search

earlephilhower / arduino-pico

Raspberry Pi Pico Arduino core, for all RP2040 boards
GNU Lesser General Public License v2.1
1.92k stars 402 forks source link

Fix PID specification for arduino-cli #2157

Closed egnor closed 3 months ago

egnor commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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.