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

Feature for a platform to add its discoverer to DiscoveryManager #7942

Open
PaulStoffregen opened this Issue Aug 28, 2018 · 14 comments

Comments

3 participants
@PaulStoffregen
Collaborator

PaulStoffregen commented Aug 28, 2018

The IDE should provide a configurable way for platforms to provide their own discoverers to the IDE's DiscoveryManager.

A recipe in platform.txt would allow a platform to specify a shell command to run for its discoverer.

Communication between the shell command and the IDE should use stdin/stdout with text in JSON format. Two fields would be mandatory, "id" for the unique port identifier to be used in the upload recipe, and "label" for the text to be shown in the Ports menu. Optional fields could provide information about the board, or FQBN, or partial QBN, depending on how much information about the hardware is known to the discoverer.

@facchinm facchinm added this to the 1.8.8 milestone Aug 29, 2018

@PaulStoffregen

This comment has been minimized.

Show comment
Hide comment
@PaulStoffregen

PaulStoffregen Sep 7, 2018

Collaborator

I started experimenting with parsing JSON as it arrives from a process's stdout stream. Using Jackson's ObjectMapper with the BoardPort class seems like the most straightforward way. The JSON would be expected to look like this when a board is connected:

{
  "address": "/dev/ttyACM0",
  "online": true,
  "label": "/dev/ttyACM0 (Arduino Uno)"
}

If the board is removed, the JSON would look like this:

{
  "address": "/dev/ttyACM0",
  "online": false
}

I didn't choose any of this... it's the natural result of using ObjectMapper and BoardPort. Hopefully that's ok?

Collaborator

PaulStoffregen commented Sep 7, 2018

I started experimenting with parsing JSON as it arrives from a process's stdout stream. Using Jackson's ObjectMapper with the BoardPort class seems like the most straightforward way. The JSON would be expected to look like this when a board is connected:

{
  "address": "/dev/ttyACM0",
  "online": true,
  "label": "/dev/ttyACM0 (Arduino Uno)"
}

If the board is removed, the JSON would look like this:

{
  "address": "/dev/ttyACM0",
  "online": false
}

I didn't choose any of this... it's the natural result of using ObjectMapper and BoardPort. Hopefully that's ok?

@cmaglie

This comment has been minimized.

Show comment
Hide comment
@cmaglie

cmaglie Sep 8, 2018

Member

(I was just looking at this)

The BoardPort class at the moment is:

