MTP Responder Contribution

[KurtE]

@all - Sometimes hard to know which thread to discuss things like integration of MTP amd MSC... and the like.

I am going to try doing some additional changes to my branch of the code to add some more support for adding MSC drives/partitions also wonder if they should apply to SD drives as well.

Examples of changes I am thinking about:

a) Something simple: (@WMXZ) - In MTP class I think that the Loop() method should instead use the Task method... That is if you look at all of our examples you will notice that in their loops they all do:

void loop()
{
  myusb.Task();

Where it is setup the main USBHost object will loop through and call each devices Task method to allow them to do any function like what you do in your Loop function...

b) Lots of stuff with MSC - First pass startup timing: Example maybe have some drives NOT at startup and/or when MSC Drive Insertion not at that time try to get the free/used space of the device, but wait to do that until after MTP is happy.. On at least a few thumb drives and maybe others this may take several seconds...

b1) I may first try a hack. Currently when I have an LittleFS drive do a format operation, I have an IntervalTimer looking for messages from the host, and this code always returns I am busy status... Wonder if I can get away with that now? That is maybe while we are looking at all of our devices at startup, have the background code try to stall the host, until we are ready for it... Currently if our startup is too slow both Windows and especially Linux won't show anything nor recover.

c) Partitions - Especially with USB Drives, but maybe also SDCards? At startup and/or insertion detection. Detect that there are multiple volumes on that device and if so maybe add storage for each one. Have some pieces in place with SDFat and MSC stuff, but not in the integration. Currently SDFat code has FatPartion and EXPartion classes, but no unifier in the FsLib (i.e no FsPartition.h/cpp files) that work with either in the same way as FsVolume works with FatVolume and ExFatVolume...

d) Again more MSC specific although again could apply to SDCards, like cards you use for RPI have multiple partitions, although so far we don't handle any of the linux file formats...
d1) Wondering about MSC object(s), How should the functionality be factored. More later in MSC thread... Or again wondering if makes sense to talk about all of this in some Integration thread...

[mjs513]

Morning @all

@KurtE - ambitious and good luck.

But to your point on SDCards - think you are going to have to implement something similar if it has more than one partition. But I think if you get it to work with USB devices the same methods can be used for SDCards.

Thinking that we should have a second MTP_test sketch that just addresses SD and USB devices. The one we have has everything in it and getting really big - maybe include prog ram as well just as a sanity check.

[KurtE]

@mjs513 - Yep I am also thinking a smaller test with fewer Storage types would be nice and/or split some of them into multiple tabs...

And yes right not I am still playing with the MSC multi partition sketch first... Again hard to know which thread to talk on...

Right now I am hacking up a new file FsPartition.h which I am part way through... Currently will be a tab in the VolumeName sketch until I get it working... Actually I am doing it in a sublime text project, but...

It is sort of a hack:

class FsPartition {
 public:
  /** Create an instance of FsPartition
   */
  FsPartition() {}

  /** \return The shift count required to multiply by bytesPerCluster. */
  uint8_t bytesPerClusterShift() const {
    if (m_fatType==FAT_TYPE_EXFAT) return m_partEx.bytesPerClusterShift();
    return m_part.bytesPerClusterShift();
  }
  /** \return Number of bytes in a cluster. */
  uint16_t bytesPerCluster() const {
    if (m_fatType==FAT_TYPE_EXFAT) return m_partEx.bytesPerCluster();
    return m_part.bytesPerCluster();
  }
  /** \return Number of bytes per sector. */
  uint16_t bytesPerSector() const {
    if (m_fatType==FAT_TYPE_EXFAT) return m_partEx.bytesPerSector();
    return m_part.bytesPerSector();
  }
...

It has 3 member variables.
The only interesting method so far is:

  bool init(BlockDevice* dev, uint8_t part = 1)
  {
    if (m_partEx.init(dev, part)) {
      m_fatType=FAT_TYPE_EXFAT;
      return true;
    }

    if (m_part.init(dev, part)) {
      m_fatType=m_part.fatType();
      return true;
    }
    return false;
  }

Still making my way through. Started off with copy of FatPartition.h and just hacking on it. Then will include in sketch and see how many screw ups... Like not sure if I can use const on these or not... Will find out soon.

EDIT: I probably should start off just with the members we call out for but... And then there is the issue of having this be a subclass of FS? Will figure that part out after I finish this part

[wwatson]

@KurtE - I am always amazed at solutions you guys come up with. What would an integration thread be? Would that just be discussions that pertain to the MTP, FS, MSC and littleFS on a new thread? If so it would be good to not have to bounce between threads. My forum savy is showing isn't it

Today I think I will on lwext4 some more.

[KurtE]

@KurtE - I am always amazed at solutions you guys come up with. What would an integration thread be? Would that just be discussions that pertain to the MTP, FS, MSC and littleFS on a new thread? If so it would be good to not have to bounce between threads. My forum savy is showing isn't it

Today I think I will on lwext4 some more.

I sort of started a thread a little bit ago: https://forum.pjrc.com/threads/66338...h-each-other-8)
Where I was maybe setting up for some integration stuff... But only myself and @mj513 posted anything...

Right now playing with your USBMscFat code: sort of relating to what I posted on the MSC thread. about having FsLib support partitions. It would be easy to get it to maybe work by changing the begin Method... But not sure if anyone would take it...

So may duplicate that class in your project with it... May also add in a volumeName method which wraps up the functionality in the thread you just added to that project.
Plus a little cleanup...

My advanced c++ skills are a bit rusty: But I think the FsVolume object leaks other objects:

class FsVolume {
 public:
  FsVolume() {}

