MTP Responder Contribution

Quick update: I added some stuff in to delete a file on this side, and notify the MTP of it...
Events sent, Windows re-enumerated some. folder still did not update automatically yet, but hitting F5 to refresh and file was gone :D

Not sure yet of this code base on if I would manually need to remove the storage record or if it will do that automatically next time someone asks to re-enumerate?

But I think progress: Again requires core change: Should I issue PR to core or do you @WMXZ which to do so?

Pushed my WIP branch up again: https://github.com/KurtE/MTP_t4/tree/SendObject_yield

Side Note: Wish some of the build directories were excluded from the project. I normally end up deleting them when working as my global search and replaces end up with lots of hits in these files.
 
As far as I remember Windows file explorer didn't automatically update any open folder and you always had to refresh an open window to see the changes that were made from somewhere else. Though I don't really use Windows anymore so something may have changed in a recent update that I don't know about.
 
Here is it attached to an apple: recognizes it but then nothing
Code:
USB MTP Device Test Program
*** Device MTPD 5ac: 12a8 - connected ***
  manufacturer: Apple Inc.
  product: iPhone
  Serial: 00008020000155080E63002E
EVENT C<-  EVENT:  OP:4005(EVT:STORE_REMOVED) TID:ffffffff P:feedface
EVENT C<-  EVENT:  OP:c001 TID:ffffffff
EVENT C<-  EVENT:  OP:4004(EVT:STORE_ADDED) TID:ffffffff P:10001

---------- Commands ----------
  s - Show storage list
  e - enum <ID>
  d - dump storage list
  R - Remove <ID> 

*** MPT connected ***
Connected to:(null)
Count of Storages: 0
Item: 0 Not found in list
Am I missing somthing?
 
As far as I remember Windows file explorer didn't automatically update any open folder and you always had to refresh an open window to see the changes that were made from somewhere else.

That is my observation also.
F5 seems to be necessary
 
That is my observation also.
F5 seems to be necessary

I just confirmed this as well with my Kindle plugged into PC. So I think some of the stuff I pushed up is starting to work pretty well.

I will do a few more experiments to see the type data differences I may see from Kindle and the like versus this code for different operations and maybe make a few changes.

Not sure what is best after this? Or How to maybe integrate some of this stuff with your "Official" version. Example I added members to the storage class or MTP class to say I deleted this file and gave it a pointer to the FS as well as a pathname, and then had the storage class do some of the work to map the path to a storage ID... and then send of notifications, and remove the node from the list and now mark it deleted...

But I can imagine a few different ways to package this functionality....
 
Just did a strange test. When my Paperwhite is connected it states USB Drive Mode. So out of curiousity I ran the mscTesting sketch and it seems to work:
Code:
msDrive1  connected/initilized
msDrive1 is NOT Mounted

   connected 1
   initialized 1
   USB Vendor ID: 1949
  USB Product ID: 0004
      HUB Number: 0
        HUB Port: 0
  Device Address: 1
Removable Device: YES
        VendorID: Kindle  
       ProductID: Internal Storage
      RevisionID: 0100
         Version: 2
    Sector Count: 6385663
     Sector size: 512
   Disk Capacity: 3269459456 Bytes

msDrive2 not connected: Code: 40


Press a key to show USB drive info:
May be why its not connecting via at all via MTP.
 
@mjs513, Note: The test sketch at times is a little temperamental ... May see if I still have the real old kindle sitting around here somewhere...

@WMXZ - Pushed up another fix to my notification code, that helped with notifications of file added at root level.

The issue was the host asked for the Parent property, which we returned the raw parent property.... First idea was if:
parent== store to return storage (converted id)... Still did not work.

Then looked at PDF again and saw:
screenshot.jpg
Sorry to do by picture but the PDF is protected so it does not allow cut and paste...

So I now return 0 when if parent==root and creating file at that level appears to work.
 
Will have to update and give the updated MTP stuff a try ...

From my phone
Code:
 B.2.11 Parent Object
 If this object exists in a hierarchy, this property will contain the object handle of tile
 parent of this object. Only objects of type Association may be identified in this field. If
 this object is in the root or the device does not support associations, this field shall be set
 to 0x00000000.

Corrected on PC - seems to hate the word THIS.
 
Got the p#551 latest github MTP.

