Inlining methods in ArduinoCoreServiceImpl (part 3: BoardList)

This patch inlines BoardList and expands the gRPC API to provide the
same output given to the cli.

Author: cmaglie

Diff for: commands/service.go

@@ -42,17 +42,6 @@ type arduinoCoreServerImpl struct {
 	versionString string
 }
 
-// BoardList FIXMEDOC
-func (s *arduinoCoreServerImpl) BoardList(ctx context.Context, req *rpc.BoardListRequest) (*rpc.BoardListResponse, error) {
-	ports, _, err := BoardList(req)
-	if err != nil {
-		return nil, err
-	}
-	return &rpc.BoardListResponse{
-		Ports: ports,
-	}, nil
-}
-
 // BoardSearch exposes to the gRPC interface the board search command
 func (s *arduinoCoreServerImpl) BoardSearch(ctx context.Context, req *rpc.BoardSearchRequest) (*rpc.BoardSearchResponse, error) {
 	return BoardSearch(ctx, req)

Diff for: commands/service_board_list.go

@@ -29,6 +29,7 @@ import (
 
 	"github.com/arduino/arduino-cli/commands/cmderrors"
 	"github.com/arduino/arduino-cli/commands/internal/instances"
+	f "github.com/arduino/arduino-cli/internal/algorithms"
 	"github.com/arduino/arduino-cli/internal/arduino/cores"
 	"github.com/arduino/arduino-cli/internal/arduino/cores/packagemanager"
 	"github.com/arduino/arduino-cli/internal/arduino/httpclient"
@@ -204,10 +205,10 @@ func identify(pme *packagemanager.Explorer, port *discovery.Port) ([]*rpc.BoardL
 // BoardList returns a list of boards found by the loaded discoveries.
 // In case of errors partial results from discoveries that didn't fail
 // are returned.
-func BoardList(req *rpc.BoardListRequest) (r []*rpc.DetectedPort, discoveryStartErrors []error, e error) {
+func (s *arduinoCoreServerImpl) BoardList(ctx context.Context, req *rpc.BoardListRequest) (*rpc.BoardListResponse, error) {
 	pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 	defer release()
 
@@ -216,19 +217,19 @@ func BoardList(req *rpc.BoardListRequest) (r []*rpc.DetectedPort, discoveryStart
 		var err error
 		fqbnFilter, err = cores.ParseFQBN(f)
 		if err != nil {
-			return nil, nil, &cmderrors.InvalidFQBNError{Cause: err}
+			return nil, &cmderrors.InvalidFQBNError{Cause: err}
 		}
 	}
 
 	dm := pme.DiscoveryManager()
-	discoveryStartErrors = dm.Start()
+	warnings := f.Map(dm.Start(), (error).Error)
 	time.Sleep(time.Duration(req.GetTimeout()) * time.Millisecond)
 
-	retVal := []*rpc.DetectedPort{}
+	ports := []*rpc.DetectedPort{}
 	for _, port := range dm.List() {
 		boards, err := identify(pme, port)
 		if err != nil {
-			return nil, discoveryStartErrors, err
+			warnings = append(warnings, err.Error())
 		}
 
 		// boards slice can be empty at this point if neither the cores nor the
@@ -239,10 +240,13 @@ func BoardList(req *rpc.BoardListRequest) (r []*rpc.DetectedPort, discoveryStart
 		}
 
 		if fqbnFilter == nil || hasMatchingBoard(b, fqbnFilter) {
-			retVal = append(retVal, b)
+			ports = append(ports, b)
 		}
 	}
-	return retVal, discoveryStartErrors, nil
+	return &rpc.BoardListResponse{
+		Ports:    ports,
+		Warnings: warnings,
+	}, nil
 }
 
 func hasMatchingBoard(b *rpc.DetectedPort, fqbnFilter *cores.FQBN) bool {

Diff for: internal/cli/arguments/completion.go

@@ -19,6 +19,7 @@ import (
 	"context"
 
 	"github.com/arduino/arduino-cli/commands"
+	f "github.com/arduino/arduino-cli/internal/algorithms"
 	"github.com/arduino/arduino-cli/internal/cli/instance"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 )
@@ -164,16 +165,11 @@ func GetInstallableLibs() []string {
 // GetAvailablePorts is an helper function useful to autocomplete.
 // It returns a list of upload port of the boards which are currently connected.
 // It will not suggests network ports because the timeout is not set.
-func GetAvailablePorts() []*rpc.Port {
+func GetAvailablePorts(srv rpc.ArduinoCoreServiceServer) []*rpc.Port {
+	// Get the port list
 	inst := instance.CreateAndInit()
+	list, _ := srv.BoardList(context.Background(), &rpc.BoardListRequest{Instance: inst})
 
-	list, _, _ := commands.BoardList(&rpc.BoardListRequest{
-		Instance: inst,
-	})
-	var res []*rpc.Port
-	// transform the data structure for the completion
-	for _, i := range list {
-		res = append(res, i.GetPort())
-	}
-	return res
+	// Transform the data structure for the completion (DetectedPort -> Port)
+	return f.Map(list.GetPorts(), (*rpc.DetectedPort).GetPort)
 }

Diff for: internal/cli/arguments/fqbn.go

@@ -69,7 +69,7 @@ func (f *Fqbn) Set(fqbn string) {
 //   - 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, defaultFQBN, defaultAddress, defaultProtocol string) (string, *rpc.Port) {
+func CalculateFQBNAndPort(portArgs *Port, fqbnArg *Fqbn, instance *rpc.Instance, srv rpc.ArduinoCoreServiceServer, defaultFQBN, defaultAddress, defaultProtocol string) (string, *rpc.Port) {
 	fqbn := fqbnArg.String()
 	if fqbn == "" {
 		fqbn = defaultFQBN
@@ -78,7 +78,7 @@ func CalculateFQBNAndPort(portArgs *Port, fqbnArg *Fqbn, instance *rpc.Instance,
 		if portArgs == nil || portArgs.address == "" {
 			feedback.FatalError(&cmderrors.MissingFQBNError{}, feedback.ErrGeneric)
 		}
-		fqbn, port := portArgs.DetectFQBN(instance)
+		fqbn, port := portArgs.DetectFQBN(instance, srv)
 		if fqbn == "" {
 			feedback.FatalError(&cmderrors.MissingFQBNError{}, feedback.ErrGeneric)
 		}

Diff for: internal/cli/arguments/port.go

@@ -39,14 +39,14 @@ type Port struct {
 }
 
 // AddToCommand adds the flags used to set port and protocol to the specified Command
-func (p *Port) AddToCommand(cmd *cobra.Command) {
+func (p *Port) AddToCommand(cmd *cobra.Command, srv rpc.ArduinoCoreServiceServer) {
 	cmd.Flags().StringVarP(&p.address, "port", "p", "", tr("Upload port address, e.g.: COM3 or /dev/ttyACM2"))
 	cmd.RegisterFlagCompletionFunc("port", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-		return f.Map(GetAvailablePorts(), (*rpc.Port).GetAddress), cobra.ShellCompDirectiveDefault
+		return f.Map(GetAvailablePorts(srv), (*rpc.Port).GetAddress), cobra.ShellCompDirectiveDefault
 	})
 	cmd.Flags().StringVarP(&p.protocol, "protocol", "l", "", tr("Upload port protocol, e.g: serial"))
 	cmd.RegisterFlagCompletionFunc("protocol", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-		return f.Map(GetAvailablePorts(), (*rpc.Port).GetProtocol), cobra.ShellCompDirectiveDefault
+		return f.Map(GetAvailablePorts(srv), (*rpc.Port).GetProtocol), cobra.ShellCompDirectiveDefault
 	})
 	p.timeout.AddToCommand(cmd)
 }
@@ -129,15 +129,15 @@ func (p *Port) GetSearchTimeout() time.Duration {
 // 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 := commands.BoardList(&rpc.BoardListRequest{
+func (p *Port) DetectFQBN(inst *rpc.Instance, srv rpc.ArduinoCoreServiceServer) (string, *rpc.Port) {
+	detectedPorts, err := srv.BoardList(context.Background(), &rpc.BoardListRequest{
 		Instance: inst,
 		Timeout:  p.timeout.Get().Milliseconds(),
 	})
 	if err != nil {
 		feedback.Fatal(tr("Error during FQBN detection: %v", err), feedback.ErrGeneric)
 	}
-	for _, detectedPort := range detectedPorts {
+	for _, detectedPort := range detectedPorts.GetPorts() {
 		port := detectedPort.GetPort()
 		if p.address != port.GetAddress() {
 			continue

Diff for: internal/cli/board/attach.go

@@ -49,7 +49,7 @@ func initAttachCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 		},
 	}
 	fqbn.AddToCommand(attachCommand, srv)
-	port.AddToCommand(attachCommand)
+	port.AddToCommand(attachCommand, srv)
 	programmer.AddToCommand(attachCommand, srv)
 
 	return attachCommand

Diff for: internal/cli/board/list.go

@@ -46,7 +46,7 @@ func initListCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 		Example: "  " + os.Args[0] + " board list --discovery-timeout 10s",
 		Args:    cobra.NoArgs,
 		Run: func(cmd *cobra.Command, args []string) {
-			runListCommand(watch, timeoutArg.Get().Milliseconds(), fqbn.String())
+			runListCommand(srv, watch, timeoutArg.Get().Milliseconds(), fqbn.String())
 		},
 	}
 
@@ -57,7 +57,7 @@ func initListCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 }
 
 // runListCommand detects and lists the connected arduino boards
-func runListCommand(watch bool, timeout int64, fqbn string) {
+func runListCommand(srv rpc.ArduinoCoreServiceServer, watch bool, timeout int64, fqbn string) {
 	inst := instance.CreateAndInit()
 
 	logrus.Info("Executing `arduino-cli board list`")
@@ -67,11 +67,13 @@ func runListCommand(watch bool, timeout int64, fqbn string) {
 		return
 	}
 
-	ports, discoveryErrors, err := commands.BoardList(&rpc.BoardListRequest{
+	list, err := srv.BoardList(context.Background(), &rpc.BoardListRequest{
 		Instance: inst,
 		Timeout:  timeout,
 		Fqbn:     fqbn,
 	})
+	ports := list.GetPorts()
+	discoveryErrors := list.GetWarnings()
 	var invalidFQBNErr *cmderrors.InvalidFQBNError
 	if errors.As(err, &invalidFQBNErr) {
 		feedback.Fatal(tr(err.Error()), feedback.ErrBadArgument)

Diff for: internal/cli/burnbootloader/burnbootloader.go

@@ -55,7 +55,7 @@ func NewCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 	}
 
 	fqbn.AddToCommand(burnBootloaderCommand, srv)
-	port.AddToCommand(burnBootloaderCommand)
+	port.AddToCommand(burnBootloaderCommand, srv)
 	programmer.AddToCommand(burnBootloaderCommand, srv)
 	burnBootloaderCommand.Flags().BoolVarP(&verify, "verify", "t", false, tr("Verify uploaded binary after the upload."))
 	burnBootloaderCommand.Flags().BoolVarP(&verbose, "verbose", "v", false, tr("Turns on verbose mode."))

Diff for: internal/cli/compile/compile.go

@@ -116,7 +116,7 @@ func NewCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 	compileCommand.Flags().BoolVarP(&verbose, "verbose", "v", false, tr("Optional, turns on verbose mode."))
 	compileCommand.Flags().BoolVar(&quiet, "quiet", false, tr("Optional, suppresses almost every output."))
 	compileCommand.Flags().BoolVarP(&uploadAfterCompile, "upload", "u", false, tr("Upload the binary after the compilation."))
-	portArgs.AddToCommand(compileCommand)
+	portArgs.AddToCommand(compileCommand, srv)
 	compileCommand.Flags().BoolVarP(&verify, "verify", "t", false, tr("Verify uploaded binary after the upload."))
 	compileCommand.Flags().StringSliceVar(&library, "library", []string{},
 		tr("Path to a single library’s root folder. Can be used multiple times or entries can be comma separated."))
@@ -179,7 +179,7 @@ func runCompileCommand(cmd *cobra.Command, args []string, srv rpc.ArduinoCoreSer
 		fqbnArg.Set(profile.GetFqbn())
 	}
 
-	fqbn, port := arguments.CalculateFQBNAndPort(&portArgs, &fqbnArg, inst, sk.GetDefaultFqbn(), sk.GetDefaultPort(), sk.GetDefaultProtocol())
+	fqbn, port := arguments.CalculateFQBNAndPort(&portArgs, &fqbnArg, inst, srv, sk.GetDefaultFqbn(), sk.GetDefaultPort(), sk.GetDefaultProtocol())
 
 	if keysKeychain != "" || signKey != "" || encryptKey != "" {
 		arguments.CheckFlagsMandatory(cmd, "keys-keychain", "sign-key", "encrypt-key")

Diff for: internal/cli/debug/debug.go

@@ -62,7 +62,7 @@ func NewCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 
 	debugCommand.AddCommand(newDebugCheckCommand(srv))
 	fqbnArg.AddToCommand(debugCommand, srv)
-	portArgs.AddToCommand(debugCommand)
+	portArgs.AddToCommand(debugCommand, srv)
 	programmer.AddToCommand(debugCommand, srv)
 	profileArg.AddToCommand(debugCommand)
 	debugCommand.Flags().StringVar(&interpreter, "interpreter", "console", tr("Debug interpreter e.g.: %s", "console, mi, mi1, mi2, mi3"))
@@ -101,7 +101,7 @@ func runDebugCommand(srv rpc.ArduinoCoreServiceServer, args []string, portArgs *
 		fqbnArg.Set(profile.GetFqbn())
 	}
 
-	fqbn, port := arguments.CalculateFQBNAndPort(portArgs, fqbnArg, inst, sk.GetDefaultFqbn(), sk.GetDefaultPort(), sk.GetDefaultProtocol())
+	fqbn, port := arguments.CalculateFQBNAndPort(portArgs, fqbnArg, inst, srv, sk.GetDefaultFqbn(), sk.GetDefaultPort(), sk.GetDefaultProtocol())
 
 	prog := profile.GetProgrammer()
 	if prog == "" || programmer.GetProgrammer() != "" {

Diff for: internal/cli/debug/debug_check.go

@@ -45,7 +45,7 @@ func newDebugCheckCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 		},
 	}
 	fqbnArg.AddToCommand(debugCheckCommand, srv)
-	portArgs.AddToCommand(debugCheckCommand)
+	portArgs.AddToCommand(debugCheckCommand, srv)
 	programmer.AddToCommand(debugCheckCommand, srv)
 	debugCheckCommand.Flags().StringVar(&interpreter, "interpreter", "console", tr("Debug interpreter e.g.: %s", "console, mi, mi1, mi2, mi3"))
 	return debugCheckCommand

Diff for: internal/cli/monitor/monitor.go

@@ -66,10 +66,10 @@ func NewCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 			if len(args) > 0 {
 				sketchPath = args[0]
 			}
-			runMonitorCmd(&portArgs, &fqbnArg, &profileArg, sketchPath, configs, describe, timestamp, quiet, raw)
+			runMonitorCmd(srv, &portArgs, &fqbnArg, &profileArg, sketchPath, configs, describe, timestamp, quiet, raw)
 		},
 	}
-	portArgs.AddToCommand(monitorCommand)
+	portArgs.AddToCommand(monitorCommand, srv)
 	profileArg.AddToCommand(monitorCommand)
 	monitorCommand.Flags().BoolVar(&raw, "raw", false, tr("Set terminal in raw mode (unbuffered)."))
 	monitorCommand.Flags().BoolVar(&describe, "describe", false, tr("Show all the settings of the communication port."))
@@ -81,6 +81,7 @@ func NewCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 }
 
 func runMonitorCmd(
+	srv rpc.ArduinoCoreServiceServer,
 	portArgs *arguments.Port, fqbnArg *arguments.Fqbn, profileArg *arguments.Profile, sketchPathArg string,
 	configs []string, describe, timestamp, quiet, raw bool,
 ) {
@@ -136,7 +137,7 @@ func runMonitorCmd(
 	case sketch.GetDefaultFqbn() != "":
 		fqbn = sketch.GetDefaultFqbn()
 	default:
-		fqbn, _ = portArgs.DetectFQBN(inst)
+		fqbn, _ = portArgs.DetectFQBN(inst, srv)
 	}
 
 	portAddress, portProtocol, err := portArgs.GetPortAddressAndProtocol(inst, defaultPort, defaultProtocol)

Diff for: internal/cli/upload/upload.go

@@ -68,7 +68,7 @@ func NewCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 	}
 
 	fqbnArg.AddToCommand(uploadCommand, srv)
-	portArgs.AddToCommand(uploadCommand)
+	portArgs.AddToCommand(uploadCommand, srv)
 	profileArg.AddToCommand(uploadCommand)
 	uploadCommand.Flags().StringVarP(&importDir, "input-dir", "", "", tr("Directory containing binaries to upload."))
 	uploadCommand.Flags().StringVarP(&importFile, "input-file", "i", "", tr("Binary file to upload."))
@@ -113,7 +113,7 @@ func runUploadCommand(srv rpc.ArduinoCoreServiceServer, args []string, uploadFie
 	defaultFQBN := sketch.GetDefaultFqbn()
 	defaultAddress := sketch.GetDefaultPort()
 	defaultProtocol := sketch.GetDefaultProtocol()
-	fqbn, port := arguments.CalculateFQBNAndPort(&portArgs, &fqbnArg, inst, defaultFQBN, defaultAddress, defaultProtocol)
+	fqbn, port := arguments.CalculateFQBNAndPort(&portArgs, &fqbnArg, inst, srv, defaultFQBN, defaultAddress, defaultProtocol)
 
 	userFieldRes, err := commands.SupportedUserFields(context.Background(), &rpc.SupportedUserFieldsRequest{
 		Instance: inst,