machine/usb: set the vid and pid to valid values supplied by Adafruit and Arduino #884

deadprogram · 2020-01-28T22:35:01Z

The PR sets the vid and pid to valid values supplied by Adafruit and Arduino for boards that support USB CDC.

It might be able to help Windows 10 users get their devices to auto-mount as USB CDC without special drivers. cc/ @torntrousers

syoder · 2020-01-29T03:18:03Z

Another thing I discovered that might be tripping up windows is that I think we are sending the string descriptors incorrectly. See here: https://github.com/tinygo-org/tinygo/blob/master/src/machine/machine_atsamd51.go#L1876-L1887

There are several problems:

the header bytes (length and descriptor type) are being overwritten with the first character of the string
the ascii string is being converted to UTF-16BE, when it should be converted to UTF-16LE
at least on macOS, the USB host requests just the header (wLength == 2), and then afterwards requests the whole thing, otherwise it throws an error (but still allows it to connect)

deadprogram · 2020-01-29T06:44:36Z

Hi @syoder I think you are totally correct about errors in that descriptor. I was looking in to that lasts night a little, but did not get quite as far as identifying specifics as you did.

I am going to try to correct a couple of those things today and see where that gets me.

torntrousers · 2020-01-29T10:23:16Z

src/machine/board_feather-m0.go