Without USB2-master it won't build :: ...\MTP_t4-master\examples\mtp-test\mtp-test.ino

With WMXZ-EU/USB2-master I get:
Code:
T:\TEMP\arduino_build_mtp-test.ino\libraries\USB2-master\usb1_mtp.c.o: In function `get_mtp_txEventcount':
T:\tCode\libraries\USB2-master\src/usb1_mtp.c:65: multiple definition of `get_mtp_txEventcount'
T:\TEMP\arduino_build_mtp-test.ino\libraries\MTP_t4-master\MTP.cpp.o:T:\tCode\libraries\MTP_t4-master\src/MTP.cpp:1621: first defined here
t:/arduino-1.8.13_t54/hardware/tools/arm/bin/../lib/gcc/arm-none-eabi/5.4.1/../../../../arm-none-eabi/bin/ld.exe: Disabling relaxation: it will not work with multiple definitions
T:\TEMP\arduino_build_mtp-test.ino\libraries\USB2-master\usb1_mtp.c.o: In function `tx_event':
T:\tCode\libraries\USB2-master\src/usb1_mtp.c:68: multiple definition of `get_mtp_rxEventcount'
T:\TEMP\arduino_build_mtp-test.ino\libraries\MTP_t4-master\MTP.cpp.o:T:\arduino-1.8.13_t54\hardware\teensy\avr\cores\teensy4/EventResponder.h:139: first defined here
T:\TEMP\arduino_build_mtp-test.ino\libraries\USB2-master\usb1_mtp.c.o: In function `tx_event':
T:\tCode\libraries\USB2-master\src/usb1_mtp.c:68: multiple definition of `usb_mtp_recvEvent'
T:\TEMP\arduino_build_mtp-test.ino\libraries\MTP_t4-master\MTP.cpp.o:T:\arduino-1.8.13_t54\hardware\teensy\avr\cores\teensy4/EventResponder.h:139: first defined here
collect2.exe: error: ld returned 1 exit status
 
Got the p#551 latest github MTP.

Without USB2-master it won't build :: ...\MTP_t4-master\examples\mtp-test\mtp-test.ino

With WMXZ-EU/USB2-master I get:
Code:
T:\TEMP\arduino_build_mtp-test.ino\libraries\USB2-master\usb1_mtp.c.o: In function `get_mtp_txEventcount':
T:\tCode\libraries\USB2-master\src/usb1_mtp.c:65: multiple definition of `get_mtp_txEventcount'
T:\TEMP\arduino_build_mtp-test.ino\libraries\MTP_t4-master\MTP.cpp.o:T:\tCode\libraries\MTP_t4-master\src/MTP.cpp:1621: first defined here
t:/arduino-1.8.13_t54/hardware/tools/arm/bin/../lib/gcc/arm-none-eabi/5.4.1/../../../../arm-none-eabi/bin/ld.exe: Disabling relaxation: it will not work with multiple definitions
T:\TEMP\arduino_build_mtp-test.ino\libraries\USB2-master\usb1_mtp.c.o: In function `tx_event':
T:\tCode\libraries\USB2-master\src/usb1_mtp.c:68: multiple definition of `get_mtp_rxEventcount'
T:\TEMP\arduino_build_mtp-test.ino\libraries\MTP_t4-master\MTP.cpp.o:T:\arduino-1.8.13_t54\hardware\teensy\avr\cores\teensy4/EventResponder.h:139: first defined here
T:\TEMP\arduino_build_mtp-test.ino\libraries\USB2-master\usb1_mtp.c.o: In function `tx_event':
T:\tCode\libraries\USB2-master\src/usb1_mtp.c:68: multiple definition of `usb_mtp_recvEvent'
T:\TEMP\arduino_build_mtp-test.ino\libraries\MTP_t4-master\MTP.cpp.o:T:\arduino-1.8.13_t54\hardware\teensy\avr\cores\teensy4/EventResponder.h:139: first defined here
collect2.exe: error: ld returned 1 exit status

Sorry I think the issue here is that I removed the code I added earlier to CORE that sent the events and instead I merged in the stuff that @WMXZ added to the MTP code, to do it.

Note the only changes I have now in Cores are to the descriptor header file. I pushed my current changes in Core to a branch:
https://github.com/KurtE/cores/tree/mtp_event_usb_descriptor

