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

Add CDC parsing host test #66

Merged
merged 2 commits into from
Oct 2, 2024
Merged

Conversation

tore-espressif
Copy link
Collaborator

@tore-espressif tore-espressif commented Sep 20, 2024

This is a refactor that separates Descriptor parsing logic to a separate file with its own private API.

This new private API is than used with newly added USB mock, so we can test the parsing logic on Linux host tests.

There is a number of hardcoded USB descriptors of tested and verified CDC devices that must work with this driver.

TODO:

  • 4G modems are not yet added. (follow-up MR?) EDIT: 4 modems added

@tore-espressif tore-espressif force-pushed the feature/cdc_acm_merge_open branch 5 times, most recently from 1fdc564 to 0dced5a Compare September 30, 2024 11:30
@tore-espressif tore-espressif self-assigned this Sep 30, 2024
@tore-espressif tore-espressif marked this pull request as ready for review September 30, 2024 13:12
@tore-espressif tore-espressif force-pushed the feature/cdc_acm_merge_open branch 2 times, most recently from a5d547c to 7d93201 Compare October 1, 2024 13:01
Copy link
Collaborator

@roma-jam roma-jam left a comment

Choose a reason for hiding this comment

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

Built locally, launched and got the line:
All tests passed (520 assertions in 11 test cases)
Cool, that we could use linux tests now!

Overall LGTM, just one basic idea about the naming. Check the notes. feel free to use the suggested names if you like them better.

Name of the file cdc_host_parsing.c seems not clear to me. Hard to say, what is inside the file. Maybe something as cdc_acm_descriptor_parsing.c or cdc_acm_host_descriptor.c would be better.

CDC ACM descriptor parsing helper calls notes left before.

host/class/cdc/usb_host_cdc_acm/cdc_host_parsing.c Outdated Show resolved Hide resolved
host/class/cdc/usb_host_cdc_acm/cdc_host_parsing.c Outdated Show resolved Hide resolved
host/class/cdc/usb_host_cdc_acm/cdc_host_parsing.c Outdated Show resolved Hide resolved
host/class/cdc/usb_host_cdc_acm/cdc_host_parsing.c Outdated Show resolved Hide resolved
@tore-espressif
Copy link
Collaborator Author

Thank you for the review @roma-jam , I'll address the comments ASAP

@tore-espressif tore-espressif merged commit 9be78b9 into master Oct 2, 2024
17 checks passed
@tore-espressif tore-espressif deleted the feature/cdc_acm_merge_open branch October 3, 2024 08:38
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.

2 participants