Added gRPC command SetSketchDefaults to change sketch attached board/port (#2217)

* Remove some direct access to sketch.Sketch

* Moved `LoadSketch` command in the proper place

* Removed some accesses to sketch.Sketch

The required information are returned from LoadSketch.

* Added SetSketchDefaults gRPC call

This allows to finally remove wrong access to `sketch.Sketch` from `cli`
package.

* Updated docs

* Fixed integration tests

* Update rpc/cc/arduino/cli/commands/v1/commands.proto

Co-authored-by: Alessio Perugini <[email protected]>

---------

Co-authored-by: Alessio Perugini <[email protected]>

Author: cmaglie

arduino/errors.go

@@ -516,6 +516,19 @@ func (e *CantCreateSketchError) Unwrap() error {
 	return e.Cause
 }
 
+// CantUpdateSketchError is returned when the sketch cannot be updated
+type CantUpdateSketchError struct {
+	Cause error
+}
+
+func (e *CantUpdateSketchError) Error() string {
+	return composeErrorMsg(tr("Can't update sketch"), e.Cause)
+}
+
+func (e *CantUpdateSketchError) Unwrap() error {
+	return e.Cause
+}
+
 // CantOpenSketchError is returned when the sketch is not found or cannot be opened
 type CantOpenSketchError struct {
 	Cause error

commands/daemon/daemon.go

@@ -198,7 +198,13 @@ func (s *ArduinoCoreServerImpl) NewSketch(ctx context.Context, req *rpc.NewSketc
 
 // LoadSketch FIXMEDOC
 func (s *ArduinoCoreServerImpl) LoadSketch(ctx context.Context, req *rpc.LoadSketchRequest) (*rpc.LoadSketchResponse, error) {
-	resp, err := commands.LoadSketch(ctx, req)
+	resp, err := sketch.LoadSketch(ctx, req)
+	return resp, convertErrorToRPCStatus(err)
+}
+
+// SetSketchDefaults FIXMEDOC
+func (s *ArduinoCoreServerImpl) SetSketchDefaults(ctx context.Context, req *rpc.SetSketchDefaultsRequest) (*rpc.SetSketchDefaultsResponse, error) {
+	resp, err := sketch.SetSketchDefaults(ctx, req)
 	return resp, convertErrorToRPCStatus(err)
 }
 

commands/instances.go

@@ -569,38 +569,6 @@ func UpdateIndex(ctx context.Context, req *rpc.UpdateIndexRequest, downloadCB rp
 	return nil
 }
 
-// LoadSketch collects and returns all files composing a sketch
-func LoadSketch(ctx context.Context, req *rpc.LoadSketchRequest) (*rpc.LoadSketchResponse, error) {
-	// TODO: This should be a ToRpc function for the Sketch struct
-	sk, err := sketch.New(paths.New(req.SketchPath))
-	if err != nil {
-		return nil, &arduino.CantOpenSketchError{Cause: err}
-	}
-
-	otherSketchFiles := make([]string, sk.OtherSketchFiles.Len())
-	for i, file := range sk.OtherSketchFiles {
-		otherSketchFiles[i] = file.String()
-	}
-
-	additionalFiles := make([]string, sk.AdditionalFiles.Len())
-	for i, file := range sk.AdditionalFiles {
-		additionalFiles[i] = file.String()
-	}
-
-	rootFolderFiles := make([]string, sk.RootFolderFiles.Len())
-	for i, file := range sk.RootFolderFiles {
-		rootFolderFiles[i] = file.String()
-	}
-
-	return &rpc.LoadSketchResponse{
-		MainFile:         sk.MainFile.String(),
-		LocationPath:     sk.FullPath.String(),
-		OtherSketchFiles: otherSketchFiles,
-		AdditionalFiles:  additionalFiles,
-		RootFolderFiles:  rootFolderFiles,
-	}, nil
-}
-
 // firstUpdate downloads libraries and packages indexes if they don't exist.
 // This ideally is only executed the first time the CLI is run.
 func firstUpdate(ctx context.Context, instance *rpc.Instance, downloadCb func(msg *rpc.DownloadProgress), externalPackageIndexes []*url.URL) error {

commands/sketch/load.go

@@ -0,0 +1,61 @@
+// This file is part of arduino-cli.
+//
+// Copyright 2020 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to [email protected].
+
+package sketch
+
+import (
+	"context"
+
+	"github.com/arduino/arduino-cli/arduino"
+	"github.com/arduino/arduino-cli/arduino/sketch"
+	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
+	paths "github.com/arduino/go-paths-helper"
+)
+
+// LoadSketch collects and returns all files composing a sketch
+func LoadSketch(ctx context.Context, req *rpc.LoadSketchRequest) (*rpc.LoadSketchResponse, error) {
+	// TODO: This should be a ToRpc function for the Sketch struct
+	sk, err := sketch.New(paths.New(req.SketchPath))
+	if err != nil {
+		return nil, &arduino.CantOpenSketchError{Cause: err}
+	}
+
+	otherSketchFiles := make([]string, sk.OtherSketchFiles.Len())
+	for i, file := range sk.OtherSketchFiles {
+		otherSketchFiles[i] = file.String()
+	}
+
+	additionalFiles := make([]string, sk.AdditionalFiles.Len())
+	for i, file := range sk.AdditionalFiles {
+		additionalFiles[i] = file.String()
+	}
+
+	rootFolderFiles := make([]string, sk.RootFolderFiles.Len())
+	for i, file := range sk.RootFolderFiles {
+		rootFolderFiles[i] = file.String()
+	}
+
+	defaultPort, defaultProtocol := sk.GetDefaultPortAddressAndProtocol()
+	return &rpc.LoadSketchResponse{
+		MainFile:         sk.MainFile.String(),
+		LocationPath:     sk.FullPath.String(),
+		OtherSketchFiles: otherSketchFiles,
+		AdditionalFiles:  additionalFiles,
+		RootFolderFiles:  rootFolderFiles,
+		DefaultFqbn:      sk.GetDefaultFQBN(),
+		DefaultPort:      defaultPort,
+		DefaultProtocol:  defaultProtocol,
+	}, nil
+}

commands/sketch/set_defaults.go

@@ -0,0 +1,57 @@
+// This file is part of arduino-cli.
+//
+// Copyright 2020 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to [email protected].
+
+package sketch
+
+import (
+	"context"
+
+	"github.com/arduino/arduino-cli/arduino"
+	"github.com/arduino/arduino-cli/arduino/sketch"
+	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
+	paths "github.com/arduino/go-paths-helper"
+)
+
+// SetSketchDefaults updates the sketch project file (sketch.yaml) with the given defaults
+// for the values `default_fqbn`, `default_port`, and `default_protocol`.
+func SetSketchDefaults(ctx context.Context, req *rpc.SetSketchDefaultsRequest) (*rpc.SetSketchDefaultsResponse, error) {
+	sk, err := sketch.New(paths.New(req.SketchPath))
+	if err != nil {
+		return nil, &arduino.CantOpenSketchError{Cause: err}
+	}
+
+	oldAddress, oldProtocol := sk.GetDefaultPortAddressAndProtocol()
+	res := &rpc.SetSketchDefaultsResponse{
+		DefaultFqbn:         sk.GetDefaultFQBN(),
+		DefaultPortAddress:  oldAddress,
+		DefaultPortProtocol: oldProtocol,
+	}
+
+	if fqbn := req.GetDefaultFqbn(); fqbn != "" {
+		if err := sk.SetDefaultFQBN(fqbn); err != nil {
+			return nil, &arduino.CantUpdateSketchError{Cause: err}
+		}
+		res.DefaultFqbn = fqbn
+	}
+	if newAddress, newProtocol := req.GetDefaultPortAddress(), req.GetDefaultPortProtocol(); newAddress != "" {
+		if err := sk.SetDefaultPort(newAddress, newProtocol); err != nil {
+			return nil, &arduino.CantUpdateSketchError{Cause: err}
+		}
+		res.DefaultPortAddress = newAddress
+		res.DefaultPortProtocol = newProtocol
+	}
+
+	return res, nil
+}

docs/UPGRADING.md

@@ -2,6 +2,13 @@
 
 Here you can find a list of migration guides to handle breaking changes between releases of the CLI.
 
+## 0.34.0
+
+### golang API: `LoadSketch` function has been moved
+
+The function `github.com/arduino/arduino-cli/commands.LoadSketch` has been moved to package
+`github.com/arduino/arduino-cli/commands/sketch.LoadSketch`. You must change the import accordingly.
+
 ## 0.33.0
 
 ### gRPC `cc.arduino.cli.commands.v1.Compile` command now return expanded build_properties by default.

internal/cli/arguments/fqbn.go

@@ -19,7 +19,6 @@ import (
 	"strings"
 
 	"github.com/arduino/arduino-cli/arduino"
-	"github.com/arduino/arduino-cli/arduino/sketch"
 	"github.com/arduino/arduino-cli/internal/cli/feedback"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/spf13/cobra"
@@ -70,12 +69,10 @@ 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, sk *sketch.Sketch) (string, *rpc.Port) {
-	// TODO: REMOVE sketch.Sketch from here
-
+func CalculateFQBNAndPort(portArgs *Port, fqbnArg *Fqbn, instance *rpc.Instance, defaultFQBN, defaultAddress, defaultProtocol string) (string, *rpc.Port) {
 	fqbn := fqbnArg.String()
-	if fqbn == "" && sk != nil {
-		fqbn = sk.GetDefaultFQBN()
+	if fqbn == "" {
+		fqbn = defaultFQBN
 	}
 	if fqbn == "" {
 		if portArgs == nil || portArgs.address == "" {
@@ -88,7 +85,7 @@ func CalculateFQBNAndPort(portArgs *Port, fqbnArg *Fqbn, instance *rpc.Instance,
 		return fqbn, port
 	}
 
-	port, err := portArgs.GetPort(instance, sk)
+	port, err := portArgs.GetPort(instance, defaultAddress, defaultProtocol)
 	if err != nil {
 		feedback.Fatal(tr("Error getting port metadata: %v", err), feedback.ErrGeneric)
 	}

internal/cli/arguments/port.go

@@ -21,7 +21,6 @@ import (
 
 	"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/commands"
 	"github.com/arduino/arduino-cli/commands/board"
 	"github.com/arduino/arduino-cli/internal/cli/feedback"
@@ -57,11 +56,12 @@ func (p *Port) AddToCommand(cmd *cobra.Command) {
 // This method allows will bypass the discoveries if:
 // - a nil instance is passed: in this case the plain port and protocol arguments are returned (even if empty)
 // - a protocol is specified: in this case the discoveries are not needed to autodetect the protocol.
-func (p *Port) GetPortAddressAndProtocol(instance *rpc.Instance, sk *sketch.Sketch) (string, string, error) {
+func (p *Port) GetPortAddressAndProtocol(instance *rpc.Instance, defaultAddress, defaultProtocol string) (string, string, error) {
 	if p.protocol != "" || instance == nil {
 		return p.address, p.protocol, nil
 	}
-	port, err := p.GetPort(instance, sk)
+
+	port, err := p.GetPort(instance, defaultAddress, defaultProtocol)
 	if err != nil {
 		return "", "", err
 	}
@@ -70,15 +70,13 @@ func (p *Port) GetPortAddressAndProtocol(instance *rpc.Instance, sk *sketch.Sket
 
 // GetPort returns the Port obtained by parsing command line arguments.
 // The extra metadata for the ports is obtained using the pluggable discoveries.
-func (p *Port) GetPort(instance *rpc.Instance, sk *sketch.Sketch) (*discovery.Port, error) {
-	// TODO: REMOVE sketch.Sketch from here
+func (p *Port) GetPort(instance *rpc.Instance, defaultAddress, defaultProtocol string) (*discovery.Port, error) {
 	// TODO: REMOVE discovery from here (use board.List instead)
 
 	address := p.address
 	protocol := p.protocol
-
-	if address == "" && sk != nil {
-		address, protocol = sk.GetDefaultPortAddressAndProtocol()
+	if address == "" && (defaultAddress != "" || defaultProtocol != "") {
+		address, protocol = defaultAddress, defaultProtocol
 	}
 	if address == "" {
 		// If no address is provided we assume the user is trying to upload

internal/cli/arguments/sketch.go

@@ -42,15 +42,6 @@ func InitSketchPath(path string) (sketchPath *paths.Path) {
 	return sketchPath
 }
 
-// NewSketch is a helper function useful to create a sketch instance
-func NewSketch(sketchPath *paths.Path) *sketch.Sketch {
-	sketch, err := sketch.New(sketchPath)
-	if err != nil {
-		feedback.Fatal(tr("Error opening sketch: %v", err), feedback.ErrGeneric)
-	}
-	return sketch
-}
-
 // WarnDeprecatedFiles warns the user that a type of sketch files are deprecated
 func WarnDeprecatedFiles(sketchPath *paths.Path) {
 	// .pde files are still supported but deprecated, this warning urges the user to rename them

internal/cli/board/attach.go

@@ -16,11 +16,14 @@
 package board
 
 import (
+	"context"
 	"fmt"
 	"os"
 
+	"github.com/arduino/arduino-cli/commands/sketch"
 	"github.com/arduino/arduino-cli/internal/cli/arguments"
 	"github.com/arduino/arduino-cli/internal/cli/feedback"
+	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/spf13/cobra"
 )
 
@@ -51,37 +54,28 @@ func initAttachCommand() *cobra.Command {
 
 func runAttachCommand(path string, port *arguments.Port, fqbn string) {
 	sketchPath := arguments.InitSketchPath(path)
-	sk := arguments.NewSketch(sketchPath)
 
-	var currentPort *boardAttachPortResult
-	if currentAddress, currentProtocol := sk.GetDefaultPortAddressAndProtocol(); currentAddress != "" {
-		currentPort = &boardAttachPortResult{
-			Address:  currentAddress,
-			Protocol: currentProtocol,
-		}
-	}
-	current := &boardAttachResult{
-		Port: currentPort,
-		Fqbn: sk.GetDefaultFQBN(),
+	portAddress, portProtocol, _ := port.GetPortAddressAndProtocol(nil, "", "")
+	newDefaults, err := sketch.SetSketchDefaults(context.Background(), &rpc.SetSketchDefaultsRequest{
+		SketchPath:          sketchPath.String(),
+		DefaultFqbn:         fqbn,
+		DefaultPortAddress:  portAddress,
+		DefaultPortProtocol: portProtocol,
+	})
+	if err != nil {
+		feedback.FatalError(err, feedback.ErrGeneric)
 	}
-	address, protocol, _ := port.GetPortAddressAndProtocol(nil, sk)
-	if address != "" {
-		if err := sk.SetDefaultPort(address, protocol); err != nil {
-			feedback.Fatal(fmt.Sprintf("%s: %s", tr("Error saving sketch metadata"), err), feedback.ErrGeneric)
-		}
-		current.Port = &boardAttachPortResult{
-			Address:  address,
-			Protocol: protocol,
-		}
+
+	res := &boardAttachResult{
+		Fqbn: newDefaults.GetDefaultFqbn(),
 	}
-	if fqbn != "" {
-		if err := sk.SetDefaultFQBN(fqbn); err != nil {
-			feedback.Fatal(fmt.Sprintf("%s: %s", tr("Error saving sketch metadata"), err), feedback.ErrGeneric)
+	if newDefaults.GetDefaultPortAddress() != "" {
+		res.Port = &boardAttachPortResult{
+			Address:  newDefaults.GetDefaultPortAddress(),
+			Protocol: newDefaults.GetDefaultPortProtocol(),
 		}
-		current.Fqbn = fqbn
 	}
-
-	feedback.PrintResult(current)
+	feedback.PrintResult(res)
 }
 
 type boardAttachPortResult struct {

internal/cli/burnbootloader/burnbootloader.go

@@ -67,7 +67,7 @@ func runBootloaderCommand(command *cobra.Command, args []string) {
 	logrus.Info("Executing `arduino-cli burn-bootloader`")
 
 	// We don't need a Sketch to upload a board's bootloader
-	discoveryPort, err := port.GetPort(instance, nil)
+	discoveryPort, err := port.GetPort(instance, "", "")
 	if err != nil {
 		feedback.Fatal(tr("Error during Upload: %v", err), feedback.ErrGeneric)
 	}