Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 38479dc

Browse filesBrowse files
cmaglieper1234
cmaglie
and
per1234
authored
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 <accounts@perglass.com> * 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 <accounts@perglass.com>
1 parent b64876c commit 38479dc
Copy full SHA for 38479dc

File tree

Expand file treeCollapse file tree

14 files changed

+717
-264
lines changed
Filter options
Expand file treeCollapse file tree

14 files changed

+717
-264
lines changed

‎arduino/discovery/discovery.go

Copy file name to clipboardExpand all lines: arduino/discovery/discovery.go
+36Lines 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

Copy file name to clipboardExpand all lines: arduino/discovery/discovery_client/main.go
+6-4Lines 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

Copy file name to clipboardExpand all lines: commands/board/list.go
+8-5Lines 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

Copy file name to clipboardExpand all lines: commands/board/list_test.go
+1-4Lines 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

Copy file name to clipboardExpand all lines: commands/daemon/daemon.go
+18-6Lines 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

Copy file name to clipboardExpand all lines: commands/upload/burnbootloader.go
+1-1Lines 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

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.