Forum Rule: Always post complete source code & details to reproduce any issue!
Page 1 of 2 1 2 LastLast
Results 1 to 25 of 39

Thread: MTP Responder Contribution

  1. #1

    MTP Responder Contribution

    While working on TeensySaber, I felt a need to access the storage without having to to pull out the sd card and put it in an sd card reader.
    Since the promised MTP responder seems to be taking a while, I wrote one myself.
    It requires applying a small patch to the teensy core libraries to work, but I hope Paul will incorporate this code into the teensy distribution to make it easy for everybody to use.
    All code attached below, enjoy.
    Attached Files Attached Files

  2. #2
    Oh, I forgot: To try it out you also need to add entries to your boards.txt file to make "MTP Disk" show up in the "USB Type" menu.
    The entries you need looks like this:

    teensy36.menu.usb.mtp=MTP Disk
    teensy36.menu.usb.mtp.build.usbtype=USB_MTPDISK
    teensy36.menu.usb.mtp.fake_serial=teensy_gateway

    (Replace 36 with 35, 31, 30 and LC as needed.)

  3. #3
    Senior Member Frank B's Avatar
    Join Date
    Apr 2014
    Location
    Germany
    Posts
    4,241
    Wow, that looks very interesting.
    Do you intend to make a library from that ?

  4. #4
    Senior Member PaulStoffregen's Avatar
    Join Date
    Nov 2012
    Posts
    15,427
    Will look at this soon! Thanks.

  5. #5
    Quote Originally Posted by Frank B View Post
    Wow, that looks very interesting.
    Do you intend to make a library from that ?
    It's intended to be a library. Not sure how you make one though.
    Also not sure if makes sense for me to do it or someone else (Paul?) as I'm probably not going to put a lot of effort into this in the future.
    (It already works well enough for my purposes.)

  6. #6
    Member
    Join Date
    Jun 2014
    Location
    Singapore
    Posts
    41
    I did this:
    1. Updated the two file as in https://github.com/PaulStoffregen/cores/pull/230
    2. Updated menu and built the code with USB_MTPDISK


    Below are my observation
    1. A lot of warning on "warning: dereferencing type-punned pointer will break strict-aliasing rules" on de-referencing on CONTAINER
    2. A 32 GB microSD wass shown as 1.00 GB microSD with 512 MB free.
    3. The microSD was shown empty but it was not.
    4. Could not write anything into the microSD


    Did I miss any step?

  7. #7
    Senior Member
    Join Date
    Jan 2013
    Posts
    843
    Quote Originally Posted by yoonghm View Post
    [*] The microSD was shown empty but it was not.
    You probably need to change the SPI pin. The sketch is set up for an external card slot.

    I could write to the SD, but large file transfers hang with the MTP device being unresponsive (Teensy 3.6 SDIO slot, Windows 7).

  8. #8
    1. I'm not sure what the right way to work around those warnings is.
    2. The SD library doesn't expose the size and free space of the SD card, so currently the code always says the card is 1Gb with 512Mb free.
    3. Make sure the SD.begin() call matches the whatever pin you use as SD_EN (BUILTIN_SDCARD if you're using teensy 3.5/3.6)
    4. (See 3)

    I could write to the SD, but large file transfers hang with the MTP device being unresponsive (Teensy 3.6 SDIO slot, Windows 7).
    This sounds suspiciously like what happened before I implemented the "Get Device Status" control message. (0x67A1).
    I guess windows 7 does something slightly different than windows 10... Like maybe it uses 0x6721 instead?
    I don't have time to test it myself right now as I'm preparing to go on vacation. (Also, I'm not sure I have a working
    Win7 machine anymore...)

  9. #9
    With a few changes I got this working on a T36 with the built-in SD slot.
    Replaced
    #include "sd.h"
    with
    #include "SdFat.h"
    SdFatSdioEX SD;

    Increased the .name field of the Record struct from 14 to 64 bytes, and then replaced
    strcpy(r.name, child.name); ~line 203
    with
    child.getName(r.name, 64);

    change the 12 in (~line 336):
    if (strlen(filename) > 12) return 0;
    to 62

    Remove the 10 from: SD.begin(10)


    After this it all looks and works faultlessly (apart from volume space reporting). I'm seeing transfer speeds of around 1Megabyte per second, which is inline with the port type. There's no lag or stuttering of any kind when accessing or copying small or large (using Win7).

    Nice work.
    Last edited by MichaelMC; 04-06-2017 at 10:24 PM.

  10. #10
    For correct size and free space replace size() and free() (~line 100) with:

    Code:
      uint64_t size(){
      	 return (uint64_t)512 * (uint64_t)SD.clusterCount() * (uint64_t)SD.vol()->blocksPerCluster();
      }
      
      uint64_t free() {
    	 uint64_t volFree = SD.vol()->freeClusterCount();
      	 uint64_t ssize = (uint64_t)512 * volFree * (uint64_t)SD.vol()->blocksPerCluster();
      	 return ssize;
      }
    and replace: ~line 377
    uint32_t size = f_.size();
    with
    uint64_t size = f_.size();

    Total size and free space/used space are now reporting correctly for me with an 8gb card.

  11. #11
    Nice, I'll have to take a look at SdFat.h, seems better than SD.h.

  12. #12
    To fix the aliasing problem:

    Replace ~line 621
    #define CONTAINER ((struct MTPContainer*)(receive_buffer->buf))

    with

    Code:
    inline MTPContainer *contains (usb_packet_t *receive_buffer){
    	return (MTPContainer*)(receive_buffer->buf);
    }
    #define CONTAINER contains(receive_buffer)

  13. #13
    Senior Member
    Join Date
    Jan 2013
    Posts
    843
    Quote Originally Posted by hubbe View Post
    This sounds suspiciously like what happened before I implemented the "Get Device Status" control message. (0x67A1).
    I guess windows 7 does something slightly different than windows 10... Like maybe it uses 0x6721 instead?
    I tried adding 0x6721 (the response would be the same, right?) - doesn't help.
    Nice, I'll have to take a look at SdFat.h, seems better than SD.h.
    It's much faster. With Teensy 3.6, SdFat-beta can do 19MB/s (not over FS USB of course).

  14. #14
    Member
    Join Date
    Jun 2014
    Location
    Singapore
    Posts
    41
    This is wonderful. MichealMC@ could you post the complete code of your changes? Thanks

  15. #15
    Quote Originally Posted by MichaelMC View Post
    To fix the aliasing problem:

    Replace ~line 621
    #define CONTAINER ((struct MTPContainer*)(receive_buffer->buf))

    with

    Code:
    inline MTPContainer *contains (usb_packet_t *receive_buffer){
    	return (MTPContainer*)(receive_buffer->buf);
    }
    #define CONTAINER contains(receive_buffer)
    Can you explain why this is better than just using a define? Or is it just hiding the aliasing so that the compiler can't see it?

  16. #16
    Member
    Join Date
    Jun 2014
    Location
    Singapore
    Posts
    41
    If you cast a pointer to a different kind of pointer and then dereference it, then your program has executed an undefined behavior.

    The in-line function returns a pointer to a real MTPContainer object instead of relying on casting. Some solutions offered in stackoverflow uses memcpy() to copy the data into a real object, but this process is slower. The solution here is superb.

  17. #17
    Quote Originally Posted by yoonghm View Post
    If you cast a pointer to a different kind of pointer and then dereference it, then your program has executed an undefined behavior.

    The in-line function returns a pointer to a real MTPContainer object instead of relying on casting. Some solutions offered in stackoverflow uses memcpy() to copy the data into a real object, but this process is slower. The solution here is superb.
    If I'm reading the standard right, this is still undefined behavior, the compiler just doesn't complain about it anymore.
    As far as I can tell, casting to a struct and then using is no longer supported in C++-11, so there goes 39 years of backwards compatibility....
    Anyways, it looks like unions is the new hotness, which is silly, because it ends up doing the same thing.

  18. #18
    Senior Member
    Join Date
    Jan 2013
    Posts
    843
    The aliasing is a time bomb. E.g.:
    Code:
    struct A {
        int s;
        int i;
    };
    
    struct B {
        int s;
        char buffer[4];
    };
    
    bool test(A& a, B& b) {
        b.buffer[0] = 1;
        a.i = 0;
        return a.i == b.buffer[0];
    }
    GCC 5.4 leaves the "a.i == b.buffer[0]" check in place. GCC 6.3 concludes that "a" and "b" cannot alias and optimizes the check away and produces the wrong result.

    Quote Originally Posted by hubbe View Post
    If I'm reading the standard right, this is still undefined behavior, the compiler just doesn't complain about it anymore.
    Yes.
    As far as I can tell, casting to a struct and then using is no longer supported in C++-11, so there goes 39 years of backwards compatibility....
    It wasn't legal in the first place. This is not something new with C++-11.
    Anyways, it looks like unions is the new hotness, which is silly, because it ends up doing the same thing.
    Using a union for type punning is not legal in C++ (it is in C99 / C11).

    GCC, CLang and ICC all support "__attribute((__may_alias__))", which gets you the correct behavior. E.g.:
    Code:
     
    struct A {
        int s;
        int i;
    } __attribute((__may_alias__));
    Last edited by tni; 04-07-2017 at 10:49 AM. Reason: fix formatting

  19. #19
    Member
    Join Date
    Jun 2014
    Location
    Singapore
    Posts
    41
    I have converted the file into library. The library could be downloaded from

    https://github.com/yoonghm/Teensy_3..../Libraries/MTP

    This library requires the following changes
    * SdFat libraries - https://github.com/greiman/SdFat-beta/tree/master/SdFat
    * <Arduino_path>/hardware/teensy/avr/cores/teensy3/usb_desc.c
    * <Arduino_path>/hardware/teensy/avr/cores/teensy3/usb_dev.c
    * <Arduino_path>/hardware/boards.txt
    Last edited by yoonghm; 04-07-2017 at 03:11 PM.

  20. #20
    Member
    Join Date
    Sep 2015
    Location
    Taiwan
    Posts
    89
    hello, gonna try this soon!
    but wondering already,

    is it possible to run my other program, after waiting for a USB response in, maybe 2 seconds?
    or switch to a MTP device if plugged.

    or is the mtpd.loop() a must to be called continuously for the MTP to work?

    thanks

  21. #21
    The best way to run something else is to put it in mtp_yield();

  22. #22
    Senior Member PaulStoffregen's Avatar
    Join Date
    Nov 2012
    Posts
    15,427
    I've uploaded Teensyduino 1.37-beta1, with the USB patch and "MTP Disk (Experimental)" in the Tools > USB Type menu.

    https://forum.pjrc.com/threads/44546...no-1-37-Beta-1

    Please give 1.37-beta1 a try and let me know how it works for you?

  23. #23
    Junior Member
    Join Date
    Jun 2017
    Posts
    8
    Many thanks for all the great contributions! Here's what I did to get a working setup for MTP in case anyone needs guidance:

    Install Teensyduino 1.37 Beta #1 over the top of 1.36 (Arduino IDE 1.8.2).
    Copy folder 'MTP' from here https://github.com/yoonghm/Teensy_3..../Libraries/MTP into my local library folder.
    Copy folder 'SdFat' from here https://github.com/greiman/SdFat-beta/tree/master/SdFat into my local library folder.
    In Arduino IDE navigate to File->Examples->(Examples from custom libraries) MTP->MTP_blinky.ino.
    In Arduino IDE navigate to Tools and ensure you have 'Board: Teensy 3.6', 'USB Type: MTP Experimental' (and I used 180MHz, Fastest)

    Regards, Ian

  24. #24
    Quote Originally Posted by PaulStoffregen View Post
    I've uploaded Teensyduino 1.37-beta1, with the USB patch and "MTP Disk (Experimental)" in the Tools > USB Type menu.

    https://forum.pjrc.com/threads/44546...no-1-37-Beta-1

    Please give 1.37-beta1 a try and let me know how it works for you?
    Sorry for the delay.
    I'm trying this out, and the MTP part seems to work, but the serial monitor seems to stop working.
    I don't think the problem is on my side, because even if I don't enable the MTP responder in my code, the serial monitor still doesn't work.
    I also tried switching to "MIDI", and then the serial monitor does work. I haven't figured out what the problem is yet, but I'll keep digging.

  25. #25
    Junior Member
    Join Date
    Jun 2017
    Posts
    8
    See here for how to create a composite MTP and CDC device, which sounds to me like what you need:

    https://forum.pjrc.com/threads/44569...l=1#post145192

    Regards, Ian

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •