builtin discoveries active when custom discovery selected

[ben-qnimble]

Bug Report

All pluggable discoveries for all installed platforms are active regardless of active platform. Instead, only discoveries related to active platform should be active. I believe the intent is for each platform to specific its discoveries and only those to be active. Quoting from platform-specification:

For backward compatibility, if a platform does not declare any discovery (using the pluggable_discovery.* properties in platform.txt) it will automatically inherit builtin:serial-discovery and builtin:mdns-discovery (but not other builtin discoveries that may be possibly added in the future).

Currently, builtin:serial-discovery and builtin:mdns-discovery always installed and actively, regardless of if any pluggable_discovery.* properties are set.

Current behavior

configure platformA with

pluggable_discovery.required=discoveryA

and configure platformB with

pluggable_discovery.required=discoveryB

Then the following discoveries are active, regardless of whether platformA or platformB is active:

• builtin:serial-discovery
• builtin:mdns-discovery
• platformA:discoveryA
• platformB:discoveryB

Expected behavior

If platformA is active, only platformA:discoveryA should be active and vice versa.

Environment

Tested with d458040

Additional context

I think there needs to be a method to control the active discoveries separately from loading them. Currently, loadDiscovery (arduino/cores/packagemanager/loader.go) installs all the discovery, but it also actives them with pm.discoveryManager.Add(d) at the end of the code. I believe this should be removed and when setting up a new platform, only the discoveries in the platform should be added.

[silvanocerza]

What do you mean with active platform?

[ben-qnimble]

In the context of the Arduino-IDE, I mean the platform of the selected board (platform.txt for active board). Really, I think this is (mostly) an issue with the Arduino IDE, but the Arduino-cli has the information necessary for fixing this.

For the Arduino-IDE, when you select a board, the discovery configuration that board should drive what ports are listed. So, if you have a board that communicates via a USB HID interface, mdns and serial ports shouldn't be shown as possible ports to connect to board. If they are selected, they may select protocols that aren't configured in the board's configuration (upload.tool.protocol) so doing an upload will yield an error. Additionally, multiple discoveries can conflict with each other (described in more detail below).

For the Arduino-cli I don't think currently has a way to know the selected board like the IDE does. However if the expected behavior is list all ports from every discovery program that is installed, this is a problem because different discoveries can use the same port. Say a new-discovery uses USB CDC (serial/com port) but does specify use the 'serial' protocol but, say, sam-ba as the protocol. Then COM5 can be claimed by both serial-discovery and new-discovery and the platform configuration cannot disable serial-discovery. Whichever discovery program lists a new port first 'wins' and gets to select the protocol for that COM5. Since the protocol is used to determine how to upload to the device, there will be no way for the user to select uploading to the device via the method that 'lost' the discovery contest. And each time the device is connected and disconnected, maybe a different discovery will 'win' with different upload behavior.

I believe that in Arduino IDE 1.8, there is isolation between different discoveries so if both serial-discovery and new-discovery use COM5, COM5 will be listed twice, once under 'serial' and another under 'new' so the user can select the port they want to use. I think this avoids the biggest problem, but from a user perspective, I think removing ports that cannot be right the active board would be preferable. And it'll save resources if there are many boards installed with numerous discoveries that do not need to be running at the same time.

I wonder if it would make sense for the BoardListWatch daemon command to (optionally?) take a platform argument to use for filtering out the discoveries to run.

[silvanocerza]

For the Arduino-IDE, when you select a board, the discovery configuration that board should drive what ports are listed. So, if you have a board that communicates via a USB HID interface, mdns and serial ports shouldn't be shown as possible ports to connect to board. If they are selected, they may select protocols that aren't configured in the board's configuration (upload.tool.protocol) so doing an upload will yield an error. Additionally, multiple discoveries can conflict with each other (described in more detail below).

The main goal of pluggable discoveries is exactly discovering which board are connected and disconnected in the shortest time possible, if we disable a discovery when a user select a certain board on the IDE we'll have no way of knowing when a new board is connected or disconnected.

The protocol selection might be a concern though, if the same port is found by two different discoveries with a different protocol the user will see two identical ports since as of now we don't show the protocol in the IDE UI.

We still need to enhance a big chunk of the UI so the users can have a better experience when selecting the board they want.

For the Arduino-cli I don't think currently has a way to know the selected board like the IDE does. However if the expected behavior is list all ports from every discovery program that is installed, this is a problem because different discoveries can use the same port. Say a new-discovery uses USB CDC (serial/com port) but does specify use the 'serial' protocol but, say, sam-ba as the protocol. Then COM5 can be claimed by both serial-discovery and new-discovery and the platform configuration cannot disable serial-discovery. Whichever discovery program lists a new port first 'wins' and gets to select the protocol for that COM5. Since the protocol is used to determine how to upload to the device, there will be no way for the user to select uploading to the device via the method that 'lost' the discovery contest. And each time the device is connected and disconnected, maybe a different discovery will 'win' with different upload behavior.

