From 7ab33de6e1a49c9294c635639ceaeeada58902a2 Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Fri, 9 Feb 2024 12:10:26 +0100
Subject: [PATCH 01/12] Added discovery client implementation from Arduino CLI

---
 client.go               | 449 ++++++++++++++++++++++++++++++++++++++++
 client_test.go          |  75 +++++++
 discovery.go            |   7 +-
 testdata/cat/.gitignore |   2 +
 testdata/cat/main.go    |  28 +++
 5 files changed, 558 insertions(+), 3 deletions(-)
 create mode 100644 client.go
 create mode 100644 client_test.go
 create mode 100644 testdata/cat/.gitignore
 create mode 100644 testdata/cat/main.go

diff --git a/client.go b/client.go
new file mode 100644
index 0000000..ed1dda0
--- /dev/null
+++ b/client.go
@@ -0,0 +1,449 @@
+//
+// This file is part of pluggable-discovery-protocol-handler.
+//
+// Copyright 2024 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to modify or
+// otherwise use the software for commercial activities involving the Arduino
+// software without disclosing the source code of your own applications. To purchase
+// a commercial license, send an email to license@arduino.cc.
+//
+
+package discovery
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"strings"
+	"sync"
+	"time"
+
+	"github.com/arduino/go-paths-helper"
+)
+
+// 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
+)
+
+// Client is a tool that detects communication ports to interact
+// with the boards.
+type Client struct {
+	id                   string
+	processArgs          []string
+	process              *paths.Process
+	outgoingCommandsPipe io.Writer
+	incomingMessagesChan <-chan *discoveryMessage
+	userAgent            string
+	logger               ClientLogger
+
+	// All the following fields are guarded by statusMutex
+	statusMutex           sync.Mutex
+	incomingMessagesError error
+	state                 int
+	eventChan             chan<- *Event
+}
+
+// ClientLogger is the interface that must be implemented by a logger
+// to be used in the discovery client.
+type ClientLogger interface {
+	Infof(format string, args ...interface{})
+	Errorf(format string, args ...interface{})
+}
+
+type nullClientLogger struct{}
+
+func (l *nullClientLogger) Infof(format string, args ...interface{})  {}
+func (l *nullClientLogger) Errorf(format string, args ...interface{}) {}
+
+type discoveryMessage struct {
+	EventType       string  `json:"eventType"`
+	Message         string  `json:"message"`
+	Error           bool    `json:"error"`
+	ProtocolVersion int     `json:"protocolVersion"` // Used in HELLO command
+	Ports           []*Port `json:"ports"`           // Used in LIST command
+	Port            *Port   `json:"port"`            // Used in add and remove events
+}
+
+func (msg discoveryMessage) String() string {
+	s := fmt.Sprintf("type: %s", msg.EventType)
+	if msg.Message != "" {
+		s = fmt.Sprintf("%[1]s, message: %[2]s", s, msg.Message)
+	}
+	if msg.ProtocolVersion != 0 {
+		s = fmt.Sprintf("%[1]s, protocol version: %[2]d", s, msg.ProtocolVersion)
+	}
+	if len(msg.Ports) > 0 {
+		s = fmt.Sprintf("%[1]s, ports: %[2]s", s, msg.Ports)
+	}
+	if msg.Port != nil {
+		s = fmt.Sprintf("%[1]s, port: %[2]s", s, msg.Port)
+	}
+	return s
+}
+
+// Equals returns true if the given port has the same address and protocol
+// of the current port.
+func (p *Port) Equals(o *Port) bool {
+	return p.Address == o.Address && p.Protocol == o.Protocol
+}
+
+func (p *Port) String() string {
+	if p == nil {
+		return "none"
+	}
+	return p.Address
+}
+
+// Clone creates a copy of this Port
+func (p *Port) Clone() *Port {
+	if p == nil {
+		return nil
+	}
+	res := *p
+	if p.Properties != nil {
+		res.Properties = p.Properties.Clone()
+	}
+	return &res
+}
+
+// Event is a pluggable discovery event
+type Event struct {
+	Type        string
+	Port        *Port
+	DiscoveryID string
+}
+
+// NewClient create a new pluggable discovery client
+func NewClient(id string, args ...string) *Client {
+	return &Client{
+		id:          id,
+		processArgs: args,
+		state:       Dead,
+		userAgent:   "pluggable-discovery-protocol-handler",
+		logger:      &nullClientLogger{},
+	}
+}
+
+// SetUserAgent sets the user agent to be used in the discovery
+func (disc *Client) SetUserAgent(userAgent string) {
+	disc.userAgent = userAgent
+}
+
+// SetLogger sets the logger to be used in the discovery
+func (disc *Client) SetLogger(logger ClientLogger) {
+	disc.logger = logger
+}
+
+// GetID returns the identifier for this discovery
+func (disc *Client) GetID() string {
+	return disc.id
+}
+
+func (disc *Client) String() string {
+	return disc.id
+}
+
+func (disc *Client) jsonDecodeLoop(in io.Reader, outChan chan<- *discoveryMessage) {
+	decoder := json.NewDecoder(in)
+	closeAndReportError := func(err error) {
+		disc.statusMutex.Lock()
+		disc.state = Dead
+		disc.incomingMessagesError = err
+		disc.statusMutex.Unlock()
+		close(outChan)
+		disc.logger.Errorf("stopped discovery %s decode loop: %v", disc.id, 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)
+			return
+		} else if err != nil {
+			closeAndReportError(err)
+			return
+		}
+		disc.logger.Infof("from discovery %s received message %s", disc.id, msg)
+		if msg.EventType == "add" {
+			if msg.Port == nil {
+				closeAndReportError(errors.New("invalid 'add' message: missing port"))
+				return
+			}
+			disc.statusMutex.Lock()
+			if disc.eventChan != nil {
+				disc.eventChan <- &Event{"add", msg.Port, disc.GetID()}
+			}
+			disc.statusMutex.Unlock()
+		} else if msg.EventType == "remove" {
+			if msg.Port == nil {
+				closeAndReportError(errors.New("invalid 'remove' message: missing port"))
+				return
+			}
+			disc.statusMutex.Lock()
+			if disc.eventChan != nil {
+				disc.eventChan <- &Event{"remove", msg.Port, disc.GetID()}
+			}
+			disc.statusMutex.Unlock()
+		} else {
+			outChan <- &msg
+		}
+	}
+}
+
+// State returns the current state of this PluggableDiscovery
+func (disc *Client) State() int {
+	disc.statusMutex.Lock()
+	defer disc.statusMutex.Unlock()
+	return disc.state
+}
+
+func (disc *Client) waitMessage(timeout time.Duration) (*discoveryMessage, error) {
+	select {
+	case msg := <-disc.incomingMessagesChan:
+		if msg == nil {
+			return nil, disc.incomingMessagesError
+		}
+		return msg, nil
+	case <-time.After(timeout):
+		return nil, fmt.Errorf("timeout waiting for message from %s", disc.id)
+	}
+}
+
+func (disc *Client) sendCommand(command string) error {
+	disc.logger.Infof("sending command %s to discovery %s", strings.TrimSpace(command), disc)
+	data := []byte(command)
+	for {
+		n, err := disc.outgoingCommandsPipe.Write(data)
+		if err != nil {
+			return err
+		}
+		if n == len(data) {
+			return nil
+		}
+		data = data[n:]
+	}
+}
+
+func (disc *Client) runProcess() error {
+	disc.logger.Infof("starting discovery %s process", disc.id)
+	proc, err := paths.NewProcess(nil, disc.processArgs...)
+	if err != nil {
+		return err
+	}
+	stdout, err := proc.StdoutPipe()
+	if err != nil {
+		return err
+	}
+	stdin, err := proc.StdinPipe()
+	if err != nil {
+		return err
+	}
+	disc.outgoingCommandsPipe = stdin
+
+	messageChan := make(chan *discoveryMessage)
+	disc.incomingMessagesChan = messageChan
+	go disc.jsonDecodeLoop(stdout, messageChan)
+
+	if err := proc.Start(); err != nil {
+		return err
+	}
+
+	disc.statusMutex.Lock()
+	defer disc.statusMutex.Unlock()
+	disc.process = proc
+	disc.state = Alive
+	disc.logger.Infof("started discovery %s process", disc.id)
+	return nil
+}
+
+func (disc *Client) killProcess() error {
+	disc.logger.Infof("killing discovery %s process", disc.id)
+	if disc.process != nil {
+		if err := disc.process.Kill(); err != nil {
+			return err
+		}
+		if err := disc.process.Wait(); err != nil {
+			return err
+		}
+	}
+	disc.statusMutex.Lock()
+	defer disc.statusMutex.Unlock()
+	disc.stopSync()
+	disc.state = Dead
+	disc.logger.Infof("killed discovery %s process", disc.id)
+	return nil
+}
+
+// Run starts the discovery executable process and sends the HELLO command to the discovery to agree on the
+// pluggable discovery protocol. This must be the first command to run in the communication with the discovery.
+// If the process is started but the HELLO command fails the process is killed.
+func (disc *Client) Run() (err error) {
+	if err = disc.runProcess(); err != nil {
+		return err
+	}
+
+	defer func() {
+		// If the discovery process is started successfully but the HELLO handshake
+		// fails the discovery is an unusable state, we kill the process to avoid
+		// further issues down the line.
+		if err == nil {
+			return
+		}
+		if err := disc.killProcess(); err != nil {
+			// Log failure to kill the process, ideally that should never happen
+			// but it's best to know it if it does
+			disc.logger.Errorf("Killing discovery %s after unsuccessful start: %s", disc.id, err)
+		}
+	}()
+
+	if err = disc.sendCommand("HELLO 1 \"arduino-cli " + disc.userAgent + "\"\n"); err != nil {
+		return err
+	}
+	if msg, err := disc.waitMessage(time.Second * 10); err != nil {
+		return fmt.Errorf("calling HELLO: %w", err)
+	} else if msg.EventType != "hello" {
+		return fmt.Errorf("event out of sync, expected 'hello', received '%s'", msg.EventType)
+	} else if msg.Error {
+		return fmt.Errorf("command failed: %s", msg.Message)
+	} else if strings.ToUpper(msg.Message) != "OK" {
+		return fmt.Errorf("communication out of sync, expected 'OK', received '%s'", msg.Message)
+	} else if msg.ProtocolVersion > 1 {
+		return fmt.Errorf("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.
+func (disc *Client) Start() error {
+	if err := disc.sendCommand("START\n"); err != nil {
+		return err
+	}
+	if msg, err := disc.waitMessage(time.Second * 10); err != nil {
+		return fmt.Errorf("calling START: %w", err)
+	} else if msg.EventType != "start" {
+		return fmt.Errorf("event out of sync, expected 'start', received '%s'", msg.EventType)
+	} else if msg.Error {
+		return fmt.Errorf("command failed: %s", msg.Message)
+	} else if strings.ToUpper(msg.Message) != "OK" {
+		return fmt.Errorf("communication out of sync, expected 'OK', received '%s'", msg.Message)
+	}
+	disc.statusMutex.Lock()
+	defer disc.statusMutex.Unlock()
+	disc.state = Running
+	return nil
+}
+
+// Stop stops the discovery internal subroutines and possibly free the internally
+// used resources. This command should be called if the client wants to pause the
+// discovery for a while.
+func (disc *Client) Stop() error {
+	if err := disc.sendCommand("STOP\n"); err != nil {
+		return err
+	}
+	if msg, err := disc.waitMessage(time.Second * 10); err != nil {
+		return fmt.Errorf("calling STOP: %w", err)
+	} else if msg.EventType != "stop" {
+		return fmt.Errorf("event out of sync, expected 'stop', received '%s'", msg.EventType)
+	} else if msg.Error {
+		return fmt.Errorf("command failed: %s", msg.Message)
+	} else if strings.ToUpper(msg.Message) != "OK" {
+		return fmt.Errorf("communication out of sync, expected 'OK', received '%s'", msg.Message)
+	}
+	disc.statusMutex.Lock()
+	defer disc.statusMutex.Unlock()
+	disc.stopSync()
+	disc.state = Idling
+	return nil
+}
+
+func (disc *Client) stopSync() {
+	if disc.eventChan != nil {
+		disc.eventChan <- &Event{"stop", nil, disc.GetID()}
+		close(disc.eventChan)
+		disc.eventChan = nil
+	}
+}
+
+// Quit terminates the discovery. No more commands can be accepted by the discovery.
+func (disc *Client) Quit() {
+	_ = disc.sendCommand("QUIT\n")
+	if _, err := disc.waitMessage(time.Second * 5); err != nil {
+		disc.logger.Errorf("Quitting discovery %s: %s", disc.id, err)
+	}
+	disc.stopSync()
+	disc.killProcess()
+}
+
+// List executes an enumeration of the ports and returns a list of the available
+// ports at the moment of the call.
+func (disc *Client) List() ([]*Port, error) {
+	if err := disc.sendCommand("LIST\n"); err != nil {
+		return nil, err
+	}
+	if msg, err := disc.waitMessage(time.Second * 10); err != nil {
+		return nil, fmt.Errorf("calling LIST: %w", err)
+	} else if msg.EventType != "list" {
+		return nil, fmt.Errorf("event out of sync, expected 'list', received '%s'", msg.EventType)
+	} else if msg.Error {
+		return nil, fmt.Errorf("command failed: %s", msg.Message)
+	} else {
+		return msg.Ports, nil
+	}
+}
+
+// StartSync puts the discovery in "events" mode: the discovery will send "add"
+// and "remove" events each time a new port is detected or removed respectively.
+// After calling StartSync an initial burst of "add" events may be generated to
+// report all the ports available at the moment of the start.
+// It also creates a channel used to receive events from the pluggable discovery.
+// The event channel must be consumed as quickly as possible since it may block the
+// discovery if it becomes full. The channel size is configurable.
+func (disc *Client) StartSync(size int) (<-chan *Event, error) {
+	disc.statusMutex.Lock()
+	defer disc.statusMutex.Unlock()
+
+	if err := disc.sendCommand("START_SYNC\n"); err != nil {
+		return nil, err
+	}
+
+	if msg, err := disc.waitMessage(time.Second * 10); err != nil {
+		return nil, fmt.Errorf("calling START_SYNC: %w", err)
+	} else if msg.EventType != "start_sync" {
+		return nil, fmt.Errorf("evemt out of sync, expected 'start_sync', received '%s'", msg.EventType)
+	} else if msg.Error {
+		return nil, fmt.Errorf("command failed: %s", msg.Message)
+	} else if strings.ToUpper(msg.Message) != "OK" {
+		return nil, fmt.Errorf("communication out of sync, expected 'OK', received '%s'", 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)
+	disc.eventChan = c
+	return c, nil
+}
diff --git a/client_test.go b/client_test.go
new file mode 100644
index 0000000..cd709b3
--- /dev/null
+++ b/client_test.go
@@ -0,0 +1,75 @@
+//
+// This file is part of pluggable-discovery-protocol-handler.
+//
+// Copyright 2024 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to modify or
+// otherwise use the software for commercial activities involving the Arduino
+// software without disclosing the source code of your own applications. To purchase
+// a commercial license, send an email to license@arduino.cc.
+//
+
+package discovery
+
+import (
+	"io"
+	"testing"
+	"time"
+
+	"github.com/arduino/go-paths-helper"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDiscoveryStdioHandling(t *testing.T) {
+	// Build `cat` helper inside testdata/cat
+	builder, err := paths.NewProcess(nil, "go", "build")
+	require.NoError(t, err)
+	builder.SetDir("testdata/cat")
+	require.NoError(t, builder.Run())
+
+	// Run cat and test if streaming json works as expected
+	disc := NewClient("test", "testdata/cat/cat") // copy stdin to stdout
+
+	err = disc.runProcess()
+	require.NoError(t, err)
+
+	_, err = disc.outgoingCommandsPipe.Write([]byte(`{ "eventType":`)) // send partial JSON
+	require.NoError(t, err)
+	msg, err := disc.waitMessage(time.Millisecond * 100)
+	require.Error(t, err)
+	require.Nil(t, msg)
+
+	_, err = disc.outgoingCommandsPipe.Write([]byte(`"ev1" }{ `)) // complete previous json and start another one
+	require.NoError(t, err)
+
+	msg, err = disc.waitMessage(time.Millisecond * 100)
+	require.NoError(t, err)
+	require.NotNil(t, msg)
+	require.Equal(t, "ev1", msg.EventType)
+
+	msg, err = disc.waitMessage(time.Millisecond * 100)
+	require.Error(t, err)
+	require.Nil(t, msg)
+
+	_, err = disc.outgoingCommandsPipe.Write([]byte(`"eventType":"ev2" }`)) // complete previous json
+	require.NoError(t, err)
+
+	msg, err = disc.waitMessage(time.Millisecond * 100)
+	require.NoError(t, err)
+	require.NotNil(t, msg)
+	require.Equal(t, "ev2", msg.EventType)
+
+	require.Equal(t, disc.State(), Alive)
+
+	err = disc.outgoingCommandsPipe.(io.ReadCloser).Close()
+	require.NoError(t, err)
+	time.Sleep(time.Millisecond * 100)
+
+	require.Equal(t, disc.State(), Dead)
+}
diff --git a/discovery.go b/discovery.go
index 54f064c..190d6b4 100644
--- a/discovery.go
+++ b/discovery.go
@@ -18,9 +18,10 @@
 // Package discovery is a library for handling the Arduino Pluggable-Discovery protocol
 // (https://github.com/arduino/tooling-rfcs/blob/main/RFCs/0002-pluggable-discovery.md#pluggable-discovery-api-via-stdinstdout)
 //
-// The library implements the state machine and the parsing logic to communicate with a pluggable-discovery client.
-// All the commands issued by the client are conveniently translated into function calls, in particular
-// the Discovery interface are the only functions that must be implemented to get a fully working pluggable discovery
+// The library implements the state machine and the parsing logic to implement a pluggable-discovery client and server.
+//
+// While implementing a server, all the commands issued by the client are conveniently translated into function calls, in particular
+// the methods of the Discovery interface are the only functions that must be implemented to get a fully working pluggable discovery
 // using this library.
 //
 // A usage example is provided in the dummy-discovery package.
diff --git a/testdata/cat/.gitignore b/testdata/cat/.gitignore
new file mode 100644
index 0000000..a7053b1
--- /dev/null
+++ b/testdata/cat/.gitignore
@@ -0,0 +1,2 @@
+cat
+cat.exe
diff --git a/testdata/cat/main.go b/testdata/cat/main.go
new file mode 100644
index 0000000..7f77910
--- /dev/null
+++ b/testdata/cat/main.go
@@ -0,0 +1,28 @@
+// This file is part of arduino-cli.
+//
+// Copyright 2023 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to license@arduino.cc.
+
+// Echo stdin to stdout.
+// This program is used for testing purposes, to make it available on all
+// OS a tool equivalent to UNIX "cat".
+package main
+
+import (
+	"io"
+	"os"
+)
+
+func main() {
+	io.Copy(os.Stdout, os.Stdin)
+}

From 2b24b07045a537a5f416eadce2423cff6256873c Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Fri, 9 Feb 2024 12:12:35 +0100
Subject: [PATCH 02/12] Moved all Port definitions in a separate file

---
 client.go    | 25 ------------------------
 discovery.go | 12 ------------
 port.go      | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 37 deletions(-)
 create mode 100644 port.go

diff --git a/client.go b/client.go
index ed1dda0..e997016 100644
--- a/client.go
+++ b/client.go
@@ -96,31 +96,6 @@ func (msg discoveryMessage) String() string {
 	return s
 }
 
-// Equals returns true if the given port has the same address and protocol
-// of the current port.
-func (p *Port) Equals(o *Port) bool {
-	return p.Address == o.Address && p.Protocol == o.Protocol
-}
-
-func (p *Port) String() string {
-	if p == nil {
-		return "none"
-	}
-	return p.Address
-}
-
-// Clone creates a copy of this Port
-func (p *Port) Clone() *Port {
-	if p == nil {
-		return nil
-	}
-	res := *p
-	if p.Properties != nil {
-		res.Properties = p.Properties.Clone()
-	}
-	return &res
-}
-
 // Event is a pluggable discovery event
 type Event struct {
 	Type        string
diff --git a/discovery.go b/discovery.go
index 190d6b4..9296286 100644
--- a/discovery.go
+++ b/discovery.go
@@ -36,20 +36,8 @@ import (
 	"strconv"
 	"strings"
 	"sync"
-
-	"github.com/arduino/go-properties-orderedmap"
 )
 
-// Port is a descriptor for a board port
-type Port struct {
-	Address       string          `json:"address"`
-	AddressLabel  string          `json:"label,omitempty"`
-	Protocol      string          `json:"protocol,omitempty"`
-	ProtocolLabel string          `json:"protocolLabel,omitempty"`
-	Properties    *properties.Map `json:"properties,omitempty"`
-	HardwareID    string          `json:"hardwareId,omitempty"`
-}
-
 // Discovery is an interface that represents the business logic that
 // a pluggable discovery must implement. The communication protocol
 // is completely hidden and it's handled by a DiscoveryServer.
diff --git a/port.go b/port.go
new file mode 100644
index 0000000..67c2bfd
--- /dev/null
+++ b/port.go
@@ -0,0 +1,55 @@
+//
+// This file is part of pluggable-discovery-protocol-handler.
+//
+// Copyright 2024 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to modify or
+// otherwise use the software for commercial activities involving the Arduino
+// software without disclosing the source code of your own applications. To purchase
+// a commercial license, send an email to license@arduino.cc.
+//
+
+package discovery
+
+import "github.com/arduino/go-properties-orderedmap"
+
+// Port is a descriptor for a board port
+type Port struct {
+	Address       string          `json:"address"`
+	AddressLabel  string          `json:"label,omitempty"`
+	Protocol      string          `json:"protocol,omitempty"`
+	ProtocolLabel string          `json:"protocolLabel,omitempty"`
+	Properties    *properties.Map `json:"properties,omitempty"`
+	HardwareID    string          `json:"hardwareId,omitempty"`
+}
+
+// Equals returns true if the given port has the same address and protocol
+// of the current port.
+func (p *Port) Equals(o *Port) bool {
+	return p.Address == o.Address && p.Protocol == o.Protocol
+}
+
+func (p *Port) String() string {
+	if p == nil {
+		return "none"
+	}
+	return p.Address
+}
+
+// Clone creates a copy of this Port
+func (p *Port) Clone() *Port {
+	if p == nil {
+		return nil
+	}
+	res := *p
+	if p.Properties != nil {
+		res.Properties = p.Properties.Clone()
+	}
+	return &res
+}

From 1c64d78fbadf964095dbb9bd77d6cc27c6a1bc0d Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Fri, 9 Feb 2024 17:39:22 +0100
Subject: [PATCH 03/12] Removed useless state tracking

---
 client.go      | 28 ----------------------------
 client_test.go |  6 ++++--
 2 files changed, 4 insertions(+), 30 deletions(-)

diff --git a/client.go b/client.go
index e997016..7b4edc0 100644
--- a/client.go
+++ b/client.go
@@ -29,17 +29,6 @@ import (
 	"github.com/arduino/go-paths-helper"
 )
 
-// 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
-)
-
 // Client is a tool that detects communication ports to interact
 // with the boards.
 type Client struct {
@@ -54,7 +43,6 @@ type Client struct {
 	// All the following fields are guarded by statusMutex
 	statusMutex           sync.Mutex
 	incomingMessagesError error
-	state                 int
 	eventChan             chan<- *Event
 }
 
@@ -108,7 +96,6 @@ func NewClient(id string, args ...string) *Client {
 	return &Client{
 		id:          id,
 		processArgs: args,
-		state:       Dead,
 		userAgent:   "pluggable-discovery-protocol-handler",
 		logger:      &nullClientLogger{},
 	}
@@ -137,7 +124,6 @@ func (disc *Client) jsonDecodeLoop(in io.Reader, outChan chan<- *discoveryMessag
 	decoder := json.NewDecoder(in)
 	closeAndReportError := func(err error) {
 		disc.statusMutex.Lock()
-		disc.state = Dead
 		disc.incomingMessagesError = err
 		disc.statusMutex.Unlock()
 		close(outChan)
@@ -149,7 +135,6 @@ func (disc *Client) jsonDecodeLoop(in io.Reader, outChan chan<- *discoveryMessag
 		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)
@@ -185,13 +170,6 @@ func (disc *Client) jsonDecodeLoop(in io.Reader, outChan chan<- *discoveryMessag
 	}
 }
 
-// State returns the current state of this PluggableDiscovery
-func (disc *Client) State() int {
-	disc.statusMutex.Lock()
-	defer disc.statusMutex.Unlock()
-	return disc.state
-}
-
 func (disc *Client) waitMessage(timeout time.Duration) (*discoveryMessage, error) {
 	select {
 	case msg := <-disc.incomingMessagesChan:
@@ -246,7 +224,6 @@ func (disc *Client) runProcess() error {
 	disc.statusMutex.Lock()
 	defer disc.statusMutex.Unlock()
 	disc.process = proc
-	disc.state = Alive
 	disc.logger.Infof("started discovery %s process", disc.id)
 	return nil
 }
@@ -264,7 +241,6 @@ func (disc *Client) killProcess() error {
 	disc.statusMutex.Lock()
 	defer disc.statusMutex.Unlock()
 	disc.stopSync()
-	disc.state = Dead
 	disc.logger.Infof("killed discovery %s process", disc.id)
 	return nil
 }
@@ -307,7 +283,6 @@ func (disc *Client) Run() (err error) {
 	}
 	disc.statusMutex.Lock()
 	defer disc.statusMutex.Unlock()
-	disc.state = Idling
 	return nil
 }
 
@@ -328,7 +303,6 @@ func (disc *Client) Start() error {
 	}
 	disc.statusMutex.Lock()
 	defer disc.statusMutex.Unlock()
-	disc.state = Running
 	return nil
 }
 
@@ -351,7 +325,6 @@ func (disc *Client) Stop() error {
 	disc.statusMutex.Lock()
 	defer disc.statusMutex.Unlock()
 	disc.stopSync()
-	disc.state = Idling
 	return nil
 }
 
@@ -415,7 +388,6 @@ func (disc *Client) StartSync(size int) (<-chan *Event, error) {
 		return nil, fmt.Errorf("communication out of sync, expected 'OK', received '%s'", 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/client_test.go b/client_test.go
index cd709b3..7781a86 100644
--- a/client_test.go
+++ b/client_test.go
@@ -65,11 +65,13 @@ func TestDiscoveryStdioHandling(t *testing.T) {
 	require.NotNil(t, msg)
 	require.Equal(t, "ev2", msg.EventType)
 
-	require.Equal(t, disc.State(), Alive)
+	// TODO:
+	// require.Equal(t, disc.State(), Alive)
 
 	err = disc.outgoingCommandsPipe.(io.ReadCloser).Close()
 	require.NoError(t, err)
 	time.Sleep(time.Millisecond * 100)
 
-	require.Equal(t, disc.State(), Dead)
+	// TODO:
+	// require.Equal(t, disc.State(), Dead)
 }

From b6925f16ca4828a19a5277654d22dec5d33f4921 Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Fri, 9 Feb 2024 17:39:50 +0100
Subject: [PATCH 04/12] Fixed comment

---
 client.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/client.go b/client.go
index 7b4edc0..2d9b772 100644
--- a/client.go
+++ b/client.go
@@ -287,7 +287,7 @@ func (disc *Client) Run() (err error) {
 }
 
 // Start initializes and start the discovery internal subroutines. This command must be
-// called before List or StartSync.
+// called before List.
 func (disc *Client) Start() error {
 	if err := disc.sendCommand("START\n"); err != nil {
 		return err

From d3577b5a5e28f20b315003d674d1078f5b3ad85c Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Fri, 9 Feb 2024 17:42:13 +0100
Subject: [PATCH 05/12] Use the Stringer interface to output discovery id

---
 client.go | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/client.go b/client.go
index 2d9b772..374ad66 100644
--- a/client.go
+++ b/client.go
@@ -127,7 +127,7 @@ func (disc *Client) jsonDecodeLoop(in io.Reader, outChan chan<- *discoveryMessag
 		disc.incomingMessagesError = err
 		disc.statusMutex.Unlock()
 		close(outChan)
-		disc.logger.Errorf("stopped discovery %s decode loop: %v", disc.id, err)
+		disc.logger.Errorf("stopped discovery %s decode loop: %v", disc, err)
 	}
 
 	for {
@@ -143,7 +143,7 @@ func (disc *Client) jsonDecodeLoop(in io.Reader, outChan chan<- *discoveryMessag
 			closeAndReportError(err)
 			return
 		}
-		disc.logger.Infof("from discovery %s received message %s", disc.id, msg)
+		disc.logger.Infof("from discovery %s received message %s", disc, msg)
 		if msg.EventType == "add" {
 			if msg.Port == nil {
 				closeAndReportError(errors.New("invalid 'add' message: missing port"))
@@ -178,7 +178,7 @@ func (disc *Client) waitMessage(timeout time.Duration) (*discoveryMessage, error
 		}
 		return msg, nil
 	case <-time.After(timeout):
-		return nil, fmt.Errorf("timeout waiting for message from %s", disc.id)
+		return nil, fmt.Errorf("timeout waiting for message from %s", disc)
 	}
 }
 
@@ -198,7 +198,7 @@ func (disc *Client) sendCommand(command string) error {
 }
 
 func (disc *Client) runProcess() error {
-	disc.logger.Infof("starting discovery %s process", disc.id)
+	disc.logger.Infof("starting discovery %s process", disc)
 	proc, err := paths.NewProcess(nil, disc.processArgs...)
 	if err != nil {
 		return err
@@ -224,12 +224,12 @@ func (disc *Client) runProcess() error {
 	disc.statusMutex.Lock()
 	defer disc.statusMutex.Unlock()
 	disc.process = proc
-	disc.logger.Infof("started discovery %s process", disc.id)
+	disc.logger.Infof("started discovery %s process", disc)
 	return nil
 }
 
 func (disc *Client) killProcess() error {
-	disc.logger.Infof("killing discovery %s process", disc.id)
+	disc.logger.Infof("killing discovery %s process", disc)
 	if disc.process != nil {
 		if err := disc.process.Kill(); err != nil {
 			return err
@@ -241,7 +241,7 @@ func (disc *Client) killProcess() error {
 	disc.statusMutex.Lock()
 	defer disc.statusMutex.Unlock()
 	disc.stopSync()
-	disc.logger.Infof("killed discovery %s process", disc.id)
+	disc.logger.Infof("killed discovery %s process", disc)
 	return nil
 }
 
@@ -263,7 +263,7 @@ func (disc *Client) Run() (err error) {
 		if err := disc.killProcess(); err != nil {
 			// Log failure to kill the process, ideally that should never happen
 			// but it's best to know it if it does
-			disc.logger.Errorf("Killing discovery %s after unsuccessful start: %s", disc.id, err)
+			disc.logger.Errorf("Killing discovery %s after unsuccessful start: %s", disc, err)
 		}
 	}()
 
@@ -340,7 +340,7 @@ func (disc *Client) stopSync() {
 func (disc *Client) Quit() {
 	_ = disc.sendCommand("QUIT\n")
 	if _, err := disc.waitMessage(time.Second * 5); err != nil {
-		disc.logger.Errorf("Quitting discovery %s: %s", disc.id, err)
+		disc.logger.Errorf("Quitting discovery %s: %s", disc, err)
 	}
 	disc.stopSync()
 	disc.killProcess()

From eb34220818e831870f7996db09ba49daf5a3d3a3 Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Fri, 9 Feb 2024 17:46:31 +0100
Subject: [PATCH 06/12] Factored exit function

---
 client.go | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/client.go b/client.go
index 374ad66..d1d53e0 100644
--- a/client.go
+++ b/client.go
@@ -127,17 +127,18 @@ func (disc *Client) jsonDecodeLoop(in io.Reader, outChan chan<- *discoveryMessag
 		disc.incomingMessagesError = err
 		disc.statusMutex.Unlock()
 		close(outChan)
-		disc.logger.Errorf("stopped discovery %s decode loop: %v", disc, err)
+		if err != nil {
+			disc.logger.Errorf("stopped discovery %s decode loop: %v", disc, err)
+		} else {
+			disc.logger.Infof("stopped discovery %s decode loop", 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.incomingMessagesError = err
-			disc.statusMutex.Unlock()
-			close(outChan)
+			// This is fine :flames: we exit gracefully
+			closeAndReportError(nil)
 			return
 		} else if err != nil {
 			closeAndReportError(err)

From ba8f6ce11e72e4bc16cc2ae81155936074d72f7d Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Fri, 9 Feb 2024 17:51:25 +0100
Subject: [PATCH 07/12] Use `process` field to dermine liveness of the
 discovery

Also, force a process kill when the decode loop exits.
---
 client.go      | 16 +++++++++++++---
 client_test.go |  6 ++----
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/client.go b/client.go
index d1d53e0..3e2d27c 100644
--- a/client.go
+++ b/client.go
@@ -126,6 +126,8 @@ func (disc *Client) jsonDecodeLoop(in io.Reader, outChan chan<- *discoveryMessag
 		disc.statusMutex.Lock()
 		disc.incomingMessagesError = err
 		disc.statusMutex.Unlock()
+		disc.stopSync()
+		disc.killProcess()
 		close(outChan)
 		if err != nil {
 			disc.logger.Errorf("stopped discovery %s decode loop: %v", disc, err)
@@ -171,6 +173,13 @@ func (disc *Client) jsonDecodeLoop(in io.Reader, outChan chan<- *discoveryMessag
 	}
 }
 
+// Alive returns true if the discovery is running and false otherwise.
+func (disc *Client) Alive() bool {
+	disc.statusMutex.Lock()
+	defer disc.statusMutex.Unlock()
+	return disc.process != nil
+}
+
 func (disc *Client) waitMessage(timeout time.Duration) (*discoveryMessage, error) {
 	select {
 	case msg := <-disc.incomingMessagesChan:
@@ -230,6 +239,9 @@ func (disc *Client) runProcess() error {
 }
 
 func (disc *Client) killProcess() error {
+	disc.statusMutex.Lock()
+	defer disc.statusMutex.Unlock()
+
 	disc.logger.Infof("killing discovery %s process", disc)
 	if disc.process != nil {
 		if err := disc.process.Kill(); err != nil {
@@ -238,10 +250,8 @@ func (disc *Client) killProcess() error {
 		if err := disc.process.Wait(); err != nil {
 			return err
 		}
+		disc.process = nil
 	}
-	disc.statusMutex.Lock()
-	defer disc.statusMutex.Unlock()
-	disc.stopSync()
 	disc.logger.Infof("killed discovery %s process", disc)
 	return nil
 }
diff --git a/client_test.go b/client_test.go
index 7781a86..f11a6ba 100644
--- a/client_test.go
+++ b/client_test.go
@@ -65,13 +65,11 @@ func TestDiscoveryStdioHandling(t *testing.T) {
 	require.NotNil(t, msg)
 	require.Equal(t, "ev2", msg.EventType)
 
-	// TODO:
-	// 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)
 
-	// TODO:
-	// require.Equal(t, disc.State(), Dead)
+	require.False(t, disc.Alive())
 }

From 31ce0cf323e408dd6fa01d356e82b501f1f46cd9 Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Fri, 9 Feb 2024 18:21:45 +0100
Subject: [PATCH 08/12] Improved logging messages

---
 client.go | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/client.go b/client.go
index 3e2d27c..74c6edd 100644
--- a/client.go
+++ b/client.go
@@ -70,16 +70,16 @@ type discoveryMessage struct {
 func (msg discoveryMessage) String() string {
 	s := fmt.Sprintf("type: %s", msg.EventType)
 	if msg.Message != "" {
-		s = fmt.Sprintf("%[1]s, message: %[2]s", s, msg.Message)
+		s += fmt.Sprintf(", message: %s", msg.Message)
 	}
 	if msg.ProtocolVersion != 0 {
-		s = fmt.Sprintf("%[1]s, protocol version: %[2]d", s, msg.ProtocolVersion)
+		s += fmt.Sprintf(", protocol version: %d", msg.ProtocolVersion)
 	}
 	if len(msg.Ports) > 0 {
-		s = fmt.Sprintf("%[1]s, ports: %[2]s", s, msg.Ports)
+		s += fmt.Sprintf(", ports: %s", msg.Ports)
 	}
 	if msg.Port != nil {
-		s = fmt.Sprintf("%[1]s, port: %[2]s", s, msg.Port)
+		s += fmt.Sprintf(", port: %s", msg.Port)
 	}
 	return s
 }
@@ -130,9 +130,9 @@ func (disc *Client) jsonDecodeLoop(in io.Reader, outChan chan<- *discoveryMessag
 		disc.killProcess()
 		close(outChan)
 		if err != nil {
-			disc.logger.Errorf("stopped discovery %s decode loop: %v", disc, err)
+			disc.logger.Errorf("Stopped decode loop: %v", err)
 		} else {
-			disc.logger.Infof("stopped discovery %s decode loop", disc, err)
+			disc.logger.Infof("Stopped decode loop")
 		}
 	}
 
@@ -146,7 +146,7 @@ func (disc *Client) jsonDecodeLoop(in io.Reader, outChan chan<- *discoveryMessag
 			closeAndReportError(err)
 			return
 		}
-		disc.logger.Infof("from discovery %s received message %s", disc, msg)
+		disc.logger.Infof("Received message %s", msg)
 		if msg.EventType == "add" {
 			if msg.Port == nil {
 				closeAndReportError(errors.New("invalid 'add' message: missing port"))
@@ -193,7 +193,7 @@ func (disc *Client) waitMessage(timeout time.Duration) (*discoveryMessage, error
 }
 
 func (disc *Client) sendCommand(command string) error {
-	disc.logger.Infof("sending command %s to discovery %s", strings.TrimSpace(command), disc)
+	disc.logger.Infof("Sending command %s", strings.TrimSpace(command))
 	data := []byte(command)
 	for {
 		n, err := disc.outgoingCommandsPipe.Write(data)
@@ -208,7 +208,7 @@ func (disc *Client) sendCommand(command string) error {
 }
 
 func (disc *Client) runProcess() error {
-	disc.logger.Infof("starting discovery %s process", disc)
+	disc.logger.Infof("Starting discovery process")
 	proc, err := paths.NewProcess(nil, disc.processArgs...)
 	if err != nil {
 		return err
@@ -234,7 +234,7 @@ func (disc *Client) runProcess() error {
 	disc.statusMutex.Lock()
 	defer disc.statusMutex.Unlock()
 	disc.process = proc
-	disc.logger.Infof("started discovery %s process", disc)
+	disc.logger.Infof("Discovery process started")
 	return nil
 }
 
@@ -242,17 +242,19 @@ func (disc *Client) killProcess() error {
 	disc.statusMutex.Lock()
 	defer disc.statusMutex.Unlock()
 
-	disc.logger.Infof("killing discovery %s process", disc)
+	disc.logger.Infof("Killing discovery process")
 	if disc.process != nil {
 		if err := disc.process.Kill(); err != nil {
+			disc.logger.Errorf("Killing discovery process: %v", err)
 			return err
 		}
 		if err := disc.process.Wait(); err != nil {
+			disc.logger.Errorf("Waiting discovery process termination: %v", err)
 			return err
 		}
 		disc.process = nil
 	}
-	disc.logger.Infof("killed discovery %s process", disc)
+	disc.logger.Infof("Discovery process killed")
 	return nil
 }
 
@@ -274,7 +276,7 @@ func (disc *Client) Run() (err error) {
 		if err := disc.killProcess(); err != nil {
 			// Log failure to kill the process, ideally that should never happen
 			// but it's best to know it if it does
-			disc.logger.Errorf("Killing discovery %s after unsuccessful start: %s", disc, err)
+			disc.logger.Errorf("Killing discovery after unsuccessful start: %s", err)
 		}
 	}()
 
@@ -351,7 +353,7 @@ func (disc *Client) stopSync() {
 func (disc *Client) Quit() {
 	_ = disc.sendCommand("QUIT\n")
 	if _, err := disc.waitMessage(time.Second * 5); err != nil {
-		disc.logger.Errorf("Quitting discovery %s: %s", disc, err)
+		disc.logger.Errorf("Quitting discovery: %s", err)
 	}
 	disc.stopSync()
 	disc.killProcess()

From b8864695e645663277a0d39389975cc46f8f030a Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Mon, 12 Feb 2024 09:58:04 +0100
Subject: [PATCH 09/12] Made stdio test more reliable by running fake discovery
 through TCP

---
 client_test.go                   | 26 ++++++++++++++++----------
 testdata/cat/.gitignore          |  2 --
 testdata/netcat/.gitignore       |  2 ++
 testdata/{cat => netcat}/main.go | 28 +++++++++++++++++++++++-----
 4 files changed, 41 insertions(+), 17 deletions(-)
 delete mode 100644 testdata/cat/.gitignore
 create mode 100644 testdata/netcat/.gitignore
 rename testdata/{cat => netcat}/main.go (57%)

diff --git a/client_test.go b/client_test.go
index f11a6ba..79faff0 100644
--- a/client_test.go
+++ b/client_test.go
@@ -18,7 +18,7 @@
 package discovery
 
 import (
-	"io"
+	"net"
 	"testing"
 	"time"
 
@@ -27,25 +27,31 @@ import (
 )
 
 func TestDiscoveryStdioHandling(t *testing.T) {
-	// Build `cat` helper inside testdata/cat
+	// Build `netcat` helper inside testdata/cat
 	builder, err := paths.NewProcess(nil, "go", "build")
 	require.NoError(t, err)
-	builder.SetDir("testdata/cat")
+	builder.SetDir("testdata/netcat")
 	require.NoError(t, builder.Run())
 
-	// Run cat and test if streaming json works as expected
-	disc := NewClient("test", "testdata/cat/cat") // copy stdin to stdout
+	// Run netcat and test if streaming json works as expected
+	listener, err := net.ListenTCP("tcp", nil)
+	require.NoError(t, err)
 
+	disc := NewClient("test", "testdata/netcat/netcat", listener.Addr().String())
 	err = disc.runProcess()
 	require.NoError(t, err)
 
-	_, err = disc.outgoingCommandsPipe.Write([]byte(`{ "eventType":`)) // send partial JSON
+	listener.SetDeadline(time.Now().Add(time.Second))
+	conn, err := listener.Accept()
+	require.NoError(t, err)
+
+	_, err = conn.Write([]byte(`{ "eventType":`)) // send partial JSON
 	require.NoError(t, err)
 	msg, err := disc.waitMessage(time.Millisecond * 100)
 	require.Error(t, err)
 	require.Nil(t, msg)
 
-	_, err = disc.outgoingCommandsPipe.Write([]byte(`"ev1" }{ `)) // complete previous json and start another one
+	_, err = conn.Write([]byte(`"ev1" }{ `)) // complete previous json and start another one
 	require.NoError(t, err)
 
 	msg, err = disc.waitMessage(time.Millisecond * 100)
@@ -57,7 +63,7 @@ func TestDiscoveryStdioHandling(t *testing.T) {
 	require.Error(t, err)
 	require.Nil(t, msg)
 
-	_, err = disc.outgoingCommandsPipe.Write([]byte(`"eventType":"ev2" }`)) // complete previous json
+	_, err = conn.Write([]byte(`"eventType":"ev2" }`)) // complete previous json
 	require.NoError(t, err)
 
 	msg, err = disc.waitMessage(time.Millisecond * 100)
@@ -67,9 +73,9 @@ func TestDiscoveryStdioHandling(t *testing.T) {
 
 	require.True(t, disc.Alive())
 
-	err = disc.outgoingCommandsPipe.(io.ReadCloser).Close()
+	err = conn.Close()
 	require.NoError(t, err)
-	time.Sleep(time.Millisecond * 100)
+	time.Sleep(time.Millisecond * 500)
 
 	require.False(t, disc.Alive())
 }
diff --git a/testdata/cat/.gitignore b/testdata/cat/.gitignore
deleted file mode 100644
index a7053b1..0000000
--- a/testdata/cat/.gitignore
+++ /dev/null
@@ -1,2 +0,0 @@
-cat
-cat.exe
diff --git a/testdata/netcat/.gitignore b/testdata/netcat/.gitignore
new file mode 100644
index 0000000..661f83c
--- /dev/null
+++ b/testdata/netcat/.gitignore
@@ -0,0 +1,2 @@
+netcat
+netcat.exe
diff --git a/testdata/cat/main.go b/testdata/netcat/main.go
similarity index 57%
rename from testdata/cat/main.go
rename to testdata/netcat/main.go
index 7f77910..c9f085a 100644
--- a/testdata/cat/main.go
+++ b/testdata/netcat/main.go
@@ -1,6 +1,6 @@
-// This file is part of arduino-cli.
+// This file is part of pluggable-discovery-protocol-handler.
 //
-// Copyright 2023 ARDUINO SA (http://www.arduino.cc/)
+// Copyright 2024 ARDUINO SA (http://www.arduino.cc/)
 //
 // This software is released under the GNU General Public License version 3,
 // which covers the main part of arduino-cli.
@@ -13,16 +13,34 @@
 // Arduino software without disclosing the source code of your own applications.
 // To purchase a commercial license, send an email to license@arduino.cc.
 
-// Echo stdin to stdout.
+// Proxy stdin/stdout through a TCP socket.
 // This program is used for testing purposes, to make it available on all
-// OS a tool equivalent to UNIX "cat".
+// OS a tool equivalent to UNIX "nc".
 package main
 
 import (
 	"io"
+	"net"
 	"os"
 )
 
 func main() {
-	io.Copy(os.Stdout, os.Stdin)
+	tcpAddr, err := net.ResolveTCPAddr("tcp", os.Args[1])
+	if err != nil {
+		println("ResolveTCPAddr failed:", err.Error())
+		os.Exit(1)
+	}
+
+	conn, err := net.DialTCP("tcp", nil, tcpAddr)
+	if err != nil {
+		println("Dial failed:", err.Error())
+		os.Exit(1)
+	}
+
+	go func() {
+		io.Copy(os.Stdout, conn)
+		os.Exit(0)
+	}()
+	io.Copy(conn, os.Stdin)
+	os.Exit(0)
 }

From 927d1417584add7dcc28708d5e70da4db08be5f0 Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Mon, 12 Feb 2024 13:19:32 +0100
Subject: [PATCH 10/12] Added logging to unit test

---
 client_test.go | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/client_test.go b/client_test.go
index 79faff0..72d25dc 100644
--- a/client_test.go
+++ b/client_test.go
@@ -18,6 +18,7 @@
 package discovery
 
 import (
+	"fmt"
 	"net"
 	"testing"
 	"time"
@@ -26,6 +27,18 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+type testLogger struct{}
+
+func (l *testLogger) Infof(msg string, args ...any) {
+	fmt.Printf(msg, args...)
+	fmt.Println()
+}
+
+func (l *testLogger) Errorf(msg string, args ...any) {
+	fmt.Printf(msg, args...)
+	fmt.Println()
+}
+
 func TestDiscoveryStdioHandling(t *testing.T) {
 	// Build `netcat` helper inside testdata/cat
 	builder, err := paths.NewProcess(nil, "go", "build")
@@ -38,6 +51,7 @@ func TestDiscoveryStdioHandling(t *testing.T) {
 	require.NoError(t, err)
 
 	disc := NewClient("test", "testdata/netcat/netcat", listener.Addr().String())
+	disc.SetLogger(&testLogger{})
 	err = disc.runProcess()
 	require.NoError(t, err)
 

From f236ff89e8c4a8b43d7160b370fc5caabe55f67b Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Mon, 12 Feb 2024 13:19:53 +0100
Subject: [PATCH 11/12] killProcess will always unset 'process' field

Even if the call to the Kill and Wait fails.
---
 client.go | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/client.go b/client.go
index 74c6edd..14f9a7a 100644
--- a/client.go
+++ b/client.go
@@ -238,24 +238,21 @@ func (disc *Client) runProcess() error {
 	return nil
 }
 
-func (disc *Client) killProcess() error {
+func (disc *Client) killProcess() {
 	disc.statusMutex.Lock()
 	defer disc.statusMutex.Unlock()
 
 	disc.logger.Infof("Killing discovery process")
-	if disc.process != nil {
-		if err := disc.process.Kill(); err != nil {
+	if process := disc.process; process != nil {
+		disc.process = nil
+		if err := process.Kill(); err != nil {
 			disc.logger.Errorf("Killing discovery process: %v", err)
-			return err
 		}
-		if err := disc.process.Wait(); err != nil {
+		if err := process.Wait(); err != nil {
 			disc.logger.Errorf("Waiting discovery process termination: %v", err)
-			return err
 		}
-		disc.process = nil
 	}
 	disc.logger.Infof("Discovery process killed")
-	return nil
 }
 
 // Run starts the discovery executable process and sends the HELLO command to the discovery to agree on the
@@ -273,11 +270,7 @@ func (disc *Client) Run() (err error) {
 		if err == nil {
 			return
 		}
-		if err := disc.killProcess(); err != nil {
-			// Log failure to kill the process, ideally that should never happen
-			// but it's best to know it if it does
-			disc.logger.Errorf("Killing discovery after unsuccessful start: %s", err)
-		}
+		disc.killProcess()
 	}()
 
 	if err = disc.sendCommand("HELLO 1 \"arduino-cli " + disc.userAgent + "\"\n"); err != nil {

From 6f3fc156bcb962768dbbe7b4cc8e7219c0a6dafe Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Mon, 12 Feb 2024 14:36:56 +0100
Subject: [PATCH 12/12] Reduced verbosity of logging

---
 client.go      | 18 +++++++++---------
 client_test.go |  2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/client.go b/client.go
index 14f9a7a..eeca09a 100644
--- a/client.go
+++ b/client.go
@@ -49,13 +49,13 @@ type Client struct {
 // ClientLogger is the interface that must be implemented by a logger
 // to be used in the discovery client.
 type ClientLogger interface {
-	Infof(format string, args ...interface{})
+	Debugf(format string, args ...interface{})
 	Errorf(format string, args ...interface{})
 }
 
 type nullClientLogger struct{}
 
-func (l *nullClientLogger) Infof(format string, args ...interface{})  {}
+func (l *nullClientLogger) Debugf(format string, args ...interface{}) {}
 func (l *nullClientLogger) Errorf(format string, args ...interface{}) {}
 
 type discoveryMessage struct {
@@ -132,7 +132,7 @@ func (disc *Client) jsonDecodeLoop(in io.Reader, outChan chan<- *discoveryMessag
 		if err != nil {
 			disc.logger.Errorf("Stopped decode loop: %v", err)
 		} else {
-			disc.logger.Infof("Stopped decode loop")
+			disc.logger.Debugf("Stopped decode loop")
 		}
 	}
 
@@ -146,7 +146,7 @@ func (disc *Client) jsonDecodeLoop(in io.Reader, outChan chan<- *discoveryMessag
 			closeAndReportError(err)
 			return
 		}
-		disc.logger.Infof("Received message %s", msg)
+		disc.logger.Debugf("Received message %s", msg)
 		if msg.EventType == "add" {
 			if msg.Port == nil {
 				closeAndReportError(errors.New("invalid 'add' message: missing port"))
@@ -193,7 +193,7 @@ func (disc *Client) waitMessage(timeout time.Duration) (*discoveryMessage, error
 }
 
 func (disc *Client) sendCommand(command string) error {
-	disc.logger.Infof("Sending command %s", strings.TrimSpace(command))
+	disc.logger.Debugf("Sending command %s", strings.TrimSpace(command))
 	data := []byte(command)
 	for {
 		n, err := disc.outgoingCommandsPipe.Write(data)
@@ -208,7 +208,7 @@ func (disc *Client) sendCommand(command string) error {
 }
 
 func (disc *Client) runProcess() error {
-	disc.logger.Infof("Starting discovery process")
+	disc.logger.Debugf("Starting discovery process")
 	proc, err := paths.NewProcess(nil, disc.processArgs...)
 	if err != nil {
 		return err
@@ -234,7 +234,7 @@ func (disc *Client) runProcess() error {
 	disc.statusMutex.Lock()
 	defer disc.statusMutex.Unlock()
 	disc.process = proc
-	disc.logger.Infof("Discovery process started")
+	disc.logger.Debugf("Discovery process started")
 	return nil
 }
 
@@ -242,7 +242,7 @@ func (disc *Client) killProcess() {
 	disc.statusMutex.Lock()
 	defer disc.statusMutex.Unlock()
 
-	disc.logger.Infof("Killing discovery process")
+	disc.logger.Debugf("Killing discovery process")
 	if process := disc.process; process != nil {
 		disc.process = nil
 		if err := process.Kill(); err != nil {
@@ -252,7 +252,7 @@ func (disc *Client) killProcess() {
 			disc.logger.Errorf("Waiting discovery process termination: %v", err)
 		}
 	}
-	disc.logger.Infof("Discovery process killed")
+	disc.logger.Debugf("Discovery process killed")
 }
 
 // Run starts the discovery executable process and sends the HELLO command to the discovery to agree on the
diff --git a/client_test.go b/client_test.go
index 72d25dc..ce945ee 100644
--- a/client_test.go
+++ b/client_test.go
@@ -29,7 +29,7 @@ import (
 
 type testLogger struct{}
 
-func (l *testLogger) Infof(msg string, args ...any) {
+func (l *testLogger) Debugf(msg string, args ...any) {
 	fmt.Printf(msg, args...)
 	fmt.Println()
 }