Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: Detect board port change after upload #2253

Merged
merged 18 commits into from
Aug 18, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions commands/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,15 +297,27 @@ func (s *ArduinoCoreServerImpl) PlatformList(ctx context.Context, req *rpc.Platf
// Upload FIXMEDOC
func (s *ArduinoCoreServerImpl) Upload(req *rpc.UploadRequest, stream rpc.ArduinoCoreService_UploadServer) error {
syncSend := NewSynchronizedSend(stream.Send)
outStream := feedStreamTo(func(data []byte) { syncSend.Send(&rpc.UploadResponse{OutStream: data}) })
errStream := feedStreamTo(func(data []byte) { syncSend.Send(&rpc.UploadResponse{ErrStream: data}) })
err := upload.Upload(stream.Context(), req, outStream, errStream)
outStream := feedStreamTo(func(data []byte) {
syncSend.Send(&rpc.UploadResponse{
Message: &rpc.UploadResponse_OutStream{OutStream: data},
})
})
errStream := feedStreamTo(func(data []byte) {
syncSend.Send(&rpc.UploadResponse{
Message: &rpc.UploadResponse_ErrStream{ErrStream: data},
})
})
res, err := upload.Upload(stream.Context(), req, outStream, errStream)
outStream.Close()
errStream.Close()
if err != nil {
return convertErrorToRPCStatus(err)
if res != nil {
syncSend.Send(&rpc.UploadResponse{
Message: &rpc.UploadResponse_Result{
Result: res,
},
})
}
return nil
return convertErrorToRPCStatus(err)
}

// UploadUsingProgrammer FIXMEDOC
Expand Down
3 changes: 2 additions & 1 deletion commands/upload/burnbootloader.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ func BurnBootloader(ctx context.Context, req *rpc.BurnBootloaderRequest, outStre
}
defer release()

err := runProgramAction(
_, err := runProgramAction(
pme,
nil, // watcher
nil, // sketch
"", // importFile
"", // importDir
Expand Down
171 changes: 139 additions & 32 deletions commands/upload/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"io"
"path/filepath"
"strings"
"time"

"github.com/arduino/arduino-cli/arduino"
"github.com/arduino/arduino-cli/arduino/cores"
Expand All @@ -29,8 +30,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"
Expand Down Expand Up @@ -123,26 +126,34 @@ func getUserFields(toolID string, platformRelease *cores.PlatformRelease) []*rpc
}

// Upload FIXMEDOC
func Upload(ctx context.Context, req *rpc.UploadRequest, outStream io.Writer, errStream io.Writer) error {
func Upload(ctx context.Context, req *rpc.UploadRequest, outStream io.Writer, errStream io.Writer) (*rpc.UploadResult, error) {
logrus.Tracef("Upload %s on %s started", req.GetSketchPath(), req.GetFqbn())

// TODO: make a generic function to extract sketch from request
// and remove duplication in commands/compile.go
sketchPath := paths.New(req.GetSketchPath())
sk, err := sketch.New(sketchPath)
if err != nil && req.GetImportDir() == "" && req.GetImportFile() == "" {
return &arduino.CantOpenSketchError{Cause: err}
return nil, &arduino.CantOpenSketchError{Cause: err}
}

pme, release := commands.GetPackageManagerExplorer(req)
pme, pmeRelease := commands.GetPackageManagerExplorer(req)
if pme == nil {
return &arduino.InvalidInstanceError{}
return nil, &arduino.InvalidInstanceError{}
}
defer release()
defer pmeRelease()

if err := runProgramAction(
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(),
Expand All @@ -155,11 +166,14 @@ func Upload(ctx context.Context, req *rpc.UploadRequest, outStream io.Writer, er
errStream,
req.GetDryRun(),
req.GetUserFields(),
); err != nil {
return err
)
if err != nil {
return nil, err
}

return nil
return &rpc.UploadResult{
UpdatedUploadPort: updatedPort,
}, nil
}

// UsingProgrammer FIXMEDOC
Expand All @@ -169,7 +183,7 @@ func UsingProgrammer(ctx context.Context, req *rpc.UploadUsingProgrammerRequest,
if req.GetProgrammer() == "" {
return &arduino.MissingProgrammerError{}
}
err := Upload(ctx, &rpc.UploadRequest{
_, err := Upload(ctx, &rpc.UploadRequest{
Instance: req.GetInstance(),
SketchPath: req.GetSketchPath(),
ImportFile: req.GetImportFile(),
Expand All @@ -186,36 +200,44 @@ func UsingProgrammer(ctx context.Context, req *rpc.UploadUsingProgrammerRequest,

func runProgramAction(pme *packagemanager.Explorer,
sk *sketch.Sketch,
importFile, importDir, fqbnIn string, port *rpc.Port,
watch <-chan *rpc.BoardListWatchResponse,
importFile, importDir, fqbnIn string, userPort *rpc.Port,
programmerID string,
verbose, verify, burnBootloader bool,
outStream, errStream io.Writer,
dryRun bool, userFields map[string]string) error {
dryRun bool, userFields map[string]string) (*rpc.Port, error) {

if burnBootloader && programmerID == "" {
return &arduino.MissingProgrammerError{}
}
// Ensure watcher events consumption in case of exit on error
defer func() {
go f.DiscardCh(watch)
}()

port := userPort
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")

if burnBootloader && programmerID == "" {
return nil, &arduino.MissingProgrammerError{}
}

fqbn, err := cores.ParseFQBN(fqbnIn)
if err != nil {
return &arduino.InvalidFQBNError{Cause: err}
return nil, &arduino.InvalidFQBNError{Cause: err}
}
logrus.WithField("fqbn", fqbn).Tracef("Detected FQBN")

// Find target board and board properties
_, boardPlatform, board, boardProperties, buildPlatform, err := pme.ResolveFQBN(fqbn)
if boardPlatform == nil {
return &arduino.PlatformNotFoundError{
return nil, &arduino.PlatformNotFoundError{
Platform: fmt.Sprintf("%s:%s", fqbn.Package, fqbn.PlatformArch),
Cause: err,
}
} else if err != nil {
return &arduino.UnknownFQBNError{Cause: err}
return nil, &arduino.UnknownFQBNError{Cause: err}
}
logrus.
WithField("boardPlatform", boardPlatform).
Expand All @@ -232,7 +254,7 @@ func runProgramAction(pme *packagemanager.Explorer,
programmer = buildPlatform.Programmers[programmerID]
}
if programmer == nil {
return &arduino.ProgrammerNotFoundError{Programmer: programmerID}
return nil, &arduino.ProgrammerNotFoundError{Programmer: programmerID}
}
}

Expand All @@ -253,7 +275,7 @@ func runProgramAction(pme *packagemanager.Explorer,
}
uploadToolID, err := getToolID(props, action, port.Protocol)
if err != nil {
return err
return nil, err
}

var uploadToolPlatform *cores.PlatformRelease
Expand All @@ -268,7 +290,7 @@ func runProgramAction(pme *packagemanager.Explorer,
Trace("Upload tool")

if split := strings.Split(uploadToolID, ":"); len(split) > 2 {
return &arduino.InvalidPlatformPropertyError{
return nil, &arduino.InvalidPlatformPropertyError{
Property: fmt.Sprintf("%s.tool.%s", action, port.Protocol), // TODO: Can be done better, maybe inline getToolID(...)
Value: uploadToolID}
} else if len(split) == 2 {
Expand All @@ -277,12 +299,12 @@ func runProgramAction(pme *packagemanager.Explorer,
PlatformArchitecture: boardPlatform.Platform.Architecture,
})
if p == nil {
return &arduino.PlatformNotFoundError{Platform: split[0] + ":" + boardPlatform.Platform.Architecture}
return nil, &arduino.PlatformNotFoundError{Platform: split[0] + ":" + boardPlatform.Platform.Architecture}
}
uploadToolID = split[1]
uploadToolPlatform = pme.GetInstalledPlatformRelease(p)
if uploadToolPlatform == nil {
return &arduino.PlatformNotFoundError{Platform: split[0] + ":" + boardPlatform.Platform.Architecture}
return nil, &arduino.PlatformNotFoundError{Platform: split[0] + ":" + boardPlatform.Platform.Architecture}
}
}

Expand All @@ -309,7 +331,7 @@ func runProgramAction(pme *packagemanager.Explorer,
}

if !uploadProperties.ContainsKey("upload.protocol") && programmer == nil {
return &arduino.ProgrammerRequiredForUploadError{}
return nil, &arduino.ProgrammerRequiredForUploadError{}
}

// Set properties for verbose upload
Expand Down Expand Up @@ -357,18 +379,32 @@ func runProgramAction(pme *packagemanager.Explorer,
if !burnBootloader {
importPath, sketchName, err := determineBuildPathAndSketchName(importFile, importDir, sk, fqbn)
if err != nil {
return &arduino.NotFoundError{Message: tr("Error finding build artifacts"), Cause: err}
return nil, &arduino.NotFoundError{Message: tr("Error finding build artifacts"), Cause: err}
}
if !importPath.Exist() {
return &arduino.NotFoundError{Message: tr("Compiled sketch not found in %s", importPath)}
return nil, &arduino.NotFoundError{Message: tr("Compiled sketch not found in %s", importPath)}
}
if !importPath.IsDir() {
return &arduino.NotFoundError{Message: tr("Expected compiled sketch in directory %s, but is a file instead", importPath)}
return nil, &arduino.NotFoundError{Message: tr("Expected compiled sketch in directory %s, but is a file instead", importPath)}
}
uploadProperties.SetPath("build.path", importPath)
uploadProperties.Set("build.project_name", sketchName)
}

// This context is kept alive for the entire duration of the upload
uploadCtx, uploadCompleted := context.WithCancel(context.Background())
defer uploadCompleted()

// By default do not return any new port but if there is an
// expected port change then run the detector.
updatedUploadPort := f.NewFuture[*rpc.Port]()
if uploadProperties.GetBoolean("upload.wait_for_upload_port") && watch != nil {
go detectUploadPort(uploadCtx, port, watch, updatedUploadPort)
} else {
updatedUploadPort.Send(nil)
go f.DiscardCh(watch)
}

// Force port wait to make easier to unbrick boards like the Arduino Leonardo, or similar with native USB,
// when a sketch causes a crash and the native USB serial port is lost.
// See https://github.com/arduino/arduino-cli/issues/1943 for the details.
Expand Down Expand Up @@ -466,23 +502,94 @@ func runProgramAction(pme *packagemanager.Explorer,
toolEnv := pme.GetEnvVarsForSpawnedProcess()
if burnBootloader {
if err := runTool("erase.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil {
return &arduino.FailedUploadError{Message: tr("Failed chip erase"), Cause: err}
return nil, &arduino.FailedUploadError{Message: tr("Failed chip erase"), Cause: err}
}
if err := runTool("bootloader.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil {
return &arduino.FailedUploadError{Message: tr("Failed to burn bootloader"), Cause: err}
return nil, &arduino.FailedUploadError{Message: tr("Failed to burn bootloader"), Cause: err}
}
} else if programmer != nil {
if err := runTool("program.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil {
return &arduino.FailedUploadError{Message: tr("Failed programming"), Cause: err}
return nil, &arduino.FailedUploadError{Message: tr("Failed programming"), Cause: err}
}
} else {
if err := runTool("upload.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil {
return &arduino.FailedUploadError{Message: tr("Failed uploading"), Cause: err}
return nil, &arduino.FailedUploadError{Message: tr("Failed uploading"), Cause: err}
}
}

uploadCompleted()
logrus.Tracef("Upload successful")
return nil

updatedPort := updatedUploadPort.Await()
if updatedPort == nil {
updatedPort = userPort
}
return userPort, nil
}

func detectUploadPort(uploadCtx context.Context, uploadPort *rpc.Port, watch <-chan *rpc.BoardListWatchResponse, result f.Future[*rpc.Port]) {
log := logrus.WithField("task", "port_detection")
log.Tracef("Detecting new board port after upload")

var candidate *rpc.Port
defer func() {
result.Send(candidate)
}()

// Ignore all events during the upload
for {
select {
case ev, ok := <-watch:
if !ok {
log.Error("Upload port detection failed, watcher closed")
return
}
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)
for {
select {
case ev, ok := <-watch:
if !ok {
log.Error("Upload port detection failed, watcher closed")
return
}
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
case <-timeout:
log.Trace("Timeout waiting for candidate port")
return
}
}
}

func runTool(recipeID string, props *properties.Map, outStream, errStream io.Writer, verbose bool, dryRun bool, toolEnv []string) error {
Expand Down
3 changes: 2 additions & 1 deletion commands/upload/upload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,10 @@ func TestUploadPropertiesComposition(t *testing.T) {
testRunner := func(t *testing.T, test test, verboseVerify bool) {
outStream := &bytes.Buffer{}
errStream := &bytes.Buffer{}
err := runProgramAction(
_, err := runProgramAction(
pme,
nil, // sketch
nil, // board watcher
"", // importFile
test.importDir.String(), // importDir
test.fqbn, // FQBN
Expand Down
Loading