Skip to content

Fix upload command not working as expected when certain flags are used #1429

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.

Sign up for GitHub

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 2 commits into from
Sep 2, 2021
Merged

Fix upload command not working as expected when certain flags are used #1429

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6cf5613
Fix upload command not working as expected when certain flags are used
silvanocerza Sep 1, 2021
51b1be6
Fix absurd upload corner case
silvanocerza Sep 2, 2021
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
4 changes: 4 additions & 0 deletions arduino/sketch/sketch.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ var tr = i18n.Tr
// New creates an Sketch instance by reading all the files composing a sketch and grouping them
// by file type.
func New(path *paths.Path) (*Sketch, error) {
if path == nil {
return nil, fmt.Errorf(tr("sketch path is not valid"))
}

path = path.Canonical()
if !path.IsDir() {
path = path.Parent()
Expand Down
6 changes: 5 additions & 1 deletion arduino/sketch/sketch_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ func TestNew(t *testing.T) {
mainFilePath := sketchFolderPath.Join(fmt.Sprintf("%s.ino", "SketchSimple"))
otherFile := sketchFolderPath.Join("other.cpp")

sketch, err := New(nil)
assert.Nil(t, sketch)
assert.Error(t, err)

// Loading using Sketch folder path
sketch, err := New(sketchFolderPath)
sketch, err = New(sketchFolderPath)
assert.Nil(t, err)
assert.True(t, mainFilePath.EquivalentTo(sketch.MainFile))
assert.True(t, sketchFolderPath.EquivalentTo(sketch.FullPath))
Expand Down
42 changes: 30 additions & 12 deletions cli/upload/upload.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/arduino/arduino-cli/commands/upload"
"github.com/arduino/arduino-cli/i18n"
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
"github.com/arduino/go-paths-helper"
"github.com/spf13/cobra"
)

Expand Down Expand Up @@ -81,27 +82,40 @@ func run(command *cobra.Command, args []string) {
if len(args) > 0 {
path = args[0]
}
sketchPath := arguments.InitSketchPath(path)
var sketchPath *paths.Path
var sk *sketch.Sketch = nil
// If the user explicitly specified a directory that contains binaries to upload
// use those, we don't really need a valid Sketch to upload in this case
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we care about this use case (which used to be supported), where the sketch argument could be used to provide sketch.json metadata when using a binary?:

$ ./arduino-cli version
arduino-cli.exe alpha Version: 0.18.3 Commit: d710b642 Date: 2021-05-14T12:36:58Z

$ ./arduino-cli sketch new /tmp/FooSketch
Sketch created in: C:\Users\per\AppData\Local\Temp\FooSketch

$ ./arduino-cli board attach arduino:avr:uno /tmp/FooSketch
Selected fqbn: arduino:avr:uno

$ ./arduino-cli compile --output-dir /tmp/FooSketchBin /tmp/FooSketch
Sketch uses 444 bytes (1%) of program storage space. Maximum is 32256 bytes.
Global variables use 9 bytes (0%) of dynamic memory, leaving 2039 bytes for local variables. Maximum is 2048 bytes.

$ ./arduino-cli upload -v -p COM10 --input-dir /tmp/FooSketchBin /tmp/FooSketch
"C:\program-files\arduino\cli\arduino-cli_nightly\directories-data\packages\arduino\tools\avrdude\6.3.0-arduino17/bin/avrdude" "-CC:\program-files\arduino\cli\arduino-cli_nightly\directories-data\packages\arduino\tools\avrdude\6.3.0-arduino17/etc/avrdude.conf" -v -V -patmega328p -carduino "-PCOM10" -b115200 -D "-Uflash:w:C:/Users/per/AppData/Local/Temp/FooSketchBin/FooSketch.ino.hex:i"

avrdude: Version 6.3-20190619
         Copyright (c) 2000-2005 Brian Dean, http://www.bdmicro.com/

[...]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh right, didn't think about that case. 🤔
I'll try to fix it.

Copy link
Contributor Author

@silvanocerza silvanocerza Sep 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Fixed with 51b1be6.

if importDir == "" && importFile == "" {
sketchPath = arguments.InitSketchPath(path)

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

sk, err := sketch.New(sketchPath)
if err != nil {
feedback.Errorf(tr("Error during Upload: %v"), err)
os.Exit(errorcodes.ErrGeneric)
var err error
sk, err = sketch.New(sketchPath)
if err != nil {
feedback.Errorf(tr("Error during Upload: %v"), err)
os.Exit(errorcodes.ErrGeneric)
}
}
discoveryPort, err := port.GetPort(instance, sk)
if err != nil {
feedback.Errorf(tr("Error during Upload: %v"), err)
os.Exit(errorcodes.ErrGeneric)
}

if fqbn == "" && sk != nil && sk.Metadata != nil {
// If the user didn't specify an FQBN and a sketch.json file is present
// read it from there.
fqbn = sk.Metadata.CPU.Fqbn
}

userFieldRes, err := upload.SupportedUserFields(context.Background(), &rpc.SupportedUserFieldsRequest{
Instance: instance,
Fqbn: fqbn,
Expand All @@ -118,10 +132,14 @@ func run(command *cobra.Command, args []string) {
fields = arguments.AskForUserFields(userFieldRes.UserFields)
}

if sketchPath != nil {
path = sketchPath.String()
}

if _, err := upload.Upload(context.Background(), &rpc.UploadRequest{
Instance: instance,
Fqbn: fqbn,
SketchPath: sketchPath.String(),
SketchPath: path,
Port: discoveryPort.ToRPC(),
Verbose: verbose,
Verify: verify,
Expand Down
56 changes: 30 additions & 26 deletions i18n/data/en.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ msgstr "Available"
msgid "Available Commands:"
msgstr "Available Commands:"

#: cli/upload/upload.go:61
#: cli/upload/upload.go:62
msgid "Binary file to upload."
msgstr "Binary file to upload."

Expand Down Expand Up @@ -552,7 +552,7 @@ msgstr "Detects and displays a list of boards connected to the current computer.
msgid "Directory containing binaries for debug."
msgstr "Directory containing binaries for debug."

#: cli/upload/upload.go:60
#: cli/upload/upload.go:61
msgid "Directory containing binaries to upload."
msgstr "Directory containing binaries to upload."

Expand All @@ -573,7 +573,7 @@ msgid "Do not install dependencies."
msgstr "Do not install dependencies."

#: cli/burnbootloader/burnbootloader.go:58
#: cli/upload/upload.go:65
#: cli/upload/upload.go:66
msgid "Do not perform the actual upload, just log out actions"
msgstr "Do not perform the actual upload, just log out actions"

Expand Down Expand Up @@ -725,10 +725,10 @@ msgstr "Error during JSON encoding of the output: %v"
#: cli/compile/compile.go:202
#: cli/compile/compile.go:212
#: cli/compile/compile.go:244
#: cli/upload/upload.go:96
#: cli/upload/upload.go:101
#: cli/upload/upload.go:111
#: cli/upload/upload.go:134
#: cli/upload/upload.go:103
#: cli/upload/upload.go:109
#: cli/upload/upload.go:125
#: cli/upload/upload.go:152
msgid "Error during Upload: %v"
msgstr "Error during Upload: %v"

Expand Down Expand Up @@ -1086,7 +1086,7 @@ msgstr "Force skip of post-install scripts (if the CLI is running interactively)
#: cli/burnbootloader/burnbootloader.go:53
#: cli/compile/compile.go:85
#: cli/debug/debug.go:62
#: cli/upload/upload.go:58
#: cli/upload/upload.go:59
msgid "Fully Qualified Board Name, e.g.: arduino:avr:uno"
msgstr "Fully Qualified Board Name, e.g.: arduino:avr:uno"

Expand Down Expand Up @@ -1575,12 +1575,12 @@ msgid "Optional, suppresses almost every output."
msgstr "Optional, suppresses almost every output."

#: cli/compile/compile.go:98
#: cli/upload/upload.go:63
#: cli/upload/upload.go:64
msgid "Optional, turns on verbose mode."
msgstr "Optional, turns on verbose mode."

#: cli/compile/compile.go:109
#: cli/upload/upload.go:64
#: cli/upload/upload.go:65
msgid "Optional, use the specified programmer to upload."
msgstr "Optional, use the specified programmer to upload."

Expand Down Expand Up @@ -1947,7 +1947,7 @@ msgstr "Sketch uses {0} bytes ({2}%%) of program storage space. Maximum is {1} b

#: cli/compile/compile.go:137
#: cli/sketch/archive.go:66
#: cli/upload/upload.go:88
#: cli/upload/upload.go:94
msgid "Sketches with .pde extension are deprecated, please rename the following files to .ino:"
msgstr "Sketches with .pde extension are deprecated, please rename the following files to .ino:"

Expand Down Expand Up @@ -2219,11 +2219,11 @@ msgstr "Upgrades one or all installed platforms to the latest version."
msgid "Upgrading platform %[1]s with %[2]s"
msgstr "Upgrading platform %[1]s with %[2]s"

#: cli/upload/upload.go:50
#: cli/upload/upload.go:51
msgid "Upload Arduino sketches."
msgstr "Upload Arduino sketches."

#: cli/upload/upload.go:51
#: cli/upload/upload.go:52
msgid "Upload Arduino sketches. This does NOT compile the sketch prior to upload."
msgstr "Upload Arduino sketches. This does NOT compile the sketch prior to upload."

Expand Down Expand Up @@ -2252,7 +2252,7 @@ msgid "Upload the bootloader."
msgstr "Upload the bootloader."

#: cli/compile/compile.go:218
#: cli/upload/upload.go:117
#: cli/upload/upload.go:131
msgid "Uploading to specified board using %s protocol requires the following info:"
msgstr "Uploading to specified board using %s protocol requires the following info:"

Expand Down Expand Up @@ -2320,7 +2320,7 @@ msgstr "VERSION_NUMBER"

#: cli/burnbootloader/burnbootloader.go:55
#: cli/compile/compile.go:102
#: cli/upload/upload.go:62
#: cli/upload/upload.go:63
msgid "Verify uploaded binary after the upload."
msgstr "Verify uploaded binary after the upload."

Expand Down Expand Up @@ -2443,7 +2443,7 @@ msgstr "calling %[1]s: %[2]w"
msgid "can't find latest release of %s"
msgstr "can't find latest release of %s"

#: arduino/sketch/sketch.go:101
#: arduino/sketch/sketch.go:105
msgid "can't find main Sketch file in %s"
msgstr "can't find main Sketch file in %s"

Expand Down Expand Up @@ -2535,7 +2535,7 @@ msgstr "creating temp dir for extraction: %s"
msgid "data section exceeds available space in board"
msgstr "data section exceeds available space in board"

#: arduino/sketch/sketch.go:207
#: arduino/sketch/sketch.go:211
msgid "decoding sketch metadata: %s"
msgstr "decoding sketch metadata: %s"

Expand Down Expand Up @@ -2587,7 +2587,7 @@ msgstr "downloading %[1]s tool: %[2]s"
msgid "empty board identifier"
msgstr "empty board identifier"

#: arduino/sketch/sketch.go:196
#: arduino/sketch/sketch.go:200
msgid "encoding sketch metadata: %s"
msgstr "encoding sketch metadata: %s"

Expand All @@ -2607,7 +2607,7 @@ msgstr "error processing response from server"
msgid "error querying Arduino Cloud Api"
msgstr "error querying Arduino Cloud Api"

#: cli/upload/upload.go:72
#: cli/upload/upload.go:73
msgid "error: %s and %s flags cannot be used together"
msgstr "error: %s and %s flags cannot be used together"

Expand Down Expand Up @@ -2710,7 +2710,7 @@ msgstr "getting parent dir of %[1]s: %[2]s"
msgid "getting tool dependencies for platform %[1]s: %[2]s"
msgstr "getting tool dependencies for platform %[1]s: %[2]s"

#: arduino/sketch/sketch.go:151
#: arduino/sketch/sketch.go:155
msgid "importing sketch metadata: %s"
msgstr "importing sketch metadata: %s"

Expand Down Expand Up @@ -2942,7 +2942,7 @@ msgstr "moving extracted archive to destination dir: %s"
msgid "multiple build artifacts found: '%[1]s' and '%[2]s'"
msgstr "multiple build artifacts found: '%[1]s' and '%[2]s'"

#: arduino/sketch/sketch.go:73
#: arduino/sketch/sketch.go:77
msgid "multiple main sketch files found (%[1]v, %[2]v)"
msgstr "multiple main sketch files found (%[1]v, %[2]v)"

Expand Down Expand Up @@ -2970,7 +2970,7 @@ msgstr "no unique root dir in archive, found '%[1]s' and '%[2]s'"
msgid "no upload port provided"
msgstr "no upload port provided"

#: arduino/sketch/sketch.go:259
#: arduino/sketch/sketch.go:263
msgid "no valid sketch found in %[1]s: missing %[2]s"
msgstr "no valid sketch found in %[1]s: missing %[2]s"

Expand Down Expand Up @@ -3095,7 +3095,7 @@ msgstr "reading directory %[1]s: %[2]s"
msgid "reading file %[1]s: %[2]s"
msgstr "reading file %[1]s: %[2]s"

#: arduino/sketch/sketch.go:229
#: arduino/sketch/sketch.go:233
msgid "reading files: %v"
msgstr "reading files: %v"

Expand Down Expand Up @@ -3123,7 +3123,7 @@ msgstr "reading library_index.json: %s"
msgid "reading package root dir: %s"
msgstr "reading package root dir: %s"

#: arduino/sketch/sketch.go:188
#: arduino/sketch/sketch.go:192
msgid "reading sketch metadata %[1]s: %[2]s"
msgstr "reading sketch metadata %[1]s: %[2]s"

Expand Down Expand Up @@ -3185,6 +3185,10 @@ msgstr "searching package root dir: %s"
msgid "setting DTR to OFF"
msgstr "setting DTR to OFF"

#: arduino/sketch/sketch.go:62
msgid "sketch path is not valid"
msgstr "sketch path is not valid"

#: cli/board/attach.go:35
#: cli/sketch/archive.go:38
msgid "sketchPath"
Expand Down Expand Up @@ -3339,7 +3343,7 @@ msgstr "unknown package %s"
msgid "unknown platform %s:%s"
msgstr "unknown platform %s:%s"

#: arduino/sketch/sketch.go:142
#: arduino/sketch/sketch.go:146
msgid "unknown sketch file extension '%s'"
msgstr "unknown sketch file extension '%s'"

Expand All @@ -3359,7 +3363,7 @@ msgstr "upgrade everything to the latest version"
msgid "uploading error: %s"
msgstr "uploading error: %s"

#: arduino/sketch/sketch.go:212
#: arduino/sketch/sketch.go:216
msgid "writing sketch metadata %[1]s: %[2]s"
msgstr "writing sketch metadata %[1]s: %[2]s"

Expand Down
8 changes: 4 additions & 4 deletions i18n/rice-box.go
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions test/test_upload.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def test_upload_with_input_dir_flag(run_command, data_dir, detected_boards):
assert run_command(f"compile -b {fqbn} {sketch_path} --output-dir {output_dir}")

# Upload with --input-dir flag
assert run_command(f"upload -b {fqbn} -p {address} --input-dir {output_dir} {sketch_path}")
assert run_command(f"upload -b {fqbn} -p {address} --input-dir {output_dir}")


def test_upload_with_input_file_flag(run_command, data_dir, detected_boards):
Expand Down Expand Up @@ -320,7 +320,7 @@ def test_upload_with_input_dir_containing_multiple_binaries(run_command, data_di
assert res.failed
assert (
"Error during Upload: "
+ "retrieving build artifacts: "
+ "Error finding build artifacts: "
+ "autodetect build artifact: "
+ "multiple build artifacts found:"
in res.stderr
Expand Down Expand Up @@ -358,7 +358,7 @@ def test_compile_and_upload_combo_sketch_with_mismatched_casing(run_command, dat
# Try to compile
res = run_command(f"compile --clean -b {board.fqbn} -u -p {board.address} {sketch_path}")
assert res.failed
assert "Error during build: opening sketch: no valid sketch found" in res.stderr
assert "Error during build: Can't open sketch: no valid sketch found in" in res.stderr


def test_upload_sketch_with_mismatched_casing(run_command, data_dir, detected_boards, wait_for_board):
Expand All @@ -381,4 +381,4 @@ def test_upload_sketch_with_mismatched_casing(run_command, data_dir, detected_bo
# searching for binaries since the sketch is not valid
res = run_command(f"upload -b {board.fqbn} -p {board.address} {sketch_path}")
assert res.failed
assert "Error during Upload: opening sketch: no valid sketch found" in res.stderr
assert "Error during Upload: no valid sketch found in" in res.stderr