Note: This new branch is based on my other branch that allowed core debug output over the Serial object (USB), which is still sitting in a pending PR
 
@WMXZ - Good Morning (probably evening there)

As I continue to play with adding code to my fork/branch, I also try to keep up on your changes in the your master branch.

I noticed a few updates you did which at least one of them I think is response to my post/code about Parent object?

I see you changed your version to simply initialize the parent object of the roots to be zero, which is probably a better solution than what I did, which was when/if the host asks for the parent property and we detect that parent==store...

I was about to merge your change into my branch. but was not sure what else it might impact...

Example:
Code:
 void MTPStorage_SD::StartGetObjectHandles(uint32_t store, uint32_t parent) 
  { 
    GenerateIndex(store);
    if (parent) 
    { if (parent == 0xFFFFFFFF) parent = store; // As per initizalization
Is that still correct?

Also wondering would it help if I pushed up a branch more closely aligned with your current stuff?

Kurt
 
@KurtE,
Yes I'm following your modifications and try them out.
There are couple of things I may have a different starting point, so I'm curious to see how they work out.
So please go ahead with modifications and if there is a stable solution, we can discuss what should go where and what to keep.
 
Sorry I think the issue here is that I removed the code I added earlier to CORE that sent the events and instead I merged in the stuff that @WMXZ added to the MTP code, to do it.

Note the only changes I have now in Cores are to the descriptor header file. I pushed my current changes in Core to a branch:
https://github.com/KurtE/cores/tree/mtp_event_usb_descriptor

Note: This new branch is based on my other branch that allowed core debug output over the Serial object (USB), which is still sitting in a pending PR

Okay using that 'CORES' :: that built and Win 10 shows online MTP Teensy with PROGRAM 960KB and RAM1 1.90MB and RAM2 3.81MB

... Going back to Other 'Frank B' and PCB Flash speed compare.
 
@WMXZ - Still playing along....

I just pushed my current changes up to my fork/branch.

With this current one I now have the code in place to create files, directories and now delete files locally and send events to the host. Which when I hit F5 the changes actually show up :D


May experiment with seeing if there is some way I can tell windows to refresh ... i.e. something that triggers a file change notify.


I was curious to see if anything will work on T3.5 or T3.6, but so far nothing showing up on PC. Are the pieces in place for this? Or do I need to do something to get it to work?

Will investigate.
 
@WMXZ - Still playing along....

I just pushed my current changes up to my fork/branch.

With this current one I now have the code in place to create files, directories and now delete files locally and send events to the host. Which when I hit F5 the changes actually show up :D


May experiment with seeing if there is some way I can tell windows to refresh ... i.e. something that triggers a file change notify.


I was curious to see if anything will work on T3.5 or T3.6, but so far nothing showing up on PC. Are the pieces in place for this? Or do I need to do something to get it to work?

Will investigate.

Great you got it working this far.

Think I read in another thread on MTP that support for events on T3.x wasn't implemented yet?
 
Thanks @mjs513 - Right now I am not getting an MTP window to open up on T3.x... Maybe it is my configuration or the like.

Also @WMXZ - Will be interesting to see how best to integrate stuff in for telling MTP that some file changed.

Right Now I am doing stuff like:
Code:
          FS *pfs = storage.getFS(storage_index);
          if (pfs) {
            Serial.printf("MTPD Try to removefile:%s from %s\n", pathname, storage.getFSName(storage_index));
            if (pfs->remove(pathname)) {
              Serial.println(" -- Succeeded try to notify");
              mtpd.notifyFileRemoved(pfs, pathname);
              Serial.printf("MTPD notified of file:%s on %s removed\n", pathname, storage.getFSName(storage_index));
            } else {
              Serial.println(" -- failed to remove");
            }
          }
Now in this case I could see adding function to MFC to delete the file instead of me doing it through the FS.

But there are at least three fingers in the pie and hard to know which finger should do what:

That is:
a) File System - We do operations on files and know nothing about The storage index and the like...
b) Storage - This keeps track of "Storage" and you keep a tree of data about which storage, which parent...
c) MTP - This talks through the USB to the Host and waits for messages and in this case needs to send events.

It of course might be nice if we had hook into FS, to know when something changes and then figure out how to filter/transform that information into information you are holding in storage.

