Refactored board autodetection in upload/compile

- use `board.List()` instead of implementing a duplicate of it
- factored the logic that calculates FQBN, Port and possibly
  autodetects FQBN, so we have a single implementation for all
  commands of the cli.

Author: cmaglie

cli/arguments/fqbn.go

@@ -16,8 +16,14 @@
 package arguments
 
 import (
+	"os"
 	"strings"
 
+	"github.com/arduino/arduino-cli/arduino"
+	"github.com/arduino/arduino-cli/arduino/sketch"
+	"github.com/arduino/arduino-cli/cli/errorcodes"
+	"github.com/arduino/arduino-cli/cli/feedback"
+	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/spf13/cobra"
 )
 
@@ -54,3 +60,44 @@ func (f *Fqbn) String() string {
 func (f *Fqbn) Set(fqbn string) {
 	f.fqbn = fqbn
 }
+
+// CalculateFQBNAndPort calculate the FQBN and Port metadata based on
+// parameters provided by the user.
+// This determine the FQBN based on:
+// - the value of the FQBN flag if explicitly specified, otherwise
+// - the FQBN value in sketch.json if available, otherwise
+// - it tries to autodetect the board connected to the given port flags
+// If all above methods fails, it returns the empty string.
+// The Port metadata are always returned except if:
+// - the port is not found, in this case nil is returned
+// - the FQBN autodetection fail, in this case the function prints an error and
+//   terminates the execution
+func CalculateFQBNAndPort(portArgs *Port, fqbnArg *Fqbn, instance *rpc.Instance, sk *sketch.Sketch) (string, *rpc.Port) {
+	// TODO: REMOVE sketch.Sketch from here
+
+	fqbn := fqbnArg.String()
+	if fqbn == "" && sk != nil && sk.Metadata != nil {
+		// If the user didn't specify an FQBN and a sketch.json file is present
+		// read it from there.
+		fqbn = sk.Metadata.CPU.Fqbn
+	}
+	if fqbn == "" {
+		if portArgs == nil || portArgs.address == "" {
+			feedback.Error(&arduino.MissingFQBNError{})
+			os.Exit(errorcodes.ErrGeneric)
+		}
+		fqbn, port := portArgs.DetectFQBN(instance)
+		if fqbn == "" {
+			feedback.Error(&arduino.MissingFQBNError{})
+			os.Exit(errorcodes.ErrGeneric)
+		}
+		return fqbn, port
+	}
+
+	port, err := portArgs.GetPort(instance, sk)
+	if err != nil {
+		feedback.Errorf(tr("Error getting port metadata: %v", err))
+		os.Exit(errorcodes.ErrGeneric)
+	}
+	return fqbn, port.ToRPC()
+}

cli/arguments/port.go

@@ -21,11 +21,13 @@ import (
 	"os"
 	"time"
 
+	"github.com/arduino/arduino-cli/arduino"
 	"github.com/arduino/arduino-cli/arduino/discovery"
 	"github.com/arduino/arduino-cli/arduino/sketch"
 	"github.com/arduino/arduino-cli/cli/errorcodes"
 	"github.com/arduino/arduino-cli/cli/feedback"
 	"github.com/arduino/arduino-cli/commands"
+	"github.com/arduino/arduino-cli/commands/board"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -152,12 +154,32 @@ func (p *Port) GetSearchTimeout() time.Duration {
 	return p.timeout
 }
 
-// GetDiscoveryPort is a helper function useful to get the port and handle possible errors
-func (p *Port) GetDiscoveryPort(instance *rpc.Instance, sk *sketch.Sketch) *discovery.Port {
-	discoveryPort, err := p.GetPort(instance, sk)
+// DetectFQBN tries to identify the board connected to the port and returns the
+// discovered Port object together with the FQBN. If the port does not match
+// exactly 1 board,
+func (p *Port) DetectFQBN(inst *rpc.Instance) (string, *rpc.Port) {
+	detectedPorts, err := board.List(&rpc.BoardListRequest{Instance: inst})
 	if err != nil {
-		feedback.Errorf(tr("Error discovering port: %v"), err)
+		feedback.Errorf(tr("Error during FQBN detection: %v", err))
 		os.Exit(errorcodes.ErrGeneric)
 	}
-	return discoveryPort
+	for _, detectedPort := range detectedPorts {
+		port := detectedPort.GetPort()
+		if p.address != port.GetAddress() {
+			continue
+		}
+		if p.protocol != "" && p.protocol != port.GetProtocol() {
+			continue
+		}
+		if len(detectedPort.MatchingBoards) > 1 {
+			feedback.Error(&arduino.MultipleBoardsDetectedError{Port: port})
+			os.Exit(errorcodes.ErrBadArgument)
+		}
+		if len(detectedPort.MatchingBoards) == 0 {
+			feedback.Error(&arduino.NoBoardsDetectedError{Port: port})
+			os.Exit(errorcodes.ErrBadArgument)
+		}
+		return detectedPort.MatchingBoards[0].Fqbn, port
+	}
+	return "", nil
 }

cli/compile/compile.go

@@ -26,8 +26,6 @@ import (
 
 	"github.com/arduino/arduino-cli/arduino"
 	"github.com/arduino/arduino-cli/arduino/cores/packagemanager"
-	"github.com/arduino/arduino-cli/arduino/discovery"
-	"github.com/arduino/arduino-cli/arduino/sketch"
 	"github.com/arduino/arduino-cli/cli/arguments"
 	"github.com/arduino/arduino-cli/cli/feedback"
 	"github.com/arduino/arduino-cli/cli/globals"
@@ -150,6 +148,8 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
 	}
 
 	sketchPath := arguments.InitSketchPath(path)
+	sk := arguments.NewSketch(sketchPath)
+	fqbn, port := arguments.CalculateFQBNAndPort(&portArgs, &fqbnArg, inst, sk)
 
 	if keysKeychain != "" || signKey != "" || encryptKey != "" {
 		arguments.CheckFlagsMandatory(cmd, "keys-keychain", "sign-key", "encrypt-key")
@@ -172,25 +172,6 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
 		overrides = o.Overrides
 	}
 
-	fqbn := fqbnArg.String()
-	var sk *sketch.Sketch
-	var port *discovery.Port
-	// If the user didn't provide an FQBN it might either mean
-	// that she forgot or that is trying to compile and upload
-	// using board autodetection.
-	if fqbn == "" && uploadAfterCompile {
-		sk = arguments.NewSketch(sketchPath)
-		port = portArgs.GetDiscoveryPort(inst, sk)
-		rpcPort := port.ToRPC()
-		var err error
-		pm := commands.GetPackageManager(inst.Id)
-		fqbn, err = upload.DetectConnectedBoard(pm, rpcPort.Address, rpcPort.Protocol)
-		if err != nil {
-			feedback.Errorf(tr("Error during FQBN detection: %v", err))
-			os.Exit(errorcodes.ErrGeneric)
-		}
-	}
-
 	compileRequest := &rpc.CompileRequest{
 		Instance:                      inst,
 		Fqbn:                          fqbn,
@@ -227,16 +208,9 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
 	}
 
 	if compileError == nil && uploadAfterCompile {
-		if sk == nil {
-			sk = arguments.NewSketch(sketchPath)
-		}
-		if port == nil {
-			port = portArgs.GetDiscoveryPort(inst, sk)
-		}
-
 		userFieldRes, err := upload.SupportedUserFields(context.Background(), &rpc.SupportedUserFieldsRequest{
 			Instance: inst,
-			Fqbn:     fqbnArg.String(),
+			Fqbn:     fqbn,
 			Address:  port.Address,
 			Protocol: port.Protocol,
 		})
@@ -255,7 +229,7 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
 			Instance:   inst,
 			Fqbn:       fqbn,
 			SketchPath: sketchPath.String(),
-			Port:       port.ToRPC(),
+			Port:       port,
 			Verbose:    verbose,
 			Verify:     verify,
 			ImportDir:  buildPath,

cli/debug/debug.go

@@ -77,13 +77,12 @@ func runDebugCommand(command *cobra.Command, args []string) {
 
 	sketchPath := arguments.InitSketchPath(path)
 	sk := arguments.NewSketch(sketchPath)
-	port := portArgs.GetDiscoveryPort(instance, sk)
-
+	fqbn, port := arguments.CalculateFQBNAndPort(&portArgs, &fqbnArg, instance, sk)
 	debugConfigRequested := &dbg.DebugConfigRequest{
 		Instance:    instance,
-		Fqbn:        fqbnArg.String(),
+		Fqbn:        fqbn,
 		SketchPath:  sketchPath.String(),
-		Port:        port.ToRPC(),
+		Port:        port,
 		Interpreter: interpreter,
 		ImportDir:   importDir,
 		Programmer:  programmer.String(),

cli/upload/upload.go

@@ -95,22 +95,11 @@ func runUploadCommand(command *cobra.Command, args []string) {
 		feedback.Errorf(tr("Error during Upload: %v"), err)
 		os.Exit(errorcodes.ErrGeneric)
 	}
-
-	port, err := portArgs.GetPort(instance, sk)
-	if err != nil {
-		feedback.Errorf(tr("Error during Upload: %v"), err)
-		os.Exit(errorcodes.ErrGeneric)
-	}
-
-	if fqbnArg.String() == "" && sk != nil && sk.Metadata != nil {
-		// If the user didn't specify an FQBN and a sketch.json file is present
-		// read it from there.
-		fqbnArg.Set(sk.Metadata.CPU.Fqbn)
-	}
+	fqbn, port := arguments.CalculateFQBNAndPort(&portArgs, &fqbnArg, instance, sk)
 
 	userFieldRes, err := upload.SupportedUserFields(context.Background(), &rpc.SupportedUserFieldsRequest{
 		Instance: instance,
-		Fqbn:     fqbnArg.String(),
+		Fqbn:     fqbn,
 		Address:  port.Address,
 		Protocol: port.Protocol,
 	})
@@ -153,9 +142,9 @@ func runUploadCommand(command *cobra.Command, args []string) {
 
 	if _, err := upload.Upload(context.Background(), &rpc.UploadRequest{
 		Instance:   instance,
-		Fqbn:       fqbnArg.String(),
+		Fqbn:       fqbn,
 		SketchPath: path,
-		Port:       port.ToRPC(),
+		Port:       port,
 		Verbose:    verbose,
 		Verify:     verify,
 		ImportFile: importFile,

commands/upload/upload.go

@@ -52,16 +52,7 @@ func SupportedUserFields(ctx context.Context, req *rpc.SupportedUserFieldsReques
 		return nil, &arduino.InvalidInstanceError{}
 	}
 
-	reqFQBN := req.GetFqbn()
-	if reqFQBN == "" {
-		var err error
-		reqFQBN, err = DetectConnectedBoard(pm, req.Address, req.Protocol)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	fqbn, err := cores.ParseFQBN(reqFQBN)
+	fqbn, err := cores.ParseFQBN(req.GetFqbn())
 	if err != nil {
 		return nil, &arduino.InvalidFQBNError{Cause: err}
 	}
@@ -86,76 +77,6 @@ func SupportedUserFields(ctx context.Context, req *rpc.SupportedUserFieldsReques
 	}, nil
 }
 
-// DetectConnectedBoard returns the FQBN of the board connected to specified address and protocol.
-// In all other cases, like when more than a possible board is detected return an error.
-func DetectConnectedBoard(pm *packagemanager.PackageManager, address, protocol string) (string, error) {
-	if address == "" {
-		return "", &arduino.MissingPortAddressError{}
-	}
-	if protocol == "" {
-		return "", &arduino.MissingPortProtocolError{}
-	}
-
-	dm := pm.DiscoveryManager()
-
-	// Run discoveries if they're not already running
-	if errs := dm.RunAll(); len(errs) > 0 {
-		// Some discovery managed to not run, just log the errors,
-		// we'll fail further down the line.
-		for _, err := range errs {
-			logrus.Error(err)
-		}
-	}
-
-	if errs := dm.StartAll(); len(errs) > 0 {
-		// Some discovery managed to not start, just log the errors,
-		// we'll fail further down the line.
-		for _, err := range errs {
-			logrus.Error(err)
-		}
-	}
-
-	ports, errs := dm.List()
-	if len(errs) > 0 {
-		// Errors at this time are not a big issue, we'll
-		// fail further down the line if we can't find a
-		// matching board and tell the user to provide
-		// an FQBN.
-		for _, err := range errs {
-			logrus.Error(err)
-		}
-	}
-
-	for _, p := range ports {
-		if p.Address == address && p.Protocol == protocol {
-			// Found matching port, let's see if we can detect
-			// which board is connected to it
-			boards := pm.IdentifyBoard(p.Properties)
-			if l := len(boards); l == 1 {
-				// We found only one board connected, that must
-				// the one the user want to upload to.
-				board := boards[0]
-				logrus.
-					WithField("board", board.String()).
-					WithField("address", address).
-					WithField("protocol", protocol).
-					Trace("Detected board")
-				return board.FQBN(), nil
-			} else if l >= 2 {
-				// There are multiple boards detected on this port,
-				// we have no way of knowing which one is the one.
-				return "", &arduino.MultipleBoardsDetectedError{Port: p.ToRPC()}
-			} else {
-				// We found a matching port but there's either
-				// no board connected or we can't recognize it.
-				// The user must provide an FQBN.
-				break
-			}
-		}
-	}
-	return "", &arduino.MissingFQBNError{}
-}
-
 // getToolID returns the ID of the tool that supports the action and protocol combination by searching in props.
 // Returns error if tool cannot be found.
 func getToolID(props *properties.Map, action, protocol string) (string, error) {
@@ -271,18 +192,6 @@ func runProgramAction(pm *packagemanager.PackageManager,
 
 	logrus.WithField("port", port).Tracef("Upload port")
 
-	if fqbnIn == "" && sk != nil && sk.Metadata != nil {
-		fqbnIn = sk.Metadata.CPU.Fqbn
-	}
-
-	if fqbnIn == "" {
-		var err error
-		fqbnIn, err = DetectConnectedBoard(pm, port.Address, port.Protocol)
-		if err != nil {
-			return err
-		}
-	}
-
 	fqbn, err := cores.ParseFQBN(fqbnIn)
 	if err != nil {
 		return &arduino.InvalidFQBNError{Cause: err}