public class BoardPort {

  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;

but most of these fields are tied to a specific Discovery (for example vid and pid are specific for SerialDiscovery, instead online, address, protocol are for NetworkDiscovery. This can't work if we want to make it modular and the number of xxxDiscovery and relative field may potentially grow.

IMHO we should leave only "label", "id" and "type/protocol", all the other values should be part of the prefs map.

Member

cmaglie commented Sep 8, 2018

(I was just looking at this)

The BoardPort class at the moment is:

public class BoardPort {

  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;

but most of these fields are tied to a specific Discovery (for example vid and pid are specific for SerialDiscovery, instead online, address, protocol are for NetworkDiscovery. This can't work if we want to make it modular and the number of xxxDiscovery and relative field may potentially grow.

IMHO we should leave only "label", "id" and "type/protocol", all the other values should be part of the prefs map.

@cmaglie

This comment has been minimized.

Show comment
Hide comment
@cmaglie

cmaglie Sep 8, 2018

Member

I started experimenting with parsing JSON as it arrives from a process's stdout stream

Did you have already implemented a CommandLineDiscovery or something like that? would you mind sharing it?

Member

cmaglie commented Sep 8, 2018

I started experimenting with parsing JSON as it arrives from a process's stdout stream

Did you have already implemented a CommandLineDiscovery or something like that? would you mind sharing it?

@PaulStoffregen

This comment has been minimized.

Show comment
Hide comment
@PaulStoffregen

PaulStoffregen Sep 8, 2018

Collaborator

Sure, with the caveat this is early & unfinished, but it does work.

https://github.com/PaulStoffregen/Arduino-1.8.6-Teensyduino/blob/master/arduino-core/src/cc/arduino/packages/discoverers/TeensyDiscovery.java

I also added a copy constructor to BoardPort, which TeensyDiscovery uses to make a copy of its list for the rest of the IDE, so changes to the copies can't corrupt the original within the discovery.

https://github.com/PaulStoffregen/Arduino-1.8.6-Teensyduino/blob/master/arduino-core/src/cc/arduino/packages/BoardPort.java#L50

Collaborator

PaulStoffregen commented Sep 8, 2018

Sure, with the caveat this is early & unfinished, but it does work.

https://github.com/PaulStoffregen/Arduino-1.8.6-Teensyduino/blob/master/arduino-core/src/cc/arduino/packages/discoverers/TeensyDiscovery.java

I also added a copy constructor to BoardPort, which TeensyDiscovery uses to make a copy of its list for the rest of the IDE, so changes to the copies can't corrupt the original within the discovery.

https://github.com/PaulStoffregen/Arduino-1.8.6-Teensyduino/blob/master/arduino-core/src/cc/arduino/packages/BoardPort.java#L50

@PaulStoffregen

This comment has been minimized.

Show comment
Hide comment
@PaulStoffregen

PaulStoffregen Sep 8, 2018

Collaborator

As you can see, the command line is hard-coded and I still have lots of debug printing left over in the code. Very much a work in progress...

Collaborator

PaulStoffregen commented Sep 8, 2018

As you can see, the command line is hard-coded and I still have lots of debug printing left over in the code. Very much a work in progress...

@PaulStoffregen

This comment has been minimized.

Show comment
Hide comment
@PaulStoffregen

PaulStoffregen Sep 8, 2018

Collaborator

I created a simple Linux command line program for testing.

https://github.com/PaulStoffregen/SerialDiscovery_JSON

Just compile and run this in a terminal, and you'll see it print JSON as you plug and unplug USB serial devices. :)

Hopefully this helps?

Collaborator

PaulStoffregen commented Sep 8, 2018

I created a simple Linux command line program for testing.

https://github.com/PaulStoffregen/SerialDiscovery_JSON

Just compile and run this in a terminal, and you'll see it print JSON as you plug and unplug USB serial devices. :)

Hopefully this helps?

@PaulStoffregen

This comment has been minimized.

Show comment
Hide comment
@PaulStoffregen

PaulStoffregen Sep 10, 2018

Collaborator

Regarding the JSON fields and BoardPort variables, I believe we should add a "partially qualified board name" field. If the discoverer knows nothing about what board is actually connected, this would left out of the JSON data and the variable in BoardPort would be null. But if the discover can learn which board is connected, and if it can deduce any of the board's optional menu settings, it would construct as much of the "fully qualified board name" as possible.

I believe the Serial discovery could populate the variable based on which board it found with a matching VID and PID numbers.

In the future, we could make use of this variable to automatically set the correct Board and some of its optional settings when the user chooses a Port. Likewise, when a user make a selection from the Board menu, DiscoveryManager could be queried to find the best matching Port (if any) and automatically select it. While not every case can be resolved, this would go a long way towards easing one of the most common new user frustrations, at least when using genuine boards.

Collaborator

PaulStoffregen commented Sep 10, 2018

Regarding the JSON fields and BoardPort variables, I believe we should add a "partially qualified board name" field. If the discoverer knows nothing about what board is actually connected, this would left out of the JSON data and the variable in BoardPort would be null. But if the discover can learn which board is connected, and if it can deduce any of the board's optional menu settings, it would construct as much of the "fully qualified board name" as possible.

I believe the Serial discovery could populate the variable based on which board it found with a matching VID and PID numbers.

In the future, we could make use of this variable to automatically set the correct Board and some of its optional settings when the user chooses a Port. Likewise, when a user make a selection from the Board menu, DiscoveryManager could be queried to find the best matching Port (if any) and automatically select it. While not every case can be resolved, this would go a long way towards easing one of the most common new user frustrations, at least when using genuine boards.

@PaulStoffregen

This comment has been minimized.

Show comment
Hide comment
@PaulStoffregen

PaulStoffregen Sep 24, 2018

Collaborator

@cmaglie - How can we move this process forward? Have you had a chance to look at the discovery code and try SerialDiscovery_JSON? Is there anything more I can do to work towards a pull request you'd merge?

Collaborator

PaulStoffregen commented Sep 24, 2018

@cmaglie - How can we move this process forward? Have you had a chance to look at the discovery code and try SerialDiscovery_JSON? Is there anything more I can do to work towards a pull request you'd merge?

@PaulStoffregen

This comment has been minimized.

Show comment
Hide comment
@PaulStoffregen

PaulStoffregen Sep 25, 2018

Collaborator

@cmaglie - Can you give me any guidance on how you'd like to see platform discoverers launched? I believe this is probably the main thing missing before I can submit a first pull request. Currently DiscoveryManager.java has a hard-coded list (of only 2, serialDiscoverer & networkDiscoverer) in its constructor. I'm pretty sure you want something more flexible and configurable to detect which platforms provide a JSON-based discoverer. I only need a little direction on how you want to see this done, and I'll try to come up with an initial implementation.

@mastrolinux - Is this moving in a direction you'll be able to use with the CLI? If you can spend just a couple minutes, please compile SerialDiscovery_JSON on any Linux system and run it in a terminal, then plug and unplug some Arduino boards while it's running. You'll see the JSON printed to stdout. I'd imagine you'll need for CLI at least some way to know when the initial enumeration of boards has ended, right?

I know you guys are probably crazy busy. Hopefully the dust has settled from 1.8.7 by now? I'd really like to move forward on this, to the point of a pull request for the IDE and at least a vision that it will also work for CLI. Really need a little input here.

Collaborator

PaulStoffregen commented Sep 25, 2018

@cmaglie - Can you give me any guidance on how you'd like to see platform discoverers launched? I believe this is probably the main thing missing before I can submit a first pull request. Currently DiscoveryManager.java has a hard-coded list (of only 2, serialDiscoverer & networkDiscoverer) in its constructor. I'm pretty sure you want something more flexible and configurable to detect which platforms provide a JSON-based discoverer. I only need a little direction on how you want to see this done, and I'll try to come up with an initial implementation.

@mastrolinux - Is this moving in a direction you'll be able to use with the CLI? If you can spend just a couple minutes, please compile SerialDiscovery_JSON on any Linux system and run it in a terminal, then plug and unplug some Arduino boards while it's running. You'll see the JSON printed to stdout. I'd imagine you'll need for CLI at least some way to know when the initial enumeration of boards has ended, right?

I know you guys are probably crazy busy. Hopefully the dust has settled from 1.8.7 by now? I'd really like to move forward on this, to the point of a pull request for the IDE and at least a vision that it will also work for CLI. Really need a little input here.

@cmaglie

This comment has been minimized.

Show comment
Hide comment
@cmaglie

cmaglie Sep 26, 2018

Member

@PaulStoffregen I'm very sorry for the delay. Here some design notes:

Discovery tool functionality

I've tried your JSON discovery, works great (do you have a similar tool also for Windows and MAC?).

In my mind I the discovery tool should be interactive, something that the IDE starts when launched (more on this later) and send command via stdin/out, for example:

START -> starts the tool internal discovery process
LIST -> output the list of active ports discovered so far
STOP -> suspend the tool internal discovery (freeing resources and CPU time)
START_SYNC -> like START, but the discovered ports are reported as they are discovered like in your tool. When START_SYNC is issued the already existing port are dumped as a first response, or an error is reported if this mode is not available on the discovery.

The rationale is that for some discovery the SYNC update may not be available or it could be expensive, so it will use the LIST as fallback.

About how discoveries are launched

All platforms that implement a custom discovery should declare in the platform.txt a recipe property like:

discovery.DISCOVERYNAME.pattern=......

for example for Teensy I guess it will be something like:

discovery.teensy-hid.pattern=/path/to/disctool/hid-discovery

The discovery should be a cross-platform tool, like the uploaders/compilers/etc. so the more natural way to deploy it is via package_index.json as any other tool.
A complete recipe could be something like:

discovery.teensy-hid.pattern={runtime.tools.hid-discovery.path}/bin/hid-discovery

The IDE at startup will search all custom discovery from all installed platforms and start them (with START_SYNC or, if not available, with START as fallback) to populate the board list.

We need some more thoughts to understand how we can share discoveries (for example what happens if two platforms uses the same discovery tool? or, worse, if they use different versions of the same tool?)

I've started to implement this on the Java IDE, it's in a very WIP state, I'll publish this in a PR for review and we can proceed from that.

What kind of information the discovery give

The discovery should give:

  • a LABEL for the IDE to display on the GUI
  • a PROTOCOL (serial, network, hid, whatever) that we will use later for upload and terminal/monitor (need some design ideas for these one too, especially on how to match the port with an uploader/monitor)
  • a list of port PROPERTIES, that are propagated trough the board configuration, down to the upload recipe.
  • a list of port IDENTIFICATION_PROPERTIES, that is a subset of the PROPERTIES above, but used to identify the board.

Example, for a serial port we will likely have:

{
  "label": "ttyACM0",
  "protocol": "serial",
  "properties": {
    "port": "/dev/ttyACM0",
    "isusb": true,
    "vid": "1234",
    "pid": "5678",
    "serial": "889327498274982739842"
  },
  "identification": {
    "vid": "1234",
    "pid": "5678"
  }
}

note that the identification part has only vid and pid but not serial, becuase we want to identify the board model, not the specific sample of the board.
The IDE may autonomously match the board by looking through the boards.txt for a board that has a matching vid and pid properties for example in the current boards.txt we already have:

# Arduino/Genuino MKR1000
# ---------------------------------------
mkr1000.name=Arduino/Genuino MKR1000
mkr1000.vid.0=0x2341
mkr1000.pid.0=0x804e
mkr1000.vid.1=0x2341
mkr1000.pid.1=0x004e

so this method will be 100% compatible with the current platforms.

Ok, for now I'll stop, waiting for your comments.

Member

cmaglie commented Sep 26, 2018

@PaulStoffregen I'm very sorry for the delay. Here some design notes:

Discovery tool functionality

I've tried your JSON discovery, works great (do you have a similar tool also for Windows and MAC?).

In my mind I the discovery tool should be interactive, something that the IDE starts when launched (more on this later) and send command via stdin/out, for example:

START -> starts the tool internal discovery process
LIST -> output the list of active ports discovered so far
STOP -> suspend the tool internal discovery (freeing resources and CPU time)
START_SYNC -> like START, but the discovered ports are reported as they are discovered like in your tool. When START_SYNC is issued the already existing port are dumped as a first response, or an error is reported if this mode is not available on the discovery.

The rationale is that for some discovery the SYNC update may not be available or it could be expensive, so it will use the LIST as fallback.

About how discoveries are launched

All platforms that implement a custom discovery should declare in the platform.txt a recipe property like:

discovery.DISCOVERYNAME.pattern=......

for example for Teensy I guess it will be something like:

discovery.teensy-hid.pattern=/path/to/disctool/hid-discovery

The discovery should be a cross-platform tool, like the uploaders/compilers/etc. so the more natural way to deploy it is via package_index.json as any other tool.
A complete recipe could be something like:

discovery.teensy-hid.pattern={runtime.tools.hid-discovery.path}/bin/hid-discovery

The IDE at startup will search all custom discovery from all installed platforms and start them (with START_SYNC or, if not available, with START as fallback) to populate the board list.

We need some more thoughts to understand how we can share discoveries (for example what happens if two platforms uses the same discovery tool? or, worse, if they use different versions of the same tool?)

I've started to implement this on the Java IDE, it's in a very WIP state, I'll publish this in a PR for review and we can proceed from that.

What kind of information the discovery give

The discovery should give:

  • a LABEL for the IDE to display on the GUI
  • a PROTOCOL (serial, network, hid, whatever) that we will use later for upload and terminal/monitor (need some design ideas for these one too, especially on how to match the port with an uploader/monitor)
  • a list of port PROPERTIES, that are propagated trough the board configuration, down to the upload recipe.
  • a list of port IDENTIFICATION_PROPERTIES, that is a subset of the PROPERTIES above, but used to identify the board.

Example, for a serial port we will likely have:

{
  "label": "ttyACM0",
  "protocol": "serial",
  "properties": {
    "port": "/dev/ttyACM0",
    "isusb": true,
    "vid": "1234",
    "pid": "5678",
    "serial": "889327498274982739842"
  },
  "identification": {
    "vid": "1234",
    "pid": "5678"
  }
}

note that the identification part has only vid and pid but not serial, becuase we want to identify the board model, not the specific sample of the board.
The IDE may autonomously match the board by looking through the boards.txt for a board that has a matching vid and pid properties for example in the current boards.txt we already have:

# Arduino/Genuino MKR1000
# ---------------------------------------
mkr1000.name=Arduino/Genuino MKR1000
mkr1000.vid.0=0x2341
mkr1000.pid.0=0x804e
mkr1000.vid.1=0x2341
mkr1000.pid.1=0x004e

so this method will be 100% compatible with the current platforms.

Ok, for now I'll stop, waiting for your comments.

@PaulStoffregen

This comment has been minimized.

Show comment
Hide comment
@PaulStoffregen

PaulStoffregen Sep 26, 2018

Collaborator

Thanks! That really charts the path forward. :)

On the new JSON format, how will device removal be reported? I don't see the "online" field. There's also no longer a unique identifier field. Removal can be tricky, because much less info may be available when the device is no longer present. A unique ID field gives the discovery tool a way to reliably refer to info it previously reported.

In the "identification" section, I would hope to be able to optionally report custom menu settings. Or maybe that would be another section?

On the SerialDiscovery tool, sadly there is no libudev for Macintosh and Windows. Macintosh does have CoreFoundation and IOKit, but they are very complex to use (at least from C/C++, I haven't tried Swift yet). Windows is the really difficult part, but Microsoft does have HID-specific APIs. Windows support for HID, and Apple's then-new HID Manager API in Leopard, were the main reason I chose to use HID instead of serial protocol when I started Teensy in 2008. In a strange full circle sort of way, back then Linux was the weakest system with only buggy libusb 0.1 that had been abandoned by libusb's developers. Around 2014 libudev became stable and universally deployed by default on pretty much all Linux distros, so Linux went from the worst to the easiest system.

Collaborator

PaulStoffregen commented Sep 26, 2018

Thanks! That really charts the path forward. :)

On the new JSON format, how will device removal be reported? I don't see the "online" field. There's also no longer a unique identifier field. Removal can be tricky, because much less info may be available when the device is no longer present. A unique ID field gives the discovery tool a way to reliably refer to info it previously reported.

In the "identification" section, I would hope to be able to optionally report custom menu settings. Or maybe that would be another section?

On the SerialDiscovery tool, sadly there is no libudev for Macintosh and Windows. Macintosh does have CoreFoundation and IOKit, but they are very complex to use (at least from C/C++, I haven't tried Swift yet). Windows is the really difficult part, but Microsoft does have HID-specific APIs. Windows support for HID, and Apple's then-new HID Manager API in Leopard, were the main reason I chose to use HID instead of serial protocol when I started Teensy in 2008. In a strange full circle sort of way, back then Linux was the weakest system with only buggy libusb 0.1 that had been abandoned by libusb's developers. Around 2014 libudev became stable and universally deployed by default on pretty much all Linux distros, so Linux went from the worst to the easiest system.

@PaulStoffregen

This comment has been minimized.

Show comment
Hide comment
@PaulStoffregen

PaulStoffregen Sep 26, 2018

Collaborator

I've started to implement this on the Java IDE, it's in a very WIP state, I'll publish this in a PR for review and we can proceed from that.

Would you like me to contribute any part of the Java code? Happy to do so.

Otherwise, I'll just focus on updating the SerialDiscovery tool to the new format. Hopefully that'll help with testing?

Collaborator

PaulStoffregen commented Sep 26, 2018

I've started to implement this on the Java IDE, it's in a very WIP state, I'll publish this in a PR for review and we can proceed from that.

Would you like me to contribute any part of the Java code? Happy to do so.

Otherwise, I'll just focus on updating the SerialDiscovery tool to the new format. Hopefully that'll help with testing?

@cmaglie

This comment has been minimized.

Show comment
Hide comment
@cmaglie

cmaglie Sep 28, 2018

Member

In the "identification" section, I would hope to be able to optionally report custom menu settings. Or maybe that would be another section?

It seems not related to the discovery, but who knows, we could possibly add some more information later, I would not exclude it now.

On the new JSON format, how will device removal be reported? I don't see the "online" field. There's also no longer a unique identifier field. Removal can be tricky, because much less info may be available when the device is no longer present. A unique ID field gives the discovery tool a way to reliably refer to info it previously reported.

I didn't thought on the specific format too much, maybe an "add" and "remove" reports, like:

{
  "action": "add", // or "remove" when they go offline
  "ports": [
    {
      "label": "ttyACM0",
      "protocol": "serial",
      "properties": {
        "port": "/dev/ttyACM0",
        "isusb": true,
        "vid": "1234",
        "pid": "5678",
        "serial": "889327498274982739842"
      },
      "identification": {
        "vid": "1234",
        "pid": "5678"
      },
      .......
  }
}

Would you like me to contribute any part of the Java code? Happy to do so.

I've just opened a PR for you!

Member

cmaglie commented Sep 28, 2018

In the "identification" section, I would hope to be able to optionally report custom menu settings. Or maybe that would be another section?

It seems not related to the discovery, but who knows, we could possibly add some more information later, I would not exclude it now.

On the new JSON format, how will device removal be reported? I don't see the "online" field. There's also no longer a unique identifier field. Removal can be tricky, because much less info may be available when the device is no longer present. A unique ID field gives the discovery tool a way to reliably refer to info it previously reported.

I didn't thought on the specific format too much, maybe an "add" and "remove" reports, like:

{
  "action": "add", // or "remove" when they go offline
  "ports": [
    {
      "label": "ttyACM0",
      "protocol": "serial",
      "properties": {
        "port": "/dev/ttyACM0",
        "isusb": true,
        "vid": "1234",
        "pid": "5678",
        "serial": "889327498274982739842"
      },
      "identification": {
        "vid": "1234",
        "pid": "5678"
      },
      .......
  }
}

Would you like me to contribute any part of the Java code? Happy to do so.

I've just opened a PR for you!

@PaulStoffregen

This comment has been minimized.

Show comment
Hide comment
@PaulStoffregen

PaulStoffregen Sep 29, 2018

Collaborator

Sent a pull request.
cmaglie#1

Collaborator

PaulStoffregen commented Sep 29, 2018

Sent a pull request.
cmaglie#1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment