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

Fix PID specification for arduino-cli #2157

Merged
merged 1 commit into from May 13, 2024

Conversation

egnor
Copy link
Contributor

@egnor egnor commented May 13, 2024

When using pluggable discovery, the arduino-cli tool expects boards.txt files to list device identification properties like USB VID and PID in boardname.upload_port.N.{vid,pid,...} properties.

There is compatibility logic to convert old style boardname.N.{vid,pid} properties into that format, however it only runs if the platform is not "pluggable discovery aware", i.e. platform.txt contains any pluggable_discovery.* properties, and this platform does include such properties, so the compatibility logic doesn't run. That means that before this change, arduino-cli wouldn't actually identify USB-connected RP2040 boards with their board type.

This change adds boardname.upload_port.N.{vid,pid} properties, while also keeping classic boardname.N.{vid,pid} properties for compatibility with older Arduino IDE versions.

Note, as a side effect of refactoring the VID/PID code slightly, boards.txt no longer includes duplicate VID/PID pairs when the PID-mangling-for-compound-devices logic OR's in bits that were already present.

@earlephilhower
Copy link
Owner

Thanks, but doesn't this end up breaking the 1.x Arduino branch? I didn't think it uses the arduino-cli and there's still a pretty good amount of folks stuck on it (the VSCode one is really heavyweight, even if it is much nicer to use).

@egnor
Copy link
Contributor Author

egnor commented May 13, 2024

The "classic" properties (boardname.#.{vid,pid}) are still present as they ever were, so nothing should break. Are you concerned that the extra properties will confuse or break older Arduino IDEs?

@earlephilhower
Copy link
Owner

Looks like some of the properties are dropped in the diff (sorry for the PNG, can't find good way to link to diff). For example, the RPIPicoW USB IDs stop at .1. now, not all .7.:
image

@egnor
Copy link
Contributor Author

egnor commented May 13, 2024

Oh right, I should have mentioned that. Since I was touching that code, I made it drop duplicate VID/PID pairs -- since the whole PID mangling for compound devices thing OR's in bits, sometimes there's no change to the PID. Previously lots of redundant VID/PID pairs would be added, now it only adds them if they're distinct. In that example, there were only ever the two pairs (0x2e8a/0xf00a and 0x2e8a/0xf10a) but they were repeated multiple times.

The PID-mangling solution should probably be revisited, but I didn't want to change that logic in this PR. Listing duplicate VID/PID pairs just seemed silly though.

(Updated the main PR description to mention this.)

@earlephilhower
Copy link
Owner

Ah, gotcha. That does make sense.

I should have just xor'd the bits to make new IDs and avoided the whole issue (or at least minimized it...still no guarantee of non-repeated IDs between boards). That's a change to makeboards and the rp2040usbsupport file to implement and should be separate from this, though.

@earlephilhower earlephilhower merged commit 819a73b into earlephilhower:master May 13, 2024
13 checks passed
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.

None yet

2 participants