Pluggable discovery: search in platform.txt (WIP)

[cmaglie]

/cc @PaulStoffregen
this is very WIP, but should be a good starting point.

This PR changes the DiscoveryManager so it search for custom discovery in the platform.txt as explained in #7942 (comment), you can try it by editing an existing platform.txt.

The DiscoveryManager then runs a PluggableDiscovery that is a stub for now but, once implemented, it should start the custom discovery tool and handle all the communication with it. If you would like to try to implement this it would be great, I'll be 100% on IDE and CLI next week (so hopefully we should make good progress on this).

[PaulStoffregen]

Thanks. I will work on a PluggableDiscovery implementation over the weekend.

How to push commits to this pull request is probably well beyond my limited knowledge of git & github, but I can certainly work on the Java code and update the SerailDiscovery tool.

[facchinm]

@ArduinoBot build this please

DNS problems on our side, no problem with the PR 😉

[PaulStoffregen]

I started an implementation of PluggableDiscovery.java. Here is the code so far:

https://github.com/PaulStoffregen/Arduino/blob/acb22b5d644ca44eca5e3a540c88d7fd68de8cfe/arduino-core/src/cc/arduino/packages/discoverers/PluggableDiscovery.java

Main question: Should I create a new class for the JSON parsing, rather than using BoardPort? And add code to translate it to BoardPort for use by DiscoveryManager? Or is the idea to change BoardPort to match the desired format?

Here's a quick screenshot with PluggableDiscovery.java using SerialDiscovery_JSON:

[PaulStoffregen]

@cmaglie - Should I add another class to use as the JSON format?

[cmaglie]

Not sure if this is the best way: we can call "LIST" just once directly in listDiscoveredBoards() (basically it will poll only when the Tools menu is triggered and not every 2.5 sec). On the other hand this may add some delay on opening the menu unless there is some internal cache in the discovery tool.

[PaulStoffregen]

We already do have the internal cache as the portList of discovered ports in PluggableDiscovery. listDiscoveredBoards() just returns a copy of the list. At least that part is easy.

To make this work, I believe DiscoverManager would need to give all discoverer instances an early notice that listDiscoveredBoards() may soon be called. Maybe this could be done by detecting mouse hover over the Tools menu? Or maybe mouse-over any part of the menus?

Early versions of Teensy's software waited in listDiscoveredBoards(). The result was usually ok, but some Windows & Mac users had a terrible experience with the GUI becoming unresponsive. I believe it's best to always quickly return the list of previously discovered ports.

[cmaglie]

Very nice!

Should I add another class to use as the JSON format?

you mean to allow passing some more metadata?
yes, something like PluggableDiscoveryMessage should do I think.

[cmaglie]

new PreferencesMap(bp.prefs) should do the job

[PaulStoffregen]

you mean to allow passing some more metadata?
yes, something like PluggableDiscoveryMessage should do I think.

Two issues.

1: Yes, metadata for the event type "add", "remove", "update" (or just "add" again with a label/address already used), "error" (currently only START_SYNC not supported). But this could be also be done easily with just 1 extra variable in BoardPort.

2: Currently BoardPort is nothing like the JSON format you described. Adding another class could give the JSON format you want without changes to BoardPort, but extra code & complexity. Using only BoardPort is simpler, but BoardPort is widely used by the rest of the IDE, so many edits needed for any changes to BoardPort.

So far, I have not tried to change BoardPort, other than adding the copy constructor.

[cmaglie]

At the moment BoardPort is defined as:

  private String address;
  private String protocol;
  private String boardName;
  private String vid;
  private String pid;
  private String iserial;
  private String label;
  private final PreferencesMap prefs;
  private boolean online;

I'd like to change it and move "fixed" fields like vid and pid inside prefs (and change where they are used with the corresponding map get).
For the online field I'm unsure I need to see where it is used and if it can be removed.
In the end the BoardPort should be something like:

  private String protocol;
  private String address;
  private String label;
  private final PreferencesMap prefs; // should be really called `props`, but let's keep it as is for now
  private final PreferencesMap identificationPrefs;
  // private boolean online; // Maybe?

