From a1b314cad99b6e1c7feb183711b47c97d6c5ef58 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 1 Nov 2019 12:32:18 +0100 Subject: [PATCH 1/4] Add tests for ResolveFQBN This adds tests for ResolveFQBN (which returns more details than the FindBoardWithFQBN that was already tested), but also adds an extra boards.txt with boards that are derived from another platform/core (i.e. have `build.core=package:core`. These new tests are currently failing, because of a bug and insufficient error handling, which will be fixed in subsequent commits. --- .../packagemanager/package_manager_test.go | 125 ++++++++++++++++++ .../extra_hardware/referenced/avr/boards.txt | 38 ++++++ .../extra_hardware/referenced/samd/boards.txt | 27 ++++ 3 files changed, 190 insertions(+) create mode 100644 arduino/cores/packagemanager/testdata/extra_hardware/referenced/avr/boards.txt create mode 100644 arduino/cores/packagemanager/testdata/extra_hardware/referenced/samd/boards.txt diff --git a/arduino/cores/packagemanager/package_manager_test.go b/arduino/cores/packagemanager/package_manager_test.go index 1dadc096a0d..37bf940117f 100644 --- a/arduino/cores/packagemanager/package_manager_test.go +++ b/arduino/cores/packagemanager/package_manager_test.go @@ -33,6 +33,9 @@ import ( var customHardware = paths.New("testdata", "custom_hardware") var dataDir1 = paths.New("testdata", "data_dir_1") +// Intended to be used alongside dataDir1 +var extraHardware = paths.New("testdata", "extra_hardware") + func TestFindBoardWithFQBN(t *testing.T) { pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware) pm.LoadHardwareFromDirectory(customHardware) @@ -48,6 +51,128 @@ func TestFindBoardWithFQBN(t *testing.T) { require.Equal(t, board.Name(), "Arduino/Genuino Mega or Mega 2560") } +func TestResolveFQBN(t *testing.T) { + // Pass nil, since these paths are only used for installing + pm := packagemanager.NewPackageManager(nil, nil, nil, nil) + // Hardware from main packages directory + pm.LoadHardwareFromDirectory(dataDir1.Join("packages")) + // This contains the arduino:avr core + pm.LoadHardwareFromDirectory(customHardware) + // This contains the referenced:avr core + pm.LoadHardwareFromDirectory(extraHardware) + + fqbn, err := cores.ParseFQBN("arduino:avr:uno") + require.Nil(t, err) + require.NotNil(t, fqbn) + pkg, platformRelease, board, props, buildPlatformRelease, err := pm.ResolveFQBN(fqbn) + require.Nil(t, err) + require.Equal(t, pkg, platformRelease.Platform.Package) + require.NotNil(t, platformRelease) + require.NotNil(t, platformRelease.Platform) + require.Equal(t, platformRelease.Platform.String(), "arduino:avr") + require.NotNil(t, board) + require.Equal(t, board.Name(), "Arduino/Genuino Uno") + require.NotNil(t, props) + require.Equal(t, platformRelease, buildPlatformRelease) + + fqbn, err = cores.ParseFQBN("arduino:avr:mega") + require.Nil(t, err) + require.NotNil(t, fqbn) + pkg, platformRelease, board, props, buildPlatformRelease, err = pm.ResolveFQBN(fqbn) + require.Nil(t, err) + require.Equal(t, pkg, platformRelease.Platform.Package) + require.NotNil(t, platformRelease) + require.NotNil(t, platformRelease.Platform) + require.Equal(t, platformRelease.Platform.String(), "arduino:avr") + require.NotNil(t, board) + require.Equal(t, board.Name(), "Arduino/Genuino Mega or Mega 2560") + require.NotNil(t, props) + require.Equal(t, platformRelease, buildPlatformRelease) + + // Test a board referenced from the main AVR arduino platform + fqbn, err = cores.ParseFQBN("referenced:avr:uno") + require.Nil(t, err) + require.NotNil(t, fqbn) + pkg, platformRelease, board, props, buildPlatformRelease, err = pm.ResolveFQBN(fqbn) + require.Nil(t, err) + require.Equal(t, pkg, platformRelease.Platform.Package) + require.NotNil(t, platformRelease) + require.NotNil(t, platformRelease.Platform) + require.Equal(t, platformRelease.Platform.String(), "referenced:avr") + require.NotNil(t, board) + require.Equal(t, board.Name(), "Referenced Uno") + require.NotNil(t, props) + require.NotNil(t, buildPlatformRelease) + require.NotNil(t, buildPlatformRelease.Platform) + require.Equal(t, buildPlatformRelease.Platform.String(), "arduino:avr") + + // Test a board referenced from the Adafruit SAMD core (this tests + // deriving where the package and core name are different) + fqbn, err = cores.ParseFQBN("referenced:samd:feather_m0") + require.Nil(t, err) + require.NotNil(t, fqbn) + pkg, platformRelease, board, props, buildPlatformRelease, err = pm.ResolveFQBN(fqbn) + require.Nil(t, err) + require.Equal(t, pkg, platformRelease.Platform.Package) + require.NotNil(t, platformRelease) + require.NotNil(t, platformRelease.Platform) + require.Equal(t, platformRelease.Platform.String(), "referenced:samd") + require.NotNil(t, board) + require.Equal(t, board.Name(), "Referenced Feather M0") + require.NotNil(t, props) + require.NotNil(t, buildPlatformRelease) + require.NotNil(t, buildPlatformRelease.Platform) + require.Equal(t, buildPlatformRelease.Platform.String(), "adafruit:samd") + + // Test a board referenced from a non-existent package + fqbn, err = cores.ParseFQBN("referenced:avr:dummy_invalid_package") + require.Nil(t, err) + require.NotNil(t, fqbn) + pkg, platformRelease, board, props, buildPlatformRelease, err = pm.ResolveFQBN(fqbn) + require.NotNil(t, err) + require.Equal(t, pkg, platformRelease.Platform.Package) + require.NotNil(t, platformRelease) + require.NotNil(t, platformRelease.Platform) + require.Equal(t, platformRelease.Platform.String(), "referenced:avr") + require.NotNil(t, board) + require.Equal(t, board.Name(), "Referenced dummy with invalid package") + require.NotNil(t, props) + require.Nil(t, buildPlatformRelease) + + // Test a board referenced from a non-existent platform/architecture + fqbn, err = cores.ParseFQBN("referenced:avr:dummy_invalid_platform") + require.Nil(t, err) + require.NotNil(t, fqbn) + pkg, platformRelease, board, props, buildPlatformRelease, err = pm.ResolveFQBN(fqbn) + require.NotNil(t, err) + require.Equal(t, pkg, platformRelease.Platform.Package) + require.NotNil(t, platformRelease) + require.NotNil(t, platformRelease.Platform) + require.Equal(t, platformRelease.Platform.String(), "referenced:avr") + require.NotNil(t, board) + require.Equal(t, board.Name(), "Referenced dummy with invalid platform") + require.NotNil(t, props) + require.Nil(t, buildPlatformRelease) + + // Test a board referenced from a non-existent core + // Note that ResolveFQBN does not actually check this currently + fqbn, err = cores.ParseFQBN("referenced:avr:dummy_invalid_core") + require.Nil(t, err) + require.NotNil(t, fqbn) + pkg, platformRelease, board, props, buildPlatformRelease, err = pm.ResolveFQBN(fqbn) + require.Nil(t, err) + require.Equal(t, pkg, platformRelease.Platform.Package) + require.NotNil(t, platformRelease) + require.NotNil(t, platformRelease.Platform) + require.Equal(t, platformRelease.Platform.String(), "referenced:avr") + require.NotNil(t, board) + require.Equal(t, board.Name(), "Referenced dummy with invalid core") + require.NotNil(t, props) + require.NotNil(t, buildPlatformRelease) + require.NotNil(t, buildPlatformRelease.Platform) + require.Equal(t, buildPlatformRelease.Platform.String(), "arduino:avr") +} + func TestBoardOptionsFunctions(t *testing.T) { pm := packagemanager.NewPackageManager(customHardware, customHardware, customHardware, customHardware) pm.LoadHardwareFromDirectory(customHardware) diff --git a/arduino/cores/packagemanager/testdata/extra_hardware/referenced/avr/boards.txt b/arduino/cores/packagemanager/testdata/extra_hardware/referenced/avr/boards.txt new file mode 100644 index 00000000000..a242ab52d1b --- /dev/null +++ b/arduino/cores/packagemanager/testdata/extra_hardware/referenced/avr/boards.txt @@ -0,0 +1,38 @@ +# Dummy board that is pretty much identical to the Uno, but defined in a +# different package (to test using a core from a different package where +# the package name and core name are the same). +uno.name=Referenced Uno + +uno.upload.tool=avrdude +uno.upload.protocol=arduino +uno.upload.maximum_size=32256 +uno.upload.maximum_data_size=2048 +uno.upload.speed=115200 + +uno.bootloader.tool=avrdude +uno.bootloader.low_fuses=0xFF +uno.bootloader.high_fuses=0xDE +uno.bootloader.extended_fuses=0xFD +uno.bootloader.unlock_bits=0x3F +uno.bootloader.lock_bits=0x0F +uno.bootloader.file=optiboot/optiboot_atmega328.hex + +uno.build.mcu=atmega328p +uno.build.f_cpu=16000000L +uno.build.board=AVR_UNO +uno.build.core=arduino:arduino +uno.build.variant=standard + +# Dummy board derived from a non-existent package +dummy_invalid_package.name=Referenced dummy with invalid package +dummy_invalid_package.build.core=nonexistent:arduino + +# Dummy board derived from a non-existent core +dummy_invalid_core.name=Referenced dummy with invalid core +dummy_invalid_core.build.core=arduino:nonexistent + +# Dummy board derived from a non-existent platform/architecture. The +# platform is avr, which is implied by the directory this file is in. The +# adafruit package in this test data only supplies a samd platform. +dummy_invalid_platform.name=Referenced dummy with invalid platform +dummy_invalid_platform.build.core=adafruit:arduino diff --git a/arduino/cores/packagemanager/testdata/extra_hardware/referenced/samd/boards.txt b/arduino/cores/packagemanager/testdata/extra_hardware/referenced/samd/boards.txt new file mode 100644 index 00000000000..e988fc204c9 --- /dev/null +++ b/arduino/cores/packagemanager/testdata/extra_hardware/referenced/samd/boards.txt @@ -0,0 +1,27 @@ +# Dummy board that is pretty much identical to the feather m0, but +# defined in a different package (to test using a core from a different +# package where the package name and core name are different). +feather_m0.name=Referenced Feather M0 +feather_m0.upload.tool=bossac +feather_m0.upload.protocol=sam-ba +feather_m0.upload.maximum_size=262144 +feather_m0.upload.offset=0x2000 +feather_m0.upload.use_1200bps_touch=true +feather_m0.upload.wait_for_upload_port=true +feather_m0.upload.native_usb=true +feather_m0.build.mcu=cortex-m0plus +feather_m0.build.f_cpu=48000000L +feather_m0.build.usb_product="Feather M0" +feather_m0.build.usb_manufacturer="Adafruit" +feather_m0.build.board=SAMD_ZERO +feather_m0.build.core=adafruit:arduino +feather_m0.build.extra_flags=-DARDUINO_SAMD_ZERO -DARM_MATH_CM0PLUS -DADAFRUIT_FEATHER_M0 -D__SAMD21G18A__ {build.usb_flags} +feather_m0.build.ldscript=linker_scripts/gcc/flash_with_bootloader.ld +feather_m0.build.openocdscript=openocd_scripts/feather_m0.cfg +feather_m0.build.variant=feather_m0 +feather_m0.build.variant_system_lib= +feather_m0.build.vid=0x239A +feather_m0.build.pid=0x800B +feather_m0.bootloader.tool=openocd +feather_m0.bootloader.file=featherM0/bootloader-feather_m0-v2.0.0-adafruit.5.bin + From f760c386511f6c8fccae59f17aeb3d9555a43475 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 1 Nov 2019 12:38:01 +0100 Subject: [PATCH 2/4] Fix resolution of referenced cores A core is referenced by specifying `build.core=package:core` in board properties. However, the code used the second part (core) both for looking up the the package, as well as for the core to use from that package. The most commonly used core reference is `arduino:arduino` which works because both parts are identical, which is probably why this bug has not shown up before. This commit fixes the bug by simply using the right part to look up the package. --- arduino/cores/packagemanager/package_manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arduino/cores/packagemanager/package_manager.go b/arduino/cores/packagemanager/package_manager.go index 44427537025..63aad7e5ce3 100644 --- a/arduino/cores/packagemanager/package_manager.go +++ b/arduino/cores/packagemanager/package_manager.go @@ -177,7 +177,7 @@ func (pm *PackageManager) ResolveFQBN(fqbn *cores.FQBN) ( buildPlatformRelease := platformRelease coreParts := strings.Split(buildProperties.Get("build.core"), ":") if len(coreParts) > 1 { - referredPackage := coreParts[1] + referredPackage := coreParts[0] buildPackage := pm.Packages[referredPackage] if buildPackage == nil { return targetPackage, platformRelease, board, buildProperties, nil, From f2f3d49989551c00ed7d2c57f1ff2bea7edd2e0d Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 1 Nov 2019 12:55:53 +0100 Subject: [PATCH 3/4] Improve error message when referencing missing package This used to say e.g. missing platform adafruit:referenced:avr required for build where two packagenames (adafruit and referenced) were joined together which makes no sense. Now, it just mentions the missing package, and the fqbn that references it. --- arduino/cores/packagemanager/package_manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arduino/cores/packagemanager/package_manager.go b/arduino/cores/packagemanager/package_manager.go index 63aad7e5ce3..dcf1d38dd23 100644 --- a/arduino/cores/packagemanager/package_manager.go +++ b/arduino/cores/packagemanager/package_manager.go @@ -181,7 +181,7 @@ func (pm *PackageManager) ResolveFQBN(fqbn *cores.FQBN) ( buildPackage := pm.Packages[referredPackage] if buildPackage == nil { return targetPackage, platformRelease, board, buildProperties, nil, - fmt.Errorf("missing package %s:%s required for build", referredPackage, platform) + fmt.Errorf("missing package %s referenced by board %s", referredPackage, fqbn) } buildPlatform := buildPackage.Platforms[fqbn.PlatformArch] buildPlatformRelease = pm.GetInstalledPlatformRelease(buildPlatform) From 2d2742c4cae1b6cc5de4614516e3b67c24e14c31 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 1 Nov 2019 12:57:45 +0100 Subject: [PATCH 4/4] Fix segfault when referenced platform is missing Before, when a platform was referenced through a `build.core` and the package was present by the platform/architecure was missing, a nullpointer was passed to GetInstalledPlatformRelease, which would segfault. Now, a proper error message is returned. --- arduino/cores/packagemanager/package_manager.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arduino/cores/packagemanager/package_manager.go b/arduino/cores/packagemanager/package_manager.go index dcf1d38dd23..ef3920a375d 100644 --- a/arduino/cores/packagemanager/package_manager.go +++ b/arduino/cores/packagemanager/package_manager.go @@ -184,6 +184,10 @@ func (pm *PackageManager) ResolveFQBN(fqbn *cores.FQBN) ( fmt.Errorf("missing package %s referenced by board %s", referredPackage, fqbn) } buildPlatform := buildPackage.Platforms[fqbn.PlatformArch] + if buildPlatform == nil { + return targetPackage, platformRelease, board, buildProperties, nil, + fmt.Errorf("missing platform %s:%s referenced by board %s", referredPackage, fqbn.PlatformArch, fqbn) + } buildPlatformRelease = pm.GetInstalledPlatformRelease(buildPlatform) }