Likewise I am not sure if the host ever sends hints back, like no one is looking at this whole branch so feel free to discard any cache you may be holding onto ...
 
Quick update: T3.6 is working. Not sure if I changed anything or I just did not notice it being open. Also this time I built inside Arduino with MTPD plus Serial..

Will go back to other build and see if it also is working using Seremu

Note: it does not look like my events are getting out on this yet... Will take look

EDIT2: it looks like my deleting a file is showing up on Windows side on T3.6, but create new file is not... still looking
Note: this is with my changes to the USB Descriptor for events...
 
Last edited:
Another Quick update. I believe I have events working with the T3.6 as well so assuming 3.5...

I pushed up changes to my fork/branch. Again probably relies on my Core file changes to the USB descriptor...

Should I issue a PR to Paul for Core changes?

If you are curious here is the send_event... Note: I hacked in some debug output which I will remove:
Code:
  #define EVENT_TX_PACKET_LIMIT 4

    int usb_mtp_sendEvent(const void *buffer, uint32_t len, uint32_t timeout)
    {
      usb_packet_t *event_packet;
      printf("usb_mtp_sendEvent: called %x %x\n", (uint32_t)buffer, len);
        struct MTPContainer {
        uint32_t len;  // 0
        uint16_t type; // 4
        uint16_t op;   // 6
        uint32_t transaction_id; // 8
        uint32_t params[5];    // 12
      } __attribute__((__may_alias__)) ;

      const  MTPContainer *pe = (const  MTPContainer *)buffer;
      printf("  op:%x len:%d type:%d tid:%d Params:  ", pe->op, pe->len, pe->type, pe->transaction_id);
      if(pe->len>12) printf(" %x", pe->params[0]); \
      if(pe->len>16) printf(" %x", pe->params[1]); \
      if(pe->len>20) printf(" %x", pe->params[2]); \
      printf("\n"); \

      if (!usb_configuration) return -1;
      elapsedMillis em = 0;
      while (1) {
        if (!usb_configuration) {
          return -1;
        }
        if (usb_tx_packet_count(MTP_EVENT_ENDPOINT) < EVENT_TX_PACKET_LIMIT) {
          event_packet = usb_malloc();
          if (event_packet) break;
        }
        if (em > timeout) {
          return -1;
        }
        yield();
      }


      memcpy(event_packet->buf, buffer, len);
      event_packet->len = len;
      usb_tx(MTP_EVENT_ENDPOINT, event_packet);
      return len;
    }
 
BTW,
My Github has also a tested T3.6 version on events included (in MTP.cpp)
I would keep event handling outside cores
I did a couple of days ago a PR on teensy3/usb_desc.h to have tx events, but no action by Paul yet
 
BTW,
My Github has also a tested T3.6 version on events included (in MTP.cpp)
I would keep event handling outside cores
I did a couple of days ago a PR on teensy3/usb_desc.h to have tx events, but no action by Paul yet
Thanks,

I am having fun and think this will be a nice enhancement for the Teensy platform!

I started off with your Teensy 3.x event code yesterday:

Code:
#if defined(__MK20DX128__) || defined(__MK20DX256__) || defined(__MK64FX512__) || defined(__MK66FX1M0__)

  #include "usb_mtp.h"
  extern "C"
  {
    usb_packet_t *tx_event_packet=NULL;

    int usb_init_events(void)
    {
      tx_event_packet = usb_malloc();
      if(tx_event_packet) return 1; else return 0; 
    }


    int usb_mtp_sendEvent(const void *buffer, uint32_t len, uint32_t timeout)
    {
      if (!usb_configuration) return -1;
      memcpy(tx_event_packet->buf, buffer, len);
      tx_event_packet->len = len;
      usb_tx(MTP_EVENT_ENDPOINT, tx_event_packet);
      return len;
    }
  }
And I was able to send one event, maybe my understanding is not correct, but my looking at other T3.x Hardware USB code, gives me the impression that,
The usb_malloc will allocate a buffer out of the usb memory pool, which you can use. And then when you call usb_tx, it will put that buffer onto a queue to be transferred and
once that transfer is complete, that usb buffer will be freed. (i.e. the allocation is only good for one send).

