Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature(esp_tinyusb): Added tusb_teardown() call while tinyusb_driver_uninstall() #39

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

roma-jam
Copy link
Collaborator

@roma-jam roma-jam commented Jun 11, 2024

Requirements

  • To uninstall tinyusb driver, a teardown tinyusb stack should be called.
  • tud_deinit() feature was implemented in upstream, starting with version v0.17.0.

Description

  • Added tusb_teardown() call while tinyusb_driver_uninstall()
  • Added test app to verify teardown feature

Testing

  1. Install TinyUSB device without any class
  2. Wait SetConfiguration() (tud_mount_cb)
  3. If attempts == 0 goto step 8
  4. Wait TEARDOWN_DEVICE_DETACH_DELAY_MS
  5. Uninstall TinyUSB device
  6. Wait TEARDOWN_DEVICE_INIT_DELAY_MS
  7. Decrease attempts by 1, goto step 3
  8. Wait TEARDOWN_DEVICE_DETACH_DELAY_MS
  9. Uninstall TinyUSB device

Note: when amount of teardown configured to 0, then only one install-uninstall routine is fulfilled.

Related issues

@roma-jam roma-jam self-assigned this Jun 11, 2024
@roma-jam
Copy link
Collaborator Author

roma-jam commented Jun 11, 2024

@tore-espressif , @peter-marcisovsky
This changes unlock the possibility to run usb device tests one by one (espressif/tinyusb#27 should be merged first, though).

I posted two questions for both of you, because it seems that we can make it better and we can use it.

I need that for enumeration driver testing (but maybe I will eliminate esp_tinyusb component from the chain, but anyway), so feel free to check it out when possible.

Meanwhile, I will go and cover the Enum Driver as much as I can.

@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch 5 times, most recently from d7cb17d to c14eebb Compare June 11, 2024 12:57
@roma-jam roma-jam marked this pull request as ready for review June 11, 2024 13:14
Copy link
Collaborator

@peter-marcisovsky peter-marcisovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some observations from using the test app

device/esp_tinyusb/test_app/sdkconfig.defaults Outdated Show resolved Hide resolved
device/esp_tinyusb/tinyusb.c Show resolved Hide resolved
@finger563
Copy link

Is there a timeline for this feature to be finalized?

@roma-jam
Copy link
Collaborator Author

Hi @finger563 ,

This feature allows the re-configure the driver with these changes: espressif/tinyusb#27
But as far as the same changes are already in the upstream, so we will rebase these changes after updating version to v0.17.0.
For current feature there is no available timeline for now, but the one thing for sure: the update to v0.17.0 will be done first.

Meanwhile, this feature should be available in the upstream of TinyUSB, if anything it is possible to use it.

Sorry for the inconvenience.

@roma-jam roma-jam marked this pull request as draft December 4, 2024 14:08
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch from 87ddae1 to 85c3227 Compare December 4, 2024 14:25
@roma-jam roma-jam added this to the esp_tinyusb v1.6.0 milestone Dec 5, 2024
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch from ef29b89 to 7bcc554 Compare December 11, 2024 11:40
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch 2 times, most recently from da569b5 to 2954367 Compare December 13, 2024 12:09
@roma-jam
Copy link
Collaborator Author

roma-jam commented Dec 13, 2024

@tore-espressif @peter-marcisovsky
This is the enabling of teardown feature via tinyusb_driver_uninstall() call from esp_tinyusb wrapper.

To enable the CI (I am expecting the teardown_device test to fail), it is required to release espressif/tinyusb component with changes from here: espressif/tinyusb#44

Feel free to review the changes in current PR and I will relaunch CI after the tinyusb release (Which I planned on 22th of December, but will do that next Monday, 16th).

UPD: we are running only cdc test, not all of them, so there is not problem with CI

Copy link
Collaborator

@peter-marcisovsky peter-marcisovsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leaving some comments.
I am having some issues with running the test locally. Will try to fix it an maybe add something else to the review.

@roma-jam roma-jam marked this pull request as draft December 16, 2024 21:21
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch 2 times, most recently from 5a9d94d to 4be9e95 Compare December 19, 2024 13:07
@roma-jam roma-jam marked this pull request as ready for review December 19, 2024 13:13
@roma-jam roma-jam marked this pull request as draft December 19, 2024 14:16
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch 4 times, most recently from f6dc83c to fae1686 Compare December 19, 2024 22:39
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch from dfd0deb to a1e0772 Compare December 20, 2024 12:27
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch from a1e0772 to d302232 Compare December 20, 2024 12:28
@roma-jam roma-jam force-pushed the feature/esp_tinyusb_stack_teardown branch from 0ca5094 to 2f2c9de Compare December 20, 2024 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

esp32s3, restarting cdc software serial doesn't work (IDFGH-12813)
6 participants