+
+// USB CDC identifiers
+const (
+	usb_STRING_PRODUCT      = "Feather M0 Express"


Should this be "Feather M0 Express" or just "Feather M0"? I'm not sure what the difference is but there are separate board definitions in the Arduino IDE for each and the board I have is just the M0 Feather not the express board.

They have the same vendor ID (VID) and product ID (PID), so the description does not need to match. In any case, the initial descriptor 0 does not seem to be getting sent correctly.

@torntrousers FYI the difference between "express" and non-"express" boards is the addition of extra SPI flash (for circuitpython)

aykevl · 2020-01-31T09:58:44Z

What is the state of this PR? Does it fix the problem on Windows?

It looks good to me but I haven't tested it.

deadprogram · 2020-02-09T12:11:13Z

We should merge in #883 first, and then I will rebase/cleanup this PR. Also I will try to correct the other errors in the sending of the descriptors.

sago35 · 2020-03-04T12:26:06Z

What is the state of this PR? Does it fix the problem on Windows?

I fixed usb-cdc on windows 10.

I'm not sure if this fix is correct.
However, it has been confirmed that it works on Windows 10.

I have confirmed that when I connect pyportal / trinket-m0 on Windows 10, it is recognized as a USB serial device.
Also, it was confirmed that serial communication was possible with the test code described below.

However, I have only tested it on Windows 10, so I'm not sure it works on mac and linux.
Could you test it out?

sago35@d13764b

test code
https://gist.github.com/sago35/785bbfa745d4734e39e8185c1d58279d

I think that URB_FUNCTION_GET_MS_FEATURE_DESCRIPTOR in the figure below was bad, but I don't know.

sago35 · 2020-03-16T15:30:57Z

Reduced call conditions for sendZlp().
And rebased to the current dev branch.

https://github.com/sago35/tinygo/tree/feature/usb-cdc-windows
- sago35@f80cc33

deadprogram · 2020-03-16T19:50:00Z

Made several updates to this branch to correct issues pointed out by @syoder and @sago35 although my fixes are slightly different.

I addressed the byte ordering and corrected skipping the header bytes when creating the descriptor strings. I also removed the case that an extra zero length packet was being sent in error along with the valid descriptor string

I did change the descriptor class being returned with the full descriptor, but I changed it to 0x02 as this is the correct base class for CDC https://www.usb.org/defined-class-codes#anchor_BaseClass02h

I have tested these changes on both Linux and Windows 10, using a Circuit Playground Express, a ItsyBitsy-M4, and a Circuit Playground Bluefruit. All performed as expected using the example/serial program.

The ATSAMD21 and ATSAMD51 boards also worked using the example/echo program. The Circuit Playground Bluefruit board did not work correctly with the example/echo program, in fact causing it to become unresponsive, however I think that issue is not related to this PR, and I will enter an issue to look into it separately.

That said, I think this PR is now ready for merge.

UPDATE: also tested all three boards on macOS and also worked.

aykevl · 2020-03-16T22:20:57Z

The Circuit Playground Bluefruit board did not work correctly with the example/echo program, in fact causing it to become unresponsive, however I think that issue is not related to this PR, and I will enter an issue to look into it separately.

Is that a regression or was the problem already present?

deadprogram · 2020-03-16T22:26:04Z

That problem is already present in the dev branch.

aykevl

You sometimes prefix the device name with the manufacturer name (e.g. "Adafruit Circuit Playground Bluefruit") and sometimes you don't (e.g. "Circuit Playground Express"). Was that intentional?

That problem is already present in the dev branch.

Ok, then it's fine.

src/machine/board_particle_argon.go

aykevl · 2020-03-17T08:22:48Z

src/machine/board_pca10056.go

+// USB CDC identifiers
+const (
+	usb_STRING_PRODUCT      = "Nordic nRF52840DK (PCA10056)"
+	usb_STRING_MANUFACTURER = "Nordic Semiconductor"
+)
+
+var (
+	usb_VID uint16 = 0x239A
+	usb_PID uint16 = 0x8029
+)


Is there a reason you're re-using the VID from Adafruit?

It looks like the nrf52*-based boards do not use these usb_STRING_PRODUCT etc constants yet.

see https://github.com/adafruit/Adafruit_nRF52_Arduino/blob/master/boards.txt#L419

That is true, I will add another commit that uses the constant.

Actually the nrf52840 boards all do use the constants. See https://github.com/tinygo-org/tinygo/blob/feature/usb-cdc-ids/src/machine/usb_nrf52840.go#L432 and https://github.com/tinygo-org/tinygo/blob/feature/usb-cdc-ids/src/machine/usb_nrf52840.go#L440

Oh okay, then it's fine.

… and Arduino for boards that support USB CDC Signed-off-by: Ron Evans <ron@hybridgroup.com>

… CDC device and string descriptors Signed-off-by: Ron Evans <ron@hybridgroup.com>

…boards Signed-off-by: Ron Evans <ron@hybridgroup.com>

…ptor for nrf52840 based boards Signed-off-by: Ron Evans <ron@hybridgroup.com>

aykevl

LGTM, I'm only wondering about the inconsistency in product naming as mentioned previously?

deadprogram · 2020-03-17T10:03:28Z

Let me modify the product names that are inconsistent, then.

…ptors consistent Signed-off-by: Ron Evans <ron@hybridgroup.com>

deadprogram · 2020-03-17T10:17:09Z

OK, added another commit for name consistency.

sago35 · 2020-03-17T14:28:38Z

@deadprogram

I tried feature/usb-cdc-ids in example/echo but it didn't work.
I'm just experimenting with swapping the source for tinygo-dev and the environment may be bad.
Please give uf2 made by tinygo build -o app.uf2 -target pyportal .
I want to check the operation with my environment (Windows10 Pro 64bit + pyportal).

# docker : tinygo/tinygo-dev
# example/echo

pushd /tinygo
git remote add gg https://github.com/tinygo-org/tinygo
git remote add gg https://github.com/tinygo-org/tinygo
git fetch gg
git checkout gg/feature/usb-cdc-ids
popd
tinygo build -o app.uf2 -target pyportal .

deadprogram · 2020-03-17T19:33:22Z

Hello @sago35 you are correct that the echo example does not yet work on Windows. But it at least solves the original problem, which is to get the correct name, and the board driver+USB Serial port drivers to be installed.

The problem whatever it might be with Windows and reading data off of the USB CDC from the device will have to be solved in another PR. This is at least progress.

torntrousers reviewed Jan 29, 2020

View reviewed changes

syoder mentioned this pull request Feb 5, 2020

nrf52840: implement USB-CDC #883

Merged

suda mentioned this pull request Feb 18, 2020

Add support for Particle Argon, Boron and Xenon boards #904

Merged

deadprogram force-pushed the feature/usb-cdc-ids branch 2 times, most recently from 2f1d7d4 to 40584c5 Compare March 16, 2020 19:33

aykevl reviewed Mar 17, 2020

View reviewed changes

deadprogram added 4 commits March 17, 2020 09:52

machine/usb: set the vid and pid to valid values supplied by Adafruit…

f7ea2dd

… and Arduino for boards that support USB CDC Signed-off-by: Ron Evans <ron@hybridgroup.com>

machine/atsamd21 and atsamd51: corrections needed to return valid UCB…

a7e89fc

… CDC device and string descriptors Signed-off-by: Ron Evans <ron@hybridgroup.com>

machine/nrf52840: add correct USB VID and PID for all nrf52840 based …

25d5ca3

…boards Signed-off-by: Ron Evans <ron@hybridgroup.com>

machine/nrf52840: use correct USB device descriptor and string descri…

e47192e

…ptor for nrf52840 based boards Signed-off-by: Ron Evans <ron@hybridgroup.com>

aykevl approved these changes Mar 17, 2020

View reviewed changes

machine/adafruit: make all USB vendor and product names string descri…

24e68b1

…ptors consistent Signed-off-by: Ron Evans <ron@hybridgroup.com>

deadprogram force-pushed the feature/usb-cdc-ids branch from 40584c5 to 24e68b1 Compare March 17, 2020 10:15

aykevl merged commit fa86108 into dev Mar 18, 2020

aykevl deleted the feature/usb-cdc-ids branch March 18, 2020 11:08

sago35 mentioned this pull request Mar 31, 2020

machine/atsamd21,atsamd51,nrf52840: correct USB CDC #1007

Merged

machine/usb: set the vid and pid to valid values supplied by Adafruit and Arduino #884

machine/usb: set the vid and pid to valid values supplied by Adafruit and Arduino #884

Uh oh!

Conversation

deadprogram commented Jan 28, 2020

Uh oh!

syoder commented Jan 29, 2020

Uh oh!

deadprogram commented Jan 29, 2020

Uh oh!

torntrousers Jan 29, 2020

Choose a reason for hiding this comment

Uh oh!

deadprogram Jan 29, 2020

Choose a reason for hiding this comment

Uh oh!

ardnew Mar 17, 2020

Choose a reason for hiding this comment

Uh oh!

aykevl commented Jan 31, 2020

Uh oh!

deadprogram commented Feb 9, 2020

Uh oh!

sago35 commented Mar 4, 2020

Uh oh!

sago35 commented Mar 16, 2020

Uh oh!

deadprogram commented Mar 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aykevl commented Mar 16, 2020

Uh oh!

deadprogram commented Mar 16, 2020

Uh oh!

aykevl left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aykevl Mar 17, 2020

Choose a reason for hiding this comment

Uh oh!

deadprogram Mar 17, 2020

Choose a reason for hiding this comment

Uh oh!

deadprogram Mar 17, 2020

Choose a reason for hiding this comment

Uh oh!

aykevl Mar 17, 2020

Choose a reason for hiding this comment

Uh oh!

aykevl left a comment

Choose a reason for hiding this comment

Uh oh!

deadprogram commented Mar 17, 2020

Uh oh!

deadprogram commented Mar 17, 2020

Uh oh!

sago35 commented Mar 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deadprogram commented Mar 17, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

deadprogram commented Mar 16, 2020 •

edited

Loading

aykevl left a comment •

edited

Loading

sago35 commented Mar 17, 2020 •

edited

Loading