Skip to content

CDCACM: Add interface association descriptor before CDC ACM control interface descriptor#12430

Merged
dylad merged 1 commit intoRIOT-OS:masterfrom
bergzand:wip/cdcacm/assoc
Oct 20, 2019
Merged

CDCACM: Add interface association descriptor before CDC ACM control interface descriptor#12430
dylad merged 1 commit intoRIOT-OS:masterfrom
bergzand:wip/cdcacm/assoc

Conversation

@bergzand
Copy link
Copy Markdown
Member

@bergzand bergzand commented Oct 11, 2019

Contribution description

This adds an interface association descriptor to the USBUS CDC ACM code. This should enable the Windows operating system (at least version 10) to correctly enumerate, detect and use the CDC ACM functionality in RIOT.

Background

Currently RIOT describes itself as a composite device to host computers. When this is used, WIndows uses interface association descriptors to detect logical interface collections on the device1.

Testing procedure

Flash the tests/usbus_cdc_acm_stdio test on a board, Windows should correctly detect the COM device and allow usage of said COM device.

More extensive test (as already done by @chrysn) would be to add the CDC ECM in different configurations and test that Windows does not enable the CDC ECM functionality, but does offer the COM functionality of the device.

(Linux and MacOS should still work just fine)

Issues/PRs references

Depends on #12429

@bergzand bergzand added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: USB Area: Universal Serial Bus labels Oct 11, 2019
@bergzand bergzand requested review from chrysn and dylad October 11, 2019 18:39
@bergzand bergzand changed the title CDCACM: Add interface association descriptor before CDC ACM control interface CDCACM: Add interface association descriptor before CDC ACM control interface descriptor Oct 14, 2019
@bergzand
Copy link
Copy Markdown
Member Author

rebased, no longer waiting for other PR's

@dylad dylad added this to the Release 2020.01 milestone Oct 20, 2019
@dylad dylad added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Oct 20, 2019
Copy link
Copy Markdown
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

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

Tested this PR on Linux and W10. cdc/acm on Linux works as expected and it also works out of the box with W10. Enumeration / serial port works perfectly !

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

dylad commented Oct 20, 2019

Here we go

@dylad dylad merged commit 2b98b12 into RIOT-OS:master Oct 20, 2019
@bergzand bergzand deleted the wip/cdcacm/assoc branch October 21, 2019 18:26
@bergzand
Copy link
Copy Markdown
Member Author

This broke support for me on Linux due to a claimed header length mismatch. Will open a PR to fix this asap. Not sure how this slipped through on bot my test and @dylad's test

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 Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants