Skip to content

Commit

Permalink
[breaking] Improved .pde warnings detection subroutines / gRPC LoadSk…
Browse files Browse the repository at this point in the history
…etch API improvement (#2490)

* sketch.GetProfile returns an error if profile is not found

* Added Profile.ToRpc helper

* Added Sketch.ToRpc helper

* Factored Sketch message in gRPC API for 'LoadSketch' call

* Inline function call

* Improved implementation of sketch.GetSketchProfiles

* Moved subroutine that warns about .pde files in feedback package

* Updated docs

* Improved 'sketch archive' parameters check

* Fixed bug detected by integration test... oops
  • Loading branch information
cmaglie committed Jan 8, 2024
1 parent 95753fc commit ea8c281
Show file tree
Hide file tree
Showing 21 changed files with 980 additions and 930 deletions.
2 changes: 1 addition & 1 deletion commands/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ 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 := sketch.LoadSketch(ctx, req)
return resp, convertErrorToRPCStatus(err)
return &rpc.LoadSketchResponse{Sketch: resp}, convertErrorToRPCStatus(err)
}

// SetSketchDefaults FIXMEDOC
Expand Down
7 changes: 4 additions & 3 deletions commands/instances.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,11 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
if err != nil {
return &cmderrors.InvalidArgumentError{Cause: err}
}
profile = sk.GetProfile(req.GetProfile())
if profile == nil {
return &cmderrors.UnknownProfileError{Profile: req.GetProfile()}
p, err := sk.GetProfile(req.GetProfile())
if err != nil {
return err
}
profile = p
responseCallback(&rpc.InitResponse{
Message: &rpc.InitResponse_Profile{
Profile: &rpc.Profile{
Expand Down
51 changes: 3 additions & 48 deletions commands/sketch/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,56 +24,11 @@ import (
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
// LoadSketch collects and returns all information about a sketch
func LoadSketch(ctx context.Context, req *rpc.LoadSketchRequest) (*rpc.Sketch, error) {
sk, err := sketch.New(paths.New(req.GetSketchPath()))
if err != nil {
return nil, &cmderrors.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()

profiles := make([](*rpc.SketchProfile), len(sk.Project.Profiles))
for i, profile := range sk.Project.Profiles {
profiles[i] = &rpc.SketchProfile{
Name: profile.Name,
Fqbn: profile.FQBN,
}
}

defaultProfileResp := &rpc.SketchProfile{}
defaultProfile := sk.GetProfile(sk.Project.DefaultProfile)
if defaultProfile != nil {
defaultProfileResp.Name = defaultProfile.Name
defaultProfileResp.Fqbn = defaultProfile.FQBN
}

return &rpc.LoadSketchResponse{
MainFile: sk.MainFile.String(),
LocationPath: sk.FullPath.String(),
OtherSketchFiles: otherSketchFiles,
AdditionalFiles: additionalFiles,
RootFolderFiles: rootFolderFiles,
DefaultFqbn: sk.GetDefaultFQBN(),
DefaultPort: defaultPort,
DefaultProtocol: defaultProtocol,
Profiles: profiles,
DefaultProfile: defaultProfileResp,
}, nil
return sk.ToRpc(), nil
}
40 changes: 40 additions & 0 deletions docs/UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,46 @@ Here you can find a list of migration guides to handle breaking changes between

## 0.36.0

### The gRPC `cc.arduino.cli.commands.v1.LoadSketchResponse` message has been changed.

Previously the `LoadSketchResponse` containted all the information about the sketch:

```proto
message LoadSketchResponse {
string main_file = 1;
string location_path = 2;
repeated string other_sketch_files = 3;
repeated string additional_files = 4;
repeated string root_folder_files = 5;
string default_fqbn = 6;
string default_port = 7;
string default_protocol = 8;
repeated SketchProfile profiles = 9;
SketchProfile default_profile = 10;
}
```

Now all the metadata have been moved into a specific `Sketch` message:

```proto
message LoadSketchResponse {
Sketch sketch = 1;
}
message Sketch {
string main_file = 1;
string location_path = 2;
repeated string other_sketch_files = 3;
repeated string additional_files = 4;
repeated string root_folder_files = 5;
string default_fqbn = 6;
string default_port = 7;
string default_protocol = 8;
repeated SketchProfile profiles = 9;
SketchProfile default_profile = 10;
}
```

### Drop support for `builtin.tools`

We're dropping the `builtin.tools` support. It was the equivalent of Arduino IDE 1.x bundled tools directory.
Expand Down
9 changes: 9 additions & 0 deletions internal/arduino/sketch/profiles.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"strings"

"github.com/arduino/arduino-cli/internal/arduino/utils"
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
"github.com/arduino/go-paths-helper"
semver "go.bug.st/relaxed-semver"
"gopkg.in/yaml.v3"
Expand Down Expand Up @@ -99,6 +100,14 @@ type Profile struct {
Libraries ProfileRequiredLibraries `yaml:"libraries"`
}

// ToRpc converts this Profile to an rpc.SketchProfile
func (p *Profile) ToRpc() *rpc.SketchProfile {
return &rpc.SketchProfile{
Name: p.Name,
Fqbn: p.FQBN,
}
}

// AsYaml outputs the profile as Yaml
func (p *Profile) AsYaml() string {
res := ""
Expand Down
49 changes: 27 additions & 22 deletions internal/arduino/sketch/sketch.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,11 @@ import (
"sort"
"strings"

"github.com/arduino/arduino-cli/commands/cmderrors"
f "github.com/arduino/arduino-cli/internal/algorithms"
"github.com/arduino/arduino-cli/internal/arduino/globals"
"github.com/arduino/arduino-cli/internal/i18n"
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
"github.com/arduino/go-paths-helper"
)

Expand Down Expand Up @@ -180,15 +183,14 @@ func (s *Sketch) supportedFiles() (*paths.PathList, error) {
return &files, nil
}

// GetProfile returns the requested profile or nil if the profile
// is not found.
func (s *Sketch) GetProfile(profileName string) *Profile {
// GetProfile returns the requested profile or an error if not found
func (s *Sketch) GetProfile(profileName string) (*Profile, error) {
for _, p := range s.Project.Profiles {
if p.Name == profileName {
return p
return p, nil
}
}
return nil
return nil, &cmderrors.UnknownProfileError{Profile: profileName}
}

// checkSketchCasing returns an error if the casing of the sketch folder and the main file are different.
Expand Down Expand Up @@ -278,23 +280,6 @@ func (e *InvalidSketchFolderNameError) Error() string {
return tr("no valid sketch found in %[1]s: missing %[2]s", e.SketchFolder, e.SketchFile)
}

// CheckForPdeFiles returns all files ending with .pde extension
// in sketch, this is mainly used to warn the user that these files
// must be changed to .ino extension.
// When .pde files won't be supported anymore this function must be removed.
func CheckForPdeFiles(sketch *paths.Path) []*paths.Path {
if sketch.IsNotDir() {
sketch = sketch.Parent()
}

files, err := sketch.ReadDirRecursive()
if err != nil {
return []*paths.Path{}
}
files.FilterSuffix(".pde")
return files
}

// DefaultBuildPath generates the default build directory for a given sketch.
// The build path is in a temporary directory and is unique for each sketch.
func (s *Sketch) DefaultBuildPath() *paths.Path {
Expand All @@ -307,3 +292,23 @@ func (s *Sketch) Hash() string {
md5SumBytes := md5.Sum([]byte(path))
return strings.ToUpper(hex.EncodeToString(md5SumBytes[:]))
}

// ToRpc converts this Sketch into a rpc.LoadSketchResponse
func (s *Sketch) ToRpc() *rpc.Sketch {
defaultPort, defaultProtocol := s.GetDefaultPortAddressAndProtocol()
res := &rpc.Sketch{
MainFile: s.MainFile.String(),
LocationPath: s.FullPath.String(),
OtherSketchFiles: s.OtherSketchFiles.AsStrings(),
AdditionalFiles: s.AdditionalFiles.AsStrings(),
RootFolderFiles: s.RootFolderFiles.AsStrings(),
DefaultFqbn: s.GetDefaultFQBN(),
DefaultPort: defaultPort,
DefaultProtocol: defaultProtocol,
Profiles: f.Map(s.Project.Profiles, (*Profile).ToRpc),
}
if defaultProfile, err := s.GetProfile(s.Project.DefaultProfile); err == nil {
res.DefaultProfile = defaultProfile.ToRpc()
}
return res
}
35 changes: 0 additions & 35 deletions internal/arduino/sketch/sketch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,41 +291,6 @@ func TestGenBuildPath(t *testing.T) {
assert.Equal(t, "ACBD18DB4CC2F85CEDEF654FCCC4A4D8", (&Sketch{FullPath: paths.New("foo")}).Hash())
}

func TestCheckForPdeFiles(t *testing.T) {
sketchPath := paths.New("testdata", "SketchSimple")
files := CheckForPdeFiles(sketchPath)
require.Empty(t, files)

sketchPath = paths.New("testdata", "SketchPde")
files = CheckForPdeFiles(sketchPath)
require.Len(t, files, 1)
require.Equal(t, sketchPath.Join("SketchPde.pde"), files[0])

sketchPath = paths.New("testdata", "SketchMultipleMainFiles")
files = CheckForPdeFiles(sketchPath)
require.Len(t, files, 1)
require.Equal(t, sketchPath.Join("SketchMultipleMainFiles.pde"), files[0])

sketchPath = paths.New("testdata", "SketchSimple", "SketchSimple.ino")
files = CheckForPdeFiles(sketchPath)
require.Empty(t, files)

sketchPath = paths.New("testdata", "SketchPde", "SketchPde.pde")
files = CheckForPdeFiles(sketchPath)
require.Len(t, files, 1)
require.Equal(t, sketchPath.Parent().Join("SketchPde.pde"), files[0])

sketchPath = paths.New("testdata", "SketchMultipleMainFiles", "SketchMultipleMainFiles.ino")
files = CheckForPdeFiles(sketchPath)
require.Len(t, files, 1)
require.Equal(t, sketchPath.Parent().Join("SketchMultipleMainFiles.pde"), files[0])

sketchPath = paths.New("testdata", "SketchMultipleMainFiles", "SketchMultipleMainFiles.pde")
files = CheckForPdeFiles(sketchPath)
require.Len(t, files, 1)
require.Equal(t, sketchPath.Parent().Join("SketchMultipleMainFiles.pde"), files[0])
}

func TestNewSketchWithSymlink(t *testing.T) {
sketchPath, _ := paths.New("testdata", "SketchWithSymlink").Abs()
mainFilePath := sketchPath.Join("SketchWithSymlink.ino")
Expand Down
22 changes: 7 additions & 15 deletions internal/cli/arguments/sketch.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"context"

"github.com/arduino/arduino-cli/commands/sketch"
f "github.com/arduino/arduino-cli/internal/algorithms"
"github.com/arduino/arduino-cli/internal/cli/feedback"
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
"github.com/arduino/go-paths-helper"
Expand All @@ -27,8 +28,7 @@ import (

// InitSketchPath returns an instance of paths.Path pointing to sketchPath.
// If sketchPath is an empty string returns the current working directory.
// In both cases it warns the user if he's using deprecated files
func InitSketchPath(path string, printWarnings bool) (sketchPath *paths.Path) {
func InitSketchPath(path string) (sketchPath *paths.Path) {
if path != "" {
sketchPath = paths.New(path)
} else {
Expand All @@ -39,11 +39,6 @@ func InitSketchPath(path string, printWarnings bool) (sketchPath *paths.Path) {
logrus.Infof("Reading sketch from dir: %s", wd)
sketchPath = wd
}
if printWarnings {
if msg := sketch.WarnDeprecatedFiles(sketchPath); msg != "" {
feedback.Warning(msg)
}
}
return sketchPath
}

Expand All @@ -57,13 +52,10 @@ func GetSketchProfiles(sketchPath string) []string {
return nil
}
}
list, _ := sketch.LoadSketch(context.Background(), &rpc.LoadSketchRequest{
SketchPath: sketchPath,
})
profiles := list.GetProfiles()
res := make([]string, len(profiles))
for i, p := range list.GetProfiles() {
res[i] = p.GetName()
sk, err := sketch.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath})
if err != nil {
return nil
}
return res
profiles := sk.GetProfiles()
return f.Map(profiles, (*rpc.SketchProfile).GetName)
}
2 changes: 1 addition & 1 deletion internal/cli/board/attach.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func initAttachCommand() *cobra.Command {
}

func runAttachCommand(path string, port *arguments.Port, fqbn string) {
sketchPath := arguments.InitSketchPath(path, true)
sketchPath := arguments.InitSketchPath(path)

portAddress, portProtocol, _ := port.GetPortAddressAndProtocol(nil, "", "")
newDefaults, err := sketch.SetSketchDefaults(context.Background(), &rpc.SetSketchDefaultsRequest{
Expand Down
4 changes: 2 additions & 2 deletions internal/cli/compile/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,12 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
path = args[0]
}

sketchPath := arguments.InitSketchPath(path, true)

sketchPath := arguments.InitSketchPath(path)
sk, err := sketch.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath.String()})
if err != nil {
feedback.FatalError(err, feedback.ErrGeneric)
}
feedback.WarnAboutDeprecatedFiles(sk)

var inst *rpc.Instance
var profile *rpc.Profile
Expand Down
4 changes: 3 additions & 1 deletion internal/cli/debug/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,13 @@ func runDebugCommand(args []string, portArgs *arguments.Port, fqbnArg *arguments
path = args[0]
}

sketchPath := arguments.InitSketchPath(path, true)
sketchPath := arguments.InitSketchPath(path)
sk, err := sketch.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath.String()})
if err != nil {
feedback.FatalError(err, feedback.ErrGeneric)
}
feedback.WarnAboutDeprecatedFiles(sk)

fqbn, port := arguments.CalculateFQBNAndPort(portArgs, fqbnArg, instance, sk.GetDefaultFqbn(), sk.GetDefaultPort(), sk.GetDefaultProtocol())
debugConfigRequested := &rpc.GetDebugConfigRequest{
Instance: instance,
Expand Down

0 comments on commit ea8c281

Please sign in to comment.