The Arduino CLI doesn't have to know the board selected nor it should, it's not meant to store that state since that information is only necessary when compiling or uploading.

Conflict is not a concern either, if two discoveries find the same port with a different protocol we treat those as two separate ports internally. The only possible conflict is if two discoveries find the same port with identical protocol, but if that happens you probably don't need another discovery to find your board. This is more of a concern for platform developers.

Remember that if you're a platform developer and need the Arduino CLI to recognize your serial or network board you just need to provide the correct identification properties as per documentation, the builtin serial-discovery or mdns-discovery will recognize the board. You don't need a custom pluggable discovery.

If I misunderstood and in fact you're seeing conflicts with multiple discoveries finding the same port with different protocols it would be great if you could provide a way to reproduce it.

I believe that in Arduino IDE 1.8, there is isolation between different discoveries so if both serial-discovery and new-discovery use COM5, COM5 will be listed twice, once under 'serial' and another under 'new' so the user can select the port they want to use. I think this avoids the biggest problem, but from a user perspective, I think removing ports that cannot be right the active board would be preferable. And it'll save resources if there are many boards installed with numerous discoveries that do not need to be running at the same time.

The discoveries in Arduino IDE 1.8 work in a similar way to the new pluggable discovery, all the discoveries are always run whichever board is selected.

I wonder if it would make sense for the BoardListWatch daemon command to (optionally?) take a platform argument to use for filtering out the discoveries to run.

I don't see how this could be useful. Most platforms don't declare and don't need a custom discovery so they use the builtin ones.

E.g. if you pass arduino:avr as platform argument you'll still going to run the builtin discoveries, so you'll receive connection and disconnection events for all serial and network ports. If you want to show the user only a certain list of boards the connected gRPC client should concern itself with that.

Some times we can't recognize at all which board has been connected because it doesn't communicate any property to be identification with certainty, the Arduino Nano is one of those.
Some other times we can't identify reliably a board because there are multiple board with identical identification properties , in those cases we tell the user that we found multiple possible board to connected to a certain port.
This happens if you have installed both Arduino AVR and Arduboy AVR platforms and connected an Arduino Leonardo.

[matthijskooijman]

I haven't followed the state of pluggable discovery implementation, but this issue sounds like something that was also discussed as part of the RFC, where I wondered if builtin discoveries should be used for all boards/platforms, or be explicitly included. I think the explicit inclusion did not make it into the spec in the end, but here's a part of the relevant discussion (I couldn't find the end of this discussion, maybe it was left open):

arduino/tooling-rfcs#2 (comment) point 4
arduino/tooling-rfcs#2 (comment) quote of point 4 and response
arduino/tooling-rfcs#2 (comment) second quote and response to it

As for the issue at hand, I can see two related but separate issues, at least when using the IDE:

1. That cli runs discoveries for other boards/platforms that you are not actually working with (i.e. have not selected in the IDE), wasting resources and potentially confusing a user if they lead to error messages.
2. That the user is offered to select ports for upload that are not actually applicable to the currently selected board.

I'm not sure which (or both) of these issues actually occur in practice, and are the subject of this issue, but it seemed good to make the distinction. Issue 2. is related to the board identification prefs and I think is mostly an issue inside the IDE / UI. Issue 1 needs the cli to be involved, but can probably be solved by making the port listing API sufficiently expressive (AFAIU the cli does not (need to) know what the currently selected board is, but I can imagine that the board listing grpc API could pass some filter parameters to let the cli only run discoveries for a specific (set of) boards or platforms.

[ben-qnimble]

I completely agree with @matthijskooijman's comments. And I think both issues described do happen in practice.

The more immediate problem I described above, where two discoveries conflict with each other if they use list devices on the same port even if they have different protocols, I was wrong that this applies to arduino-cli. The arduino-cli handles this properly where if they have different protocols, they are isolated. The Arduino IDE 2.0 treats them the same, so I'll file a bug report there.

I think it would make sense for an arduino-cli API command to list to ports but only for discoveries associated with a specific platform. Then the Arduino-IDE (or other programs) could decide to list ports that apply for a specific board.

Alternatively, the arduino-cli could return information about the discovery that found the port. Then Arduino-IDE could filter based on this if so desired.

[ben-qnimble]

For reference, the bug report on the IDE is arduino/arduino-ide#711

[silvanocerza]

Since we identified that the problem is in the Arduino IDE I'll close this in favor of arduino/arduino-ide#711.
Thanks for the report @ben-qnimble. 🙏