Inlining methods in ArduinoCoreServiceImpl (part 1: BoardListAll)

This commit is composed of many parts: * ArduinoCoreServiceImpl has been made private * A new function NewArduinoCoreService() has been added to instatiate a new service. * The service is created at CLI startup and sent down the initialization chain where needed. In this commit only the BoardListAll command has been ported to prove the feasability of the change.
arduino · cmaglie · May 15, 2024 · Mar 13, 2024 · Mar 13, 2024 · Mar 13, 2024
commit 23ccd4332dcbe8b8153824381ae79b96de419a20
diff --git a/commands/service.go b/commands/service.go
diff --git a/commands/service_board_listall.go b/commands/service_board_listall.go
@@ -26,8 +26,8 @@ import (
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 )
 
-// BoardListAll FIXMEDOC
-func BoardListAll(ctx context.Context, req *rpc.BoardListAllRequest) (*rpc.BoardListAllResponse, error) {
+// BoardListAll list all the boards provided by installed platforms.
+func (s *arduinoCoreServerImpl) BoardListAll(ctx context.Context, req *rpc.BoardListAllRequest) (*rpc.BoardListAllResponse, error) {
 	pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
 	if err != nil {
 		return nil, err

diff --git a/commands/service_debug.go b/commands/service_debug.go
@@ -26,7 +26,7 @@ import (
 // Debug returns a stream response that can be used to fetch data from the
 // target. The first message passed through the `Debug` request must
 // contain DebugRequest configuration params, not data.
-func (s *ArduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer) error {
+func (s *arduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer) error {
 	// Grab the first message
 	msg, err := stream.Recv()
 	if err != nil {
@@ -61,11 +61,11 @@ func (s *ArduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer)
 }
 
 // GetDebugConfig return metadata about a debug session
-func (s *ArduinoCoreServerImpl) GetDebugConfig(ctx context.Context, req *rpc.GetDebugConfigRequest) (*rpc.GetDebugConfigResponse, error) {
+func (s *arduinoCoreServerImpl) GetDebugConfig(ctx context.Context, req *rpc.GetDebugConfigRequest) (*rpc.GetDebugConfigResponse, error) {
 	return GetDebugConfig(ctx, req)
 }
 
 // IsDebugSupported checks if debugging is supported for a given configuration
-func (s *ArduinoCoreServerImpl) IsDebugSupported(ctx context.Context, req *rpc.IsDebugSupportedRequest) (*rpc.IsDebugSupportedResponse, error) {
+func (s *arduinoCoreServerImpl) IsDebugSupported(ctx context.Context, req *rpc.IsDebugSupportedRequest) (*rpc.IsDebugSupportedResponse, error) {
 	return IsDebugSupported(ctx, req)
 }
diff --git a/commands/service_settings.go b/commands/service_settings.go
@@ -28,7 +28,7 @@ import (
 
 // SettingsGetAll returns a message with a string field containing all the settings
 // currently in use, marshalled in JSON format.
-func (s *ArduinoCoreServerImpl) SettingsGetAll(ctx context.Context, req *rpc.SettingsGetAllRequest) (*rpc.SettingsGetAllResponse, error) {
+func (s *arduinoCoreServerImpl) SettingsGetAll(ctx context.Context, req *rpc.SettingsGetAllRequest) (*rpc.SettingsGetAllResponse, error) {
 	b, err := json.Marshal(configuration.Settings.AllSettings())
 	if err == nil {
 		return &rpc.SettingsGetAllResponse{
@@ -68,7 +68,7 @@ func mapper(toMap map[string]interface{}) map[string]interface{} {
 }
 
 // SettingsMerge applies multiple settings values at once.
-func (s *ArduinoCoreServerImpl) SettingsMerge(ctx context.Context, req *rpc.SettingsMergeRequest) (*rpc.SettingsMergeResponse, error) {
+func (s *arduinoCoreServerImpl) SettingsMerge(ctx context.Context, req *rpc.SettingsMergeRequest) (*rpc.SettingsMergeResponse, error) {
 	var toMerge map[string]interface{}
 	if err := json.Unmarshal([]byte(req.GetJsonData()), &toMerge); err != nil {
 		return nil, err
@@ -93,7 +93,7 @@ func (s *ArduinoCoreServerImpl) SettingsMerge(ctx context.Context, req *rpc.Sett
 // SettingsGetValue returns a settings value given its key. If the key is not present
 // an error will be returned, so that we distinguish empty settings from missing
 // ones.
-func (s *ArduinoCoreServerImpl) SettingsGetValue(ctx context.Context, req *rpc.SettingsGetValueRequest) (*rpc.SettingsGetValueResponse, error) {
+func (s *arduinoCoreServerImpl) SettingsGetValue(ctx context.Context, req *rpc.SettingsGetValueRequest) (*rpc.SettingsGetValueResponse, error) {
 	key := req.GetKey()
 
 	// Check if settings key actually existing, we don't use Viper.InConfig()
@@ -121,7 +121,7 @@ func (s *ArduinoCoreServerImpl) SettingsGetValue(ctx context.Context, req *rpc.S
 }
 
 // SettingsSetValue updates or set a value for a certain key.
-func (s *ArduinoCoreServerImpl) SettingsSetValue(ctx context.Context, val *rpc.SettingsSetValueRequest) (*rpc.SettingsSetValueResponse, error) {
+func (s *arduinoCoreServerImpl) SettingsSetValue(ctx context.Context, val *rpc.SettingsSetValueRequest) (*rpc.SettingsSetValueResponse, error) {
 	key := val.GetKey()
 	var value interface{}
 
@@ -137,15 +137,15 @@ func (s *ArduinoCoreServerImpl) SettingsSetValue(ctx context.Context, val *rpc.S
 // We don't have a Read() function, that's not necessary since we only want one config file to be used
 // and that's picked up when the CLI is run as daemon, either using the default path or a custom one
 // set with the --config-file flag.
-func (s *ArduinoCoreServerImpl) SettingsWrite(ctx context.Context, req *rpc.SettingsWriteRequest) (*rpc.SettingsWriteResponse, error) {
+func (s *arduinoCoreServerImpl) SettingsWrite(ctx context.Context, req *rpc.SettingsWriteRequest) (*rpc.SettingsWriteResponse, error) {
 	if err := configuration.Settings.WriteConfigAs(req.GetFilePath()); err != nil {
 		return nil, err
 	}
 	return &rpc.SettingsWriteResponse{}, nil
 }
 
 // SettingsDelete removes a key from the config file
-func (s *ArduinoCoreServerImpl) SettingsDelete(ctx context.Context, req *rpc.SettingsDeleteRequest) (*rpc.SettingsDeleteResponse, error) {
+func (s *arduinoCoreServerImpl) SettingsDelete(ctx context.Context, req *rpc.SettingsDeleteRequest) (*rpc.SettingsDeleteResponse, error) {
 	toDelete := req.GetKey()
 
 	// Check if settings key actually existing, we don't use Viper.InConfig()

diff --git a/commands/service_settings_test.go b/commands/service_settings_test.go
@@ -27,7 +27,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-var svc = ArduinoCoreServerImpl{}
+var svc = NewArduinoCoreServer("")
 
 func init() {
 	configuration.Settings = configuration.Init(filepath.Join("testdata", "arduino-cli.yaml"))

diff --git a/internal/cli/arguments/completion.go b/internal/cli/arguments/completion.go
@@ -26,10 +26,10 @@ import (
 // GetInstalledBoards is an helper function useful to autocomplete.
 // It returns a list of fqbn
 // it's taken from cli/board/listall.go
-func GetInstalledBoards() []string {
+func GetInstalledBoards(srv rpc.ArduinoCoreServiceServer) []string {
 	inst := instance.CreateAndInit()
 
-	list, _ := commands.BoardListAll(context.Background(), &rpc.BoardListAllRequest{
+	list, _ := srv.BoardListAll(context.Background(), &rpc.BoardListAllRequest{
 		Instance:            inst,
 		SearchArgs:          nil,
 		IncludeHiddenBoards: false,
@@ -44,7 +44,7 @@ func GetInstalledBoards() []string {
 
 // GetInstalledProgrammers is an helper function useful to autocomplete.
 // It returns a list of programmers available based on the installed boards
-func GetInstalledProgrammers() []string {
+func GetInstalledProgrammers(srv rpc.ArduinoCoreServiceServer) []string {
 	inst := instance.CreateAndInit()
 
 	// we need the list of the available fqbn in order to get the list of the programmers
@@ -53,7 +53,7 @@ func GetInstalledProgrammers() []string {
 		SearchArgs:          nil,
 		IncludeHiddenBoards: false,
 	}
-	list, _ := commands.BoardListAll(context.Background(), listAllReq)
+	list, _ := srv.BoardListAll(context.Background(), listAllReq)
 
 	installedProgrammers := make(map[string]string)
 	for _, board := range list.GetBoards() {

diff --git a/internal/cli/arguments/fqbn.go b/internal/cli/arguments/fqbn.go
@@ -33,10 +33,10 @@ type Fqbn struct {
 }
 
 // AddToCommand adds the flags used to set fqbn to the specified Command
-func (f *Fqbn) AddToCommand(cmd *cobra.Command) {
+func (f *Fqbn) AddToCommand(cmd *cobra.Command, srv rpc.ArduinoCoreServiceServer) {
 	cmd.Flags().StringVarP(&f.fqbn, "fqbn", "b", "", tr("Fully Qualified Board Name, e.g.: arduino:avr:uno"))
 	cmd.RegisterFlagCompletionFunc("fqbn", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-		return GetInstalledBoards(), cobra.ShellCompDirectiveDefault
+		return GetInstalledBoards(srv), cobra.ShellCompDirectiveDefault
 	})
 	cmd.Flags().StringSliceVar(&f.boardOptions, "board-options", []string{},
 		tr("List of board options separated by commas. Or can be used multiple times for multiple options."))

diff --git a/internal/cli/arguments/programmer.go b/internal/cli/arguments/programmer.go
@@ -31,10 +31,10 @@ type Programmer struct {
 }
 
 // AddToCommand adds the flags used to set the programmer to the specified Command
-func (p *Programmer) AddToCommand(cmd *cobra.Command) {
+func (p *Programmer) AddToCommand(cmd *cobra.Command, srv rpc.ArduinoCoreServiceServer) {
 	cmd.Flags().StringVarP(&p.programmer, "programmer", "P", "", tr("Programmer to use, e.g: atmel_ice"))
 	cmd.RegisterFlagCompletionFunc("programmer", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
-		return GetInstalledProgrammers(), cobra.ShellCompDirectiveDefault
+		return GetInstalledProgrammers(srv), cobra.ShellCompDirectiveDefault
 	})
 }
 

diff --git a/internal/cli/board/attach.go b/internal/cli/board/attach.go
@@ -27,7 +27,7 @@ import (
 	"github.com/spf13/cobra"
 )
 
-func initAttachCommand() *cobra.Command {
+func initAttachCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 	var port arguments.Port
 	var fqbn arguments.Fqbn
 	var programmer arguments.Programmer
@@ -48,9 +48,9 @@ func initAttachCommand() *cobra.Command {
 			runAttachCommand(sketchPath, &port, fqbn.String(), &programmer)
 		},
 	}
-	fqbn.AddToCommand(attachCommand)
+	fqbn.AddToCommand(attachCommand, srv)
 	port.AddToCommand(attachCommand)
-	programmer.AddToCommand(attachCommand)
+	programmer.AddToCommand(attachCommand, srv)
 
 	return attachCommand
 }

diff --git a/internal/cli/board/board.go b/internal/cli/board/board.go
@@ -19,13 +19,14 @@ import (
 	"os"
 
 	"github.com/arduino/arduino-cli/internal/i18n"
+	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/spf13/cobra"
 )
 
 var tr = i18n.Tr
 
 // NewCommand created a new `board` command
-func NewCommand() *cobra.Command {
+func NewCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 	boardCommand := &cobra.Command{
 		Use:   "board",
 		Short: tr("Arduino board commands."),
@@ -34,10 +35,10 @@ func NewCommand() *cobra.Command {
 			"  " + os.Args[0] + " board list",
 	}
 
-	boardCommand.AddCommand(initAttachCommand())
-	boardCommand.AddCommand(initDetailsCommand())
-	boardCommand.AddCommand(initListCommand())
-	boardCommand.AddCommand(initListAllCommand())
+	boardCommand.AddCommand(initAttachCommand(srv))
+	boardCommand.AddCommand(initDetailsCommand(srv))
+	boardCommand.AddCommand(initListCommand(srv))
+	boardCommand.AddCommand(initListAllCommand(srv))
 	boardCommand.AddCommand(initSearchCommand())
 
 	return boardCommand

diff --git a/internal/cli/board/details.go b/internal/cli/board/details.go
@@ -32,7 +32,7 @@ import (
 	"github.com/spf13/cobra"
 )
 
-func initDetailsCommand() *cobra.Command {
+func initDetailsCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 	var showFullDetails bool
 	var listProgrammers bool
 	var fqbn arguments.Fqbn
@@ -48,7 +48,7 @@ func initDetailsCommand() *cobra.Command {
 		},
 	}
 
-	fqbn.AddToCommand(detailsCommand)
+	fqbn.AddToCommand(detailsCommand, srv)
 	detailsCommand.Flags().BoolVarP(&showFullDetails, "full", "f", false, tr("Show full board details"))
 	detailsCommand.Flags().BoolVarP(&listProgrammers, "list-programmers", "", false, tr("Show list of available programmers"))
 	detailsCommand.MarkFlagRequired("fqbn")

diff --git a/internal/cli/board/list.go b/internal/cli/board/list.go
@@ -35,7 +35,7 @@ import (
 	"github.com/spf13/cobra"
 )
 
-func initListCommand() *cobra.Command {
+func initListCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 	var timeoutArg arguments.DiscoveryTimeout
 	var watch bool
 	var fqbn arguments.Fqbn
@@ -51,7 +51,7 @@ func initListCommand() *cobra.Command {
 	}
 
 	timeoutArg.AddToCommand(listCommand)
-	fqbn.AddToCommand(listCommand)
+	fqbn.AddToCommand(listCommand, srv)
 	listCommand.Flags().BoolVarP(&watch, "watch", "w", false, tr("Command keeps running and prints list of connected boards whenever there is a change."))
 	return listCommand
 }

diff --git a/internal/cli/board/listall.go b/internal/cli/board/listall.go
@@ -21,7 +21,6 @@ import (
 	"os"
 	"sort"
 
-	"github.com/arduino/arduino-cli/commands"
 	"github.com/arduino/arduino-cli/internal/cli/feedback"
 	"github.com/arduino/arduino-cli/internal/cli/feedback/result"
 	"github.com/arduino/arduino-cli/internal/cli/feedback/table"
@@ -33,7 +32,7 @@ import (
 
 var showHiddenBoard bool
 
-func initListAllCommand() *cobra.Command {
+func initListAllCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 	var listAllCommand = &cobra.Command{
 		Use:   fmt.Sprintf("listall [%s]", tr("boardname")),
 		Short: tr("List all known boards and their corresponding FQBN."),
@@ -43,19 +42,21 @@ for a specific board if you specify the board name`),
 			"  " + os.Args[0] + " board listall\n" +
 			"  " + os.Args[0] + " board listall zero",
 		Args: cobra.ArbitraryArgs,
-		Run:  runListAllCommand,
+		Run: func(cmd *cobra.Command, args []string) {
+			runListAllCommand(args, srv)
+		},
 	}
 	listAllCommand.Flags().BoolVarP(&showHiddenBoard, "show-hidden", "a", false, tr("Show also boards marked as 'hidden' in the platform"))
 	return listAllCommand
 }
 
 // runListAllCommand list all installed boards
-func runListAllCommand(cmd *cobra.Command, args []string) {
+func runListAllCommand(args []string, srv rpc.ArduinoCoreServiceServer) {
 	inst := instance.CreateAndInit()
 
 	logrus.Info("Executing `arduino-cli board listall`")
 
-	list, err := commands.BoardListAll(context.Background(), &rpc.BoardListAllRequest{
+	list, err := srv.BoardListAll(context.Background(), &rpc.BoardListAllRequest{
 		Instance:            inst,
 		SearchArgs:          args,
 		IncludeHiddenBoards: showHiddenBoard,

diff --git a/internal/cli/burnbootloader/burnbootloader.go b/internal/cli/burnbootloader/burnbootloader.go
@@ -42,7 +42,7 @@ var (
 )
 
 // NewCommand created a new `burn-bootloader` command
-func NewCommand() *cobra.Command {
+func NewCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 	burnBootloaderCommand := &cobra.Command{
 		Use:     "burn-bootloader",
 		Short:   tr("Upload the bootloader."),
@@ -52,9 +52,9 @@ func NewCommand() *cobra.Command {
 		Run:     runBootloaderCommand,
 	}
 
-	fqbn.AddToCommand(burnBootloaderCommand)
+	fqbn.AddToCommand(burnBootloaderCommand, srv)
 	port.AddToCommand(burnBootloaderCommand)
-	programmer.AddToCommand(burnBootloaderCommand)
+	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."))
 	burnBootloaderCommand.Flags().BoolVar(&dryRun, "dry-run", false, tr("Do not perform the actual upload, just log out actions"))

diff --git a/internal/cli/cli.go b/internal/cli/cli.go
@@ -64,13 +64,13 @@ var (
 )
 
 // NewCommand creates a new ArduinoCli command root
-func NewCommand() *cobra.Command {
+func NewCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
 	cobra.AddTemplateFunc("tr", i18n.Tr)
 
 	var updaterMessageChan chan *semver.Version
 
 	// ArduinoCli is the root command
-	arduinoCli := &cobra.Command{
+	cmd := &cobra.Command{
 		Use:     "arduino-cli",
 		Short:   tr("Arduino CLI."),
 		Long:    tr("Arduino Command Line Interface (arduino-cli)."),
@@ -112,35 +112,27 @@ func NewCommand() *cobra.Command {
 		},
 	}
 
-	arduinoCli.SetUsageTemplate(getUsageTemplate())
+	cmd.SetUsageTemplate(getUsageTemplate())
 
-	createCliCommandTree(arduinoCli)
-
-	return arduinoCli
-}
-
-// this is here only for testing
-func createCliCommandTree(cmd *cobra.Command) {
-	cmd.AddCommand(board.NewCommand())
+	cmd.AddCommand(board.NewCommand(srv))
 	cmd.AddCommand(cache.NewCommand())
-	cmd.AddCommand(compile.NewCommand())
+	cmd.AddCommand(compile.NewCommand(srv))
 	cmd.AddCommand(completion.NewCommand())
 	cmd.AddCommand(config.NewCommand())
 	cmd.AddCommand(core.NewCommand())
 	cmd.AddCommand(daemon.NewCommand())
 	cmd.AddCommand(generatedocs.NewCommand())
-	cmd.AddCommand(lib.NewCommand())
-	cmd.AddCommand(monitor.NewCommand())
+	cmd.AddCommand(lib.NewCommand(srv))
+	cmd.AddCommand(monitor.NewCommand(srv))
 	cmd.AddCommand(outdated.NewCommand())
 	cmd.AddCommand(sketch.NewCommand())
 	cmd.AddCommand(update.NewCommand())
 	cmd.AddCommand(upgrade.NewCommand())
-	cmd.AddCommand(upload.NewCommand())
-	cmd.AddCommand(debug.NewCommand())
-	cmd.AddCommand(burnbootloader.NewCommand())
+	cmd.AddCommand(upload.NewCommand(srv))
+	cmd.AddCommand(debug.NewCommand(srv))
+	cmd.AddCommand(burnbootloader.NewCommand(srv))
 	cmd.AddCommand(version.NewCommand())
 	cmd.AddCommand(feedback.NewCommand())
-
 	cmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, tr("Print the logs on the standard output."))
 	cmd.Flag("verbose").Hidden = true
 	cmd.PersistentFlags().BoolVar(&verbose, "log", false, tr("Print the logs on the standard output."))
@@ -166,6 +158,8 @@ func createCliCommandTree(cmd *cobra.Command) {
 	cmd.PersistentFlags().StringSlice("additional-urls", []string{}, tr("Comma-separated list of additional URLs for the Boards Manager."))
 	cmd.PersistentFlags().Bool("no-color", false, "Disable colored output.")
 	configuration.BindFlags(cmd, configuration.Settings)
+
+	return cmd
 }
 
 // convert the string passed to the `--log-level` option to the corresponding