search

eclipse-threadx / usbx

Eclipse ThreadX - USBX is a high-performance USB host, device, and on-the-go (OTG) embedded stack, that is fully integrated with Eclipse ThreadX RTOS
https://github.com/eclipse-threadx/rtos-docs/blob/main/rtos-docs/usbx/index.md
MIT License
148 stars 88 forks source link

UX_SLAVE_REQUEST_DATA_MAX_LENGTH has hard constrains #79

Open josesimoes opened 1 year ago

josesimoes commented 1 year ago

Describe the bug

When trying to define UX_SLAVE_REQUEST_DATA_MAX_LENGTH in us_user.h to low value in order to save memory, it hits on a check of UX_DEVICE_CLASS_RNDIS_MAX_MSG_LENGTH.

This is on the latest v6.2.0_rel.

To Reproduce Steps to reproduce the behavior:

  1. Set UX_SLAVE_REQUEST_DATA_MAX_LENGTH to 64 for example
  2. The build will fail with an error: #error "Error: the maximum-sized RNDIS response cannot fit inside the control endpoint's data buffer. Increase UX_SLAVE_REQUEST_DATA_MAX_LENGTH."

Expected behavior It should be possible to save memory (even aggressively) by use of UX_SLAVE_REQUEST_DATA_MAX_LENGTH.

Impact Mild impact as this is wasting memory for nothing.

Logs and console output N/A

Additional context A nice approach to fix this would be to have configuration options in ux_user.h to enable (or disable) the slave (and master, if that's the case) classes that should be added to the build. As it is now, everything gets added (which is a bit of a waste if you ask me).

Maybe a quicker fix would be to have a specific config to signal that RNDIS is not used and therefore it wouldn't hit that check.

josesimoes commented 1 year ago

Trying to add a workaround on that, I found out the there is another check that colides with this:

#if (UX_SLAVE_REQUEST_DATA_MAX_LENGTH < USBX_DEVICE_CLASS_STORAGE_CONFIGURATION_ACTIVE_PROFILE_LENGTH) ... @ line 295 common\usbx_device_classes\src\ux_device_class_storage_get_configuration.c

xiaocq2001 commented 1 year ago

Thanks for the feedback. We will discuss and fix the issue in next release.

BTW it's safe to exclude classes files that you don't need while building.

josesimoes commented 1 year ago

Thanks for the feedback. We will discuss and fix the issue in next release.

Great! Appreciate you looking into this.

BTW it's safe to exclude classes files that you don't need while building.

Sure. The thing is that for accomplish that it requires hacking into CMake list of build files for the USBX being build as a library. Unless there is another recommended way for doing this that you can suggest. If that's the case can you please share that?

xiaocq2001 commented 1 year ago

Plan to fix compile time error in unused classes:

  1. The size check is moved to device class initialize function instead of pre-processor conditions so there will never be compile time error reported for classes.
  2. If buffer option conflicts, an error trap with code of UX_INVALID_BUILD_OPTION is reported (if callback is set) and device class registration fails (it should never happen since classes not used are not registered by application).
xiaocq2001 commented 3 months ago

It should have been fixed in 6.3.0