Add support for MTP device class #3023
Conversation
nbmaurizio
force-pushed
the
mtp
branch
4 times, most recently
from
b734659 to
8c0a674
Compare
roundby
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Congrats!
|
|
@WerWolv thank you for sharing! This code has been designed after the need to expose a set of files and folders stored in an SPI memory managed by littlefs, and we took the chance to do it within this library (we already used TinyUSB on some of our designs, and we hope this is a way to contribute back). While reading the code and the example, one should remind that the specification requires the device to provide a session unique 32 bit identifier for each object (e.g. file or directory), cached by the host when traversing storage content, able to be recalled any time and never re-used if an object is deleted. The implementation of this on the application side and in case of static files is easy, but it's not trivial on mutable file system where file I/O is performed by both host and (internally) by the device. Moreover, we found that, under Windows (appearing in contrast with MTP requirements) the object IDs are still cached if the session is closed and reopened, unless a bus reset or physical unplug is performed. Comparing with your code:
The code is functional, and it is expected to be ready for exposing file and directory level I/O when a block device is not advisable:
I recognize that the introduction of a new class (and it's the first time we're contributing to TinyUSB) could require some time for a review and refinement, so, we'll wait for indications from the maintainers about the interest in merging this, asking for changes, moving to a different branch for testing/integration with additional features or any other suggestion. So far, we corrected all the notified build errors and most of the code scanning notices. |
|
Hi @roundby |
littlefs must work for us, to be useful. |
i guess it will be hard with littlefs. |
|
Hi, I'm having some issues testing the features from https://github.com/ennebi/tinyusb/tree/mtp, copying or creating files in Windows 10 doesn't work properly. |
Hi @diancity025. Can you check if the bulk endpoints are numbered and opened correctly in the initialization sequence? The command phase is supposed to be followed by a data phase on bulk EP, unless there is an issue with the sequence. |
|
Hi @roundby Read-only works normally. The problem I encountered before could not create files and folders in WIN10. I added some changes through debugging and now it works fine. I'm not sure if this is a BUG. |
|
Hi @roundby, thanks very much for this! I have encountered the same thing as @diancity025. I've extended the read-only example to allow adding new objects to the list (while ignoring the file contents for now). Without @diancity025's changes, I just get a progress bar stuck at 0% while copying a file to the device. With their changes everything works as expected. I'm running this on an RP2350 connected to a Windows machine (it's also sort of working on Linux using android-file-transfer, and not at all with libmtp/nemo - will look into that properly later). Thanks @diancity025 for that fix edit: file copying is also broken on Linux without the change, but is working as expected with it. Perhaps this is something specific to the RP2040/RP2350 that we are using. |
|
Hi @diancity025. We tested the fix together with some corrections along with other refinements, a better example code with read and write capabilities and the fix on the typo identified by @kaimac1. I just pushed the changes. Tests on a RBPI Pico 2 are confirmed it to be working on Windows 11, Windows 10 and briefly checked under Ubuntu (within nautilus). |
|
Hi @f-hoepfinger-hr-agrartechnik
Lately, I am replying to this. We used this change in an internal test project, specifically designed to expose files from an SPI memory managed by littlefs. I can confirm that this setup is perfectly feasible, though there are some limitations depending on platform constraints. You can imagine that including a complete example with littlefs as a dependency would be too heavy for many microcontrollers, and likely wouldn’t be accepted. However, implementation is straightforward—as long as you account for the system's limitations. One option is to implement a flat filesystem (i.e., avoid creating associations), storing the unique identifier in either the file name or file metadata. If the platform has sufficient RAM, you could instead track queried objects and construct a mapping between identifiers and file paths. Of course there might be other valid approaches. This mapping strategy is likely the only major design decision you'll need to make. Everything else should be relatively straightforward. |
|
Hi @roundby, thanks very much for this! |
Can confirm this works with littlefs. Made some adjustments but can read all files from NOR flash |
|
@hathach ? :) |
simplify container field name
There was a problem hiding this comment.
I have reworked the driver and change how it interact and API with application. I think we should keep the driver simpler and give the data/response to application user since it is too specific. I think it make the API easy to use and maintain. It is still WIP @roundby let me know what you think.
|
Thank you for the deep review and rework on the driver, @hathach. I had a look at the functional changes, from what I see you are adding a Since when we proposed this pull request, we thought there was a tradeoff between providing a simple API that does not require deep protocol understanding and flexibility in multiple use cases. It's more a project strategy, and your directions in this sense are needed and valid. Especially thanks to the addition of The only downside I may think about is that, by providing a hybrid approach or letting on the user the responsibility of command handling, the implementer needs to precisely follow some documentation in order to know how to use the driver, what kind of operations are mandatory and how to overcome some platform specific differences (MTP host is implemented in very different ways under multiple platforms). As an example, the driver works on Windows platform without handling the property I'll wait to know if you are moving other APIs for further understanding on how you are changing the rest of the driver, and to perform some testing on a building example. |
|
@roundby thank you, I am glad the refactor make sense to you. Indeed I think we should let application take full control of the data/response. Since it will be different from app to apps. Some device will has slow block storage such as SDcard and will only need to call the tud_mtp_data_send() when it got the data asynchronously. Actually the driver can handle part of the request e.g get_device_info() the data is pre-filled according to the CFG_TUD_MTP_DEVICE_INFO_* but still presented the whole buffer to application, in case it want to do the full changes. Like msc, I think the driver shouldd focus on making reading/writing large data (e.g few MBs) easier for application. Still working on more update, hopefully we can merge this soon enough. |
| switch (command->code) { | ||
| case MTP_OP_GET_OBJECT: { | ||
| // File contents span over multiple xfers | ||
| const uint32_t obj_handle = command->params[0]; | ||
| fs_file_t* f = fs_get_file(obj_handle); | ||
| if (f == NULL) { | ||
| resp_code = MTP_RESP_INVALID_OBJECT_HANDLE; | ||
| } else { | ||
| // file contents offset is xferred byte minus header size | ||
| const uint32_t offset = cb_data->total_xferred_bytes - sizeof(mtp_container_header_t); | ||
| const uint32_t xact_len = tu_min32(f->size - offset, io_container->payload_bytes); | ||
| memcpy(io_container->payload, f->data + offset, xact_len); | ||
| tud_mtp_data_send(io_container); | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| case MTP_OP_SEND_OBJECT_INFO: { | ||
| mtp_object_info_header_t* obj_info = (mtp_object_info_header_t*) io_container->payload; | ||
| if (obj_info->storage_id != 0 && obj_info->storage_id != SUPPORTED_STORAGE_ID) { | ||
| resp_code = MTP_RESP_INVALID_STORAGE_ID; | ||
| break; | ||
| } | ||
|
|
||
| if (obj_info->parent_object) { | ||
| fs_file_t* parent = fs_get_file(obj_info->parent_object); | ||
| if (parent == NULL || !parent->association_type) { | ||
| resp_code = MTP_RESP_INVALID_PARENT_OBJECT; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| uint8_t* f_buf = fs_malloc(obj_info->object_compressed_size); | ||
| if (f_buf == NULL) { | ||
| resp_code = MTP_RESP_STORE_FULL; | ||
| break; | ||
| } | ||
| fs_file_t* f = fs_create_file(); | ||
| if (f == NULL) { | ||
| resp_code = MTP_RESP_STORE_FULL; | ||
| break; | ||
| } | ||
|
|
||
| f->object_format = obj_info->object_format; | ||
| f->protection_status = obj_info->protection_status; | ||
| f->image_pix_width = obj_info->image_pix_width; | ||
| f->image_pix_height = obj_info->image_pix_height; | ||
| f->image_bit_depth = obj_info->image_bit_depth; | ||
| f->parent = obj_info->parent_object; | ||
| f->association_type = obj_info->association_type; | ||
| f->size = obj_info->object_compressed_size; | ||
| f->data = f_buf; | ||
| uint8_t* buf = io_container->payload + sizeof(mtp_object_info_header_t); | ||
| mtp_container_get_string(buf, f->name); | ||
| // ignore date created/modified/keywords | ||
| break; | ||
| } | ||
|
|
||
| case MTP_OP_SEND_OBJECT: { | ||
| fs_file_t* f = fs_get_file(send_obj_handle); | ||
| if (f == NULL) { | ||
| resp_code = MTP_RESP_INVALID_OBJECT_HANDLE; | ||
| } else { | ||
| // file contents offset is total xferred minus header size minus last received chunk | ||
| const uint32_t offset = cb_data->total_xferred_bytes - sizeof(mtp_container_header_t) - io_container->payload_bytes; | ||
| memcpy(f->data + offset, io_container->payload, io_container->payload_bytes); | ||
| tud_mtp_data_receive(io_container); // receive more data if needed | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| default: | ||
| resp_code = MTP_RESP_OPERATION_NOT_SUPPORTED; | ||
| break; | ||
| } |
Check notice
Code scanning / CodeQL
Long switch case Note
| switch (command->code) { | ||
| case MTP_OP_SEND_OBJECT_INFO: { | ||
| fs_file_t* f = fs_get_file(send_obj_handle); | ||
| if (f == NULL) { | ||
| resp->header->code = MTP_RESP_GENERAL_ERROR; | ||
| break; | ||
| } | ||
| // parameter is: storage id, parent handle, new handle | ||
| mtp_container_add_uint32(resp, SUPPORTED_STORAGE_ID); | ||
| mtp_container_add_uint32(resp, f->parent); | ||
| mtp_container_add_uint32(resp, send_obj_handle); | ||
| resp->header->code = MTP_RESP_OK; | ||
| break; | ||
| } | ||
|
|
||
| default: | ||
| resp->header->code = (cb_data->xfer_result == XFER_RESULT_SUCCESS) ? MTP_RESP_OK : MTP_RESP_GENERAL_ERROR; | ||
| break; | ||
| } |
Check notice
Code scanning / CodeQL
No trivial switch statements Note
| switch (p_mtp->phase) { | ||
| case MTP_PHASE_IDLE: | ||
| // received new command | ||
| TU_VERIFY(ep_addr == p_mtp->ep_out && p_container->header.type == MTP_CONTAINER_TYPE_COMMAND_BLOCK); | ||
| p_mtp->phase = MTP_PHASE_COMMAND; | ||
| TU_ATTR_FALLTHROUGH; // handle in the next case | ||
|
|
||
| case MTP_PHASE_COMMAND: { | ||
| memcpy(&p_mtp->command, p_container, sizeof(mtp_container_command_t)); // save new command | ||
| p_container->header.len = sizeof(mtp_container_header_t); // default container to header only | ||
| process_cmd(p_mtp, &cb_data); | ||
| if (tud_mtp_command_received_cb(&cb_data) < 0) { | ||
| p_mtp->phase = MTP_PHASE_ERROR; | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| case MTP_PHASE_DATA: { | ||
| const uint16_t bulk_mps = (tud_speed_get() == TUSB_SPEED_HIGH) ? 512 : 64; | ||
| p_mtp->xferred_len += xferred_bytes; | ||
| cb_data.total_xferred_bytes = p_mtp->xferred_len; | ||
|
|
||
| bool is_complete = false; | ||
| // complete if ZLP or short packet or overflow | ||
| if (xferred_bytes == 0 || // ZLP | ||
| (xferred_bytes & (bulk_mps - 1)) || // short packet | ||
| p_mtp->xferred_len > p_mtp->total_len) { | ||
| is_complete = true; | ||
| } | ||
|
|
||
| if (ep_addr == p_mtp->ep_in) { | ||
| // Data In | ||
| if (is_complete) { | ||
| cb_data.io_container.header->len = sizeof(mtp_container_header_t); | ||
| tud_mtp_data_complete_cb(&cb_data); | ||
| } else { | ||
| // 2nd+ packet: payload only | ||
| cb_data.io_container = headerless_packet; | ||
| tud_mtp_data_xfer_cb(&cb_data); | ||
| } | ||
| } else { | ||
| // Data Out | ||
| if (p_mtp->xferred_len == xferred_bytes) { | ||
| // 1st OUT packet: header + payload | ||
| p_mtp->io_header = p_container->header; // save header for subsequent transaction | ||
| cb_data.io_container.payload_bytes = xferred_bytes - sizeof(mtp_container_header_t); | ||
| } else { | ||
| // 2nd+ packet: payload only | ||
| cb_data.io_container = headerless_packet; | ||
| cb_data.io_container.payload_bytes = xferred_bytes; | ||
| } | ||
| tud_mtp_data_xfer_cb(&cb_data); | ||
|
|
||
| if (is_complete) { | ||
| // back to header + payload for response | ||
| cb_data.io_container = headered_packet; | ||
| cb_data.io_container.header->len = sizeof(mtp_container_header_t); | ||
| tud_mtp_data_complete_cb(&cb_data); | ||
| } | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| case MTP_PHASE_RESPONSE_QUEUED: | ||
| // response phase is complete -> prepare for new command | ||
| TU_ASSERT(ep_addr == p_mtp->ep_in); | ||
| tud_mtp_response_complete_cb(&cb_data); | ||
| prepare_new_command(p_mtp); | ||
| break; | ||
|
|
||
| case MTP_PHASE_RESPONSE: | ||
| case MTP_PHASE_ERROR: | ||
| // supposedly to be empty | ||
| break; | ||
| default: return false; | ||
| } |
Check notice
Code scanning / CodeQL
Long switch case Note
| case MTP_PHASE_RESPONSE_QUEUED: | ||
| // response phase is complete -> prepare for new command | ||
| TU_ASSERT(ep_addr == p_mtp->ep_in); | ||
| tud_mtp_response_complete_cb(&cb_data); |
Check warning
Code scanning / CodeQL
Expression has no effect Warning
Describe the PR
Add Media Transfer Protocol (MTP) device class support
Additional context
Feature request reference: #1079
The implementation has been tested and found functional at prototype level. We provide an example tested on STM32F3. It makes use of blulk and interrupt EPs only, so it should be supported by most platforms.
The class supports a single session only, but it allows multiple objects and nested associations (i.e. files and folders). We did not add support for object properties access. There are some other limitations but for the purpose of exposing file trees when mass storage class is not advised, the library is fully functional.
This implementation has been written from the ground up, with minimal requirements on buffers and keeping the code as simple as possible. Most of the buffer allocation for object I/O is demanded to the underlying implementation, as such, the RAM footprint of the library is limited.
The working mechanism of MTP (persistent 32 bit file handles) may not scale well on microcontrollers with constrained resources, depending on the application requirements. In the example, we just provide a fixed filesystem with files residing in RAM. It would be possible, depending on the application requirements, to build filesystem based I/O on top of this, by using the API provided in mtp_device.h.
The sources are available here