From 4b2d5327da763190a6ec531a061a2ca991d610a6 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 18 Jun 2021 17:10:01 +0200 Subject: [PATCH 1/2] Removed now useless boilerplate for BoardList --- commands/board/list.go | 13 ++-- commands/board/list_test.go | 6 +- commands/bundled_tools_serial_discovery.go | 84 ++++------------------ 3 files changed, 19 insertions(+), 84 deletions(-) diff --git a/commands/board/list.go b/commands/board/list.go index 63c25fece11..26f74ad81ba 100644 --- a/commands/board/list.go +++ b/commands/board/list.go @@ -26,6 +26,7 @@ import ( "sync" "github.com/arduino/arduino-cli/arduino/cores/packagemanager" + "github.com/arduino/arduino-cli/arduino/discovery" "github.com/arduino/arduino-cli/commands" "github.com/arduino/arduino-cli/httpclient" rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" @@ -101,7 +102,7 @@ func apiByVidPid(vid, pid string) ([]*rpc.BoardListItem, error) { return retVal, nil } -func identifyViaCloudAPI(port *commands.BoardPort) ([]*rpc.BoardListItem, error) { +func identifyViaCloudAPI(port *discovery.Port) ([]*rpc.BoardListItem, error) { // If the port is not USB do not try identification via cloud id := port.Properties if !id.ContainsKey("vid") || !id.ContainsKey("pid") { @@ -113,7 +114,7 @@ func identifyViaCloudAPI(port *commands.BoardPort) ([]*rpc.BoardListItem, error) } // identify returns a list of boards checking first the installed platforms or the Cloud API -func identify(pm *packagemanager.PackageManager, port *commands.BoardPort) ([]*rpc.BoardListItem, error) { +func identify(pm *packagemanager.PackageManager, port *discovery.Port) ([]*rpc.BoardListItem, error) { boards := []*rpc.BoardListItem{} // first query installed cores through the Package Manager @@ -236,13 +237,7 @@ func Watch(instanceID int32, interrupt <-chan bool) (<-chan *rpc.BoardListWatchR boards := []*rpc.BoardListItem{} boardsError := "" if event.Type == "add" { - boards, err = identify(pm, &commands.BoardPort{ - Address: event.Port.Address, - Label: event.Port.AddressLabel, - Properties: event.Port.Properties, - Protocol: event.Port.Protocol, - ProtocolLabel: event.Port.ProtocolLabel, - }) + boards, err = identify(pm, event.Port) if err != nil { boardsError = err.Error() } diff --git a/commands/board/list_test.go b/commands/board/list_test.go index 78db9ff83b1..02212c4e119 100644 --- a/commands/board/list_test.go +++ b/commands/board/list_test.go @@ -23,7 +23,7 @@ import ( "testing" "github.com/arduino/arduino-cli/arduino/cores/packagemanager" - "github.com/arduino/arduino-cli/commands" + "github.com/arduino/arduino-cli/arduino/discovery" "github.com/arduino/arduino-cli/configuration" "github.com/arduino/go-paths-helper" "github.com/arduino/go-properties-orderedmap" @@ -106,7 +106,7 @@ func TestGetByVidPidMalformedResponse(t *testing.T) { } func TestBoardDetectionViaAPIWithNonUSBPort(t *testing.T) { - port := &commands.BoardPort{ + port := &discovery.Port{ Properties: properties.NewMap(), } items, err := identifyViaCloudAPI(port) @@ -152,7 +152,7 @@ func TestBoardIdentifySorting(t *testing.T) { idPrefs := properties.NewMap() idPrefs.Set("vid", "0x0000") idPrefs.Set("pid", "0x0000") - res, err := identify(pm, &commands.BoardPort{Properties: idPrefs}) + res, err := identify(pm, &discovery.Port{Properties: idPrefs}) require.NoError(t, err) require.NotNil(t, res) require.Len(t, res, 4) diff --git a/commands/bundled_tools_serial_discovery.go b/commands/bundled_tools_serial_discovery.go index 00f9b38f470..91ab85c8c30 100644 --- a/commands/bundled_tools_serial_discovery.go +++ b/commands/bundled_tools_serial_discovery.go @@ -16,18 +16,13 @@ package commands import ( - "encoding/json" "fmt" "sync" - "time" "github.com/arduino/arduino-cli/arduino/cores" "github.com/arduino/arduino-cli/arduino/cores/packagemanager" "github.com/arduino/arduino-cli/arduino/discovery" "github.com/arduino/arduino-cli/arduino/resources" - "github.com/arduino/arduino-cli/executils" - "github.com/arduino/go-properties-orderedmap" - "github.com/pkg/errors" semver "go.bug.st/relaxed-semver" ) @@ -107,24 +102,10 @@ var ( } ) -// BoardPort is a generic port descriptor -type BoardPort struct { - Address string `json:"address"` - Label string `json:"label"` - Properties *properties.Map `json:"properties"` - Protocol string `json:"protocol"` - ProtocolLabel string `json:"protocolLabel"` -} - -type eventJSON struct { - EventType string `json:"eventType,required"` - Ports []*BoardPort `json:"ports"` -} - var listBoardMutex sync.Mutex // ListBoards foo -func ListBoards(pm *packagemanager.PackageManager) ([]*BoardPort, error) { +func ListBoards(pm *packagemanager.PackageManager) ([]*discovery.Port, error) { // ensure the connection to the discoverer is unique to avoid messing up // the messages exchanged listBoardMutex.Lock() @@ -141,67 +122,26 @@ func ListBoards(pm *packagemanager.PackageManager) ([]*BoardPort, error) { return nil, fmt.Errorf("missing serial-discovery tool") } - // build the command to be executed - cmd, err := executils.NewProcessFromPath(t.InstallDir.Join("serial-discovery")) - if err != nil { - return nil, errors.Wrap(err, "creating discovery process") - } - - // attach in/out pipes to the process - in, err := cmd.StdinPipe() - if err != nil { - return nil, fmt.Errorf("creating stdin pipe for discovery: %s", err) - } - - out, err := cmd.StdoutPipe() + disc, err := discovery.New("serial-discovery", t.InstallDir.Join(t.Tool.Name).String()) if err != nil { - return nil, fmt.Errorf("creating stdout pipe for discovery: %s", err) + return nil, err } - outJSON := json.NewDecoder(out) + defer disc.Quit() - // start the process - if err := cmd.Start(); err != nil { - return nil, fmt.Errorf("starting discovery process: %s", err) + if err = disc.Hello(); err != nil { + return nil, fmt.Errorf("starting discovery: %v", err) } - // send the LIST command - if _, err := in.Write([]byte("LIST\n")); err != nil { - return nil, fmt.Errorf("sending LIST command to discovery: %s", err) + if err = disc.Start(); err != nil { + return nil, fmt.Errorf("starting discovery: %v", err) } - // read the response from the pipe - decodeResult := make(chan error) - var event eventJSON - go func() { - decodeResult <- outJSON.Decode(&event) - }() - - var finalError error - var retVal []*BoardPort - - // wait for the response - select { - case err := <-decodeResult: - if err == nil { - retVal = event.Ports - } else { - finalError = err - } - case <-time.After(10 * time.Second): - finalError = fmt.Errorf("decoding LIST command: timeout") + res, err := disc.List() + if err != nil { + return nil, fmt.Errorf("getting port list from discovery: %v", err) } - // tell the process to quit - in.Write([]byte("QUIT\n")) - in.Close() - out.Close() - // kill the process if it takes too long to quit - time.AfterFunc(time.Second, func() { - cmd.Kill() - }) - cmd.Wait() - - return retVal, finalError + return res, nil } // WatchListBoards returns a channel that receives events from the bundled discovery tool From 90f3825d6d8c1ea751eb0f3c5e9633d1944c0af8 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 18 Jun 2021 17:12:43 +0200 Subject: [PATCH 2/2] Fixed wrong 'break' I guess it was meant to exit from the outer loop and not the select. --- commands/board/list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/board/list.go b/commands/board/list.go index 26f74ad81ba..79cecd691d2 100644 --- a/commands/board/list.go +++ b/commands/board/list.go @@ -260,7 +260,7 @@ func Watch(instanceID int32, interrupt <-chan bool) (<-chan *rpc.BoardListWatchR Error: boardsError, } case <-interrupt: - break + return } } }()