Skip to content

Add back support for sketches with .pde extension and deprecate it #1157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion arduino/globals/globals.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@ package globals
var (
empty struct{}

// MainFileValidExtension is the extension that must be used for files in new sketches
MainFileValidExtension string = ".ino"

// MainFileValidExtensions lists valid extensions for a sketch file
MainFileValidExtensions = map[string]struct{}{
".ino": empty,
MainFileValidExtension: empty,
// .pde extension is deprecated and must not be used for new sketches
".pde": empty,
}

Expand Down
10 changes: 3 additions & 7 deletions arduino/libraries/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
"strings"

"github.com/arduino/arduino-cli/arduino/sketches"
"github.com/arduino/go-paths-helper"
properties "github.com/arduino/go-properties-orderedmap"
"github.com/pkg/errors"
Expand Down Expand Up @@ -172,7 +173,8 @@ func addExamplesToPathList(examplesPath *paths.Path, list *paths.PathList) error
return err
}
for _, file := range files {
if isExample(file) {
_, err := sketches.NewSketchFromPath(file)
if err == nil {
list.Add(file)
} else if file.IsDir() {
if err := addExamplesToPathList(file, list); err != nil {
Expand All @@ -182,9 +184,3 @@ func addExamplesToPathList(examplesPath *paths.Path, list *paths.PathList) error
}
return nil
}

// isExample returns true if examplePath contains an example
func isExample(examplePath *paths.Path) bool {
mainIno := examplePath.Join(examplePath.Base() + ".ino")
return mainIno.Exist() && mainIno.IsNotDir()
}
55 changes: 46 additions & 9 deletions arduino/sketches/sketches.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,17 @@ import (
"fmt"

"github.com/arduino/arduino-cli/arduino/builder"
"github.com/arduino/arduino-cli/arduino/globals"
"github.com/arduino/go-paths-helper"
"github.com/pkg/errors"
)

// Sketch is a sketch for Arduino
type Sketch struct {
Name string
FullPath *paths.Path
Metadata *Metadata
Name string
MainFileExtension string
FullPath *paths.Path
Metadata *Metadata
}

// Metadata is the kind of data associated to a project such as the connected board
Expand All @@ -52,14 +54,32 @@ func NewSketchFromPath(path *paths.Path) (*Sketch, error) {
if !path.IsDir() {
path = path.Parent()
}
sketchFile := path.Join(path.Base() + ".ino")
if !sketchFile.Exist() {
return nil, errors.Errorf("no valid sketch found in %s: missing %s", path, sketchFile.Base())

var mainSketchFile *paths.Path
for ext := range globals.MainFileValidExtensions {
candidateSketchMainFile := path.Join(path.Base() + ext)
if candidateSketchMainFile.Exist() {
if mainSketchFile == nil {
mainSketchFile = candidateSketchMainFile
} else {
return nil, errors.Errorf("multiple main sketch files found (%v, %v)",
mainSketchFile,
candidateSketchMainFile,
)
}
}
}

if mainSketchFile == nil {
sketchFile := path.Join(path.Base() + globals.MainFileValidExtension)
return nil, errors.Errorf("no valid sketch found in %s: missing %s", path, sketchFile)
}

sketch := &Sketch{
FullPath: path,
Name: path.Base(),
Metadata: &Metadata{},
FullPath: path,
MainFileExtension: mainSketchFile.Ext(),
Name: path.Base(),
Metadata: &Metadata{},
}
sketch.ImportMetadata()
return sketch, nil
Expand Down Expand Up @@ -108,3 +128,20 @@ func (s *Sketch) BuildPath() (*paths.Path, error) {
}
return builder.GenBuildPath(s.FullPath), nil
}

// CheckForPdeFiles returns all files ending with .pde extension
// in dir, 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
}
50 changes: 50 additions & 0 deletions arduino/sketches/sketches_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,59 @@ func TestSketchBuildPath(t *testing.T) {
require.NoError(t, err)
require.Contains(t, buildPath.String(), "arduino-sketch-")

// Verifies sketch path is returned if sketch has .pde extension
sketchPath = paths.New("testdata", "SketchPde")
sketch, err = NewSketchFromPath(sketchPath)
require.NoError(t, err)
require.NotNil(t, sketch)
buildPath, err = sketch.BuildPath()
require.NoError(t, err)
require.Contains(t, buildPath.String(), "arduino-sketch-")

// Verifies error is returned if there are multiple main files
sketchPath = paths.New("testdata", "SketchMultipleMainFiles")
sketch, err = NewSketchFromPath(sketchPath)
require.Nil(t, sketch)
require.Error(t, err, "multiple main sketch files found")

// Verifies error is returned if sketch path is not set
sketch = &Sketch{}
buildPath, err = sketch.BuildPath()
require.Nil(t, buildPath)
require.Error(t, err, "sketch path is empty")
}

func TestCheckForPdeFiles(t *testing.T) {
sketchPath := paths.New("testdata", "Sketch1")
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", "Sketch1", "Sketch1.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])
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

void setup() {}
void loop() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

void setup() {}
void loop() {}
3 changes: 3 additions & 0 deletions arduino/sketches/testdata/SketchPde/SketchPde.pde
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

void setup() {}
void loop() {}
10 changes: 10 additions & 0 deletions cli/compile/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"encoding/json"
"os"

"github.com/arduino/arduino-cli/arduino/sketches"
"github.com/arduino/arduino-cli/cli/feedback"
"github.com/arduino/arduino-cli/cli/output"
"github.com/arduino/arduino-cli/configuration"
Expand Down Expand Up @@ -127,6 +128,15 @@ func run(cmd *cobra.Command, args []string) {
}

sketchPath := initSketchPath(path)

// .pde files are still supported but deprecated, this warning urges the user to rename them
if files := sketches.CheckForPdeFiles(sketchPath); len(files) > 0 {
feedback.Error("Sketches with .pde extension are deprecated, please rename the following files to .ino:")
for _, f := range files {
feedback.Error(f)
}
}

Comment on lines 130 to +139
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This piece is duplicated in compile.go, upload.go and archive.go.
On compile.go and upload.go there is also initSketchPath() that is duplicated as well.
This makes me wonder two things:

  • in archive.go we are missing some cases because we miss an initSketchPath(...)
  • we must move everything (the current initSketchPath and the pde check) into a common function args.InitSketchParh(path) or something like that...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably something that we need to tackle in a systematic way and not in this PR or it would make it explode.

// We must read this from settings since the value is set when the binding is accessed from viper,
// accessing it from cobra would only read it if the flag is explicitly set by the user and ignore
// the config file and the env vars.
Expand Down
12 changes: 11 additions & 1 deletion cli/sketch/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ import (
"context"
"os"

"github.com/arduino/arduino-cli/arduino/sketches"
"github.com/arduino/arduino-cli/cli/errorcodes"
"github.com/arduino/arduino-cli/cli/feedback"
"github.com/arduino/arduino-cli/commands/sketch"
rpc "github.com/arduino/arduino-cli/rpc/commands"
"github.com/arduino/go-paths-helper"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -53,11 +55,19 @@ func initArchiveCommand() *cobra.Command {
func runArchiveCommand(cmd *cobra.Command, args []string) {
logrus.Info("Executing `arduino sketch archive`")

sketchPath := ""
sketchPath := "."
if len(args) >= 1 {
sketchPath = args[0]
}

// .pde files are still supported but deprecated, this warning urges the user to rename them
if files := sketches.CheckForPdeFiles(paths.New(sketchPath)); len(files) > 0 {
feedback.Error("Sketches with .pde extension are deprecated, please rename the following files to .ino:")
for _, f := range files {
feedback.Error(f)
}
}

archivePath := ""
if len(args) == 2 {
archivePath = args[1]
Expand Down
9 changes: 9 additions & 0 deletions cli/upload/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"context"
"os"

"github.com/arduino/arduino-cli/arduino/sketches"
"github.com/arduino/arduino-cli/cli/errorcodes"
"github.com/arduino/arduino-cli/cli/feedback"
"github.com/arduino/arduino-cli/cli/instance"
Expand Down Expand Up @@ -83,6 +84,14 @@ func run(command *cobra.Command, args []string) {
}
sketchPath := initSketchPath(path)

// .pde files are still supported but deprecated, this warning urges the user to rename them
if files := sketches.CheckForPdeFiles(sketchPath); len(files) > 0 {
feedback.Error("Sketches with .pde extension are deprecated, please rename the following files to .ino:")
for _, f := range files {
feedback.Error(f)
}
}

if _, err := upload.Upload(context.Background(), &rpc.UploadReq{
Instance: instance,
Fqbn: fqbn,
Expand Down
27 changes: 9 additions & 18 deletions commands/sketch/archive.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"path/filepath"
"strings"

"github.com/arduino/arduino-cli/arduino/sketches"
rpc "github.com/arduino/arduino-cli/rpc/commands"
paths "github.com/arduino/go-paths-helper"
)
Expand All @@ -37,27 +38,17 @@ func ArchiveSketch(ctx context.Context, req *rpc.ArchiveSketchReq) (*rpc.Archive
sketchPath = paths.New(".")
}

sketchPath, err := sketchPath.Clean().Abs()
sketch, err := sketches.NewSketchFromPath(sketchPath)
if err != nil {
return nil, fmt.Errorf("Error getting absolute sketch path %v", err)
return nil, err
}

// Get the sketch name and make sketchPath point to the ino file
if sketchPath.IsDir() {
sketchName = sketchPath.Base()
sketchPath = sketchPath.Join(sketchName + ".ino")
} else if sketchPath.Ext() == ".ino" {
sketchName = strings.TrimSuffix(sketchPath.Base(), ".ino")
}

// Checks if it's really a sketch
if sketchPath.NotExist() {
return nil, fmt.Errorf("specified path is not a sketch: %v", sketchPath.String())
}
sketchPath = sketch.FullPath
sketchName = sketch.Name

archivePath := paths.New(req.ArchivePath)
if archivePath == nil {
archivePath = sketchPath.Parent().Parent()
archivePath = sketchPath.Parent()
}

archivePath, err = archivePath.Clean().Abs()
Expand All @@ -76,7 +67,7 @@ func ArchiveSketch(ctx context.Context, req *rpc.ArchiveSketchReq) (*rpc.Archive
return nil, fmt.Errorf("archive already exists")
}

filesToZip, err := sketchPath.Parent().ReadDirRecursive()
filesToZip, err := sketchPath.ReadDirRecursive()
if err != nil {
return nil, fmt.Errorf("Error retrieving sketch files: %v", err)
}
Expand All @@ -94,7 +85,7 @@ func ArchiveSketch(ctx context.Context, req *rpc.ArchiveSketchReq) (*rpc.Archive
for _, f := range filesToZip {

if !req.IncludeBuildDir {
filePath, err := sketchPath.Parent().Parent().RelTo(f)
filePath, err := sketchPath.Parent().RelTo(f)
if err != nil {
return nil, fmt.Errorf("Error calculating relative file path: %v", err)
}
Expand All @@ -107,7 +98,7 @@ func ArchiveSketch(ctx context.Context, req *rpc.ArchiveSketchReq) (*rpc.Archive

// We get the parent path since we want the archive to unpack as a folder.
// If we don't do this the archive would contain all the sketch files as top level.
err = addFileToSketchArchive(zipWriter, f, sketchPath.Parent().Parent())
err = addFileToSketchArchive(zipWriter, f, sketchPath.Parent())
if err != nil {
return nil, fmt.Errorf("Error adding file to archive: %v", err)
}
Expand Down
17 changes: 10 additions & 7 deletions commands/upload/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
bldr "github.com/arduino/arduino-cli/arduino/builder"
"github.com/arduino/arduino-cli/arduino/cores"
"github.com/arduino/arduino-cli/arduino/cores/packagemanager"
"github.com/arduino/arduino-cli/arduino/globals"
"github.com/arduino/arduino-cli/arduino/serialutils"
"github.com/arduino/arduino-cli/arduino/sketches"
"github.com/arduino/arduino-cli/commands"
Expand Down Expand Up @@ -452,7 +453,7 @@ func determineBuildPathAndSketchName(importFile, importDir string, sketch *sketc

// Case 4: only sketch specified. In this case we use the generated build path
// and the given sketch name.
return bldr.GenBuildPath(sketch.FullPath), sketch.Name + ".ino", nil
return bldr.GenBuildPath(sketch.FullPath), sketch.Name + sketch.MainFileExtension, nil
}

func detectSketchNameFromBuildPath(buildPath *paths.Path) (string, error) {
Expand All @@ -462,11 +463,13 @@ func detectSketchNameFromBuildPath(buildPath *paths.Path) (string, error) {
}

if absBuildPath, err := buildPath.Abs(); err == nil {
candidateName := absBuildPath.Base() + ".ino"
f := files.Clone()
f.FilterPrefix(candidateName + ".")
if f.Len() > 0 {
return candidateName, nil
for ext := range globals.MainFileValidExtensions {
candidateName := absBuildPath.Base() + ext
f := files.Clone()
f.FilterPrefix(candidateName + ".")
if f.Len() > 0 {
return candidateName, nil
}
}
}

Expand All @@ -479,7 +482,7 @@ func detectSketchNameFromBuildPath(buildPath *paths.Path) (string, error) {

// Sometimes we may have particular files like:
// Blink.ino.with_bootloader.bin
if filepath.Ext(name) != ".ino" {
if _, ok := globals.MainFileValidExtensions[filepath.Ext(name)]; !ok {
// just ignore those files
continue
}
Expand Down
Loading