If we reach this point we can change also the board detector so it can match the board model using the identificationPrefs and the boards.txt content.

Does the Teensy hid use some identifiers similar to vid and pid?

[PaulStoffregen]

I'd like to change it and move "fixed" fields like vid and pid inside prefs (and change where they are used with the corresponding map get).

I'll start working on this. Do you want it to be on this pull request?

If we reach this point we can change also the board detector so it can match the board model using the identificationPrefs and the boards.txt content.

I do not have a clear understanding how you want identificationPrefs to work. My guess is you're imagining adding a separate class or package for board detection? Maybe you're thinking identificationPrefs info would be the input to this board detection class?

So far, I have understood the board detection process happens within each discoverer instance. Both SerialDiscovery and NetworkDiscovery have this detection process built in. So I have imagined any board detection logic for PluggableDiscovery would occur within the external discovery tool and be communicated to the PluggableDiscovery instance as JSON fields.

I'm imagining identificationPrefs would be the output of board detection logic. Rather than leaving the rest of the IDE to struggle with how VID, PID, HID descriptors, bluetooth info and other arbitrary info matches, all that info-to-board logic would be inside each discoverer. Then identificationPrefs would have fields the rest of the IDE could readily use without guesswork, like the board name.

Before I do any work with identificationPrefs, I need to clearly understand how you want it to work. Where the properties-to-identification logic will exist is the critical decision.

[PaulStoffregen]

In the end the BoardPort should be something like: ...

I added a commit to move closer to this goal. Moving vid, pid, serial number into prefs is easy, because everything uses the getter & setting functions. I've updated SerialDiscovery_JSON. Now it looks like this:

{
  "eventType": "add",
  "address": "_/dev/ttyUSB0",
  "label": "/dev/ttyUSB0 (FT232R USB UART)",
  "boardName": "FT232R USB UART",
  "prefs": {
    "vendorId": "0403",
    "productId": "6001",
    "serialNumber": "A800I6T1"
  },
  "protocol": "Serial Device"
}
{
  "eventType": "add",
  "address": "_/dev/ttyACM0",
  "label": "/dev/ttyACM0 (USB Serial)",
  "boardName": "USB Serial",
  "prefs": {
    "vendorId": "16c0",
    "productId": "0483",
    "serialNumber": "3990740"
  },
  "protocol": "Serial Device"
}

PluggableDiscovery is no longer using the "online" boolean. But it seems to be used by SerialBoardsLister. I don't fully understand it, but the use appears to be related to making a port inaccessible during upload, and perhaps handling the case where the same board reappears as a different port shortly after the upload completes. Pretty sure this can't be removed from BoardPort without breaking stuff.

Not sure what to do with boardName. Need your opinion on how you want identificationPrefs to work and where you envision the properties-to-board logic implemented. My hope for PluggableDiscovery would be to put that logic into the discovery tool and send the board info as JSON fields. But if you're planning to add another package or some other layer to do that stuff, need to understand how you want it to work.

[cmaglie]

I do not have a clear understanding how you want identificationPrefs to work. My guess is you're imagining adding a separate class or package for board detection? Maybe you're thinking identificationPrefs info would be the input to this board detection class?

exactly, let me expand a bit on this one, the idea is really simple:

• each board should have the discovery information written in the boards.txt as generic properties, for example the Arduino Uno has:

uno.name=Arduino/Genuino Uno
uno.vid.0=0x2341
uno.pid.0=0x0043
uno.vid.1=0x2341
uno.pid.1=0x0001
uno.vid.2=0x2A03
uno.pid.2=0x0043
uno.vid.3=0x2341
uno.pid.3=0x0243

