diff --git a/cmd/arduino-app-cli/app/app.go b/cmd/arduino-app-cli/app/app.go index fca105b8..48c98c7a 100644 --- a/cmd/arduino-app-cli/app/app.go +++ b/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 diff --git a/cmd/arduino-app-cli/main.go b/cmd/arduino-app-cli/main.go index c9dfddca..849e4178 100644 --- a/cmd/arduino-app-cli/main.go +++ b/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() diff --git a/cmd/arduino-app-cli/app/monitor.go b/cmd/arduino-app-cli/monitor/monitor.go similarity index 51% rename from cmd/arduino-app-cli/app/monitor.go rename to cmd/arduino-app-cli/monitor/monitor.go index cdf057e1..df6946e6 100644 --- a/cmd/arduino-app-cli/app/monitor.go +++ b/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 license@arduino.cc. -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 +} diff --git a/internal/api/handlers/monitor.go b/internal/api/handlers/monitor.go index 5aaf8f4d..08492cd1 100644 --- a/internal/api/handlers/monitor.go +++ b/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 } diff --git a/internal/monitor/monitor.go b/internal/monitor/monitor.go new file mode 100644 index 00000000..1c4a33b0 --- /dev/null +++ b/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 license@arduino.cc. + +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 + } + } + }() +} diff --git a/internal/monitor/monitor_test.go b/internal/monitor/monitor_test.go new file mode 100644 index 00000000..2906a347 --- /dev/null +++ b/internal/monitor/monitor_test.go @@ -0,0 +1,79 @@ +package monitor + +import ( + "fmt" + "io" + "net" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/arduino/arduino-app-cli/pkg/x/ports" +) + +func TestMonitorHandler(t *testing.T) { + addr := startEcoMonitor(t) + + rIn, wIn, rwOut := getReadWriteCloser() + + handler, err := NewMonitorHandler(rwOut, addr.String()) + assert.NoError(t, err) + go handler() + + // Write data to the pipe writer + message := "Hello, Monitor!" + n, err := wIn.Write([]byte(message)) + assert.NoError(t, err) + assert.Equal(t, len(message), n) + + // Read data from the pipe reader + buf := [128]byte{} + n, err = rIn.Read(buf[:]) + assert.NoError(t, err) + assert.Equal(t, len(message), n) + assert.Equal(t, message, string(buf[:n])) +} + +func getReadWriteCloser() (io.Reader, io.Writer, io.ReadWriteCloser) { + rOut, wIn := io.Pipe() + rIn, wOut := io.Pipe() + + type pipeReadWriteCloser struct { + io.Reader + io.Writer + io.Closer + } + pr := &pipeReadWriteCloser{ + Reader: rOut, + Writer: wOut, + Closer: io.NopCloser(nil), + } + return rIn, wIn, pr +} + +func startEcoMonitor(t *testing.T) net.Addr { + t.Helper() + + port, err := ports.GetAvailable() + assert.NoError(t, err) + + ln, err := net.Listen("tcp", fmt.Sprintf("localhost:%d", port)) + assert.NoError(t, err) + t.Cleanup(func() { _ = ln.Close() }) + + go func() { + for { + conn, err := ln.Accept() + if err != nil { + return + } + + go func() { + defer conn.Close() + _, _ = io.Copy(conn, conn) // Echo server + }() + } + }() + + return ln.Addr() +}