So I modeled my version after the suff in usb_keyboard.c...
Code:
static int usb_keymedia_send(void)
{
	uint32_t wait_count=0;
	usb_packet_t *tx_packet;
	const uint16_t *consumer;

	while (1) {
		if (!usb_configuration) {
			return -1;
		}
		if (usb_tx_packet_count(KEYMEDIA_ENDPOINT) < TX_PACKET_LIMIT) {
			tx_packet = usb_malloc();
			if (tx_packet) break;
		}
		if (++wait_count > TX_TIMEOUT || transmit_previous_timeout) {
			transmit_previous_timeout = 1;
			return -1;
		}
		yield();
	}
	// 44444444 44333333 33332222 22222211 11111111
	// 98765432 10987654 32109876 54321098 76543210
	consumer = keymedia_consumer_keys;
	*(tx_packet->buf + 0) = consumer[0];
	*(tx_packet->buf + 1) = (consumer[1] << 2) | ((consumer[0] >> 8) & 0x03);
	*(tx_packet->buf + 2) = (consumer[2] << 4) | ((consumer[1] >> 6) & 0x0F);
	*(tx_packet->buf + 3) = (consumer[3] << 6) | ((consumer[2] >> 4) & 0x3F);
	*(tx_packet->buf + 4) = consumer[3] >> 2;
	*(tx_packet->buf + 5) = keymedia_system_keys[0];
	*(tx_packet->buf + 6) = keymedia_system_keys[1];
	*(tx_packet->buf + 7) = keymedia_system_keys[2];
	tx_packet->len = 8;
	usb_tx(KEYMEDIA_ENDPOINT, tx_packet);
	return 0;
}
Where they check to see how may packets they still have pending to send on their endpoint, and wait until it is under some max value, and then they allocate a new one, fill it in and call the usb_tx.

