Upload port change detection (first draft)

arduino · cmaglie · Aug 18, 2023 · Jul 31, 2023 · Jul 31, 2023 · Aug 1, 2023
commit bfb2fb4447ae179375a6cb9517ef72c668c1c41c
diff --git a/commands/upload/burnbootloader.go b/commands/upload/burnbootloader.go
@@ -41,6 +41,7 @@ func BurnBootloader(ctx context.Context, req *rpc.BurnBootloaderRequest, outStre
 
 	_, err := runProgramAction(
 		pme,
+		nil, // watcher
 		nil, // sketch
 		"",  // importFile
 		"",  // importDir

diff --git a/commands/upload/upload.go b/commands/upload/upload.go
@@ -21,6 +21,8 @@ import (
 	"io"
 	"path/filepath"
 	"strings"
+	"sync"
+	"time"
 
 	"github.com/arduino/arduino-cli/arduino"
 	"github.com/arduino/arduino-cli/arduino/cores"
@@ -29,8 +31,10 @@ import (
 	"github.com/arduino/arduino-cli/arduino/serialutils"
 	"github.com/arduino/arduino-cli/arduino/sketch"
 	"github.com/arduino/arduino-cli/commands"
+	"github.com/arduino/arduino-cli/commands/board"
 	"github.com/arduino/arduino-cli/executils"
 	"github.com/arduino/arduino-cli/i18n"
+	f "github.com/arduino/arduino-cli/internal/algorithms"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	paths "github.com/arduino/go-paths-helper"
 	properties "github.com/arduino/go-properties-orderedmap"
@@ -134,15 +138,23 @@ func Upload(ctx context.Context, req *rpc.UploadRequest, outStream io.Writer, er
 		return nil, &arduino.CantOpenSketchError{Cause: err}
 	}
 
-	pme, release := commands.GetPackageManagerExplorer(req)
+	pme, pmeRelease := commands.GetPackageManagerExplorer(req)
 	if pme == nil {
 		return nil, &arduino.InvalidInstanceError{}
 	}
-	defer release()
+	defer pmeRelease()
+
+	watch, watchRelease, err := board.Watch(&rpc.BoardListWatchRequest{Instance: req.GetInstance()})
+	if err != nil {
+		logrus.WithError(err).Error("Error watching board ports")
+	} else {
+		defer watchRelease()
+	}
 
 	updatedPort, err := runProgramAction(
 		pme,
 		sk,
+		watch,
 		req.GetImportFile(),
 		req.GetImportDir(),
 		req.GetFqbn(),
@@ -189,21 +201,43 @@ func UsingProgrammer(ctx context.Context, req *rpc.UploadUsingProgrammerRequest,
 
 func runProgramAction(pme *packagemanager.Explorer,
 	sk *sketch.Sketch,
+	watch <-chan *rpc.BoardListWatchResponse,
 	importFile, importDir, fqbnIn string, port *rpc.Port,
 	programmerID string,
 	verbose, verify, burnBootloader bool,
 	outStream, errStream io.Writer,
 	dryRun bool, userFields map[string]string) (*rpc.Port, error) {
 
-	if burnBootloader && programmerID == "" {
-		return nil, &arduino.MissingProgrammerError{}
-	}
 	if port == nil || (port.Address == "" && port.Protocol == "") {
 		// For no-port uploads use "default" protocol
 		port = &rpc.Port{Protocol: "default"}
 	}
 	logrus.WithField("port", port).Tracef("Upload port")
 
+	// Default newPort
+	uploadCompleted := func() *rpc.Port { return nil }
+	if watch != nil {
+		// Run port detector
+		uploadCompletedCtx, cancel := context.WithCancel(context.Background())
+		var newUploadPort *rpc.Port
+		var wg sync.WaitGroup
+		wg.Add(1)
+		go func() {
+			newUploadPort = detectUploadPort(port, watch, uploadCompletedCtx)
+			wg.Done()
+		}()
+		uploadCompleted = func() *rpc.Port {
+			cancel()
+			wg.Wait()
+			return newUploadPort
+		}
+		defer uploadCompleted() // defer in case of exit on error (ensures goroutine completion)
+	}
+
+	if burnBootloader && programmerID == "" {
+		return nil, &arduino.MissingProgrammerError{}
+	}
+
 	fqbn, err := cores.ParseFQBN(fqbnIn)
 	if err != nil {
 		return nil, &arduino.InvalidFQBNError{Cause: err}
@@ -485,13 +519,73 @@ func runProgramAction(pme *packagemanager.Explorer,
 	}
 
 	logrus.Tracef("Upload successful")
-	return nil, nil // TODO: return new port
+	return uploadCompleted(), nil
 }
 
-func detectNewUploadPort(oldPort *rpc.Port) *rpc.Port {
-	logrus.Tracef("Detecting new board port")
-	// TODO
-	return nil
+func detectUploadPort(uploadPort *rpc.Port, watch <-chan *rpc.BoardListWatchResponse, uploadCtx context.Context) *rpc.Port {
+	log := logrus.WithField("task", "port_detection")
+	log.Tracef("Detecting new board port after upload")
+
+	defer func() {
+		// On exit, discard all events until the watcher is closed
+		go f.DiscardCh(watch)
+	}()
+
+	// Ignore all events during the upload
+	for {
+		select {
+		case ev, ok := <-watch:
+			if !ok {
+				log.Error("Upload port detection failed, watcher closed")
+				return nil
+			}
+			log.WithField("event", ev).Trace("Ignored watcher event before upload")
+			continue
+		case <-uploadCtx.Done():
+			// Upload completed, move to the next phase
+		}
+		break
+	}
+
+	// Pick the first port that is detected after the upload
+	desiredHwID := uploadPort.HardwareId
+	timeout := time.After(5 * time.Second)
+	var candidate *rpc.Port
+	for {
+		select {
+		case ev, ok := <-watch:
+			if !ok {
+				log.Error("Upload port detection failed, watcher closed")
+				return candidate
+			}
+			if ev.EventType == "remove" && candidate != nil {
+				if candidate.Equals(ev.Port.GetPort()) {
+					log.WithField("event", ev).Trace("Candidate port is no more available")
+					candidate = nil
+					continue
+				}
+			}
+			if ev.EventType != "add" {
+				log.WithField("event", ev).Trace("Ignored non-add event")
+				continue
+			}
+			candidate = ev.Port.GetPort()
+			log.WithField("event", ev).Trace("New upload port candidate")
+
+			// If the current candidate port does not have the desired HW-ID do
+			// not return it immediately.
+			if desiredHwID != "" && candidate.GetHardwareId() != desiredHwID {
+				log.Trace("New candidate port did not match desired HW ID, keep watching...")
+				continue
+			}
+
+			log.Trace("Found new upload port!")
+			return candidate
+		case <-timeout:
+			log.Trace("Timeout waiting for candidate port")
+			return candidate
+		}
+	}
 }
 
 func runTool(recipeID string, props *properties.Map, outStream, errStream io.Writer, verbose bool, dryRun bool, toolEnv []string) error {

diff --git a/commands/upload/upload_test.go b/commands/upload/upload_test.go
@@ -187,6 +187,7 @@ func TestUploadPropertiesComposition(t *testing.T) {
 		_, err := runProgramAction(
 			pme,
 			nil,                     // sketch
+			nil,                     // board watcher
 			"",                      // importFile
 			test.importDir.String(), // importDir
 			test.fqbn,               // FQBN

diff --git a/internal/algorithms/channels.go b/internal/algorithms/channels.go
@@ -0,0 +1,22 @@
+// This file is part of arduino-cli.
+//
+// Copyright 2023 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to license@arduino.cc.
+
+package f
+
+// DiscardCh consume all incoming messages from the given channel until its closed.
+func DiscardCh[T any](ch <-chan T) {
+	for range ch {
+	}
+}
diff --git a/rpc/cc/arduino/cli/commands/v1/port.go b/rpc/cc/arduino/cli/commands/v1/port.go
@@ -0,0 +1,22 @@
+// This file is part of arduino-cli.
+//
+// Copyright 2023 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to license@arduino.cc.
+
+package commands
+
+// Equals return true if the given port has the same address and protocol
+// of the current port.
+func (p *Port) Equals(o *Port) bool {
+	return p.GetAddress() == o.GetAddress() && p.GetProtocol() == o.GetProtocol()
+}