  ~FsVolume() {end();}
,,,
  /** free dynamic memory and end access to volume */
  void end() {
    m_fVol = nullptr;
    m_xVol = nullptr;
  }
...
bool FsVolume::begin(BlockDevice* blockDev, bool setCwv, uint8_t part) {
  Serial.printf("FsVolume::begin(%x)\n", (uint32_t)blockDev);
  m_blockDev = blockDev;
  m_fVol = nullptr;
  m_xVol = new (m_volMem) ExFatVolume;
  if (m_xVol && m_xVol->begin(m_blockDev, setCwv, part)) {
    goto done;
  }
  m_xVol = nullptr;
  m_fVol = new (m_volMem) FatVolume;
  if (m_fVol && m_fVol->begin(m_blockDev, setCwv, part)) {
    goto done;
  }
  m_cwv = nullptr;
  m_fVol = nullptr;
  return false;

 done:
  m_cwv = this;
  return true;
}

I believe that the end function setting the pointers to null frees the object that was created by new? Could be wrong.
Also dito in the begin method, that if the begin for exFatVolume fails... then it should delete the exFatVolume before it sets nullptr...

[wwatson]

I sort of started a thread a little bit ago: https://forum.pjrc.com/threads/66338...h-each-other-8)
Where I was maybe setting up for some integration stuff... But only myself and @mj513 posted anything...

Right now playing with your USBMscFat code: sort of relating to what I posted on the MSC thread. about having FsLib support partitions. It would be easy to get it to maybe work by changing the begin Method... But not sure if anyone would take it...

So may duplicate that class in your project with it... May also add in a volumeName method which wraps up the functionality in the thread you just added to that project.
Plus a little cleanup...

My advanced c++ skills are a bit rusty: But I think the FsVolume object leaks other objects:

class FsVolume {
 public:
  FsVolume() {}

  ~FsVolume() {end();}
,,,
  /** free dynamic memory and end access to volume */
  void end() {
    m_fVol = nullptr;
    m_xVol = nullptr;
  }
...
bool FsVolume::begin(BlockDevice* blockDev, bool setCwv, uint8_t part) {
  Serial.printf("FsVolume::begin(%x)\n", (uint32_t)blockDev);
  m_blockDev = blockDev;
  m_fVol = nullptr;
  m_xVol = new (m_volMem) ExFatVolume;
  if (m_xVol && m_xVol->begin(m_blockDev, setCwv, part)) {
    goto done;
  }
  m_xVol = nullptr;
  m_fVol = new (m_volMem) FatVolume;
  if (m_fVol && m_fVol->begin(m_blockDev, setCwv, part)) {
    goto done;
  }
  m_cwv = nullptr;
  m_fVol = nullptr;
  return false;

 done:
  m_cwv = this;
  return true;
}

I believe that the end function setting the pointers to null frees the object that was created by new? Could be wrong.
Also dito in the begin method, that if the begin for exFatVolume fails... then it should delete the exFatVolume before it sets nullptr...

I had seen that thread and of course forgot about it. That would be a good thread I think IMHO.

[mjs513]

@KurtE
My advanced c++ skills are limited to looking things up as I go

Anyway, what you say seems to make sense, at least to me. Now just to decided where to put all this. At this point maybe stick to the integration thread since we are jumping between libraries.

[KurtE]

@KurtE
My advanced c++ skills are limited to looking things up as I go

Anyway, what you say seems to make sense, at least to me. Now just to decided where to put all this. At this point maybe stick to the integration thread since we are jumping between libraries.

Actually I received a response on github from Bill... Now there is not a leak as the new is not actually allocating memory...
As the code like: m_fVol = new (m_volMem) FatVolume;

is actually just assigning the memory in a memory storage of the object to the pointer... More or a less a cast...

No I don't really use dynamic memory. For embedded systems in my world it was forbidden. SdFat never uses the heap.

I define my own new operator to share RAM between FAT and exFAT since only one can be active.
Here is the magic:
FsNew.h
/** 32-bit alignment */
typedef uint32_t newalign_t;

/** Custom new placement operator */
void* operator new(size_t size, newalign_t* ptr);
FsNew.cpp
#include "FsNew.h"
void* operator new(size_t size, newalign_t* ptr) {
(void)size;
return ptr;
}
It's allocated in FsVolume.h here. FS_ALIGN_DIM is the max size required.
newalign_t m_volMem[FS_ALIGN_DIM(ExFatVolume, FatVolume)];

[mjs513]

Actually I received a response on github from Bill... Now there is not a leak as the new is not actually allocating memory...
As the code like: m_fVol = new (m_volMem) FatVolume;

is actually just assigning the memory in a memory storage of the object to the pointer... More or a less a ca

Guess that answers that question

[mborgerson]

I just transferred a solar energy evaluation program from the T3.6 that I used last summer to the T3.5. I ran into some compilation issues in MTP.h and MTP.cpp in the tests for processor type:

#if defined(__MK66FX1M0__)..... code

There was no path to compile the code for the T3.5. I solved the problem by changing the #define:

#if defined(__MK66FX1M0__) || defined(__MK64FX512__)

The compilation proceeded and the program seems to work fine.

If you have made modifications to the MTP code over the last six months, please check to see that you have adjusted the #define statements to allow for the T3.5. I was using code from about 6 months ago which I had modified to allow passing in an initialized file system when setting up the storage. This saves the data space that would result if you had a file system in your foreground code and initialized a new file system inside the MTP startup. This may become even more important on the T3.5 with its smaller memory and the proliferating number of storage devices that are being added to the MTP system.