• each discovery will report the discovered port with all the identification information, for example (slightly changing the output of your Serial Discovery) a report may be:

{
  "eventType": "add",
  "address": "_/dev/ttyACM0",
  "label": "/dev/ttyACM0 (USB Serial)",
  "boardName": "USB Serial",
  "prefs": {
    "vendorId": "0x2341",
    "productId": "0x0043",
    "serialNumber": "3990740"
  },
  "identificationPrefs": {
    "vid": "0x2341",
    "pid": "0x0043"
  }
  "protocol": "Serial Device"
}

• the hypothetical BoardPortMatcher will scan all boards.txt searching for the board that match the identificationPrefs. In other words, looking at the example above, the matcher will search for a board that has the propertes vid.x and pid.x set respectively to 0x2341 and 0x0043 (in this case the Arduino Uno will be matched).

• this example has vid and pid but it really may be anything... macaddressprefix, mdsntype... oranges and apple :-) it just needs the discovery to provide these info and the matcher will do the rest.

So far, I have understood the board detection process happens within each discoverer instance. Both SerialDiscovery and NetworkDiscovery have this detection process built in. So I have imagined any board detection logic for PluggableDiscovery would occur within the external discovery tool and be communicated to the PluggableDiscovery instance as JSON fields.

Doing this will not scale well, for example the discovery will not match 3rd party boards (for example the serial discovery will match only Arduino boards because they are the only one known to him), unless we find a way to pass the content of each installed boards.txt to the discovery tool, but this seems to complicate things IMHO.

[PaulStoffregen]

Oh, ok, I believe I understand now. Will try to add this later today.

[PaulStoffregen]

I added identificationPrefs and a function which searches for a board which matches all the identificationPrefs fields. Hopefully this is close to what you were thinking?

So far, it's only actually being used within PluggableDiscovery to set the board name.

[PaulStoffregen]

@cmaglie - Is there anything more you wanted to see done here? Other than when to send "LIST", not sure what more I can do here?

Maybe getting close to being able to merge this?

[cmaglie]

The algorithm is correct, but I'd like to move it away from the BoardPort class.
BTW I'm on it now, will report back soon.

[cmaglie]

The SerialDiscovery_JSON repo is updated with the version you used to test?

[PaulStoffregen]

Yes, SerialDiscovery_JSON is up to date. The last change added identificationPrefs, so if you see that in the JSON output you can be sure you've got the latest.

[ArduinoBot]

✅ Build completed.

Please test this code using one of the following:

⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8038-BUILD-821-linux32.tar.xz
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8038-BUILD-821-linux64.tar.xz
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8038-BUILD-821-windows.zip
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8038-BUILD-821-macosx.zip
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8038-BUILD-821-linuxarm.tar.xz

ℹ️ The linuxarm build is still experimental and may not be always available.

[PaulStoffregen]

Trying to work with Pluggable Discovery in just-released 1.8.9. Quick question about how I'm supposed to write the pattern in platform.txt.

What I want to do is something like this:

discovery.teensy.pattern="{runtime.hardware.path}/tools/teensy_ports" -J2

The problem is {runtime.hardware.path} doesn't get replaced with the pathname to the files I've installed. I believe there are 2 issues. First, discoverers are started very early, before the runtime.hardware.path pref exists. Second, even if it did exist, this refers to the path for the currently selected board, which wouldn't let my discoverer start unless the user had selects the board and then quits & restarts the IDE.

I was able to make it work with this:

discovery.teensy.pattern="{runtime.ide.path}/hardware/tools/teensy_ports" -J2

But this pattern will only work if my platform is installed inside the IDE, at that specific location.

I did write a patch for DiscoveryManager.java to create a special PreferencesMap with the platforms paths and then do a 2nd StringReplacer.replaceFromMapping() on the pattern. That works great... but patching DiscoveryManager.java can't be the proper answer!

Hoping you can tell me how you intended for the pattern to be written for the path a platform-provided discoverer?

