Skip to content

cdcacm: Activate data out endpoint on DTE present signal#12536

Merged
benpicco merged 2 commits intoRIOT-OS:masterfrom
bergzand:pr/usbus/cdcacm_activate_on_dte_present
Oct 29, 2019
Merged

cdcacm: Activate data out endpoint on DTE present signal#12536
benpicco merged 2 commits intoRIOT-OS:masterfrom
bergzand:pr/usbus/cdcacm_activate_on_dte_present

Conversation

@bergzand
Copy link
Copy Markdown
Member

Contribution description

This PR modifies the CDC ACM code to signal the data out endpoint to be ready when the host has indicated that an application opened the serial console, DTE present in CDC ACM terminology.

This mainly prevents the code from calling the ready function when no USB host is present and at least after the low level endpoint is initialized by usbus. The downside is that the cdcacm struct requires a reference to the endpoint.

Testing procedure

Verify that tests/usbus_cdc_acm is still working and that RIOT shell commands still work over the CDC ACM function.

Issues/PRs references

Testing should be a bit easier with #12535 included

@bergzand bergzand added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: USB Area: Universal Serial Bus labels Oct 21, 2019
@bergzand bergzand requested review from benpicco and dylad October 21, 2019 18:42
@dylad
Copy link
Copy Markdown
Member

dylad commented Oct 22, 2019

The downside is that the cdcacm struct requires a reference to the endpoint.

Why add a reference to the out endpoint ? I thought this endpoint could be accessible through usbus_interface_t field ?

@dylad dylad added this to the Release 2020.01 milestone Oct 22, 2019
@bergzand
Copy link
Copy Markdown
Member Author

bergzand commented Oct 22, 2019

Why add a reference to the out endpoint ? I thought this endpoint could be accessible through usbus_interface_t field ?

Good point, however, I think I need a proper getter for that first. Something along the lines of

usbus_endpoint_t *usbus_interface_find_endpoint(usb_ep_type_t type, usb_ep_dir_t dir);

Which would return a reference to the first endpoint matching the spec. This way we don't depend too much on the order the endpoints are added to the interface.

@dylad
Copy link
Copy Markdown
Member

dylad commented Oct 23, 2019

I think I need a proper getter for that first

I think this is the way to go, this is more generic and this may be reuse in another implementation later.

@bergzand bergzand force-pushed the pr/usbus/cdcacm_activate_on_dte_present branch from 0a55de9 to 1d5d1cb Compare October 23, 2019 08:10
@bergzand
Copy link
Copy Markdown
Member Author

rebased to include the descriptor bug fix from #12535

@bergzand bergzand force-pushed the pr/usbus/cdcacm_activate_on_dte_present branch from 1d5d1cb to e2ce187 Compare October 23, 2019 08:11
@bergzand
Copy link
Copy Markdown
Member Author

And reordered the commits to keep stuff properly incremental

@benpicco
Copy link
Copy Markdown
Contributor

This looks good, please squash.

@bergzand bergzand force-pushed the pr/usbus/cdcacm_activate_on_dte_present branch from e2ce187 to 1d7032a Compare October 29, 2019 07:45
@bergzand
Copy link
Copy Markdown
Member Author

squashed!

@bergzand bergzand added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 29, 2019
Copy link
Copy Markdown
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Code looks good, tested successfully on same54-xpro and Linux

@benpicco benpicco merged commit acaaee9 into RIOT-OS:master Oct 29, 2019
@bergzand bergzand deleted the pr/usbus/cdcacm_activate_on_dte_present branch June 5, 2020 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: USB Area: Universal Serial Bus CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants