Add support for Pluggable Discoveries (#1333)

* [skip changelog] Add DiscoveryManager to PackageManager

* Add loading of PluggableDiscoveries when loading a platform release

* Added compatibility layer for non-pluggable platforms

* Implemented board list with discoveries

* Implemented discovery loading after initialization

* Implemented board watch with discoveries

* Fix load discoveries tests

* Fix some issues with board list watcher

* Fix FindToolsRequiredFromPlatformRelease not returning discoveries

* Enhanced handling of some discoveries states

* Fix PackageManager reset

* Add function to convert discovery.Port to rpc.Port

* Moved reference argument parsing to new package

* Fix functions docstrings

* Remove duplicated code to initialize Sketch path

* Add property conversion for platform not supporting pluggable discovery

* Fix board list watch not working

* Fix crash when converting Port to rpc struct

* Add generic Port argument

* Change gRPC upload functions to use new Port message

* Add support for upload user fields

* Fix upload unit tests

* Fix code naming issues

* Added builtin:mdns-discovery

* Do not panic if discovery tool is not installed

* Implemented port/protocol detection at CLI startup time

* Perform 1200bps-touch only on serial ports

* Added missing properties for pluggable upload

* Correctly implemented 'board list' timeout option

* Updated mdns-discovery to 0.9.2

* Add documentation

* Add board properties to board list command and gRPC function

* Fix documentation and code comments

Co-authored-by: per1234 <accounts@perglass.com>

* Fix crash when attempting upload without specifying port address

* Fix unit tests

* Update go-properties-orderedmap to fix discovery properties issues

* Fix more documentation

Co-authored-by: per1234 <accounts@perglass.com>

* Clarify pluggable discovery specification

* More documentation fixes

* Add upload_port properties docs in platform specification

* Change links from pluggable discovery RFC to official docs

* Add more upload mock integration tests

* Fix integration tests

* Change property to declare pluggable discoveries

* Change property to declare pluggable discoveries

* Fix documentation

Co-authored-by: per1234 <accounts@perglass.com>

* Fix loading of platform not supporting pluggable discovery

* Fix more documentation

Co-authored-by: per1234 <accounts@perglass.com>

* Add pluggable discovery states documentation

* Enhanced handling of pluggable discoveries states

* Discoveries processes are now killed if the HELLO command fails

* Add pluggable discovery logging

* Enhanced handling of failing pluggable discoveries

* Fix pluggable discoveries parallelization

* Discoveries event channels are now created when start sync is called

* Cached ports are now reset on discovery stop

* Renamed ListSync methods to ListCachedPorts

* Pluggable discovery upload user fields are now limited to 50 chars

* Fix i18n strings

* Fix failing integration tests

* Fix i18n data

* Fix integration tests again

* [skip changelog] Internationalize strings added for pluggable discovery support (#1384)

* Update docs/pluggable-discovery-specification.md

Co-authored-by: per1234 <accounts@perglass.com>

* Fix failing workflows

* Updated upload-mock tests for generation

* Added a lot of mock upload test (also with programmer option)

* test_upload_mock: Handle '{' and '}' in recipes

* network ota: autoconvert network_patter from legacy

* Automatically add port detection properties for network discovery

* Slightly improved 'board list' text output

* Default 'board list' timeout to 1s

* Added some code review fixes

* Added unit test for legacy-package conversion to pluggable discovery

Co-authored-by: Cristian Maglie <c.maglie@arduino.cc>
Co-authored-by: per1234 <accounts@perglass.com>

Author: 3 people

.flake8

@@ -5,6 +5,8 @@
 
 [flake8]
 doctests = True
+per-file-ignores =
+    test/test_upload_mock.py:E501
 ignore =
     E741,
     # W503 and W504 are mutually exclusive. PEP 8 recommends line break before.

arduino/cores/packagemanager/loader.go

@@ -23,6 +23,7 @@ import (
 	"strings"
 
 	"github.com/arduino/arduino-cli/arduino/cores"
+	"github.com/arduino/arduino-cli/arduino/discovery"
 	"github.com/arduino/arduino-cli/configuration"
 	"github.com/arduino/go-paths-helper"
 	properties "github.com/arduino/go-properties-orderedmap"
@@ -320,8 +321,11 @@ func (pm *PackageManager) loadPlatformRelease(platform *cores.PlatformRelease, p
 		return fmt.Errorf(tr("loading %[1]s: %[2]s"), platformTxtLocalPath, err)
 	}
 
-	if platform.Properties.SubTree("discovery").Size() > 0 {
+	if platform.Properties.SubTree("pluggable_discovery").Size() > 0 {
 		platform.PluggableDiscoveryAware = true
+	} else {
+		platform.Properties.Set("pluggable_discovery.required.0", "builtin:serial-discovery")
+		platform.Properties.Set("pluggable_discovery.required.1", "builtin:mdns-discovery")
 	}
 
 	if platform.Platform.Name == "" {
@@ -337,8 +341,11 @@ func (pm *PackageManager) loadPlatformRelease(platform *cores.PlatformRelease, p
 
 	// Create programmers properties
 	if programmersProperties, err := properties.SafeLoad(programmersTxtPath.String()); err == nil {
-		for programmerID, programmerProperties := range programmersProperties.FirstLevelOf() {
-			platform.Programmers[programmerID] = pm.loadProgrammer(programmerProperties)
+		for programmerID, programmerProps := range programmersProperties.FirstLevelOf() {
+			if !platform.PluggableDiscoveryAware {
+				convertUploadToolsToPluggableDiscovery(programmerProps)
+			}
+			platform.Programmers[programmerID] = pm.loadProgrammer(programmerProps)
 			platform.Programmers[programmerID].PlatformRelease = platform
 		}
 	} else {
@@ -349,9 +356,71 @@ func (pm *PackageManager) loadPlatformRelease(platform *cores.PlatformRelease, p
 		return fmt.Errorf(tr("loading boards: %s"), err)
 	}
 
+	if !platform.PluggableDiscoveryAware {
+		convertLegacyPlatformToPluggableDiscovery(platform)
+	}
 	return nil
 }
 
+func convertLegacyPlatformToPluggableDiscovery(platform *cores.PlatformRelease) {
+	toolsProps := platform.Properties.SubTree("tools").FirstLevelOf()
+	for toolName, toolProps := range toolsProps {
+		if !toolProps.ContainsKey("upload.network_pattern") {
+			continue
+		}
+
+		// Convert network_pattern configuration to pluggable discovery
+		convertedToolName := toolName + "__pluggable_network"
+		convertedProps := convertLegacyNetworkPatternToPluggableDiscovery(toolProps, convertedToolName)
+
+		// Merge the converted properties in the root configuration
+		platform.Properties.Merge(convertedProps)
+
+		// Add the network upload to the boards using the old method
+		for _, board := range platform.Boards {
+			oldUploadTool := board.Properties.Get("upload.tool")
+			if oldUploadTool == toolName && !board.Properties.ContainsKey("upload.tool.network") {
+				board.Properties.Set("upload.tool.network", convertedToolName)
+
+				// Add identification properties for network protocol
+				i := 0
+				for {
+					if !board.Properties.ContainsKey(fmt.Sprintf("upload_port.%d.vid", i)) {
+						break
+					}
+					i++
+				}
+				board.Properties.Set(fmt.Sprintf("upload_port.%d.board", i), board.BoardID)
+			}
+		}
+	}
+}
+
+func convertLegacyNetworkPatternToPluggableDiscovery(props *properties.Map, newToolName string) *properties.Map {
+	pattern, ok := props.GetOk("upload.network_pattern")
+	if !ok {
+		return nil
+	}
+	props.Remove("upload.network_pattern")
+	pattern = strings.ReplaceAll(pattern, "{serial.port}", "{upload.port.address}")
+	pattern = strings.ReplaceAll(pattern, "{network.port}", "{upload.port.properties.port}")
+	if strings.Contains(pattern, "{network.password}") {
+		props.Set("upload.field.password", "Password")
+		props.Set("upload.field.password.secret", "true")
+		pattern = strings.ReplaceAll(pattern, "{network.password}", "{upload.field.password}")
+	}
+	props.Set("upload.pattern", pattern)
+
+	prefix := "tools." + newToolName + "."
+	res := properties.NewMap()
+	for _, k := range props.Keys() {
+		v := props.Get(k)
+		res.Set(prefix+k, v)
+		// fmt.Println("ADDED:", prefix+k+"="+v)
+	}
+	return res
+}
+
 func (pm *PackageManager) loadProgrammer(programmerProperties *properties.Map) *cores.Programmer {
 	return &cores.Programmer{
 		Name:       programmerProperties.Get("name"),
@@ -388,12 +457,6 @@ func (pm *PackageManager) loadBoards(platform *cores.PlatformRelease) error {
 	// set all other boards properties
 	delete(propertiesByBoard, "menu")
 
-	if !platform.PluggableDiscoveryAware {
-		for _, boardProperties := range propertiesByBoard {
-			convertVidPidIdentificationPropertiesToPluggableDiscovery(boardProperties)
-		}
-	}
-
 	skippedBoards := []string{}
 	for boardID, boardProperties := range propertiesByBoard {
 		var board *cores.Board
@@ -412,6 +475,12 @@ func (pm *PackageManager) loadBoards(platform *cores.PlatformRelease) error {
 				goto next_board
 			}
 		}
+
+		if !platform.PluggableDiscoveryAware {
+			convertVidPidIdentificationPropertiesToPluggableDiscovery(boardProperties)
+			convertUploadToolsToPluggableDiscovery(boardProperties)
+		}
+
 		// The board's ID must be available in a board's properties since it can
 		// be used in all configuration files for several reasons, like setting compilation
 		// flags depending on the board id.
@@ -468,6 +537,22 @@ func convertVidPidIdentificationPropertiesToPluggableDiscovery(boardProperties *
 	}
 }
 
+func convertUploadToolsToPluggableDiscovery(props *properties.Map) {
+	actions := []string{"upload", "bootloader", "program"}
+	for _, action := range actions {
+		if !props.ContainsKey(fmt.Sprintf("%s.tool.default", action)) {
+			tool, found := props.GetOk(fmt.Sprintf("%s.tool", action))
+			if !found {
+				// Just skip it, ideally this must never happen but if a platform
+				// doesn't define an expected upload.tool, bootloader.tool or program.tool
+				// there will be other issues further down the road after this conversion
+				continue
+			}
+			props.Set(fmt.Sprintf("%s.tool.default", action), tool)
+		}
+	}
+}
+
 func (pm *PackageManager) loadToolsFromPackage(targetPackage *cores.Package, toolsPath *paths.Path) []*status.Status {
 	pm.Log.Infof("Loading tools from dir: %s", toolsPath)
 
@@ -587,3 +672,100 @@ func (pm *PackageManager) LoadToolsFromBundleDirectory(toolsPath *paths.Path) er
 	}
 	return nil
 }
+
+// LoadDiscoveries load all discoveries for all loaded platforms
+// Returns error if:
+// * A PluggableDiscovery instance can't be created
+// * Tools required by the PlatformRelease cannot be found
+// * Command line to start PluggableDiscovery has malformed or mismatched quotes
+func (pm *PackageManager) LoadDiscoveries() []*status.Status {
+	statuses := []*status.Status{}
+	for _, platform := range pm.InstalledPlatformReleases() {
+		statuses = append(statuses, pm.loadDiscoveries(platform)...)
+	}
+	return statuses
+}
+
+func (pm *PackageManager) loadDiscoveries(release *cores.PlatformRelease) []*status.Status {
+	statuses := []*status.Status{}
+	discoveryProperties := release.Properties.SubTree("pluggable_discovery")
+
+	if discoveryProperties.Size() == 0 {
+		return nil
+	}
+
+	// Handles discovery properties formatted like so:
+	//
+	// Case 1:
+	//    "pluggable_discovery.required": "PLATFORM:DISCOVERY_NAME",
+	//
+	// Case 2:
+	//    "pluggable_discovery.required.0": "PLATFORM:DISCOVERY_ID_1",
+	//    "pluggable_discovery.required.1": "PLATFORM:DISCOVERY_ID_2",
+	//
+	// If both indexed and unindexed properties are found the unindexed are ignored
+	for _, id := range discoveryProperties.ExtractSubIndexLists("required") {
+		tool := pm.GetTool(id)
+		if tool == nil {
+			statuses = append(statuses, status.Newf(codes.FailedPrecondition, tr("discovery not found: %s"), id))
+			continue
+		}
+		toolRelease := tool.GetLatestInstalled()
+		if toolRelease == nil {
+			statuses = append(statuses, status.Newf(codes.FailedPrecondition, tr("discovery not installed: %s"), id))
+			continue
+		}
+		discoveryPath := toolRelease.InstallDir.Join(tool.Name).String()
+		d, err := discovery.New(id, discoveryPath)
+		if err != nil {
+			statuses = append(statuses, status.Newf(codes.FailedPrecondition, tr("creating discovery: %s"), err))
+			continue
+		}
+		pm.discoveryManager.Add(d)
+	}
+
+	discoveryIDs := discoveryProperties.FirstLevelOf()
+	delete(discoveryIDs, "required")
+	// Get the list of tools only if there are discoveries that use Direct discovery integration.
+	// See:
+	// https://arduino.github.io/arduino-cli/latest/platform-specification/#pluggable-discovery
+	// We need the tools only in that case since we might need some tool's
+	// runtime properties to expand the discovery pattern to run it correctly.
+	var tools []*cores.ToolRelease
+	if len(discoveryIDs) > 0 {
+		var err error
+		tools, err = pm.FindToolsRequiredFromPlatformRelease(release)
+		if err != nil {
+			statuses = append(statuses, status.New(codes.Internal, err.Error()))
+		}
+	}
+
+	// Handles discovery properties formatted like so:
+	//
+	// discovery.DISCOVERY_ID.pattern: "COMMAND_TO_EXECUTE"
+	for discoveryID, props := range discoveryIDs {
+		pattern, ok := props.GetOk("pattern")
+		if !ok {
+			statuses = append(statuses, status.Newf(codes.FailedPrecondition, tr("can't find pattern for discovery with id %s"), discoveryID))
+			continue
+		}
+		configuration := release.Properties.Clone()
+		configuration.Merge(release.RuntimeProperties())
+		configuration.Merge(props)
+
+		for _, tool := range tools {
+			configuration.Merge(tool.RuntimeProperties())
+		}
+
+		cmd := configuration.ExpandPropsInString(pattern)
+		if cmdArgs, err := properties.SplitQuotedString(cmd, `"'`, true); err != nil {
+			statuses = append(statuses, status.New(codes.Internal, err.Error()))
+		} else if d, err := discovery.New(discoveryID, cmdArgs...); err != nil {
+			statuses = append(statuses, status.New(codes.Internal, err.Error()))
+		} else {
+			pm.discoveryManager.Add(d)
+		}
+	}
+
+	return statuses
+}