[cmaglie]

The idea is to deliver the discovery as a board-manager tool and use {runtime.tools.TOOLNAME.path} or {runtime.tools.TOOL_NAME-TOOL_VERSION.path} (as explained here and here)

BTW we could allow also {runtime.hardware.path}, the problem of the selected board may be solved by setting the variable to the platform path that is currently scanned.

[PaulStoffregen]

Ah, ok, now I understand. The problem comes because I am just copying my platform stuff into the hardware folder, not using a JSON index file.

[omzlo]

This is awesome. I've been following this new feature with great interest. I've built an IoT platform called NoCAN that identifies nodes with a special unique identifier (instead of a serial port or a ip address). After playing a bit with 1.8.9, I'm able to list all the nodes in the network in the Arduino IDE, with the Pluggable Discovery you have implemented.

I have one question though: once the user selects a port, how to I fetch the port information in my tools.....upload.pattern definition. For example, how do I pass the "address" defined for the selected port to my uploader command?

[PaulStoffregen]

Use {serial.port}. So it should look something like this:

tools.yourtoolname.upload.pattern="{runtime.hardware.path}/programname" "{build.path}/{build.project_name}" "{serial.port}"

It's still called "serial.port" in the prefs, even though we're now able to do any arbitrary protocol and any "address".

[PaulStoffregen]

Unfortunately, there is still no pluggable serial monitor. I have some code for Teensy which hard-codes this, using stdin/stdout pipes for the data, and stderr for status. Thinking about converting it over to use these recipes, and probably JSON instead of my ad-hoc text messages. @cmaglie - maybe that can serve as a starting point for a pluggable serial monitor in 1.8.10?

[omzlo]

Use {serial.port}. So it should look something like this:

tools.yourtoolname.upload.pattern="{runtime.hardware.path}/programname" "{build.path}/{build.project_name}" "{serial.port}"

It's still called "serial.port" in the prefs, even though we're now able to do any arbitrary protocol and any "address".

@PaulStoffregen This is perfect thanks! I just tried it and it works in my proof of concept.

[omzlo]

Hi. My POC works well with the START_SYNC mechanism, however it failed with the fallback LIST mechanism.
After looking at the code, I believe that line 146 in PluggableDiscovery.java

        BoardPort port = mapJsonNodeToBoardPort(mapper, node);

shoud be changed to

        BoardPort port = mapJsonNodeToBoardPort(mapper, portNode);

This made things work in my setup.
I hope this helps.

[facchinm]

@cmaglie what do you think about this 🔝 ?

[omzlo]

I've been trying to release a package that takes advantage of the new discovery features of the Arduino IDE.
My tool is called "nocanc". I run into the following issue.

The following fails in platform.txt:

discovery.nocanc.path={runtime.tools.nocanc.path}
discovery.nocanc.cmd=nocanc
discovery.nocanc.cmd.windows=nocanc.exe
discovery.nocanc.pattern="{path}/{cmd}" arduino-discovery

However, the following works (at least on Mac OSX):

discovery.nocanc.cmd=nocanc
discovery.nocanc.cmd.windows=nocanc.exe
discovery.nocanc.pattern="{runtime.tools.nocanc.path}/{cmd}" arduino-discovery

The reason seems to be that variable substitution using PreferencesData only happens on the pattern and not on the other discovery.* fields, as the code in DiscoveryManager suggests:

  String pattern = discoveryPrefs.get("pattern");
  ...
  pattern = StringReplacer.replaceFromMapping(pattern, PreferencesData.getMap());`

I believe that this will cause further issues on Windows where the cmd.windows substitution won't happen as well.
This could explain some problems previously reported by @PaulStoffregen above.

[facchinm]

@omzlo the differentiation between windows and unix, as far as the filename is the same (without the extension), is not needed anymore (at least from IDE 1.6.something).
However, I understand your point; the discoverer should try to resolve exactly as the normal platform.txt parser