From 308e5074925f5e05c9319d08aa3a6e6173e2985f Mon Sep 17 00:00:00 2001 From: Massimiliano Pippi Date: Wed, 28 Aug 2019 15:28:24 +0200 Subject: [PATCH 1/4] make the serial discovery execution atomic --- commands/board/list.go | 9 ++++----- commands/bundled_tools_serial_discovery.go | 10 +++++----- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/commands/board/list.go b/commands/board/list.go index 11e4c3cc3d8..4d8ae99c363 100644 --- a/commands/board/list.go +++ b/commands/board/list.go @@ -27,6 +27,7 @@ import ( "github.com/arduino/arduino-cli/commands" rpc "github.com/arduino/arduino-cli/rpc/commands" "github.com/pkg/errors" + "github.com/sirupsen/logrus" ) var ( @@ -87,11 +88,6 @@ func List(instanceID int32) ([]*rpc.DetectedPort, error) { return nil, errors.Wrap(err, "unable to instance serial-discovery") } - if err := serialDiscovery.Start(); err != nil { - return nil, errors.Wrap(err, "unable to start serial-discovery") - } - defer serialDiscovery.Close() - ports, err := serialDiscovery.List() if err != nil { return nil, errors.Wrap(err, "error getting port list from serial-discovery") @@ -102,6 +98,7 @@ func List(instanceID int32) ([]*rpc.DetectedPort, error) { b := []*rpc.BoardListItem{} // first query installed cores through the Package Manager + logrus.Info("Querying installed cores for board identification...") for _, board := range pm.IdentifyBoard(port.IdentificationPrefs) { b = append(b, &rpc.BoardListItem{ Name: board.Name(), @@ -112,12 +109,14 @@ func List(instanceID int32) ([]*rpc.DetectedPort, error) { // if installed cores didn't recognize the board, try querying // the builder API if len(b) == 0 { + logrus.Info("Querying builder API for board identification...") url := fmt.Sprintf("https://builder.arduino.cc/v3/boards/byVidPid/%s/%s", port.IdentificationPrefs.Get("vid"), port.IdentificationPrefs.Get("pid")) items, err := apiByVidPid(url) if err == ErrNotFound { // the board couldn't be detected, keep going with the next port + logrus.Info("Board not recognized") continue } else if err != nil { // this is bad, bail out diff --git a/commands/bundled_tools_serial_discovery.go b/commands/bundled_tools_serial_discovery.go index 25724064f66..63119858fcc 100644 --- a/commands/bundled_tools_serial_discovery.go +++ b/commands/bundled_tools_serial_discovery.go @@ -153,7 +153,7 @@ func NewBuiltinSerialDiscovery(pm *packagemanager.PackageManager) (*SerialDiscov } // Start starts the specified discovery -func (d *SerialDiscovery) Start() error { +func (d *SerialDiscovery) start() error { if in, err := d.cmd.StdinPipe(); err == nil { d.in = in } else { @@ -181,8 +181,8 @@ func (d *SerialDiscovery) List() ([]*BoardPort, error) { d.Lock() defer d.Unlock() - if d.cmd.Process == nil { - return nil, fmt.Errorf("discovery hasn't started") + if err := d.start(); err != nil { + return nil, fmt.Errorf("discovery hasn't started: %v", err) } if _, err := d.in.Write([]byte("LIST\n")); err != nil { @@ -196,7 +196,7 @@ func (d *SerialDiscovery) List() ([]*BoardPort, error) { case <-done: case <-time.After(2000 * time.Millisecond): timeout = true - d.Close() + d.close() } }() if err := d.outJSON.Decode(&event); err != nil { @@ -210,7 +210,7 @@ func (d *SerialDiscovery) List() ([]*BoardPort, error) { } // Close stops the Discovery and free the resources -func (d *SerialDiscovery) Close() error { +func (d *SerialDiscovery) close() error { _, _ = d.in.Write([]byte("QUIT\n")) _ = d.in.Close() _ = d.out.Close() From 436fb106cfbefe52a25f68da0571702b735fe4e4 Mon Sep 17 00:00:00 2001 From: Massimiliano Pippi Date: Wed, 28 Aug 2019 15:36:21 +0200 Subject: [PATCH 2/4] close upon done --- commands/bundled_tools_serial_discovery.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/bundled_tools_serial_discovery.go b/commands/bundled_tools_serial_discovery.go index 63119858fcc..504727dd92d 100644 --- a/commands/bundled_tools_serial_discovery.go +++ b/commands/bundled_tools_serial_discovery.go @@ -206,7 +206,7 @@ func (d *SerialDiscovery) List() ([]*BoardPort, error) { return nil, fmt.Errorf("decoding LIST command: %s", err) } done <- true - return event.Ports, nil + return event.Ports, d.close() } // Close stops the Discovery and free the resources From a805d229e3f2933de69649c707abaea903408a8e Mon Sep 17 00:00:00 2001 From: Massimiliano Pippi Date: Wed, 28 Aug 2019 16:33:00 +0200 Subject: [PATCH 3/4] increase timeout to 10s --- commands/bundled_tools_serial_discovery.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/bundled_tools_serial_discovery.go b/commands/bundled_tools_serial_discovery.go index 504727dd92d..ee5e8a976b0 100644 --- a/commands/bundled_tools_serial_discovery.go +++ b/commands/bundled_tools_serial_discovery.go @@ -194,7 +194,7 @@ func (d *SerialDiscovery) List() ([]*BoardPort, error) { go func() { select { case <-done: - case <-time.After(2000 * time.Millisecond): + case <-time.After(10 * time.Second): timeout = true d.close() } From 920da6f930ffd1bb927b0d038693b57685b56c67 Mon Sep 17 00:00:00 2001 From: Massimiliano Pippi Date: Wed, 28 Aug 2019 17:11:53 +0200 Subject: [PATCH 4/4] lower debug level --- commands/board/list.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/commands/board/list.go b/commands/board/list.go index 4d8ae99c363..e8157422f7a 100644 --- a/commands/board/list.go +++ b/commands/board/list.go @@ -98,7 +98,7 @@ func List(instanceID int32) ([]*rpc.DetectedPort, error) { b := []*rpc.BoardListItem{} // first query installed cores through the Package Manager - logrus.Info("Querying installed cores for board identification...") + logrus.Debug("Querying installed cores for board identification...") for _, board := range pm.IdentifyBoard(port.IdentificationPrefs) { b = append(b, &rpc.BoardListItem{ Name: board.Name(), @@ -109,14 +109,14 @@ func List(instanceID int32) ([]*rpc.DetectedPort, error) { // if installed cores didn't recognize the board, try querying // the builder API if len(b) == 0 { - logrus.Info("Querying builder API for board identification...") + logrus.Debug("Querying builder API for board identification...") url := fmt.Sprintf("https://builder.arduino.cc/v3/boards/byVidPid/%s/%s", port.IdentificationPrefs.Get("vid"), port.IdentificationPrefs.Get("pid")) items, err := apiByVidPid(url) if err == ErrNotFound { // the board couldn't be detected, keep going with the next port - logrus.Info("Board not recognized") + logrus.Debug("Board not recognized") continue } else if err != nil { // this is bad, bail out