Fix upload properties not overridden by protocol specific ones (#1422)

Author: silvanocerza

commands/upload/upload.go

@@ -275,7 +275,7 @@ func runProgramAction(pm *packagemanager.PackageManager,
 	uploadProperties.Set("runtime.os", properties.GetOSSuffix())
 	uploadProperties.Merge(boardPlatform.Properties)
 	uploadProperties.Merge(boardPlatform.RuntimeProperties())
-	uploadProperties.Merge(boardProperties)
+	uploadProperties.Merge(overrideProtocolProperties(action, port.Protocol, boardProperties))
 	uploadProperties.Merge(uploadProperties.SubTree("tools." + uploadToolID))
 	if programmer != nil {
 		uploadProperties.Merge(programmer.Properties)
@@ -616,3 +616,24 @@ func detectSketchNameFromBuildPath(buildPath *paths.Path) (string, error) {
 	}
 	return candidateName, nil
 }
+
+// overrideProtocolProperties returns a copy of props overriding action properties with
+// specified protocol properties.
+//
+// For example passing the below properties and "upload" as action and "serial" as protocol:
+//	upload.speed=256
+//	upload.serial.speed=57600
+//	upload.network.speed=19200
+//
+// will return:
+//	upload.speed=57600
+//	upload.serial.speed=57600
+//	upload.network.speed=19200
+func overrideProtocolProperties(action, protocol string, props *properties.Map) *properties.Map {
+	res := props.Clone()
+	subtree := props.SubTree(fmt.Sprintf("%s.%s", action, protocol))
+	for k, v := range subtree.AsMap() {
+		res.Set(fmt.Sprintf("%s.%s", action, k), v)
+	}
+	return res
+}

commands/upload/upload_test.go

@@ -311,3 +311,37 @@ tools.arduino_ota.upload.field.some_field=This is a really long label that ideal
 	require.Equal(t, userFields[0].Label, "This is a really long label that ideally must nev…")
 	require.False(t, userFields[0].Secret)
 }
+
+func TestOverrideProtocolProperties(t *testing.T) {
+	props, err := properties.LoadFromBytes([]byte(`
+	upload.speed=256
+	upload.serial.speed=57600
+	upload.network.speed=19200
+	upload.unrelated_property=ok`))
+	require.NoError(t, err)
+
+	res := overrideProtocolProperties("upload", "serial", props)
+	require.Equal(t, res.Get("upload.speed"), "57600")
+	require.Equal(t, res.Get("upload.serial.speed"), "57600")
+	require.Equal(t, res.Get("upload.network.speed"), "19200")
+	require.Equal(t, res.Get("upload.unrelated_property"), "ok")
+
+	res = overrideProtocolProperties("upload", "network", props)
+	require.Equal(t, res.Get("upload.speed"), "19200")
+	require.Equal(t, res.Get("upload.serial.speed"), "57600")
+	require.Equal(t, res.Get("upload.network.speed"), "19200")
+	require.Equal(t, res.Get("upload.unrelated_property"), "ok")
+
+	res = overrideProtocolProperties("upload", "some_other_protocol", props)
+	require.Equal(t, res.Get("upload.speed"), "256")
+	require.Equal(t, res.Get("upload.serial.speed"), "57600")
+	require.Equal(t, res.Get("upload.network.speed"), "19200")
+	require.Equal(t, res.Get("upload.unrelated_property"), "ok")
+
+	res = overrideProtocolProperties("bootloader", "serial", props)
+	require.Equal(t, res.Get("upload.speed"), "256")
+	require.Equal(t, res.Get("upload.serial.speed"), "57600")
+	require.Equal(t, res.Get("upload.network.speed"), "19200")
+	require.Equal(t, res.Get("upload.unrelated_property"), "ok")
+
+}

docs/platform-specification.md

@@ -857,11 +857,11 @@ tools.avrdude.upload.pattern="{cmd.path}" "-C{config.path}" {upload.verbose} -p{
 If necessary the same property can be defined multiple times for different protocols:
 
 ```
-leonardo.upload.serial.maximum_size=28672
-leonardo.upload.network.maximum_size=256
+leonardo.upload.serial.speed=57600
+leonardo.upload.network.speed=19200
 ```
 
-The two above properties will be available as **{upload.serial.maximum_size}** and **{upload.network.maximum_size}**.
+The two above properties will be available as **{upload.speed}**, the value will depend on the protocol used to upload.
 
 #### Properties from pluggable discovery