Skip to content
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

Reintroduce the `--input-file` flag for the `upload` command #777

Merged
merged 16 commits into from Sep 7, 2020
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -15,8 +15,8 @@ venv

# gRPC client example folder
/client_example/client_example
*.bin
*.elf
/client_example/**/*.bin
/client_example/**/*.elf

# Misc.
.DS_Store
@@ -20,6 +20,7 @@ import (
"fmt"

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

// Sketch is a sketch for Arduino
@@ -43,9 +44,17 @@ type BoardMetadata struct {

// NewSketchFromPath loads a sketch from the specified path
func NewSketchFromPath(path *paths.Path) (*Sketch, error) {
path, err := path.Abs()
if err != nil {
return nil, errors.Errorf("getting sketch path: %s", err)
}
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())
}
sketch := &Sketch{
FullPath: path,
Name: path.Base(),
@@ -36,6 +36,7 @@ var (
verbose bool
verify bool
importDir string
importFile string
programmer string
)

@@ -47,19 +48,28 @@ func NewCommand() *cobra.Command {
Long: "Upload Arduino sketches. This does NOT compile the sketch prior to upload.",
Example: " " + os.Args[0] + " upload /home/user/Arduino/MySketch",
Args: cobra.MaximumNArgs(1),
PreRun: checkFlagsConflicts,
Run: run,
}

uploadCommand.Flags().StringVarP(&fqbn, "fqbn", "b", "", "Fully Qualified Board Name, e.g.: arduino:avr:uno")
uploadCommand.Flags().StringVarP(&port, "port", "p", "", "Upload port, e.g.: COM10 or /dev/ttyACM0")
uploadCommand.Flags().StringVarP(&importDir, "input-dir", "", "", "Directory containing binaries to upload.")
uploadCommand.Flags().StringVarP(&importFile, "input-file", "i", "", "Binary file to upload.")
uploadCommand.Flags().BoolVarP(&verify, "verify", "t", false, "Verify uploaded binary after the upload.")
uploadCommand.Flags().BoolVarP(&verbose, "verbose", "v", false, "Optional, turns on verbose mode.")
uploadCommand.Flags().StringVarP(&programmer, "programmer", "P", "", "Optional, use the specified programmer to upload or 'list' to list supported programmers.")

return uploadCommand
}

func checkFlagsConflicts(command *cobra.Command, args []string) {
This conversation was marked as resolved by cmaglie

This comment has been minimized.

@rsora

rsora Sep 7, 2020
Author Contributor

Since we have this check here at lower level https://github.com/arduino/arduino-cli/pull/777/files#diff-2cb7c2b612feb221e1a0e4c63af8ba4bR456

Is this function redundant?

This comment has been minimized.

@cmaglie

cmaglie Sep 7, 2020
Member

The error message from the lower level is:
Error: retrieving build artifacts: importFile and importDir cannot be used together

that is ok-ish but the message in the preprun function is:
error: --input-file and --input-dir flags cannot be used together

that is much more clear in the command line context, so I will keep it even if redundant.

if importFile != "" && importDir != "" {
feedback.Errorf("error: --input-file and --input-dir flags cannot be used together")
os.Exit(errorcodes.ErrBadArgument)
}
}

func run(command *cobra.Command, args []string) {
instance, err := instance.CreateInstance()
if err != nil {
@@ -80,6 +90,7 @@ func run(command *cobra.Command, args []string) {
Port: port,
Verbose: verbose,
Verify: verify,
ImportFile: importFile,
ImportDir: importDir,
Programmer: programmer,
}, os.Stdout, os.Stderr); err != nil {
@@ -26,14 +26,16 @@ import (
dbg "github.com/arduino/arduino-cli/rpc/debug"
"github.com/arduino/go-paths-helper"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

var customHardware = paths.New("testdata", "custom_hardware")
var dataDir = paths.New("testdata", "data_dir", "packages")
var sketch = "hello"
var sketchPath = paths.New("testdata", sketch)

func TestGetCommandLine(t *testing.T) {
customHardware := paths.New("testdata", "custom_hardware")
dataDir := paths.New("testdata", "data_dir", "packages")
sketch := "hello"
sketchPath := paths.New("testdata", sketch)
require.NoError(t, sketchPath.ToAbs())

pm := packagemanager.NewPackageManager(nil, nil, nil, nil)
pm.LoadHardwareFromDirectory(customHardware)
pm.LoadHardwareFromDirectory(dataDir)
@@ -59,9 +61,9 @@ func TestGetCommandLine(t *testing.T) {
fmt.Sprintf(" -c \"gdb_port pipe\" -c \"telnet_port 0\" -c init -c halt %s/build/arduino-test.samd.arduino_zero_edbg/hello.ino.elf", sketchPath)

command, err := getCommandLine(req, pm)
assert.Nil(t, err)
require.Nil(t, err)
commandToTest := strings.Join(command[:], " ")
assert.Equal(t, filepath.FromSlash(goldCommand), filepath.FromSlash(commandToTest))
require.Equal(t, filepath.FromSlash(goldCommand), filepath.FromSlash(commandToTest))

// Other samd boards such as mkr1000 can be debugged using an external tool such as Atmel ICE connected to
// the board debug port
@@ -83,5 +85,4 @@ func TestGetCommandLine(t *testing.T) {
assert.Nil(t, err)
commandToTest2 := strings.Join(command2[:], " ")
assert.Equal(t, filepath.FromSlash(goldCommand2), filepath.FromSlash(commandToTest2))

}
Empty file.
@@ -37,6 +37,7 @@ func BurnBootloader(ctx context.Context, req *rpc.BurnBootloaderReq, outStream i
err := runProgramAction(
pm,
nil, // sketch
"", // importFile
"", // importDir
req.GetFqbn(),
req.GetPort(),
Empty file.
@@ -20,6 +20,7 @@ import (
"fmt"
"io"
"net/url"
"path/filepath"
"strings"
"time"

@@ -32,6 +33,7 @@ import (
rpc "github.com/arduino/arduino-cli/rpc/commands"
paths "github.com/arduino/go-paths-helper"
properties "github.com/arduino/go-properties-orderedmap"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"go.bug.st/serial"
)
@@ -42,12 +44,9 @@ func Upload(ctx context.Context, req *rpc.UploadReq, outStream io.Writer, errStr

// TODO: make a generic function to extract sketch from request
// and remove duplication in commands/compile.go
if req.GetSketchPath() == "" {
return nil, fmt.Errorf("missing sketchPath")
}
sketchPath := paths.New(req.GetSketchPath())
sketch, err := sketches.NewSketchFromPath(sketchPath)
if err != nil {
if err != nil && req.GetImportDir() == "" && req.GetImportFile() == "" {
return nil, fmt.Errorf("opening sketch: %s", err)
}

@@ -56,6 +55,7 @@ func Upload(ctx context.Context, req *rpc.UploadReq, outStream io.Writer, errStr
err = runProgramAction(
pm,
sketch,
req.GetImportFile(),
req.GetImportDir(),
req.GetFqbn(),
req.GetPort(),
@@ -73,10 +73,11 @@ func Upload(ctx context.Context, req *rpc.UploadReq, outStream io.Writer, errStr
}

func runProgramAction(pm *packagemanager.PackageManager,
sketch *sketches.Sketch, importDir string, fqbnIn string, port string,
sketch *sketches.Sketch,
importFile, importDir, fqbnIn, port string,
programmerID string,
verbose, verify, burnBootloader bool,
outStream io.Writer, errStream io.Writer) error {
outStream, errStream io.Writer) error {

if burnBootloader && programmerID == "" {
return fmt.Errorf("no programmer specified for burning bootloader")
@@ -239,30 +240,19 @@ func runProgramAction(pm *packagemanager.PackageManager,
uploadProperties.Set("program.verify", uploadProperties.Get("program.params.noverify"))
}

var importPath *paths.Path
if !burnBootloader {
if sketch == nil {
return fmt.Errorf(("no sketch specified"))
}

if importDir != "" {
importPath = paths.New(importDir)
} else {
// TODO: Create a function to obtain importPath from sketch
importPath = sketch.FullPath
// Add FQBN (without configs part) to export path
fqbnSuffix := strings.Replace(fqbn.StringWithoutConfig(), ":", ".", -1)
importPath = importPath.Join("build").Join(fqbnSuffix)
importPath, sketchName, err := determineBuildPathAndSketchName(importFile, importDir, sketch, fqbn)
if err != nil {
return errors.Errorf("retrieving build artifacts: %s", err)
}

if !importPath.Exist() {
return fmt.Errorf("compiled sketch not found in %s", importPath)
}
if !importPath.IsDir() {
return fmt.Errorf("expected compiled sketch in directory %s, but is a file instead", importPath)
}
uploadProperties.SetPath("build.path", importPath)
uploadProperties.Set("build.project_name", sketch.Name+".ino")
uploadProperties.Set("build.project_name", sketchName)
}

// If not using programmer perform some action required
@@ -449,3 +439,102 @@ func waitForNewSerialPort() (string, error) {

return "", nil
}

func determineBuildPathAndSketchName(importFile, importDir string, sketch *sketches.Sketch, fqbn *cores.FQBN) (*paths.Path, string, error) {
// In general, compiling a sketch will produce a set of files that are
// named as the sketch but have different extensions, for example Sketch.ino
// may produce: Sketch.ino.bin; Sketch.ino.hex; Sketch.ino.zip; etc...
// These files are created together in the build directory and anyone of
// them may be required for upload.

// The upload recipes are already written using the 'build.project_name' property
// concatenated with an explicit extension. To perform the upload we should now
// determine the project name (e.g. 'sketch.ino) and set the 'build.project_name'
// property accordingly, together with the 'build.path' property to point to the
// directory containing the build artifacts.

// Case 1: importFile flag has been specified
if importFile != "" {
if importDir != "" {
return nil, "", fmt.Errorf("importFile and importDir cannot be used together")
}

// We have a path like "path/to/my/build/SketchName.ino.bin". We are going to
// ignore the extension and set:
// - "build.path" as "path/to/my/build"
// - "build.project_name" as "SketchName.ino"

importFilePath := paths.New(importFile)
if !importFilePath.Exist() {
return nil, "", fmt.Errorf("binary file not found in %s", importFilePath)
}
return importFilePath.Parent(), strings.TrimSuffix(importFilePath.Base(), importFilePath.Ext()), nil
}

if importDir != "" {
// Case 2: importDir flag has been specified

// In this case we have a build path but ignore the sketch name, we'll
// try to determine the sketch name by applying some euristics to the build folder.
// - "build.path" as importDir
// - "build.project_name" after trying to autodetect it from the build folder.
buildPath := paths.New(importDir)
sketchName, err := detectSketchNameFromBuildPath(buildPath)
if err != nil {
return nil, "", errors.Errorf("autodetect build artifact: %s", err)
}
return buildPath, sketchName, nil
}

// Case 3: nothing given...
if sketch == nil {
return nil, "", fmt.Errorf("no sketch or build directory/file specified")
}

// Case 4: only sketch specified. In this case we use the default sketch build path
// and the given sketch name.

// TODO: Create a function to obtain importPath from sketch
// Add FQBN (without configs part) to export path
if fqbn == nil {
return nil, "", fmt.Errorf("missing FQBN")
}
fqbnSuffix := strings.Replace(fqbn.StringWithoutConfig(), ":", ".", -1)
return sketch.FullPath.Join("build").Join(fqbnSuffix), sketch.Name + ".ino", nil
}

func detectSketchNameFromBuildPath(buildPath *paths.Path) (string, error) {
files, err := buildPath.ReadDir()
if err != nil {
return "", err
}

candidateName := ""
var candidateFile *paths.Path
for _, file := range files {
// Build artifacts are usually names as "Blink.ino.hex" or "Blink.ino.bin".
// Extract the "Blink.ino" part
name := strings.TrimSuffix(file.Base(), file.Ext())

// Sometimes we may have particular files like:
// Blink.ino.with_bootloader.bin
if filepath.Ext(name) != ".ino" {
// just ignore those files
continue
}

if candidateName == "" {
candidateName = name
candidateFile = file
}

if candidateName != name {
return "", errors.Errorf("multiple build artifacts found: '%s' and '%s'", candidateFile, file)
}
}

if candidateName == "" {
return "", errors.New("could not find a valid build artifact")
}
return candidateName, nil
}
ProTip! Use n and p to navigate between commits in a pull request.