Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 38479dc

Browse files
cmaglieper1234
andauthoredAug 18, 2023
feature: Detect board port change after upload (#2253)
* UploadResponse now has 'oneof' clause for better API design * Added scaffolding to return updated-port after upload * Upload port change detection (first draft) * Simplified port detection using a Future-style abstraction * Perform watcher-flush higher in the call tree * Do not infer upload port if 'upload.wait_for_upload_port' is false * Further simplified port detection subroutine structure * fixed linter issue * Always return an updatedUploadPort. Arduino CLI should always return the port after an upload, even in the case where no port change is expected. The consumer shouldn't be required to implement "if not updated_upload_port, use original port" logic. The whole point is that all the logic for determining which port should be selected after an upload should be implemented in Arduino CLI. The consumer should be able to simply select the port Arduino CLI tells it to select in all cases. * Updated docs * Perform a deep-copy of upload ports where needed. Previously only the pointer was copied, thus making changes in `actualPort` to be reflected also to `port`. This lead to some weird result in the `updatedUploadPort` result: { "stdout": "Verify 11344 bytes of flash with checksum.\nVerify successful\ndone in 0.010 seconds\nCPU reset.\n", "stderr": "", "updated_upload_port": { "address": "/dev/tty.usbmodem14101", <------- this address... "label": "/dev/cu.usbmodem14101", <------- ...is different from the label "protocol": "serial", "protocol_label": "Serial Port (USB)", "properties": { "pid": "0x804E", "serialNumber": "94A3397C5150435437202020FF150838", "vid": "0x2341" }, "hardware_id": "94A3397C5150435437202020FF150838" } } * When updating `actualPort` address, update also the address label. * Fixed some potential nil pointer exceptions * Further simplified board watcher We must acesss the gRPC API only until we cross the `command` package border. Once we are inside the `command` package we should use the internal API only. * Before returning from upload, check if the port is still alive Now the upload detects cases when the upload port is "unstable", i.e. the port changes even if it shouldn't (because the wait_for_upload_port property in boards.txt is set to false). This change should make the upload process more resilient. * Apply suggestions from code review Co-authored-by: per1234 <[email protected]> * Fixed nil exception * Improved tracking algorithm for upload-port reconnection The new algorithm takes into account the case where a single board may expose multiple ports, in this case the selection will increase priority to ports that: 1. have the same HW id as the user specified port for upload 2. have the same protocol as the user specified port for upload 3. have the same address as the user specified port for upload --------- Co-authored-by: per1234 <[email protected]>
1 parent b64876c commit 38479dc

File tree

14 files changed

+717
-264
lines changed

14 files changed

+717
-264
lines changed
 

‎arduino/discovery/discovery.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,12 @@ type Port struct {
9797

9898
var tr = i18n.Tr
9999

100+
// Equals returns true if the given port has the same address and protocol
101+
// of the current port.
102+
func (p *Port) Equals(o *Port) bool {
103+
return p.Address == o.Address && p.Protocol == o.Protocol
104+
}
105+
100106
// ToRPC converts Port into rpc.Port
101107
func (p *Port) ToRPC() *rpc.Port {
102108
props := p.Properties
@@ -113,13 +119,43 @@ func (p *Port) ToRPC() *rpc.Port {
113119
}
114120
}
115121

122+
// PortFromRPCPort converts an *rpc.Port to a *Port
123+
func PortFromRPCPort(o *rpc.Port) (p *Port) {
124+
if o == nil {
125+
return nil
126+
}
127+
res := &Port{
128+
Address: o.Address,
129+
AddressLabel: o.Label,
130+
Protocol: o.Protocol,
131+
ProtocolLabel: o.ProtocolLabel,
132+
HardwareID: o.HardwareId,
133+
}
134+
if o.Properties != nil {
135+
res.Properties = properties.NewFromHashmap(o.Properties)
136+
}
137+
return res
138+
}
139+
116140
func (p *Port) String() string {
117141
if p == nil {
118142
return "none"
119143
}
120144
return p.Address
121145
}
122146

147+
// Clone creates a copy of this Port
148+
func (p *Port) Clone() *Port {
149+
if p == nil {
150+
return nil
151+
}
152+
var res Port = *p
153+
if p.Properties != nil {
154+
res.Properties = p.Properties.Clone()
155+
}
156+
return &res
157+
}
158+
123159
// Event is a pluggable discovery event
124160
type Event struct {
125161
Type string

‎arduino/discovery/discovery_client/main.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,12 @@ func main() {
5151
fmt.Printf(" Address: %s\n", port.Address)
5252
fmt.Printf(" Protocol: %s\n", port.Protocol)
5353
if ev.Type == "add" {
54-
keys := port.Properties.Keys()
55-
sort.Strings(keys)
56-
for _, k := range keys {
57-
fmt.Printf(" %s=%s\n", k, port.Properties.Get(k))
54+
if port.Properties != nil {
55+
keys := port.Properties.Keys()
56+
sort.Strings(keys)
57+
for _, k := range keys {
58+
fmt.Printf(" %s=%s\n", k, port.Properties.Get(k))
59+
}
5860
}
5961
}
6062
fmt.Println()

‎commands/board/list.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"github.com/arduino/arduino-cli/commands"
3535
"github.com/arduino/arduino-cli/internal/inventory"
3636
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
37+
"github.com/arduino/go-properties-orderedmap"
3738
"github.com/pkg/errors"
3839
"github.com/sirupsen/logrus"
3940
)
@@ -128,20 +129,22 @@ func apiByVidPid(vid, pid string) ([]*rpc.BoardListItem, error) {
128129
}, nil
129130
}
130131

131-
func identifyViaCloudAPI(port *discovery.Port) ([]*rpc.BoardListItem, error) {
132+
func identifyViaCloudAPI(props *properties.Map) ([]*rpc.BoardListItem, error) {
132133
// If the port is not USB do not try identification via cloud
133-
id := port.Properties
134-
if !id.ContainsKey("vid") || !id.ContainsKey("pid") {
134+
if !props.ContainsKey("vid") || !props.ContainsKey("pid") {
135135
return nil, nil
136136
}
137137

138138
logrus.Debug("Querying builder API for board identification...")
139-
return cachedAPIByVidPid(id.Get("vid"), id.Get("pid"))
139+
return cachedAPIByVidPid(props.Get("vid"), props.Get("pid"))
140140
}
141141

142142
// identify returns a list of boards checking first the installed platforms or the Cloud API
143143
func identify(pme *packagemanager.Explorer, port *discovery.Port) ([]*rpc.BoardListItem, error) {
144144
boards := []*rpc.BoardListItem{}
145+
if port.Properties == nil {
146+
return boards, nil
147+
}
145148

146149
// first query installed cores through the Package Manager
147150
logrus.Debug("Querying installed cores for board identification...")
@@ -167,7 +170,7 @@ func identify(pme *packagemanager.Explorer, port *discovery.Port) ([]*rpc.BoardL
167170
// if installed cores didn't recognize the board, try querying
168171
// the builder API if the board is a USB device port
169172
if len(boards) == 0 {
170-
items, err := identifyViaCloudAPI(port)
173+
items, err := identifyViaCloudAPI(port.Properties)
171174
if err != nil {
172175
// this is bad, but keep going
173176
logrus.WithError(err).Debug("Error querying builder API")

‎commands/board/list_test.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,7 @@ func TestGetByVidPidMalformedResponse(t *testing.T) {
103103
}
104104

105105
func TestBoardDetectionViaAPIWithNonUSBPort(t *testing.T) {
106-
port := &discovery.Port{
107-
Properties: properties.NewMap(),
108-
}
109-
items, err := identifyViaCloudAPI(port)
106+
items, err := identifyViaCloudAPI(properties.NewMap())
110107
require.NoError(t, err)
111108
require.Empty(t, items)
112109
}

‎commands/daemon/daemon.go

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -297,15 +297,27 @@ func (s *ArduinoCoreServerImpl) PlatformList(ctx context.Context, req *rpc.Platf
297297
// Upload FIXMEDOC
298298
func (s *ArduinoCoreServerImpl) Upload(req *rpc.UploadRequest, stream rpc.ArduinoCoreService_UploadServer) error {
299299
syncSend := NewSynchronizedSend(stream.Send)
300-
outStream := feedStreamTo(func(data []byte) { syncSend.Send(&rpc.UploadResponse{OutStream: data}) })
301-
errStream := feedStreamTo(func(data []byte) { syncSend.Send(&rpc.UploadResponse{ErrStream: data}) })
302-
err := upload.Upload(stream.Context(), req, outStream, errStream)
300+
outStream := feedStreamTo(func(data []byte) {
301+
syncSend.Send(&rpc.UploadResponse{
302+
Message: &rpc.UploadResponse_OutStream{OutStream: data},
303+
})
304+
})
305+
errStream := feedStreamTo(func(data []byte) {
306+
syncSend.Send(&rpc.UploadResponse{
307+
Message: &rpc.UploadResponse_ErrStream{ErrStream: data},
308+
})
309+
})
310+
res, err := upload.Upload(stream.Context(), req, outStream, errStream)
303311
outStream.Close()
304312
errStream.Close()
305-
if err != nil {
306-
return convertErrorToRPCStatus(err)
313+
if res != nil {
314+
syncSend.Send(&rpc.UploadResponse{
315+
Message: &rpc.UploadResponse_Result{
316+
Result: res,
317+
},
318+
})
307319
}
308-
return nil
320+
return convertErrorToRPCStatus(err)
309321
}
310322

311323
// UploadUsingProgrammer FIXMEDOC

‎commands/upload/burnbootloader.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ func BurnBootloader(ctx context.Context, req *rpc.BurnBootloaderRequest, outStre
3939
}
4040
defer release()
4141

42-
err := runProgramAction(
42+
_, err := runProgramAction(
4343
pme,
4444
nil, // sketch
4545
"", // importFile

‎commands/upload/upload.go

Lines changed: 174 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,19 @@ import (
2121
"io"
2222
"path/filepath"
2323
"strings"
24+
"time"
2425

2526
"github.com/arduino/arduino-cli/arduino"
2627
"github.com/arduino/arduino-cli/arduino/cores"
2728
"github.com/arduino/arduino-cli/arduino/cores/packagemanager"
29+
"github.com/arduino/arduino-cli/arduino/discovery"
2830
"github.com/arduino/arduino-cli/arduino/globals"
2931
"github.com/arduino/arduino-cli/arduino/serialutils"
3032
"github.com/arduino/arduino-cli/arduino/sketch"
3133
"github.com/arduino/arduino-cli/commands"
3234
"github.com/arduino/arduino-cli/executils"
3335
"github.com/arduino/arduino-cli/i18n"
36+
f "github.com/arduino/arduino-cli/internal/algorithms"
3437
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
3538
paths "github.com/arduino/go-paths-helper"
3639
properties "github.com/arduino/go-properties-orderedmap"
@@ -123,24 +126,24 @@ func getUserFields(toolID string, platformRelease *cores.PlatformRelease) []*rpc
123126
}
124127

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

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

137-
pme, release := commands.GetPackageManagerExplorer(req)
140+
pme, pmeRelease := commands.GetPackageManagerExplorer(req)
138141
if pme == nil {
139-
return &arduino.InvalidInstanceError{}
142+
return nil, &arduino.InvalidInstanceError{}
140143
}
141-
defer release()
144+
defer pmeRelease()
142145

143-
if err := runProgramAction(
146+
updatedPort, err := runProgramAction(
144147
pme,
145148
sk,
146149
req.GetImportFile(),
@@ -155,11 +158,14 @@ func Upload(ctx context.Context, req *rpc.UploadRequest, outStream io.Writer, er
155158
errStream,
156159
req.GetDryRun(),
157160
req.GetUserFields(),
158-
); err != nil {
159-
return err
161+
)
162+
if err != nil {
163+
return nil, err
160164
}
161165

162-
return nil
166+
return &rpc.UploadResult{
167+
UpdatedUploadPort: updatedPort,
168+
}, nil
163169
}
164170

165171
// UsingProgrammer FIXMEDOC
@@ -169,7 +175,7 @@ func UsingProgrammer(ctx context.Context, req *rpc.UploadUsingProgrammerRequest,
169175
if req.GetProgrammer() == "" {
170176
return &arduino.MissingProgrammerError{}
171177
}
172-
err := Upload(ctx, &rpc.UploadRequest{
178+
_, err := Upload(ctx, &rpc.UploadRequest{
173179
Instance: req.GetInstance(),
174180
SketchPath: req.GetSketchPath(),
175181
ImportFile: req.GetImportFile(),
@@ -186,36 +192,38 @@ func UsingProgrammer(ctx context.Context, req *rpc.UploadUsingProgrammerRequest,
186192

187193
func runProgramAction(pme *packagemanager.Explorer,
188194
sk *sketch.Sketch,
189-
importFile, importDir, fqbnIn string, port *rpc.Port,
195+
importFile, importDir, fqbnIn string, userPort *rpc.Port,
190196
programmerID string,
191197
verbose, verify, burnBootloader bool,
192198
outStream, errStream io.Writer,
193-
dryRun bool, userFields map[string]string) error {
194-
195-
if burnBootloader && programmerID == "" {
196-
return &arduino.MissingProgrammerError{}
197-
}
199+
dryRun bool, userFields map[string]string,
200+
) (*rpc.Port, error) {
201+
port := discovery.PortFromRPCPort(userPort)
198202
if port == nil || (port.Address == "" && port.Protocol == "") {
199203
// For no-port uploads use "default" protocol
200-
port = &rpc.Port{Protocol: "default"}
204+
port = &discovery.Port{Protocol: "default"}
201205
}
202206
logrus.WithField("port", port).Tracef("Upload port")
203207

208+
if burnBootloader && programmerID == "" {
209+
return nil, &arduino.MissingProgrammerError{}
210+
}
211+
204212
fqbn, err := cores.ParseFQBN(fqbnIn)
205213
if err != nil {
206-
return &arduino.InvalidFQBNError{Cause: err}
214+
return nil, &arduino.InvalidFQBNError{Cause: err}
207215
}
208216
logrus.WithField("fqbn", fqbn).Tracef("Detected FQBN")
209217

210218
// Find target board and board properties
211219
_, boardPlatform, board, boardProperties, buildPlatform, err := pme.ResolveFQBN(fqbn)
212220
if boardPlatform == nil {
213-
return &arduino.PlatformNotFoundError{
221+
return nil, &arduino.PlatformNotFoundError{
214222
Platform: fmt.Sprintf("%s:%s", fqbn.Package, fqbn.PlatformArch),
215223
Cause: err,
216224
}
217225
} else if err != nil {
218-
return &arduino.UnknownFQBNError{Cause: err}
226+
return nil, &arduino.UnknownFQBNError{Cause: err}
219227
}
220228
logrus.
221229
WithField("boardPlatform", boardPlatform).
@@ -232,7 +240,7 @@ func runProgramAction(pme *packagemanager.Explorer,
232240
programmer = buildPlatform.Programmers[programmerID]
233241
}
234242
if programmer == nil {
235-
return &arduino.ProgrammerNotFoundError{Programmer: programmerID}
243+
return nil, &arduino.ProgrammerNotFoundError{Programmer: programmerID}
236244
}
237245
}
238246

@@ -253,7 +261,7 @@ func runProgramAction(pme *packagemanager.Explorer,
253261
}
254262
uploadToolID, err := getToolID(props, action, port.Protocol)
255263
if err != nil {
256-
return err
264+
return nil, err
257265
}
258266

259267
var uploadToolPlatform *cores.PlatformRelease
@@ -268,7 +276,7 @@ func runProgramAction(pme *packagemanager.Explorer,
268276
Trace("Upload tool")
269277

270278
if split := strings.Split(uploadToolID, ":"); len(split) > 2 {
271-
return &arduino.InvalidPlatformPropertyError{
279+
return nil, &arduino.InvalidPlatformPropertyError{
272280
Property: fmt.Sprintf("%s.tool.%s", action, port.Protocol), // TODO: Can be done better, maybe inline getToolID(...)
273281
Value: uploadToolID}
274282
} else if len(split) == 2 {
@@ -277,12 +285,12 @@ func runProgramAction(pme *packagemanager.Explorer,
277285
PlatformArchitecture: boardPlatform.Platform.Architecture,
278286
})
279287
if p == nil {
280-
return &arduino.PlatformNotFoundError{Platform: split[0] + ":" + boardPlatform.Platform.Architecture}
288+
return nil, &arduino.PlatformNotFoundError{Platform: split[0] + ":" + boardPlatform.Platform.Architecture}
281289
}
282290
uploadToolID = split[1]
283291
uploadToolPlatform = pme.GetInstalledPlatformRelease(p)
284292
if uploadToolPlatform == nil {
285-
return &arduino.PlatformNotFoundError{Platform: split[0] + ":" + boardPlatform.Platform.Architecture}
293+
return nil, &arduino.PlatformNotFoundError{Platform: split[0] + ":" + boardPlatform.Platform.Architecture}
286294
}
287295
}
288296

@@ -309,7 +317,7 @@ func runProgramAction(pme *packagemanager.Explorer,
309317
}
310318

311319
if !uploadProperties.ContainsKey("upload.protocol") && programmer == nil {
312-
return &arduino.ProgrammerRequiredForUploadError{}
320+
return nil, &arduino.ProgrammerRequiredForUploadError{}
313321
}
314322

315323
// Set properties for verbose upload
@@ -357,18 +365,35 @@ func runProgramAction(pme *packagemanager.Explorer,
357365
if !burnBootloader {
358366
importPath, sketchName, err := determineBuildPathAndSketchName(importFile, importDir, sk, fqbn)
359367
if err != nil {
360-
return &arduino.NotFoundError{Message: tr("Error finding build artifacts"), Cause: err}
368+
return nil, &arduino.NotFoundError{Message: tr("Error finding build artifacts"), Cause: err}
361369
}
362370
if !importPath.Exist() {
363-
return &arduino.NotFoundError{Message: tr("Compiled sketch not found in %s", importPath)}
371+
return nil, &arduino.NotFoundError{Message: tr("Compiled sketch not found in %s", importPath)}
364372
}
365373
if !importPath.IsDir() {
366-
return &arduino.NotFoundError{Message: tr("Expected compiled sketch in directory %s, but is a file instead", importPath)}
374+
return nil, &arduino.NotFoundError{Message: tr("Expected compiled sketch in directory %s, but is a file instead", importPath)}
367375
}
368376
uploadProperties.SetPath("build.path", importPath)
369377
uploadProperties.Set("build.project_name", sketchName)
370378
}
371379

380+
// This context is kept alive for the entire duration of the upload
381+
uploadCtx, uploadCompleted := context.WithCancel(context.Background())
382+
defer uploadCompleted()
383+
384+
// Start the upload port change detector.
385+
watcher, err := pme.DiscoveryManager().Watch()
386+
if err != nil {
387+
return nil, err
388+
}
389+
defer watcher.Close()
390+
updatedUploadPort := f.NewFuture[*discovery.Port]()
391+
go detectUploadPort(
392+
uploadCtx,
393+
port, watcher.Feed(),
394+
uploadProperties.GetBoolean("upload.wait_for_upload_port"),
395+
updatedUploadPort)
396+
372397
// Force port wait to make easier to unbrick boards like the Arduino Leonardo, or similar with native USB,
373398
// when a sketch causes a crash and the native USB serial port is lost.
374399
// See https://github.com/arduino/arduino-cli/issues/1943 for the details.
@@ -385,7 +410,7 @@ func runProgramAction(pme *packagemanager.Explorer,
385410

386411
// If not using programmer perform some action required
387412
// to set the board in bootloader mode
388-
actualPort := port
413+
actualPort := port.Clone()
389414
if programmer == nil && !burnBootloader && (port.Protocol == "serial" || forcedSerialPortWait) {
390415
// Perform reset via 1200bps touch if requested and wait for upload port also if requested.
391416
touch := uploadProperties.GetBoolean("upload.use_1200bps_touch")
@@ -439,6 +464,7 @@ func runProgramAction(pme *packagemanager.Explorer,
439464
} else {
440465
if newPortAddress != "" {
441466
actualPort.Address = newPortAddress
467+
actualPort.AddressLabel = newPortAddress
442468
}
443469
}
444470
}
@@ -455,34 +481,144 @@ func runProgramAction(pme *packagemanager.Explorer,
455481

456482
// Get Port properties gathered using pluggable discovery
457483
uploadProperties.Set("upload.port.address", port.Address)
458-
uploadProperties.Set("upload.port.label", port.Label)
484+
uploadProperties.Set("upload.port.label", port.AddressLabel)
459485
uploadProperties.Set("upload.port.protocol", port.Protocol)
460486
uploadProperties.Set("upload.port.protocolLabel", port.ProtocolLabel)
461-
for prop, value := range actualPort.Properties {
462-
uploadProperties.Set(fmt.Sprintf("upload.port.properties.%s", prop), value)
487+
if actualPort.Properties != nil {
488+
for prop, value := range actualPort.Properties.AsMap() {
489+
uploadProperties.Set(fmt.Sprintf("upload.port.properties.%s", prop), value)
490+
}
463491
}
464492

465493
// Run recipes for upload
466494
toolEnv := pme.GetEnvVarsForSpawnedProcess()
467495
if burnBootloader {
468496
if err := runTool("erase.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil {
469-
return &arduino.FailedUploadError{Message: tr("Failed chip erase"), Cause: err}
497+
return nil, &arduino.FailedUploadError{Message: tr("Failed chip erase"), Cause: err}
470498
}
471499
if err := runTool("bootloader.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil {
472-
return &arduino.FailedUploadError{Message: tr("Failed to burn bootloader"), Cause: err}
500+
return nil, &arduino.FailedUploadError{Message: tr("Failed to burn bootloader"), Cause: err}
473501
}
474502
} else if programmer != nil {
475503
if err := runTool("program.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil {
476-
return &arduino.FailedUploadError{Message: tr("Failed programming"), Cause: err}
504+
return nil, &arduino.FailedUploadError{Message: tr("Failed programming"), Cause: err}
477505
}
478506
} else {
479507
if err := runTool("upload.pattern", uploadProperties, outStream, errStream, verbose, dryRun, toolEnv); err != nil {
480-
return &arduino.FailedUploadError{Message: tr("Failed uploading"), Cause: err}
508+
return nil, &arduino.FailedUploadError{Message: tr("Failed uploading"), Cause: err}
481509
}
482510
}
483511

512+
uploadCompleted()
484513
logrus.Tracef("Upload successful")
485-
return nil
514+
515+
updatedPort := updatedUploadPort.Await()
516+
if updatedPort == nil {
517+
return nil, nil
518+
}
519+
return updatedPort.ToRPC(), nil
520+
}
521+
522+
func detectUploadPort(
523+
uploadCtx context.Context,
524+
uploadPort *discovery.Port, watch <-chan *discovery.Event,
525+
waitForUploadPort bool,
526+
result f.Future[*discovery.Port],
527+
) {
528+
log := logrus.WithField("task", "port_detection")
529+
log.Tracef("Detecting new board port after upload")
530+
531+
candidate := uploadPort.Clone()
532+
defer func() {
533+
result.Send(candidate)
534+
}()
535+
536+
// Ignore all events during the upload
537+
for {
538+
select {
539+
case ev, ok := <-watch:
540+
if !ok {
541+
log.Error("Upload port detection failed, watcher closed")
542+
return
543+
}
544+
if candidate != nil && ev.Type == "remove" && ev.Port.Equals(candidate) {
545+
log.WithField("event", ev).Trace("User-specified port has been disconnected, forcing wait for upload port")
546+
waitForUploadPort = true
547+
candidate = nil
548+
} else {
549+
log.WithField("event", ev).Trace("Ignored watcher event before upload")
550+
}
551+
continue
552+
case <-uploadCtx.Done():
553+
// Upload completed, move to the next phase
554+
}
555+
break
556+
}
557+
558+
// Pick the first port that is detected after the upload
559+
timeout := time.After(5 * time.Second)
560+
if !waitForUploadPort {
561+
timeout = time.After(time.Second)
562+
}
563+
for {
564+
select {
565+
case ev, ok := <-watch:
566+
if !ok {
567+
log.Error("Upload port detection failed, watcher closed")
568+
return
569+
}
570+
if candidate != nil && ev.Type == "remove" && candidate.Equals(ev.Port) {
571+
log.WithField("event", ev).Trace("Candidate port is no longer available")
572+
candidate = nil
573+
if !waitForUploadPort {
574+
waitForUploadPort = true
575+
timeout = time.After(5 * time.Second)
576+
log.Trace("User-specified port has been disconnected, now waiting for upload port, timeout extended by 5 seconds")
577+
}
578+
continue
579+
}
580+
if ev.Type != "add" {
581+
log.WithField("event", ev).Trace("Ignored non-add event")
582+
continue
583+
}
584+
585+
portPriority := func(port *discovery.Port) int {
586+
if port == nil {
587+
return 0
588+
}
589+
prio := 0
590+
if port.HardwareID == uploadPort.HardwareID {
591+
prio += 1000
592+
}
593+
if port.Protocol == uploadPort.Protocol {
594+
prio += 100
595+
}
596+
if port.Address == uploadPort.Address {
597+
prio += 10
598+
}
599+
return prio
600+
}
601+
evPortPriority := portPriority(ev.Port)
602+
candidatePriority := portPriority(candidate)
603+
if evPortPriority <= candidatePriority {
604+
log.WithField("event", ev).Tracef("New upload port candidate is worse than the current one (prio=%d)", evPortPriority)
605+
continue
606+
}
607+
log.WithField("event", ev).Tracef("Found new upload port candidate (prio=%d)", evPortPriority)
608+
candidate = ev.Port
609+
610+
// If the current candidate have the desired HW-ID return it quickly.
611+
if candidate.HardwareID == ev.Port.HardwareID {
612+
timeout = time.After(time.Second)
613+
log.Trace("New candidate port match the desired HW ID, timeout reduced to 1 second.")
614+
continue
615+
}
616+
617+
case <-timeout:
618+
log.WithField("selected_port", candidate).Trace("Timeout waiting for candidate port")
619+
return
620+
}
621+
}
486622
}
487623

488624
func runTool(recipeID string, props *properties.Map, outStream, errStream io.Writer, verbose bool, dryRun bool, toolEnv []string) error {

‎commands/upload/upload_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ func TestUploadPropertiesComposition(t *testing.T) {
184184
testRunner := func(t *testing.T, test test, verboseVerify bool) {
185185
outStream := &bytes.Buffer{}
186186
errStream := &bytes.Buffer{}
187-
err := runProgramAction(
187+
_, err := runProgramAction(
188188
pme,
189189
nil, // sketch
190190
"", // importFile

‎docs/UPGRADING.md

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,58 @@ Here you can find a list of migration guides to handle breaking changes between
44

55
## 0.34.0
66

7+
### The gRPC `cc.arduino.cli.commands.v1.UploadRepsonse` command response has been changed.
8+
9+
Previously the `UploadResponse` was used only to stream the tool output:
10+
11+
```
12+
message UploadResponse {
13+
// The output of the upload process.
14+
bytes out_stream = 1;
15+
// The error output of the upload process.
16+
bytes err_stream = 2;
17+
}
18+
```
19+
20+
Now the API logic has been clarified using the `oneof` clause and another field has been added providing an
21+
`UploadResult` message that is sent when a successful upload completes.
22+
23+
```
24+
message UploadResponse {
25+
oneof message {
26+
// The output of the upload process.
27+
bytes out_stream = 1;
28+
// The error output of the upload process.
29+
bytes err_stream = 2;
30+
// The upload result
31+
UploadResult result = 3;
32+
}
33+
}
34+
35+
message UploadResult {
36+
// When a board requires a port disconnection to perform the upload, this
37+
// field returns the port where the board reconnects after the upload.
38+
Port updated_upload_port = 1;
39+
}
40+
```
41+
42+
### golang API: method `github.com/arduino/arduino-cli/commands/upload.Upload` changed signature
43+
44+
The `Upload` method signature has been changed from:
45+
46+
```go
47+
func Upload(ctx context.Context, req *rpc.UploadRequest, outStream io.Writer, errStream io.Writer) error { ... }
48+
```
49+
50+
to:
51+
52+
```go
53+
func Upload(ctx context.Context, req *rpc.UploadRequest, outStream io.Writer, errStream io.Writer) (*rpc.UploadResult, error) { ... }
54+
```
55+
56+
Now an `UploadResult` structure is returned together with the error. If you are not interested in the information
57+
contained in the structure you can safely ignore it.
58+
759
### golang package `github.com/arduino/arduino-cli/inventory` removed from public API
860

961
The package `inventory` is no more a public golang API.

‎internal/algorithms/channels.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
// This file is part of arduino-cli.
2+
//
3+
// Copyright 2023 ARDUINO SA (http://www.arduino.cc/)
4+
//
5+
// This software is released under the GNU General Public License version 3,
6+
// which covers the main part of arduino-cli.
7+
// The terms of this license can be found at:
8+
// https://www.gnu.org/licenses/gpl-3.0.en.html
9+
//
10+
// You can be released from the requirements of the above licenses by purchasing
11+
// a commercial license. Buying such a license is mandatory if you want to
12+
// modify or otherwise use the software for commercial activities involving the
13+
// Arduino software without disclosing the source code of your own applications.
14+
// To purchase a commercial license, send an email to license@arduino.cc.
15+
16+
package f
17+
18+
import "sync"
19+
20+
// DiscardCh consumes all incoming messages from the given channel until it's closed.
21+
func DiscardCh[T any](ch <-chan T) {
22+
for range ch {
23+
}
24+
}
25+
26+
// Future is an object that holds a result value. The value may be read and
27+
// written asynchronously.
28+
type Future[T any] interface {
29+
Send(T)
30+
Await() T
31+
}
32+
33+
type future[T any] struct {
34+
wg sync.WaitGroup
35+
value T
36+
}
37+
38+
// NewFuture creates a new Future[T]
39+
func NewFuture[T any]() Future[T] {
40+
res := &future[T]{}
41+
res.wg.Add(1)
42+
return res
43+
}
44+
45+
// Send a result in the Future. Threads waiting for result will be unlocked.
46+
func (f *future[T]) Send(value T) {
47+
f.value = value
48+
f.wg.Done()
49+
}
50+
51+
// Await for a result from the Future, blocks until a result is available.
52+
func (f *future[T]) Await() T {
53+
f.wg.Wait()
54+
return f.value
55+
}

‎internal/cli/compile/compile.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,8 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
236236
DoNotExpandBuildProperties: showProperties == arguments.ShowPropertiesUnexpanded,
237237
}
238238
compileRes, compileError := compile.Compile(context.Background(), compileRequest, stdOut, stdErr, nil)
239+
240+
var uploadRes *rpc.UploadResult
239241
if compileError == nil && uploadAfterCompile {
240242
userFieldRes, err := upload.SupportedUserFields(context.Background(), &rpc.SupportedUserFieldsRequest{
241243
Instance: inst,
@@ -268,8 +270,10 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
268270
UserFields: fields,
269271
}
270272

271-
if err := upload.Upload(context.Background(), uploadRequest, stdOut, stdErr); err != nil {
273+
if res, err := upload.Upload(context.Background(), uploadRequest, stdOut, stdErr); err != nil {
272274
feedback.Fatal(tr("Error during Upload: %v", err), feedback.ErrGeneric)
275+
} else {
276+
uploadRes = res
273277
}
274278
}
275279

@@ -330,6 +334,7 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
330334
CompilerOut: stdIO.Stdout,
331335
CompilerErr: stdIO.Stderr,
332336
BuilderResult: compileRes,
337+
UploadResult: uploadRes,
333338
ProfileOut: profileOut,
334339
Success: compileError == nil,
335340
showPropertiesMode: showProperties,
@@ -375,6 +380,7 @@ type compileResult struct {
375380
CompilerOut string `json:"compiler_out"`
376381
CompilerErr string `json:"compiler_err"`
377382
BuilderResult *rpc.CompileResponse `json:"builder_result"`
383+
UploadResult *rpc.UploadResult `json:"upload_result"`
378384
Success bool `json:"success"`
379385
ProfileOut string `json:"profile_out,omitempty"`
380386
Error string `json:"error,omitempty"`

‎internal/cli/upload/upload.go

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,31 @@ func runUploadCommand(command *cobra.Command, args []string) {
168168
DryRun: dryRun,
169169
UserFields: fields,
170170
}
171-
if err := upload.Upload(context.Background(), req, stdOut, stdErr); err != nil {
171+
if res, err := upload.Upload(context.Background(), req, stdOut, stdErr); err != nil {
172172
feedback.FatalError(err, feedback.ErrGeneric)
173+
} else {
174+
io := stdIOResult()
175+
feedback.PrintResult(&uploadResult{
176+
Stdout: io.Stdout,
177+
Stderr: io.Stderr,
178+
UpdatedUploadPort: res.UpdatedUploadPort,
179+
})
173180
}
174-
feedback.PrintResult(stdIOResult())
181+
}
182+
183+
type uploadResult struct {
184+
Stdout string `json:"stdout"`
185+
Stderr string `json:"stderr"`
186+
UpdatedUploadPort *rpc.Port `json:"updated_upload_port,omitempty"`
187+
}
188+
189+
func (r *uploadResult) Data() interface{} {
190+
return r
191+
}
192+
193+
func (r *uploadResult) String() string {
194+
if r.UpdatedUploadPort == nil {
195+
return ""
196+
}
197+
return tr("New upload port: %[1]s (%[2]s)", r.UpdatedUploadPort.Address, r.UpdatedUploadPort.Protocol)
175198
}

‎rpc/cc/arduino/cli/commands/v1/upload.pb.go

Lines changed: 319 additions & 198 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎rpc/cc/arduino/cli/commands/v1/upload.proto

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,20 @@ message UploadRequest {
6262
}
6363

6464
message UploadResponse {
65-
// The output of the upload process.
66-
bytes out_stream = 1;
67-
// The error output of the upload process.
68-
bytes err_stream = 2;
65+
oneof message {
66+
// The output of the upload process.
67+
bytes out_stream = 1;
68+
// The error output of the upload process.
69+
bytes err_stream = 2;
70+
// The upload result
71+
UploadResult result = 3;
72+
}
73+
}
74+
75+
message UploadResult {
76+
// When a board requires a port disconnection to perform the upload, this
77+
// field returns the port where the board reconnects after the upload.
78+
Port updated_upload_port = 1;
6979
}
7080

7181
message ProgrammerIsRequiredForUploadError {}

0 commit comments

Comments
 (0)
Please sign in to comment.