Which is why mine (minus debug stuff looks like:
Code:
  #define EVENT_TX_PACKET_LIMIT 4

    int usb_mtp_sendEvent(const void *buffer, uint32_t len, uint32_t timeout)
    {
      usb_packet_t *event_packet;
      elapsedMillis em = 0;
      while (1) {
        if (!usb_configuration) {
          return -1;
        }
        if (usb_tx_packet_count(MTP_EVENT_ENDPOINT) < EVENT_TX_PACKET_LIMIT) {
          event_packet = usb_malloc();
          if (event_packet) break;
        }
        if (em > timeout) {
          return -1;
        }
        yield();
      }

      memcpy(event_packet->buf, buffer, len);
      event_packet->len = len;
      usb_tx(MTP_EVENT_ENDPOINT, event_packet);
      return len;
    }

And then yesterday I was able to run the T3.6 and do events. Where I created a file, and did the F5 and it was there, likewise create a directory... And delete a file.
Likewise am able to do the same events on Windows.

Note: at some point it would be interesting to have the ability for a Teensy sketch that is setup to use MTP to know in their sketch when a file is added or changed from the PC side.

Example the user sends a new configuration file or maybe you are running a python interpreter and want to know if a new python script has been downloaded...

Again having fun!
 
@KurtE
On T3.x, my interpretation was that after malloc you need also a free, but I may be wrong.
So I use always the same buffer for multiple events, assuming that I will not send multiple events one after the other. Events will not span multiple buffers, so multiple mallocs would not be necessary.
Anyhow,
could not get storage_info_changed event working.
I see, you send it after every file creation, together with object_added event, but what I need sending event after multiple file creation (say after long time of logging). For me reset event is sufficient.
 
Again I could be wrong, but my look through teensy3... USB stuff, I did not notice any of the USB classes ever doing a free of a packet used for TX, only ones used for RX.

I also noticed in the ISR:
Code:
			if (stat & 0x08) { // transmit
				usb_free(packet);
				packet = tx_first[endpoint];
				if (packet) {
					//serial_print("tx packet\n");
So again it is my impression that once a TX happens that Packet is freed and some other device or usage could easily claim it.

Yes - my test version of code always sends both events one after another. But this could easily be changed to for example may be keep an elapsedMillis item for how long since the last file was updated, and if a timeout then the update is sent... Or add logical support like: begin transaction, change all of the files, end transaction...

Will play some more. The bright side of the updated code is it does not take a buffer out of the USB memory pool unless it is actually going to send an event.

Now back to playing.
 
I also noticed in the ISR:
Code:
			if (stat & 0x08) { // transmit
				usb_free(packet);
				packet = tx_first[endpoint];
				if (packet) {
					//serial_print("tx packet\n");
So again it is my impression that once a TX happens that Packet is freed and some other device or usage could easily claim it.
maybe one needs an ISR also for events. don't think TX ISR is called.
 
maybe one needs an ISR also for events. don't think TX ISR is called.

Note the ISR I am mentioning is the main USB ISR in usb_dev.c
Code:
void usb_isr(void)
{
	uint8_t status, stat, t;

	//serial_print("isr");
	//status = USB0_ISTAT;
	//serial_phex(status);
	//serial_print("\n");
	restart:
	status = USB0_ISTAT;

	if ((status & USB_ISTAT_SOFTOK /* 04 */ )) {
		if (usb_configuration) {
			t = usb_reboot_timer;
			if (t) {
				usb_reboot_timer = --t;
				if (!t) _reboot_Teensyduino_();
			}
...
#ifdef AUDIO_INTERFACE
			if ((endpoint == AUDIO_TX_ENDPOINT-1) && (stat & 0x08)) {
				unsigned int len;
				len = usb_audio_transmit_callback();
				if (len > 0) {
					b = (bdt_t *)((uint32_t)b ^ 8);
					b->addr = usb_audio_transmit_buffer;
					b->desc = (len << 16) | BDT_OWN;
					tx_state[endpoint] ^= 1;
				}
			} else if ((endpoint == AUDIO_RX_ENDPOINT-1) && !(stat & 0x08)) {
				usb_audio_receive_callback(b->desc >> 16);
				b->addr = usb_audio_receive_buffer;
				b->desc = (AUDIO_RX_SIZE << 16) | BDT_OWN;
			} else if ((endpoint == AUDIO_SYNC_ENDPOINT-1) && (stat & 0x08)) {
				b = (bdt_t *)((uint32_t)b ^ 8);
				b->addr = &usb_audio_sync_feedback;
				b->desc = (3 << 16) | BDT_OWN;
				tx_state[endpoint] ^= 1;
			} else
#endif
			if (stat & 0x08) { // transmit
				usb_free(packet);
				packet = tx_first[endpoint];
				if (packet) {
					//serial_print("tx packet\n");
					tx_first[endpoint] = packet->next;
Again I might be misinterpreting this, but...
 
Side note on the notifications. I am not really sure what the send_StorageInfoChangedEvent is actually getting us yet...

That is I am not noticing anywhere that the Host is then asking for updated information.
Code:
*** List of Storages ***
    0: sdio
    1: nspif-3
    2: nspif-4

Commands
  Create File: f <storage index> pathname [size]
  Create directory: d <SI> pathname
  Remove file.dir: r <SI> pathname
  print storage index list: i

 *** Command line: f 2 /ANewFile.txt
MTPStorage_SD::MapFileNameToIndex 2 /ANewFile.txt dir:0 add:1
2: 2 1 1 -1 0 0 /
Looking for: ANewFile.txt
Node Not found
New node created: 24
24: 2 0 0 -1 0 0 ANewFile.txt
notifyFileCreated: 1fff202c:2002fdd0 maps to handle: 18
usb_mtp_sendEvent: called 2002fc40 10
  op:4002 len:16 type:4 tid:184 Params:   18
  parent: ffffffff storage: 2 -> 10000003
usb_mtp_sendEvent: called 2002fc40 10
  op:400c len:16 type:4 tid:184 Params:   10000003
MTPD notified of file:/ANewFile.txt size:1024 on nspif-4 created
9803 20 1 185:  18 dc02
9803 20 1 186:  18 dc01
9803 20 1 187:  18 dc07
9802 20 1 188:  dc0b 3000
9803 20 1 189:  18 dc0b
9803 20 1 190:  18 dc41
9803 20 1 191:  18 dc44
9803 20 1 192:  18 dc03
9803 20 1 193:  18 dc04
1008 16 1 194:  18
Note: you probably already noticed I changed my StorageIDs to have the high word by non zero, like: 10000003

I would have expected that there would be a message asking for the updated information on that storage object...
All of the above appear to be asking for data on the new object (0x18)...
As well as asking about format of the DC0b parameter (Parent)...
 
Back
Top