Skip to content

Commit a510d2d

Browse files
cmagliecmaglie
committed
Inlining methods in ArduinoCoreServiceImpl (part 1)
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.
1 parent ee52f06 commit a510d2d

28 files changed

+138
-135
lines changed

Diff for: ‎commands/service.go

+49-48
Large diffs are not rendered by default.

Diff for: ‎commands/service_board_listall.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ import (
2626
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
2727
)
2828

29-
// BoardListAll FIXMEDOC
30-
func BoardListAll(ctx context.Context, req *rpc.BoardListAllRequest) (*rpc.BoardListAllResponse, error) {
29+
// BoardListAll list all the boards provided by installed platforms.
30+
func (s *arduinoCoreServerImpl) BoardListAll(ctx context.Context, req *rpc.BoardListAllRequest) (*rpc.BoardListAllResponse, error) {
3131
pme, release, err := instances.GetPackageManagerExplorer(req.GetInstance())
3232
if err != nil {
3333
return nil, err

Diff for: ‎commands/service_debug.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626
// Debug returns a stream response that can be used to fetch data from the
2727
// target. The first message passed through the `Debug` request must
2828
// contain DebugRequest configuration params, not data.
29-
func (s *ArduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer) error {
29+
func (s *arduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer) error {
3030
// Grab the first message
3131
msg, err := stream.Recv()
3232
if err != nil {
@@ -61,11 +61,11 @@ func (s *ArduinoCoreServerImpl) Debug(stream rpc.ArduinoCoreService_DebugServer)
6161
}
6262

6363
// GetDebugConfig return metadata about a debug session
64-
func (s *ArduinoCoreServerImpl) GetDebugConfig(ctx context.Context, req *rpc.GetDebugConfigRequest) (*rpc.GetDebugConfigResponse, error) {
64+
func (s *arduinoCoreServerImpl) GetDebugConfig(ctx context.Context, req *rpc.GetDebugConfigRequest) (*rpc.GetDebugConfigResponse, error) {
6565
return GetDebugConfig(ctx, req)
6666
}
6767

6868
// IsDebugSupported checks if debugging is supported for a given configuration
69-
func (s *ArduinoCoreServerImpl) IsDebugSupported(ctx context.Context, req *rpc.IsDebugSupportedRequest) (*rpc.IsDebugSupportedResponse, error) {
69+
func (s *arduinoCoreServerImpl) IsDebugSupported(ctx context.Context, req *rpc.IsDebugSupportedRequest) (*rpc.IsDebugSupportedResponse, error) {
7070
return IsDebugSupported(ctx, req)
7171
}

Diff for: ‎commands/service_settings.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import (
2828

2929
// SettingsGetAll returns a message with a string field containing all the settings
3030
// currently in use, marshalled in JSON format.
31-
func (s *ArduinoCoreServerImpl) SettingsGetAll(ctx context.Context, req *rpc.SettingsGetAllRequest) (*rpc.SettingsGetAllResponse, error) {
31+
func (s *arduinoCoreServerImpl) SettingsGetAll(ctx context.Context, req *rpc.SettingsGetAllRequest) (*rpc.SettingsGetAllResponse, error) {
3232
b, err := json.Marshal(configuration.Settings.AllSettings())
3333
if err == nil {
3434
return &rpc.SettingsGetAllResponse{
@@ -68,7 +68,7 @@ func mapper(toMap map[string]interface{}) map[string]interface{} {
6868
}
6969

7070
// SettingsMerge applies multiple settings values at once.
71-
func (s *ArduinoCoreServerImpl) SettingsMerge(ctx context.Context, req *rpc.SettingsMergeRequest) (*rpc.SettingsMergeResponse, error) {
71+
func (s *arduinoCoreServerImpl) SettingsMerge(ctx context.Context, req *rpc.SettingsMergeRequest) (*rpc.SettingsMergeResponse, error) {
7272
var toMerge map[string]interface{}
7373
if err := json.Unmarshal([]byte(req.GetJsonData()), &toMerge); err != nil {
7474
return nil, err
@@ -93,7 +93,7 @@ func (s *ArduinoCoreServerImpl) SettingsMerge(ctx context.Context, req *rpc.Sett
9393
// SettingsGetValue returns a settings value given its key. If the key is not present
9494
// an error will be returned, so that we distinguish empty settings from missing
9595
// ones.
96-
func (s *ArduinoCoreServerImpl) SettingsGetValue(ctx context.Context, req *rpc.SettingsGetValueRequest) (*rpc.SettingsGetValueResponse, error) {
96+
func (s *arduinoCoreServerImpl) SettingsGetValue(ctx context.Context, req *rpc.SettingsGetValueRequest) (*rpc.SettingsGetValueResponse, error) {
9797
key := req.GetKey()
9898

9999
// 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
121121
}
122122

123123
// SettingsSetValue updates or set a value for a certain key.
124-
func (s *ArduinoCoreServerImpl) SettingsSetValue(ctx context.Context, val *rpc.SettingsSetValueRequest) (*rpc.SettingsSetValueResponse, error) {
124+
func (s *arduinoCoreServerImpl) SettingsSetValue(ctx context.Context, val *rpc.SettingsSetValueRequest) (*rpc.SettingsSetValueResponse, error) {
125125
key := val.GetKey()
126126
var value interface{}
127127

@@ -137,15 +137,15 @@ func (s *ArduinoCoreServerImpl) SettingsSetValue(ctx context.Context, val *rpc.S
137137
// We don't have a Read() function, that's not necessary since we only want one config file to be used
138138
// and that's picked up when the CLI is run as daemon, either using the default path or a custom one
139139
// set with the --config-file flag.
140-
func (s *ArduinoCoreServerImpl) SettingsWrite(ctx context.Context, req *rpc.SettingsWriteRequest) (*rpc.SettingsWriteResponse, error) {
140+
func (s *arduinoCoreServerImpl) SettingsWrite(ctx context.Context, req *rpc.SettingsWriteRequest) (*rpc.SettingsWriteResponse, error) {
141141
if err := configuration.Settings.WriteConfigAs(req.GetFilePath()); err != nil {
142142
return nil, err
143143
}
144144
return &rpc.SettingsWriteResponse{}, nil
145145
}
146146

147147
// SettingsDelete removes a key from the config file
148-
func (s *ArduinoCoreServerImpl) SettingsDelete(ctx context.Context, req *rpc.SettingsDeleteRequest) (*rpc.SettingsDeleteResponse, error) {
148+
func (s *arduinoCoreServerImpl) SettingsDelete(ctx context.Context, req *rpc.SettingsDeleteRequest) (*rpc.SettingsDeleteResponse, error) {
149149
toDelete := req.GetKey()
150150

151151
// Check if settings key actually existing, we don't use Viper.InConfig()

Diff for: ‎commands/service_settings_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import (
2727
"github.com/stretchr/testify/require"
2828
)
2929

30-
var svc = ArduinoCoreServerImpl{}
30+
var svc = NewArduinoCoreServer("")
3131

3232
func init() {
3333
configuration.Settings = configuration.Init(filepath.Join("testdata", "arduino-cli.yaml"))

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ import (
2626
// GetInstalledBoards is an helper function useful to autocomplete.
2727
// It returns a list of fqbn
2828
// it's taken from cli/board/listall.go
29-
func GetInstalledBoards() []string {
29+
func GetInstalledBoards(srv rpc.ArduinoCoreServiceServer) []string {
3030
inst := instance.CreateAndInit()
3131

32-
list, _ := commands.BoardListAll(context.Background(), &rpc.BoardListAllRequest{
32+
list, _ := srv.BoardListAll(context.Background(), &rpc.BoardListAllRequest{
3333
Instance: inst,
3434
SearchArgs: nil,
3535
IncludeHiddenBoards: false,
@@ -44,7 +44,7 @@ func GetInstalledBoards() []string {
4444

4545
// GetInstalledProgrammers is an helper function useful to autocomplete.
4646
// It returns a list of programmers available based on the installed boards
47-
func GetInstalledProgrammers() []string {
47+
func GetInstalledProgrammers(srv rpc.ArduinoCoreServiceServer) []string {
4848
inst := instance.CreateAndInit()
4949

5050
// we need the list of the available fqbn in order to get the list of the programmers
@@ -53,7 +53,7 @@ func GetInstalledProgrammers() []string {
5353
SearchArgs: nil,
5454
IncludeHiddenBoards: false,
5555
}
56-
list, _ := commands.BoardListAll(context.Background(), listAllReq)
56+
list, _ := srv.BoardListAll(context.Background(), listAllReq)
5757

5858
installedProgrammers := make(map[string]string)
5959
for _, board := range list.GetBoards() {

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,10 @@ type Fqbn struct {
3333
}
3434

3535
// AddToCommand adds the flags used to set fqbn to the specified Command
36-
func (f *Fqbn) AddToCommand(cmd *cobra.Command) {
36+
func (f *Fqbn) AddToCommand(cmd *cobra.Command, srv rpc.ArduinoCoreServiceServer) {
3737
cmd.Flags().StringVarP(&f.fqbn, "fqbn", "b", "", tr("Fully Qualified Board Name, e.g.: arduino:avr:uno"))
3838
cmd.RegisterFlagCompletionFunc("fqbn", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
39-
return GetInstalledBoards(), cobra.ShellCompDirectiveDefault
39+
return GetInstalledBoards(srv), cobra.ShellCompDirectiveDefault
4040
})
4141
cmd.Flags().StringSliceVar(&f.boardOptions, "board-options", []string{},
4242
tr("List of board options separated by commas. Or can be used multiple times for multiple options."))

Diff for: ‎internal/cli/arguments/programmer.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ type Programmer struct {
3131
}
3232

3333
// AddToCommand adds the flags used to set the programmer to the specified Command
34-
func (p *Programmer) AddToCommand(cmd *cobra.Command) {
34+
func (p *Programmer) AddToCommand(cmd *cobra.Command, srv rpc.ArduinoCoreServiceServer) {
3535
cmd.Flags().StringVarP(&p.programmer, "programmer", "P", "", tr("Programmer to use, e.g: atmel_ice"))
3636
cmd.RegisterFlagCompletionFunc("programmer", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
37-
return GetInstalledProgrammers(), cobra.ShellCompDirectiveDefault
37+
return GetInstalledProgrammers(srv), cobra.ShellCompDirectiveDefault
3838
})
3939
}
4040

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import (
2727
"github.com/spf13/cobra"
2828
)
2929

30-
func initAttachCommand() *cobra.Command {
30+
func initAttachCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
3131
var port arguments.Port
3232
var fqbn arguments.Fqbn
3333
var programmer arguments.Programmer
@@ -48,9 +48,9 @@ func initAttachCommand() *cobra.Command {
4848
runAttachCommand(sketchPath, &port, fqbn.String(), &programmer)
4949
},
5050
}
51-
fqbn.AddToCommand(attachCommand)
51+
fqbn.AddToCommand(attachCommand, srv)
5252
port.AddToCommand(attachCommand)
53-
programmer.AddToCommand(attachCommand)
53+
programmer.AddToCommand(attachCommand, srv)
5454

5555
return attachCommand
5656
}

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

+6-5
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,14 @@ import (
1919
"os"
2020

2121
"github.com/arduino/arduino-cli/internal/i18n"
22+
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
2223
"github.com/spf13/cobra"
2324
)
2425

2526
var tr = i18n.Tr
2627

2728
// NewCommand created a new `board` command
28-
func NewCommand() *cobra.Command {
29+
func NewCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
2930
boardCommand := &cobra.Command{
3031
Use: "board",
3132
Short: tr("Arduino board commands."),
@@ -34,10 +35,10 @@ func NewCommand() *cobra.Command {
3435
" " + os.Args[0] + " board list",
3536
}
3637

37-
boardCommand.AddCommand(initAttachCommand())
38-
boardCommand.AddCommand(initDetailsCommand())
39-
boardCommand.AddCommand(initListCommand())
40-
boardCommand.AddCommand(initListAllCommand())
38+
boardCommand.AddCommand(initAttachCommand(srv))
39+
boardCommand.AddCommand(initDetailsCommand(srv))
40+
boardCommand.AddCommand(initListCommand(srv))
41+
boardCommand.AddCommand(initListAllCommand(srv))
4142
boardCommand.AddCommand(initSearchCommand())
4243

4344
return boardCommand

Diff for: ‎internal/cli/board/details.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import (
3232
"github.com/spf13/cobra"
3333
)
3434

35-
func initDetailsCommand() *cobra.Command {
35+
func initDetailsCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
3636
var showFullDetails bool
3737
var listProgrammers bool
3838
var fqbn arguments.Fqbn
@@ -48,7 +48,7 @@ func initDetailsCommand() *cobra.Command {
4848
},
4949
}
5050

51-
fqbn.AddToCommand(detailsCommand)
51+
fqbn.AddToCommand(detailsCommand, srv)
5252
detailsCommand.Flags().BoolVarP(&showFullDetails, "full", "f", false, tr("Show full board details"))
5353
detailsCommand.Flags().BoolVarP(&listProgrammers, "list-programmers", "", false, tr("Show list of available programmers"))
5454
detailsCommand.MarkFlagRequired("fqbn")

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ import (
3535
"github.com/spf13/cobra"
3636
)
3737

38-
func initListCommand() *cobra.Command {
38+
func initListCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
3939
var timeoutArg arguments.DiscoveryTimeout
4040
var watch bool
4141
var fqbn arguments.Fqbn
@@ -51,7 +51,7 @@ func initListCommand() *cobra.Command {
5151
}
5252

5353
timeoutArg.AddToCommand(listCommand)
54-
fqbn.AddToCommand(listCommand)
54+
fqbn.AddToCommand(listCommand, srv)
5555
listCommand.Flags().BoolVarP(&watch, "watch", "w", false, tr("Command keeps running and prints list of connected boards whenever there is a change."))
5656
return listCommand
5757
}

Diff for: ‎internal/cli/board/listall.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import (
2121
"os"
2222
"sort"
2323

24-
"github.com/arduino/arduino-cli/commands"
2524
"github.com/arduino/arduino-cli/internal/cli/feedback"
2625
"github.com/arduino/arduino-cli/internal/cli/feedback/result"
2726
"github.com/arduino/arduino-cli/internal/cli/feedback/table"
@@ -33,7 +32,7 @@ import (
3332

3433
var showHiddenBoard bool
3534

36-
func initListAllCommand() *cobra.Command {
35+
func initListAllCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
3736
var listAllCommand = &cobra.Command{
3837
Use: fmt.Sprintf("listall [%s]", tr("boardname")),
3938
Short: tr("List all known boards and their corresponding FQBN."),
@@ -43,19 +42,21 @@ for a specific board if you specify the board name`),
4342
" " + os.Args[0] + " board listall\n" +
4443
" " + os.Args[0] + " board listall zero",
4544
Args: cobra.ArbitraryArgs,
46-
Run: runListAllCommand,
45+
Run: func(cmd *cobra.Command, args []string) {
46+
runListAllCommand(args, srv)
47+
},
4748
}
4849
listAllCommand.Flags().BoolVarP(&showHiddenBoard, "show-hidden", "a", false, tr("Show also boards marked as 'hidden' in the platform"))
4950
return listAllCommand
5051
}
5152

5253
// runListAllCommand list all installed boards
53-
func runListAllCommand(cmd *cobra.Command, args []string) {
54+
func runListAllCommand(args []string, srv rpc.ArduinoCoreServiceServer) {
5455
inst := instance.CreateAndInit()
5556

5657
logrus.Info("Executing `arduino-cli board listall`")
5758

58-
list, err := commands.BoardListAll(context.Background(), &rpc.BoardListAllRequest{
59+
list, err := srv.BoardListAll(context.Background(), &rpc.BoardListAllRequest{
5960
Instance: inst,
6061
SearchArgs: args,
6162
IncludeHiddenBoards: showHiddenBoard,

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ var (
4242
)
4343

4444
// NewCommand created a new `burn-bootloader` command
45-
func NewCommand() *cobra.Command {
45+
func NewCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
4646
burnBootloaderCommand := &cobra.Command{
4747
Use: "burn-bootloader",
4848
Short: tr("Upload the bootloader."),
@@ -52,9 +52,9 @@ func NewCommand() *cobra.Command {
5252
Run: runBootloaderCommand,
5353
}
5454

55-
fqbn.AddToCommand(burnBootloaderCommand)
55+
fqbn.AddToCommand(burnBootloaderCommand, srv)
5656
port.AddToCommand(burnBootloaderCommand)
57-
programmer.AddToCommand(burnBootloaderCommand)
57+
programmer.AddToCommand(burnBootloaderCommand, srv)
5858
burnBootloaderCommand.Flags().BoolVarP(&verify, "verify", "t", false, tr("Verify uploaded binary after the upload."))
5959
burnBootloaderCommand.Flags().BoolVarP(&verbose, "verbose", "v", false, tr("Turns on verbose mode."))
6060
burnBootloaderCommand.Flags().BoolVar(&dryRun, "dry-run", false, tr("Do not perform the actual upload, just log out actions"))

Diff for: ‎internal/cli/cli.go

+13-18
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import (
4444
"github.com/arduino/arduino-cli/internal/cli/version"
4545
"github.com/arduino/arduino-cli/internal/i18n"
4646
"github.com/arduino/arduino-cli/internal/inventory"
47+
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
4748
versioninfo "github.com/arduino/arduino-cli/version"
4849
"github.com/fatih/color"
4950
"github.com/mattn/go-colorable"
@@ -61,11 +62,11 @@ var (
6162
)
6263

6364
// NewCommand creates a new ArduinoCli command root
64-
func NewCommand() *cobra.Command {
65+
func NewCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
6566
cobra.AddTemplateFunc("tr", i18n.Tr)
6667

6768
// ArduinoCli is the root command
68-
arduinoCli := &cobra.Command{
69+
cmd := &cobra.Command{
6970
Use: "arduino-cli",
7071
Short: tr("Arduino CLI."),
7172
Long: tr("Arduino Command Line Interface (arduino-cli)."),
@@ -74,35 +75,27 @@ func NewCommand() *cobra.Command {
7475
PersistentPostRun: postRun,
7576
}
7677

77-
arduinoCli.SetUsageTemplate(getUsageTemplate())
78+
cmd.SetUsageTemplate(getUsageTemplate())
7879

79-
createCliCommandTree(arduinoCli)
80-
81-
return arduinoCli
82-
}
83-
84-
// this is here only for testing
85-
func createCliCommandTree(cmd *cobra.Command) {
86-
cmd.AddCommand(board.NewCommand())
80+
cmd.AddCommand(board.NewCommand(srv))
8781
cmd.AddCommand(cache.NewCommand())
88-
cmd.AddCommand(compile.NewCommand())
82+
cmd.AddCommand(compile.NewCommand(srv))
8983
cmd.AddCommand(completion.NewCommand())
9084
cmd.AddCommand(config.NewCommand())
9185
cmd.AddCommand(core.NewCommand())
9286
cmd.AddCommand(daemon.NewCommand())
9387
cmd.AddCommand(generatedocs.NewCommand())
94-
cmd.AddCommand(lib.NewCommand())
95-
cmd.AddCommand(monitor.NewCommand())
88+
cmd.AddCommand(lib.NewCommand(srv))
89+
cmd.AddCommand(monitor.NewCommand(srv))
9690
cmd.AddCommand(outdated.NewCommand())
9791
cmd.AddCommand(sketch.NewCommand())
9892
cmd.AddCommand(update.NewCommand())
9993
cmd.AddCommand(upgrade.NewCommand())
100-
cmd.AddCommand(upload.NewCommand())
101-
cmd.AddCommand(debug.NewCommand())
102-
cmd.AddCommand(burnbootloader.NewCommand())
94+
cmd.AddCommand(upload.NewCommand(srv))
95+
cmd.AddCommand(debug.NewCommand(srv))
96+
cmd.AddCommand(burnbootloader.NewCommand(srv))
10397
cmd.AddCommand(version.NewCommand())
10498
cmd.AddCommand(feedback.NewCommand())
105-
10699
cmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, tr("Print the logs on the standard output."))
107100
cmd.Flag("verbose").Hidden = true
108101
cmd.PersistentFlags().BoolVar(&verbose, "log", false, tr("Print the logs on the standard output."))
@@ -126,6 +119,8 @@ func createCliCommandTree(cmd *cobra.Command) {
126119
cmd.PersistentFlags().StringSlice("additional-urls", []string{}, tr("Comma-separated list of additional URLs for the Boards Manager."))
127120
cmd.PersistentFlags().Bool("no-color", false, "Disable colored output.")
128121
configuration.BindFlags(cmd, configuration.Settings)
122+
123+
return cmd
129124
}
130125

131126
// convert the string passed to the `--log-level` option to the corresponding

0 commit comments

Comments
 (0)