diff --git a/commands/upload/testdata/hardware/alice/avr/boards.txt b/commands/upload/testdata/hardware/alice/avr/boards.txt new file mode 100644 index 00000000000..651d9639e88 --- /dev/null +++ b/commands/upload/testdata/hardware/alice/avr/boards.txt @@ -0,0 +1,23 @@ +board1.name=board1 +board1.conf.board=conf-board1 +board1.upload.tool=one +board1.upload.protocol=protocol +board1.upload.speed=speed +board1.bootloader.tool=one +board1.bootloader.fuses=0xFF +board1.bootloader.file=niceboot/niceboot.hex + + +board2.name=board2 +board2.conf.board=conf-board1 +board2.upload.tool=one-noport +board2.upload.protocol=protocol +board2.upload.speed=speed + +board2.bootloader.tool=one +board2.bootloader.low_fuses=0xFF +board2.bootloader.high_fuses=0xDE +board2.bootloader.extended_fuses=0xFD +board2.bootloader.unlock_bits=0x3F +board2.bootloader.lock_bits=0x0F +board2.bootloader.file=optiboot/optiboot_atmega328.hex diff --git a/commands/upload/testdata/hardware/alice/avr/platform.txt b/commands/upload/testdata/hardware/alice/avr/platform.txt new file mode 100644 index 00000000000..4e55d111915 --- /dev/null +++ b/commands/upload/testdata/hardware/alice/avr/platform.txt @@ -0,0 +1,79 @@ +name=Upload Test Platform (Alice) +version=1.0.0 + +# Upload test 1 +tools.one.cmd.path=echo +tools.one.conf.general=conf-general +tools.one.upload.conf=conf-upload +tools.one.upload.params.verbose=verbose +tools.one.upload.params.quiet=quiet +tools.one.upload.params.verify=verify +tools.one.upload.params.noverify=noverify +tools.one.upload.pattern={cmd.path} {conf.board} {conf.general} {upload.conf} {upload.verbose} {upload.verify} {upload.protocol} "{serial.port}" -b{upload.speed} "{build.path}/{build.project_name}.hex" + +tools.one.program.conf=conf-program +tools.one.program.params.verbose=verbose +tools.one.program.params.quiet=quiet +tools.one.program.params.verify=verify +tools.one.program.params.noverify=noverify +tools.one.program.pattern={cmd.path} {conf.board} {conf.general} {program.conf} {program.verbose} {program.verify} {program.protocol} "{serial.port}" -b{upload.speed} "{build.path}/{build.project_name}.hex" + +tools.one.erase.conf=conf-erase +tools.one.erase.params.verbose=verbose +tools.one.erase.params.quiet=quiet +tools.one.erase.params.verify=verify +tools.one.erase.params.noverify=noverify +tools.one.erase.pattern={cmd.path} ERASE {conf.board} {conf.general} {erase.conf} {erase.verbose} {erase.verify} {protocol} "{serial.port}" -b{upload.speed} + +tools.one.bootloader.conf=conf-bootloader +tools.one.bootloader.params.verbose=verbose +tools.one.bootloader.params.quiet=quiet +tools.one.bootloader.params.verify=verify +tools.one.bootloader.params.noverify=noverify +tools.one.bootloader.pattern={cmd.path} BURN {conf.board} {conf.general} {bootloader.conf} {bootloader.verbose} {bootloader.verify} {protocol} "{serial.port}" -b{upload.speed} -F{bootloader.fuses} "{runtime.platform.path}/bootloaders/{bootloader.file}" + +# Upload test 2 +tools.one-noport.cmd.path=echo +tools.one-noport.conf.general=conf-general +tools.one-noport.upload.conf=conf-upload +tools.one-noport.upload.params.verbose=verbose +tools.one-noport.upload.params.quiet=quiet +tools.one-noport.upload.params.verify=verify +tools.one-noport.upload.params.noverify=noverify +tools.one-noport.upload.pattern={cmd.path} {conf.board} {conf.general} {upload.conf} {upload.verbose} {upload.verify} {upload.protocol} -b{upload.speed} "{build.path}/{build.project_name}.hex" + +tools.one-noport.program.conf=conf-program +tools.one-noport.program.params.verbose=verbose +tools.one-noport.program.params.quiet=quiet +tools.one-noport.program.params.verify=verify +tools.one-noport.program.params.noverify=noverify +tools.one-noport.program.pattern={cmd.path} {conf.board} {conf.general} {program.conf} {program.verbose} {program.verify} {program.protocol} -b{upload.speed} "{build.path}/{build.project_name}.hex" + +# Recipes for "tool.one-extra-params" +tools.one-extra-params.cmd.path=echo +tools.one-extra-params.conf.general=conf-general + +tools.one-extra-params.program.conf=conf-program +tools.one-extra-params.program.params.verbose=verbose +tools.one-extra-params.program.params.quiet=quiet +tools.one-extra-params.program.params.verify=verify +tools.one-extra-params.program.params.noverify=noverify +tools.one-extra-params.program.pattern={cmd.path} {conf.board} {conf.general} {program.conf} {program.verbose} {program.verify} {program.protocol} {program.extra_params} -b{upload.speed} "{build.path}/{build.project_name}.hex" + +# Upload with programmer test 1 +tools.two.cmd.path=echo +tools.two.conf.general=conf-two-general + +tools.two.erase.conf=conf-two-erase +tools.two.erase.params.verbose=verbose +tools.two.erase.params.quiet=quiet +tools.two.erase.params.verify=verify +tools.two.erase.params.noverify=noverify +tools.two.erase.pattern={cmd.path} ERASE {conf.board} {conf.general} {erase.conf} {erase.verbose} {erase.verify} {bootloader.protocol} "{serial.port}" -b{upload.speed} + +tools.two.bootloader.conf=conf-two-bootloader +tools.two.bootloader.params.verbose=verbose +tools.two.bootloader.params.quiet=quiet +tools.two.bootloader.params.verify=verify +tools.two.bootloader.params.noverify=noverify +tools.two.bootloader.pattern={cmd.path} BURN {conf.board} {conf.general} {bootloader.conf} {bootloader.verbose} {bootloader.verify} {bootloader.protocol} "{serial.port}" -b{upload.speed} -F{bootloader.fuses} "{runtime.platform.path}/bootloaders/{bootloader.file}" diff --git a/commands/upload/testdata/hardware/alice/avr/programmers.txt b/commands/upload/testdata/hardware/alice/avr/programmers.txt new file mode 100644 index 00000000000..cb5fa415154 --- /dev/null +++ b/commands/upload/testdata/hardware/alice/avr/programmers.txt @@ -0,0 +1,19 @@ +progr1.name=Programmer 1 +progr1.program.protocol=progprotocol +progr1.program.tool=one +progr1.protocol=genprog1protocol + +progr2.name=Programmer 2 +progr2.program.protocol=prog2protocol +progr2.program.tool=one-noport + +progr3.name=Programmer 3 +progr3.program.protocol=prog3protocol +progr3.program.tool=one-extra-params +progr3.program.extra_params={serial.port} + +progr4.name=Programmer 4 +progr4.program.protocol=prog4protocol-upload +progr4.program.tool=one +progr4.bootloader.protocol=prog4protocol-bootloader +progr4.bootloader.tool=two diff --git a/commands/upload/upload.go b/commands/upload/upload.go index a26bdb6ceff..917865af2f9 100644 --- a/commands/upload/upload.go +++ b/commands/upload/upload.go @@ -117,23 +117,8 @@ func runProgramAction(pm *packagemanager.PackageManager, WithField("buildPlatform", buildPlatform). Tracef("Upload data") - // Load upload tool definitions - var uploadToolName string - var uploadToolPlatform *cores.PlatformRelease + // Extract programmer properties (when specified) var programmer *cores.Programmer - - if burnBootloader { - uploadToolName = boardProperties.Get("bootloader.tool") - uploadToolPlatform = boardPlatform - if uploadToolName == "" { - return fmt.Errorf("cannot get programmer tool: undefined 'bootloader.tool' in boards.txt") - } - logrus. - WithField("uploadToolName", uploadToolName). - WithField("uploadToolPlatform", uploadToolPlatform). - Trace("Upload tool from 'bootloader.tool' property") - } - if programmerID != "" { programmer = boardPlatform.Programmers[programmerID] if programmer == nil { @@ -143,36 +128,54 @@ func runProgramAction(pm *packagemanager.PackageManager, if programmer == nil { return fmt.Errorf("programmer '%s' not available", programmerID) } - uploadToolName = programmer.Properties.Get("program.tool") - uploadToolPlatform = programmer.PlatformRelease - if uploadToolName == "" { - return fmt.Errorf("cannot get programmer tool: undefined 'program.tool' property") + } + + // Determine upload tool + var uploadToolID string + { + toolProperty := "upload.tool" + if burnBootloader { + toolProperty = "bootloader.tool" + } else if programmer != nil { + toolProperty = "program.tool" + } + + // create a temporary configuration only for the selection of upload tool + props := properties.NewMap() + props.Merge(boardPlatform.Properties) + props.Merge(boardPlatform.RuntimeProperties()) + props.Merge(boardProperties) + if programmer != nil { + props.Merge(programmer.Properties) + } + if t, ok := props.GetOk(toolProperty); ok { + uploadToolID = t + } else { + return fmt.Errorf("cannot get programmer tool: undefined '%s' property", toolProperty) } - logrus. - WithField("uploadToolName", uploadToolName). - WithField("uploadToolPlatform", uploadToolPlatform). - Trace("Upload tool from --programmer parameter") + } + + var uploadToolPlatform *cores.PlatformRelease + if programmer != nil { + uploadToolPlatform = programmer.PlatformRelease } else { - uploadToolName = boardProperties.Get("upload.tool") uploadToolPlatform = boardPlatform - if uploadToolName == "" { - return fmt.Errorf("cannot get upload tool: undefined 'upload.tool' property") - } - if split := strings.Split(uploadToolName, ":"); len(split) > 2 { - return fmt.Errorf("invalid 'upload.tool' property: %s", uploadToolName) - } else if len(split) == 2 { - uploadToolName = split[1] - uploadToolPlatform = pm.GetInstalledPlatformRelease( - pm.FindPlatform(&packagemanager.PlatformReference{ - Package: split[0], - PlatformArchitecture: boardPlatform.Platform.Architecture, - }), - ) - } - logrus. - WithField("uploadToolName", uploadToolName). - WithField("uploadToolPlatform", uploadToolPlatform). - Trace("Upload tool") + } + logrus. + WithField("uploadToolID", uploadToolID). + WithField("uploadToolPlatform", uploadToolPlatform). + Trace("Upload tool") + + if split := strings.Split(uploadToolID, ":"); len(split) > 2 { + return fmt.Errorf("invalid 'upload.tool' property: %s", uploadToolID) + } else if len(split) == 2 { + uploadToolID = split[1] + uploadToolPlatform = pm.GetInstalledPlatformRelease( + pm.FindPlatform(&packagemanager.PlatformReference{ + Package: split[0], + PlatformArchitecture: boardPlatform.Platform.Architecture, + }), + ) } // Build configuration for upload @@ -183,9 +186,7 @@ func runProgramAction(pm *packagemanager.PackageManager, uploadProperties.Merge(boardPlatform.Properties) uploadProperties.Merge(boardPlatform.RuntimeProperties()) uploadProperties.Merge(boardProperties) - - uploadToolProperties := uploadProperties.SubTree("tools." + uploadToolName) - uploadProperties.Merge(uploadToolProperties) + uploadProperties.Merge(uploadProperties.SubTree("tools." + uploadToolID)) if programmer != nil { uploadProperties.Merge(programmer.Properties) } @@ -234,9 +235,13 @@ func runProgramAction(pm *packagemanager.PackageManager, if verify { uploadProperties.Set("upload.verify", uploadProperties.Get("upload.params.verify")) uploadProperties.Set("program.verify", uploadProperties.Get("program.params.verify")) + uploadProperties.Set("erase.verify", uploadProperties.Get("erase.params.verify")) + uploadProperties.Set("bootloader.verify", uploadProperties.Get("bootloader.params.verify")) } else { uploadProperties.Set("upload.verify", uploadProperties.Get("upload.params.noverify")) uploadProperties.Set("program.verify", uploadProperties.Get("program.params.noverify")) + uploadProperties.Set("erase.verify", uploadProperties.Get("erase.params.noverify")) + uploadProperties.Set("bootloader.verify", uploadProperties.Get("bootloader.params.noverify")) } if !burnBootloader { @@ -307,7 +312,7 @@ func runProgramAction(pm *packagemanager.PackageManager, } } - // Build recipe for upload + // Run recipes for upload if burnBootloader { if err := runTool("erase.pattern", uploadProperties, outStream, errStream, verbose); err != nil { return fmt.Errorf("chip erase error: %s", err) diff --git a/commands/upload/upload_test.go b/commands/upload/upload_test.go index 54f34155e7e..29cc5e6be77 100644 --- a/commands/upload/upload_test.go +++ b/commands/upload/upload_test.go @@ -16,12 +16,16 @@ package upload import ( + "bytes" "fmt" + "strings" "testing" "github.com/arduino/arduino-cli/arduino/cores" + "github.com/arduino/arduino-cli/arduino/cores/packagemanager" "github.com/arduino/arduino-cli/arduino/sketches" paths "github.com/arduino/go-paths-helper" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" ) @@ -119,3 +123,95 @@ func TestDetermineBuildPathAndSketchName(t *testing.T) { }) } } + +func TestUploadPropertiesComposition(t *testing.T) { + pm := packagemanager.NewPackageManager(nil, nil, nil, nil) + err := pm.LoadHardwareFromDirectory(paths.New("testdata", "hardware")) + require.NoError(t, err) + buildPath1 := paths.New("testdata", "build_path_1") + logrus.SetLevel(logrus.TraceLevel) + type test struct { + importDir *paths.Path + fqbn string + port string + programmer string + burnBootloader bool + expectedOutput string + expectedOutput2 string + } + + cwdPath, err := paths.Getwd() + require.NoError(t, err) + cwd := strings.ReplaceAll(cwdPath.String(), "\\", "/") + + tests := []test{ + // 0: classic upload, requires port + {buildPath1, "alice:avr:board1", "port", "", false, "conf-board1 conf-general conf-upload $$VERBOSE-VERIFY$$ protocol port -bspeed testdata/build_path_1/sketch.ino.hex\n", ""}, + {buildPath1, "alice:avr:board1", "", "", false, "FAIL", ""}, + // 2: classic upload, no port + {buildPath1, "alice:avr:board2", "port", "", false, "conf-board1 conf-general conf-upload $$VERBOSE-VERIFY$$ protocol -bspeed testdata/build_path_1/sketch.ino.hex\n", ""}, + {buildPath1, "alice:avr:board2", "", "", false, "conf-board1 conf-general conf-upload $$VERBOSE-VERIFY$$ protocol -bspeed testdata/build_path_1/sketch.ino.hex\n", ""}, + + // 4: upload with programmer, requires port + {buildPath1, "alice:avr:board1", "port", "progr1", false, "conf-board1 conf-general conf-program $$VERBOSE-VERIFY$$ progprotocol port -bspeed testdata/build_path_1/sketch.ino.hex\n", ""}, + {buildPath1, "alice:avr:board1", "", "progr1", false, "FAIL", ""}, + // 6: upload with programmer, no port + {buildPath1, "alice:avr:board1", "port", "progr2", false, "conf-board1 conf-general conf-program $$VERBOSE-VERIFY$$ prog2protocol -bspeed testdata/build_path_1/sketch.ino.hex\n", ""}, + {buildPath1, "alice:avr:board1", "", "progr2", false, "conf-board1 conf-general conf-program $$VERBOSE-VERIFY$$ prog2protocol -bspeed testdata/build_path_1/sketch.ino.hex\n", ""}, + // 8: upload with programmer, require port through extra params + {buildPath1, "alice:avr:board1", "port", "progr3", false, "conf-board1 conf-general conf-program $$VERBOSE-VERIFY$$ prog3protocol port -bspeed testdata/build_path_1/sketch.ino.hex\n", ""}, + {buildPath1, "alice:avr:board1", "", "progr3", false, "FAIL", ""}, + + // 10: burn bootloader, require port + {buildPath1, "alice:avr:board1", "port", "", true, "FAIL", ""}, // requires programmer + {buildPath1, "alice:avr:board1", "port", "progr1", true, + "ERASE conf-board1 conf-general conf-erase $$VERBOSE-VERIFY$$ genprog1protocol port -bspeed\n", + "BURN conf-board1 conf-general conf-bootloader $$VERBOSE-VERIFY$$ genprog1protocol port -bspeed -F0xFF " + cwd + "/testdata/hardware/alice/avr/bootloaders/niceboot/niceboot.hex\n"}, + + // 12: burn bootloader, preferences override from programmers.txt + {buildPath1, "alice:avr:board1", "port", "progr4", true, + "ERASE conf-board1 conf-two-general conf-two-erase $$VERBOSE-VERIFY$$ prog4protocol-bootloader port -bspeed\n", + "BURN conf-board1 conf-two-general conf-two-bootloader $$VERBOSE-VERIFY$$ prog4protocol-bootloader port -bspeed -F0xFF " + cwd + "/testdata/hardware/alice/avr/bootloaders/niceboot/niceboot.hex\n"}, + } + + testRunner := func(t *testing.T, test test, verboseVerify bool) { + outStream := &bytes.Buffer{} + errStream := &bytes.Buffer{} + err := runProgramAction( + pm, + nil, // sketch + "", // importFile + test.importDir.String(), // importDir + test.fqbn, // FQBN + test.port, // port + test.programmer, // programmer + verboseVerify, // verbose + verboseVerify, // verify + test.burnBootloader, // burnBootloader + outStream, + errStream, + ) + verboseVerifyOutput := "verbose verify" + if !verboseVerify { + verboseVerifyOutput = "quiet noverify" + } + if test.expectedOutput == "FAIL" { + require.Error(t, err) + } else { + require.NoError(t, err) + outFiltered := strings.ReplaceAll(outStream.String(), "\r", "") + outFiltered = strings.ReplaceAll(outFiltered, "\\", "/") + require.Contains(t, outFiltered, strings.ReplaceAll(test.expectedOutput, "$$VERBOSE-VERIFY$$", verboseVerifyOutput)) + require.Contains(t, outFiltered, strings.ReplaceAll(test.expectedOutput2, "$$VERBOSE-VERIFY$$", verboseVerifyOutput)) + } + } + + for i, test := range tests { + t.Run(fmt.Sprintf("SubTest%02d", i), func(t *testing.T) { + testRunner(t, test, false) + }) + t.Run(fmt.Sprintf("SubTest%02d-WithVerifyAndVerbose", i), func(t *testing.T) { + testRunner(t, test, true) + }) + } +}