Make the serial discovery execution atomic (#365)

* make the serial discovery execution atomic

* increase timeout to 10s

* lower debug level

Author: masci

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.Debug("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.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.Debug("Board not recognized")
 				continue
 			} else if err != nil {
 				// this is bad, bail out

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 {
@@ -194,9 +194,9 @@ 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()
+			d.close()
 		}
 	}()
 	if err := d.outJSON.Decode(&event); err != nil {
@@ -206,11 +206,11 @@ 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
-func (d *SerialDiscovery) Close() error {
+func (d *SerialDiscovery) close() error {
 	_, _ = d.in.Write([]byte("QUIT\n"))
 	_ = d.in.Close()
 	_ = d.out.Close()