feat: add monitor command (#66)

* feat: add bridge monitor command

* Update internal/monitor/monitor.go

Co-authored-by: Cristian Maglie <[email protected]>

* improve description

* apply code review suggestions

* propertly implement the internal function

* handle also text type

* Update internal/monitor/monitor.go

Co-authored-by: Davide <[email protected]>

* Update internal/monitor/monitor.go

Co-authored-by: Davide <[email protected]>

* Update internal/monitor/monitor.go

Co-authored-by: Davide <[email protected]>

* add monitor test

* fixup! add monitor test

* fixup! fixup! add monitor test

* fixup! fixup! fixup! add monitor test

* fixup! fixup! fixup! fixup! add monitor test

---------

Co-authored-by: Cristian Maglie <[email protected]>
Co-authored-by: Davide <[email protected]>

Author: 3 people

cmd/arduino-app-cli/app/app.go

@@ -38,7 +38,6 @@ func NewAppCmd(cfg config.Configuration) *cobra.Command {
 	appCmd.AddCommand(newRestartCmd(cfg))
 	appCmd.AddCommand(newLogsCmd(cfg))
 	appCmd.AddCommand(newListCmd(cfg))
-	appCmd.AddCommand(newMonitorCmd(cfg))
 	appCmd.AddCommand(newCacheCleanCmd(cfg))
 
 	return appCmd

cmd/arduino-app-cli/main.go

@@ -30,6 +30,7 @@ import (
 	"github.com/arduino/arduino-app-cli/cmd/arduino-app-cli/config"
 	"github.com/arduino/arduino-app-cli/cmd/arduino-app-cli/daemon"
 	"github.com/arduino/arduino-app-cli/cmd/arduino-app-cli/internal/servicelocator"
+	"github.com/arduino/arduino-app-cli/cmd/arduino-app-cli/monitor"
 	"github.com/arduino/arduino-app-cli/cmd/arduino-app-cli/properties"
 	"github.com/arduino/arduino-app-cli/cmd/arduino-app-cli/system"
 	"github.com/arduino/arduino-app-cli/cmd/arduino-app-cli/version"
@@ -78,6 +79,7 @@ func run(configuration cfg.Configuration) error {
 		config.NewConfigCmd(configuration),
 		system.NewSystemCmd(configuration),
 		version.NewVersionCmd(Version),
+		monitor.NewMonitorCmd(),
 	)
 
 	ctx := context.Background()

cmd/arduino-app-cli/app/monitor.go cmd/arduino-app-cli/monitor/monitor.gocmd/arduino-app-cli/app/monitor.go renamed to cmd/arduino-app-cli/monitor/monitor.go

@@ -13,22 +13,51 @@
 // Arduino software without disclosing the source code of your own applications.
 // To purchase a commercial license, send an email to [email protected].
 
-package app
+package monitor
 
 import (
+	"io"
+	"os"
+
 	"github.com/spf13/cobra"
 
-	"github.com/arduino/arduino-app-cli/cmd/arduino-app-cli/completion"
-	"github.com/arduino/arduino-app-cli/internal/orchestrator/config"
+	"github.com/arduino/arduino-app-cli/cmd/feedback"
+	"github.com/arduino/arduino-app-cli/internal/monitor"
 )
 
-func newMonitorCmd(cfg config.Configuration) *cobra.Command {
+func NewMonitorCmd() *cobra.Command {
 	return &cobra.Command{
 		Use:   "monitor",
-		Short: "Monitor the Arduino app",
+		Short: "Attach to the microcontroller serial monitor",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			panic("not implemented")
+			stdout, _, err := feedback.DirectStreams()
+			if err != nil {
+				return err
+			}
+			start, err := monitor.NewMonitorHandler(&combinedReadWrite{r: os.Stdin, w: stdout}) // nolint:forbidigo
+			if err != nil {
+				return err
+			}
+			go start()
+			<-cmd.Context().Done()
+			return nil
 		},
-		ValidArgsFunction: completion.ApplicationNames(cfg),
 	}
 }
+
+type combinedReadWrite struct {
+	r io.Reader
+	w io.Writer
+}
+
+func (crw *combinedReadWrite) Read(p []byte) (n int, err error) {
+	return crw.r.Read(p)
+}
+
+func (crw *combinedReadWrite) Write(p []byte) (n int, err error) {
+	return crw.w.Write(p)
+}
+
+func (crw *combinedReadWrite) Close() error {
+	return nil
+}

internal/api/handlers/monitor.go

@@ -16,71 +16,50 @@
 package handlers
 
 import (
-	"errors"
 	"fmt"
-	"io"
 	"log/slog"
 	"net"
 	"net/http"
 	"strings"
-	"time"
 
 	"github.com/gorilla/websocket"
 
 	"github.com/arduino/arduino-app-cli/internal/api/models"
+	"github.com/arduino/arduino-app-cli/internal/monitor"
 	"github.com/arduino/arduino-app-cli/internal/render"
 )
 
-func monitorStream(mon net.Conn, ws *websocket.Conn) {
-	logWebsocketError := func(msg string, err error) {
-		// Do not log simple close or interruption errors
-		if websocket.IsUnexpectedCloseError(err, websocket.CloseNormalClosure, websocket.CloseGoingAway, websocket.CloseNoStatusReceived, websocket.CloseAbnormalClosure) {
-			if e, ok := err.(*websocket.CloseError); ok {
-				slog.Error(msg, slog.String("closecause", fmt.Sprintf("%d: %s", e.Code, err)))
-			} else {
-				slog.Error(msg, slog.String("error", err.Error()))
-			}
-		}
-	}
-	logSocketError := func(msg string, err error) {
-		if !errors.Is(err, net.ErrClosed) && !errors.Is(err, io.EOF) {
-			slog.Error(msg, slog.String("error", err.Error()))
-		}
+func HandleMonitorWS(allowedOrigins []string) http.HandlerFunc {
+	upgrader := websocket.Upgrader{
+		ReadBufferSize:  1024,
+		WriteBufferSize: 1024,
+		CheckOrigin: func(r *http.Request) bool {
+			return checkOrigin(r.Header.Get("Origin"), allowedOrigins)
+		},
 	}
-	go func() {
-		defer mon.Close()
-		defer ws.Close()
-		for {
-			// Read from websocket and write to monitor
-			_, msg, err := ws.ReadMessage()
-			if err != nil {
-				logWebsocketError("Error reading from websocket", err)
-				return
-			}
-			if _, err := mon.Write(msg); err != nil {
-				logSocketError("Error writing to monitor", err)
-				return
-			}
+
+	return func(w http.ResponseWriter, r *http.Request) {
+		// Upgrade the connection to websocket
+		conn, err := upgrader.Upgrade(w, r, nil)
+		if err != nil {
+			// Remember to close monitor connection if websocket upgrade fails.
+
+			slog.Error("Failed to upgrade connection", slog.String("error", err.Error()))
+			render.EncodeResponse(w, http.StatusInternalServerError, map[string]string{"error": "Failed to upgrade connection: " + err.Error()})
+			return
 		}
-	}()
-	go func() {
-		defer mon.Close()
-		defer ws.Close()
-		buff := [1024]byte{}
-		for {
-			// Read from monitor and write to websocket
-			n, err := mon.Read(buff[:])
-			if err != nil {
-				logSocketError("Error reading from monitor", err)
-				return
-			}
-
-			if err := ws.WriteMessage(websocket.BinaryMessage, buff[:n]); err != nil {
-				logWebsocketError("Error writing to websocket", err)
-				return
-			}
+
+		// Now the connection is managed by the websocket library, let's move the handlers in the goroutine
+		start, err := monitor.NewMonitorHandler(&wsReadWriteCloser{conn: conn})
+		if err != nil {
+			slog.Error("Unable to start monitor handler", slog.String("error", err.Error()))
+			render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "Unable to start monitor handler: " + err.Error()})
+			return
 		}
-	}()
+		go start()
+
+		// and return nothing to the http library
+	}
 }
 
 func splitOrigin(origin string) (scheme, host, port string, err error) {
@@ -126,41 +105,47 @@ func checkOrigin(origin string, allowedOrigins []string) bool {
 	return false
 }
 
-func HandleMonitorWS(allowedOrigins []string) http.HandlerFunc {
-	// Do a dry-run of checkorigin, so it can panic if misconfigured now, not on first request
-	_ = checkOrigin("http://localhost", allowedOrigins)
+type wsReadWriteCloser struct {
+	conn *websocket.Conn
 
-	upgrader := websocket.Upgrader{
-		ReadBufferSize:  1024,
-		WriteBufferSize: 1024,
-		CheckOrigin: func(r *http.Request) bool {
-			return checkOrigin(r.Header.Get("Origin"), allowedOrigins)
-		},
-	}
+	buff []byte
+}
 
-	return func(w http.ResponseWriter, r *http.Request) {
-		// Connect to monitor
-		mon, err := net.DialTimeout("tcp", "127.0.0.1:7500", time.Second)
-		if err != nil {
-			slog.Error("Unable to connect to monitor", slog.String("error", err.Error()))
-			render.EncodeResponse(w, http.StatusServiceUnavailable, models.ErrorResponse{Details: "Unable to connect to monitor: " + err.Error()})
-			return
-		}
+func (w *wsReadWriteCloser) Read(p []byte) (n int, err error) {
+	if len(w.buff) > 0 {
+		n = copy(p, w.buff)
+		w.buff = w.buff[n:]
+		return n, nil
+	}
 
-		// Upgrade the connection to websocket
-		conn, err := upgrader.Upgrade(w, r, nil)
-		if err != nil {
-			// Remember to close monitor connection if websocket upgrade fails.
-			mon.Close()
+	ty, message, err := w.conn.ReadMessage()
+	if err != nil {
+		return 0, mapWebSocketErrors(err)
+	}
+	if ty != websocket.BinaryMessage && ty != websocket.TextMessage {
+		return
+	}
+	n = copy(p, message)
+	w.buff = message[n:]
+	return n, nil
+}
 
-			slog.Error("Failed to upgrade connection", slog.String("error", err.Error()))
-			render.EncodeResponse(w, http.StatusInternalServerError, map[string]string{"error": "Failed to upgrade connection: " + err.Error()})
-			return
-		}
+func (w *wsReadWriteCloser) Write(p []byte) (n int, err error) {
+	err = w.conn.WriteMessage(websocket.BinaryMessage, p)
+	if err != nil {
+		return 0, mapWebSocketErrors(err)
+	}
+	return len(p), nil
+}
 
-		// Now the connection is managed by the websocket library, let's move the handlers in the goroutine
-		go monitorStream(mon, conn)
+func (w *wsReadWriteCloser) Close() error {
+	w.buff = nil
+	return w.conn.Close()
+}
 
-		// and return nothing to the http library
+func mapWebSocketErrors(err error) error {
+	if websocket.IsUnexpectedCloseError(err, websocket.CloseNormalClosure, websocket.CloseGoingAway, websocket.CloseNoStatusReceived, websocket.CloseAbnormalClosure) {
+		return net.ErrClosed
 	}
+	return err
 }

internal/monitor/monitor.go

@@ -0,0 +1,90 @@
+// This file is part of arduino-app-cli.
+//
+// Copyright 2025 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-app-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 [email protected].
+
+package monitor
+
+import (
+	"errors"
+	"io"
+	"log/slog"
+	"net"
+	"time"
+
+	"go.bug.st/f"
+)
+
+const defaultArduinoRouterMonitorAddress = "127.0.0.1:7500"
+
+func NewMonitorHandler(rw io.ReadWriteCloser, address ...string) (func(), error) {
+	f.Assert(len(address) <= 1, "NewMonitorHandler accepts at most one address argument")
+
+	addr := defaultArduinoRouterMonitorAddress
+	if len(address) == 1 {
+		addr = address[0]
+	}
+
+	// Connect to monitor
+	monitor, err := net.DialTimeout("tcp", addr, time.Second)
+	if err != nil {
+		return nil, err
+	}
+
+	return func() {
+		monitorStream(monitor, rw)
+	}, nil
+}
+
+func monitorStream(mon net.Conn, rw io.ReadWriteCloser) {
+	logSocketError := func(msg string, err error) {
+		if !errors.Is(err, net.ErrClosed) && !errors.Is(err, io.EOF) {
+			slog.Error(msg, slog.String("error", err.Error()))
+		}
+	}
+	go func() {
+		defer mon.Close()
+		defer rw.Close()
+		buff := [1024]byte{}
+		for {
+			// Read from reader and write to monitor
+			n, err := rw.Read(buff[:])
+			if err != nil {
+				logSocketError("Error reading from websocket", err)
+				return
+			}
+			if _, err := mon.Write(buff[:n]); err != nil {
+				logSocketError("Error writing to monitor", err)
+				return
+			}
+		}
+	}()
+	go func() {
+		defer mon.Close()
+		defer rw.Close()
+		buff := [1024]byte{}
+		for {
+			// Read from monitor and write to writer
+			n, err := mon.Read(buff[:])
+			if err != nil {
+				logSocketError("Error reading from monitor", err)
+				return
+			}
+
+			if _, err := rw.Write(buff[:n]); err != nil {
+				logSocketError("Error writing to buffer", err)
+				return
+			}
+		}
+	}()
+}