diff --git a/arduino/discovery/discovery.go b/arduino/discovery/discovery.go index 4ba26d91121..c35a81812f1 100644 --- a/arduino/discovery/discovery.go +++ b/arduino/discovery/discovery.go @@ -32,17 +32,6 @@ import ( "github.com/sirupsen/logrus" ) -// To work correctly a Pluggable Discovery must respect the state machine specified on the documentation: -// https://arduino.github.io/arduino-cli/latest/pluggable-discovery-specification/#state-machine -// States a PluggableDiscovery can be in -const ( - Alive int = iota - Idling - Running - Syncing - Dead -) - // PluggableDiscovery is a tool that detects communication ports to interact // with the boards. type PluggableDiscovery struct { @@ -55,7 +44,6 @@ type PluggableDiscovery struct { // All the following fields are guarded by statusMutex statusMutex sync.Mutex incomingMessagesError error - state int eventChan chan<- *Event } @@ -168,7 +156,6 @@ func New(id string, args ...string) *PluggableDiscovery { return &PluggableDiscovery{ id: id, processArgs: args, - state: Dead, } } @@ -185,28 +172,26 @@ func (disc *PluggableDiscovery) jsonDecodeLoop(in io.Reader, outChan chan<- *dis decoder := json.NewDecoder(in) closeAndReportError := func(err error) { disc.statusMutex.Lock() - disc.state = Dead disc.incomingMessagesError = err disc.statusMutex.Unlock() + disc.killProcess() close(outChan) - logrus.Errorf("stopped discovery %s decode loop: %v", disc.id, err) + if err != nil { + logrus.Errorf("stopped discovery %s decode loop: %v", disc, err) + } } for { var msg discoveryMessage if err := decoder.Decode(&msg); errors.Is(err, io.EOF) { // This is fine, we exit gracefully - disc.statusMutex.Lock() - disc.state = Dead - disc.incomingMessagesError = err - disc.statusMutex.Unlock() - close(outChan) + closeAndReportError(nil) return } else if err != nil { closeAndReportError(err) return } - logrus.Infof("from discovery %s received message %s", disc.id, msg) + logrus.Infof("from discovery %s received message %s", disc, msg) if msg.EventType == "add" { if msg.Port == nil { closeAndReportError(errors.New(tr("invalid 'add' message: missing port"))) @@ -233,11 +218,10 @@ func (disc *PluggableDiscovery) jsonDecodeLoop(in io.Reader, outChan chan<- *dis } } -// State returns the current state of this PluggableDiscovery -func (disc *PluggableDiscovery) State() int { +func (disc *PluggableDiscovery) Alive() bool { disc.statusMutex.Lock() defer disc.statusMutex.Unlock() - return disc.state + return disc.process != nil } func (disc *PluggableDiscovery) waitMessage(timeout time.Duration) (*discoveryMessage, error) { @@ -268,7 +252,7 @@ func (disc *PluggableDiscovery) sendCommand(command string) error { } func (disc *PluggableDiscovery) runProcess() error { - logrus.Infof("starting discovery %s process", disc.id) + logrus.Infof("starting discovery %s process", disc) proc, err := executils.NewProcess(nil, disc.processArgs...) if err != nil { return err @@ -294,13 +278,14 @@ func (disc *PluggableDiscovery) runProcess() error { disc.statusMutex.Lock() defer disc.statusMutex.Unlock() disc.process = proc - disc.state = Alive - logrus.Infof("started discovery %s process", disc.id) + logrus.Infof("started discovery %s process", disc) return nil } func (disc *PluggableDiscovery) killProcess() error { - logrus.Infof("killing discovery %s process", disc.id) + disc.statusMutex.Lock() + defer disc.statusMutex.Unlock() + logrus.Infof("killing discovery %s process", disc) if disc.process != nil { if err := disc.process.Kill(); err != nil { return err @@ -308,12 +293,10 @@ func (disc *PluggableDiscovery) killProcess() error { if err := disc.process.Wait(); err != nil { return err } + disc.process = nil } - disc.statusMutex.Lock() - defer disc.statusMutex.Unlock() disc.stopSync() - disc.state = Dead - logrus.Infof("killed discovery %s process", disc.id) + logrus.Infof("killed discovery %s process", disc) return nil } @@ -353,14 +336,11 @@ func (disc *PluggableDiscovery) Run() (err error) { } else if msg.ProtocolVersion > 1 { return errors.Errorf(tr("protocol version not supported: requested 1, got %d"), msg.ProtocolVersion) } - disc.statusMutex.Lock() - defer disc.statusMutex.Unlock() - disc.state = Idling return nil } // Start initializes and start the discovery internal subroutines. This command must be -// called before List or StartSync. +// called before List. func (disc *PluggableDiscovery) Start() error { if err := disc.sendCommand("START\n"); err != nil { return err @@ -374,9 +354,6 @@ func (disc *PluggableDiscovery) Start() error { } else if strings.ToUpper(msg.Message) != "OK" { return errors.Errorf(tr("communication out of sync, expected '%[1]s', received '%[2]s'"), "OK", msg.Message) } - disc.statusMutex.Lock() - defer disc.statusMutex.Unlock() - disc.state = Running return nil } @@ -399,7 +376,6 @@ func (disc *PluggableDiscovery) Stop() error { disc.statusMutex.Lock() defer disc.statusMutex.Unlock() disc.stopSync() - disc.state = Idling return nil } @@ -415,7 +391,7 @@ func (disc *PluggableDiscovery) stopSync() { func (disc *PluggableDiscovery) Quit() { _ = disc.sendCommand("QUIT\n") if _, err := disc.waitMessage(time.Second * 5); err != nil { - logrus.Errorf("Quitting discovery %s: %s", disc.id, err) + logrus.Errorf("Quitting discovery %s: %s", disc, err) } disc.stopSync() disc.killProcess() @@ -463,7 +439,6 @@ func (disc *PluggableDiscovery) StartSync(size int) (<-chan *Event, error) { return nil, errors.Errorf(tr("communication out of sync, expected '%[1]s', received '%[2]s'"), "OK", msg.Message) } - disc.state = Syncing // In case there is already an existing event channel in use we close it before creating a new one. disc.stopSync() c := make(chan *Event, size) diff --git a/arduino/discovery/discovery_test.go b/arduino/discovery/discovery_test.go index 27791da6c89..2cf800a2a55 100644 --- a/arduino/discovery/discovery_test.go +++ b/arduino/discovery/discovery_test.go @@ -63,11 +63,11 @@ func TestDiscoveryStdioHandling(t *testing.T) { require.NotNil(t, msg) require.Equal(t, "ev2", msg.EventType) - require.Equal(t, disc.State(), Alive) + require.True(t, disc.Alive()) err = disc.outgoingCommandsPipe.(io.ReadCloser).Close() require.NoError(t, err) time.Sleep(time.Millisecond * 100) - require.Equal(t, disc.State(), Dead) + require.False(t, disc.Alive()) }