From 82e3356a4217b8144e7d5bbd79c9cfebac280758 Mon Sep 17 00:00:00 2001 From: MatteoPologruto Date: Wed, 20 Sep 2023 11:45:07 +0200 Subject: [PATCH 1/7] Get default port address and protocol from sketch profile --- internal/cli/monitor/monitor.go | 35 ++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/internal/cli/monitor/monitor.go b/internal/cli/monitor/monitor.go index c7cdac4810a..5df63077e72 100644 --- a/internal/cli/monitor/monitor.go +++ b/internal/cli/monitor/monitor.go @@ -27,6 +27,7 @@ import ( "time" "github.com/arduino/arduino-cli/commands/monitor" + "github.com/arduino/arduino-cli/commands/sketch" "github.com/arduino/arduino-cli/configuration" "github.com/arduino/arduino-cli/i18n" "github.com/arduino/arduino-cli/internal/cli/arguments" @@ -45,13 +46,14 @@ var tr = i18n.Tr // NewCommand created a new `monitor` command func NewCommand() *cobra.Command { var ( - raw bool - portArgs arguments.Port - describe bool - configs []string - quiet bool - timestamp bool - fqbn arguments.Fqbn + raw bool + portArgs arguments.Port + describe bool + configs []string + quiet bool + timestamp bool + fqbn arguments.Fqbn + sketchPath string ) monitorCommand := &cobra.Command{ Use: "monitor", @@ -61,7 +63,7 @@ func NewCommand() *cobra.Command { " " + os.Args[0] + " monitor -p /dev/ttyACM0\n" + " " + os.Args[0] + " monitor -p /dev/ttyACM0 --describe", Run: func(cmd *cobra.Command, args []string) { - runMonitorCmd(&portArgs, &fqbn, configs, describe, timestamp, quiet, raw) + runMonitorCmd(&portArgs, &fqbn, configs, describe, timestamp, quiet, raw, sketchPath) }, } portArgs.AddToCommand(monitorCommand) @@ -70,12 +72,12 @@ func NewCommand() *cobra.Command { monitorCommand.Flags().StringSliceVarP(&configs, "config", "c", []string{}, tr("Configure communication port settings. The format is =[,=]...")) monitorCommand.Flags().BoolVarP(&quiet, "quiet", "q", false, tr("Run in silent mode, show only monitor input and output.")) monitorCommand.Flags().BoolVar(×tamp, "timestamp", false, tr("Timestamp each incoming line.")) + monitorCommand.Flags().StringVarP(&sketchPath, "sketch", "s", "", tr("Path to the sketch")) fqbn.AddToCommand(monitorCommand) - monitorCommand.MarkFlagRequired("port") return monitorCommand } -func runMonitorCmd(portArgs *arguments.Port, fqbn *arguments.Fqbn, configs []string, describe, timestamp, quiet, raw bool) { +func runMonitorCmd(portArgs *arguments.Port, fqbn *arguments.Fqbn, configs []string, describe, timestamp, quiet, raw bool, sketchPath string) { instance := instance.CreateAndInit() logrus.Info("Executing `arduino-cli monitor`") @@ -83,8 +85,17 @@ func runMonitorCmd(portArgs *arguments.Port, fqbn *arguments.Fqbn, configs []str quiet = true } - // TODO: Should use sketch default_port/protocol? - portAddress, portProtocol, err := portArgs.GetPortAddressAndProtocol(instance, "", "") + addressDefault := "" + protocolDefault := "" + if sketchPath != "" { + sketch, err := sketch.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath}) + if err != nil { + feedback.FatalError(err, feedback.ErrGeneric) + } + addressDefault = sketch.GetDefaultPort() + protocolDefault = sketch.GetDefaultProtocol() + } + portAddress, portProtocol, err := portArgs.GetPortAddressAndProtocol(instance, addressDefault, protocolDefault) if err != nil { feedback.FatalError(err, feedback.ErrGeneric) } From 85b1a538e3fd7bebee87b18d9d53d78fb4d06035 Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Wed, 18 Oct 2023 17:42:09 +0200 Subject: [PATCH 2/7] remove sketch flag --- internal/cli/monitor/monitor.go | 86 +++++++++++++++++++++++++-------- 1 file changed, 65 insertions(+), 21 deletions(-) diff --git a/internal/cli/monitor/monitor.go b/internal/cli/monitor/monitor.go index 5df63077e72..11219183ca6 100644 --- a/internal/cli/monitor/monitor.go +++ b/internal/cli/monitor/monitor.go @@ -27,7 +27,7 @@ import ( "time" "github.com/arduino/arduino-cli/commands/monitor" - "github.com/arduino/arduino-cli/commands/sketch" + sk "github.com/arduino/arduino-cli/commands/sketch" "github.com/arduino/arduino-cli/configuration" "github.com/arduino/arduino-cli/i18n" "github.com/arduino/arduino-cli/internal/cli/arguments" @@ -46,14 +46,14 @@ var tr = i18n.Tr // NewCommand created a new `monitor` command func NewCommand() *cobra.Command { var ( - raw bool portArgs arguments.Port + fqbnArg arguments.Fqbn + profileArg arguments.Profile + raw bool describe bool configs []string quiet bool timestamp bool - fqbn arguments.Fqbn - sketchPath string ) monitorCommand := &cobra.Command{ Use: "monitor", @@ -63,47 +63,91 @@ func NewCommand() *cobra.Command { " " + os.Args[0] + " monitor -p /dev/ttyACM0\n" + " " + os.Args[0] + " monitor -p /dev/ttyACM0 --describe", Run: func(cmd *cobra.Command, args []string) { - runMonitorCmd(&portArgs, &fqbn, configs, describe, timestamp, quiet, raw, sketchPath) + sketchPath := "" + if len(args) > 0 { + sketchPath = args[0] + } + var portProvidedFromFlag bool + if p := cmd.Flags().Lookup("port"); p != nil && p.Changed { + portProvidedFromFlag = true + } + runMonitorCmd(&portArgs, &fqbnArg, &profileArg, sketchPath, configs, describe, timestamp, quiet, raw, portProvidedFromFlag) }, } portArgs.AddToCommand(monitorCommand) + profileArg.AddToCommand(monitorCommand) monitorCommand.Flags().BoolVar(&raw, "raw", false, tr("Set terminal in raw mode (unbuffered).")) monitorCommand.Flags().BoolVar(&describe, "describe", false, tr("Show all the settings of the communication port.")) monitorCommand.Flags().StringSliceVarP(&configs, "config", "c", []string{}, tr("Configure communication port settings. The format is =[,=]...")) monitorCommand.Flags().BoolVarP(&quiet, "quiet", "q", false, tr("Run in silent mode, show only monitor input and output.")) monitorCommand.Flags().BoolVar(×tamp, "timestamp", false, tr("Timestamp each incoming line.")) - monitorCommand.Flags().StringVarP(&sketchPath, "sketch", "s", "", tr("Path to the sketch")) - fqbn.AddToCommand(monitorCommand) + fqbnArg.AddToCommand(monitorCommand) return monitorCommand } -func runMonitorCmd(portArgs *arguments.Port, fqbn *arguments.Fqbn, configs []string, describe, timestamp, quiet, raw bool, sketchPath string) { - instance := instance.CreateAndInit() +func runMonitorCmd( + portArgs *arguments.Port, fqbnArg *arguments.Fqbn, profileArg *arguments.Profile, sketchPathArg string, + configs []string, describe, timestamp, quiet, raw bool, portProvidedFromFlag bool, +) { logrus.Info("Executing `arduino-cli monitor`") if !configuration.HasConsole { quiet = true } - addressDefault := "" - protocolDefault := "" - if sketchPath != "" { - sketch, err := sketch.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath}) + var ( + inst *rpc.Instance + fqbn string + defaultPort, defaultProtocol string + ) + + if !portProvidedFromFlag { + sketchPath := arguments.InitSketchPath(sketchPathArg) + sketch, err := sk.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath.String()}) if err != nil { - feedback.FatalError(err, feedback.ErrGeneric) + feedback.Fatal( + tr("Error getting default port from `sketch.yaml`. Check if you're in the correct sketch folder or provide the --port flag: %s", err), + feedback.ErrGeneric, + ) + } + defaultPort = sketch.GetDefaultPort() + defaultProtocol = sketch.GetDefaultProtocol() + + var profile *rpc.Profile + if profileArg.Get() == "" { + inst, profile = instance.CreateAndInitWithProfile(sketch.GetDefaultProfile().GetName(), sketchPath) + } else { + inst, profile = instance.CreateAndInitWithProfile(profileArg.Get(), sketchPath) + } + + // Priority on how to retrieve the fqbn + // 1. from flag + // 2. from profile + // 3. from default_fqbn specified in the sketch.yaml + // 4. try to detect from the port + switch { + case fqbnArg.String() != "": + fqbn = fqbnArg.String() + case profile.GetFqbn() != "": + fqbn = profile.GetFqbn() + case sketch.GetDefaultFqbn() != "": + fqbn = sketch.GetDefaultFqbn() + default: + fqbn, _ = portArgs.DetectFQBN(inst) } - addressDefault = sketch.GetDefaultPort() - protocolDefault = sketch.GetDefaultProtocol() + } else { + inst = instance.CreateAndInit() + fqbn = fqbnArg.String() } - portAddress, portProtocol, err := portArgs.GetPortAddressAndProtocol(instance, addressDefault, protocolDefault) + portAddress, portProtocol, err := portArgs.GetPortAddressAndProtocol(inst, defaultPort, defaultProtocol) if err != nil { feedback.FatalError(err, feedback.ErrGeneric) } enumerateResp, err := monitor.EnumerateMonitorPortSettings(context.Background(), &rpc.EnumerateMonitorPortSettingsRequest{ - Instance: instance, + Instance: inst, PortProtocol: portProtocol, - Fqbn: fqbn.String(), + Fqbn: fqbn, }) if err != nil { feedback.Fatal(tr("Error getting port settings details: %s", err), feedback.ErrGeneric) @@ -155,9 +199,9 @@ func runMonitorCmd(portArgs *arguments.Port, fqbn *arguments.Fqbn, configs []str } } portProxy, _, err := monitor.Monitor(context.Background(), &rpc.MonitorRequest{ - Instance: instance, + Instance: inst, Port: &rpc.Port{Address: portAddress, Protocol: portProtocol}, - Fqbn: fqbn.String(), + Fqbn: fqbn, PortConfiguration: configuration, }) if err != nil { From 93b2b86d92d40551acd5b4e7e3cf8e26c155f006 Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Tue, 24 Oct 2023 10:09:00 +0200 Subject: [PATCH 3/7] introduve IsPortFlagSet method recevier --- internal/cli/arguments/port.go | 5 +++++ internal/cli/monitor/monitor.go | 10 +++------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/internal/cli/arguments/port.go b/internal/cli/arguments/port.go index 14f5846d80d..80df4b5c22c 100644 --- a/internal/cli/arguments/port.go +++ b/internal/cli/arguments/port.go @@ -155,3 +155,8 @@ func (p *Port) DetectFQBN(inst *rpc.Instance) (string, *rpc.Port) { } return "", nil } + +// IsPortFlagSet returns true if the port address is provided +func (p *Port) IsPortFlagSet() bool { + return p.address != "" +} diff --git a/internal/cli/monitor/monitor.go b/internal/cli/monitor/monitor.go index 11219183ca6..0d203c5aed4 100644 --- a/internal/cli/monitor/monitor.go +++ b/internal/cli/monitor/monitor.go @@ -67,11 +67,7 @@ func NewCommand() *cobra.Command { if len(args) > 0 { sketchPath = args[0] } - var portProvidedFromFlag bool - if p := cmd.Flags().Lookup("port"); p != nil && p.Changed { - portProvidedFromFlag = true - } - runMonitorCmd(&portArgs, &fqbnArg, &profileArg, sketchPath, configs, describe, timestamp, quiet, raw, portProvidedFromFlag) + runMonitorCmd(&portArgs, &fqbnArg, &profileArg, sketchPath, configs, describe, timestamp, quiet, raw) }, } portArgs.AddToCommand(monitorCommand) @@ -87,7 +83,7 @@ func NewCommand() *cobra.Command { func runMonitorCmd( portArgs *arguments.Port, fqbnArg *arguments.Fqbn, profileArg *arguments.Profile, sketchPathArg string, - configs []string, describe, timestamp, quiet, raw bool, portProvidedFromFlag bool, + configs []string, describe, timestamp, quiet, raw bool, ) { logrus.Info("Executing `arduino-cli monitor`") @@ -101,7 +97,7 @@ func runMonitorCmd( defaultPort, defaultProtocol string ) - if !portProvidedFromFlag { + if !portArgs.IsPortFlagSet() { sketchPath := arguments.InitSketchPath(sketchPathArg) sketch, err := sk.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath.String()}) if err != nil { From 65eb91df7050ec18b7de6d3c654eeb8251458640 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Fri, 20 Oct 2023 16:11:27 +0200 Subject: [PATCH 4/7] Added integration tests --- .../integrationtest/monitor/monitor_test.go | 188 +++++++++++++++++- .../SketchWithDefaultFQBN.ino | 3 + .../SketchWithDefaultFQBN/sketch.yaml | 1 + .../SketchWithDefaultPort.ino | 3 + .../SketchWithDefaultPort/sketch.yaml | 1 + .../SketchWithDefaultPortAndFQBN.ino | 3 + .../SketchWithDefaultPortAndFQBN/sketch.yaml | 2 + .../SketchWithNoProfiles.ino | 3 + 8 files changed, 193 insertions(+), 11 deletions(-) create mode 100644 internal/integrationtest/monitor/testdata/SketchWithDefaultFQBN/SketchWithDefaultFQBN.ino create mode 100644 internal/integrationtest/monitor/testdata/SketchWithDefaultFQBN/sketch.yaml create mode 100644 internal/integrationtest/monitor/testdata/SketchWithDefaultPort/SketchWithDefaultPort.ino create mode 100644 internal/integrationtest/monitor/testdata/SketchWithDefaultPort/sketch.yaml create mode 100644 internal/integrationtest/monitor/testdata/SketchWithDefaultPortAndFQBN/SketchWithDefaultPortAndFQBN.ino create mode 100644 internal/integrationtest/monitor/testdata/SketchWithDefaultPortAndFQBN/sketch.yaml create mode 100644 internal/integrationtest/monitor/testdata/SketchWithNoProfiles/SketchWithNoProfiles.ino diff --git a/internal/integrationtest/monitor/monitor_test.go b/internal/integrationtest/monitor/monitor_test.go index 0f5593cb9aa..0521281769f 100644 --- a/internal/integrationtest/monitor/monitor_test.go +++ b/internal/integrationtest/monitor/monitor_test.go @@ -21,9 +21,16 @@ import ( "testing" "github.com/arduino/arduino-cli/internal/integrationtest" + "github.com/arduino/go-paths-helper" "github.com/stretchr/testify/require" ) +// returns a reader that tells the mocked monitor to exit +func quitMonitor() io.Reader { + // tells mocked monitor to exit + return bytes.NewBufferString("QUIT\n") +} + func TestMonitorConfigFlags(t *testing.T) { env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) defer env.CleanUp() @@ -38,14 +45,8 @@ func TestMonitorConfigFlags(t *testing.T) { cli.InstallMockedSerialDiscovery(t) cli.InstallMockedSerialMonitor(t) - // Test monitor command - quit := func() io.Reader { - // tells mocked monitor to exit - return bytes.NewBufferString("QUIT\n") - } - t.Run("NoArgs", func(t *testing.T) { - stdout, _, err := cli.RunWithCustomInput(quit(), "monitor", "-p", "/dev/ttyARG", "--raw") + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARG", "--raw") require.NoError(t, err) require.Contains(t, string(stdout), "Opened port: /dev/ttyARG") require.Contains(t, string(stdout), "Configuration baudrate = 9600") @@ -54,7 +55,7 @@ func TestMonitorConfigFlags(t *testing.T) { }) t.Run("BaudConfig", func(t *testing.T) { - stdout, _, err := cli.RunWithCustomInput(quit(), "monitor", "-p", "/dev/ttyARG", "-c", "baudrate=115200", "--raw") + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARG", "-c", "baudrate=115200", "--raw") require.NoError(t, err) require.Contains(t, string(stdout), "Opened port: /dev/ttyARG") require.Contains(t, string(stdout), "Configuration baudrate = 115200") @@ -64,7 +65,7 @@ func TestMonitorConfigFlags(t *testing.T) { }) t.Run("BaudAndParitfyConfig", func(t *testing.T) { - stdout, _, err := cli.RunWithCustomInput(quit(), "monitor", "-p", "/dev/ttyARG", + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARG", "-c", "baudrate=115200", "-c", "parity=even", "--raw") require.NoError(t, err) require.Contains(t, string(stdout), "Opened port: /dev/ttyARG") @@ -75,16 +76,181 @@ func TestMonitorConfigFlags(t *testing.T) { }) t.Run("InvalidConfigKey", func(t *testing.T) { - _, stderr, err := cli.RunWithCustomInput(quit(), "monitor", "-p", "/dev/ttyARG", + _, stderr, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARG", "-c", "baud=115200", "-c", "parity=even", "--raw") require.Error(t, err) require.Contains(t, string(stderr), "invalid port configuration: baud=115200") }) t.Run("InvalidConfigValue", func(t *testing.T) { - _, stderr, err := cli.RunWithCustomInput(quit(), "monitor", "-p", "/dev/ttyARG", + _, stderr, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARG", "-c", "parity=9600", "--raw") require.Error(t, err) require.Contains(t, string(stderr), "invalid port configuration value for parity: 9600") }) } + +func TestMonitorCommandFlagsAndDefaultPortFQBNSelection(t *testing.T) { + env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) + defer env.CleanUp() + + // Install AVR platform + _, _, err := cli.Run("core", "install", "arduino:avr@1.8.6") + require.NoError(t, err) + + // Patch the Yun board to require special RTS/DTR serial configuration + f, err := cli.DataDir().Join("packages", "arduino", "hardware", "avr", "1.8.6", "boards.txt").Append() + require.NoError(t, err) + _, err = f.WriteString(` +uno.serial.disableRTS=true +uno.serial.disableDTR=false +yun.serial.disableRTS=true +yun.serial.disableDTR=true +`) + require.NoError(t, err) + require.NoError(t, f.Close()) + + // Install mocked discovery and monitor for testing + cli.InstallMockedSerialDiscovery(t) + cli.InstallMockedSerialMonitor(t) + + // Create test sketches + getSketchPath := func(sketch string) string { + p, err := paths.New("testdata", sketch).Abs() + require.NoError(t, err) + require.True(t, p.IsDir()) + return p.String() + } + sketch := getSketchPath("SketchWithNoProfiles") + sketchWithPort := getSketchPath("SketchWithDefaultPort") + sketchWithFQBN := getSketchPath("SketchWithDefaultFQBN") + sketchWithPortAndFQBN := getSketchPath("SketchWithDefaultPortAndFQBN") + + t.Run("NoFlags", func(t *testing.T) { + t.Run("NoDefaultPortNoDefaultFQBN", func(t *testing.T) { + _, stderr, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "--raw", sketch) + require.Error(t, err) + require.Contains(t, string(stderr), "No monitor available for the port protocol default") + }) + + t.Run("WithDefaultPort", func(t *testing.T) { + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "--raw", sketchWithPort) + require.NoError(t, err) + require.Contains(t, string(stdout), "Opened port: /dev/ttyDEF") + require.Contains(t, string(stdout), "Configuration rts = on") + require.Contains(t, string(stdout), "Configuration dtr = on") + }) + + t.Run("WithDefaultFQBN", func(t *testing.T) { + _, stderr, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "--raw", sketchWithFQBN) + require.Error(t, err) + require.Contains(t, string(stderr), "No monitor available for the port protocol default") + }) + + t.Run("WithDefaultPortAndQBN", func(t *testing.T) { + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "--raw", sketchWithPortAndFQBN) + require.NoError(t, err) + require.Contains(t, string(stdout), "Opened port: /dev/ttyDEF") + require.Contains(t, string(stdout), "Configuration rts = off") + require.Contains(t, string(stdout), "Configuration dtr = off") + }) + }) + + t.Run("WithPortFlag", func(t *testing.T) { + t.Run("NoDefaultPortNoDefaultFQBN", func(t *testing.T) { + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARGS", "--raw", sketch) + require.NoError(t, err) + require.Contains(t, string(stdout), "Opened port: /dev/ttyARGS") + require.Contains(t, string(stdout), "Configuration rts = on") + require.Contains(t, string(stdout), "Configuration dtr = on") + }) + + t.Run("WithDefaultPort", func(t *testing.T) { + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARGS", "--raw", sketchWithPort) + require.NoError(t, err) + require.Contains(t, string(stdout), "Opened port: /dev/ttyARGS") + require.Contains(t, string(stdout), "Configuration rts = on") + require.Contains(t, string(stdout), "Configuration dtr = on") + }) + + t.Run("WithDefaultFQBN", func(t *testing.T) { + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARGS", "--raw", sketchWithFQBN) + require.NoError(t, err) + require.Contains(t, string(stdout), "Opened port: /dev/ttyARGS") + require.Contains(t, string(stdout), "Configuration rts = off") + require.Contains(t, string(stdout), "Configuration dtr = off") + }) + + t.Run("WithDefaultPortAndQBN", func(t *testing.T) { + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARGS", "--raw", sketchWithPortAndFQBN) + require.NoError(t, err) + require.Contains(t, string(stdout), "Opened port: /dev/ttyARGS") + require.Contains(t, string(stdout), "Configuration rts = off") + require.Contains(t, string(stdout), "Configuration dtr = off") + }) + }) + + t.Run("WithFQBNFlag", func(t *testing.T) { + t.Run("NoDefaultPortNoDefaultFQBN", func(t *testing.T) { + _, stderr, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-b", "arduino:avr:uno", "--raw", sketch) + require.Error(t, err) + require.Contains(t, string(stderr), "No monitor available for the port protocol default") + }) + + t.Run("WithDefaultPort", func(t *testing.T) { + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-b", "arduino:avr:uno", "--raw", sketchWithPort) + require.NoError(t, err) + require.Contains(t, string(stdout), "Opened port: /dev/ttyDEF") + require.Contains(t, string(stdout), "Configuration rts = off") + require.Contains(t, string(stdout), "Configuration dtr = on") + }) + + t.Run("WithDefaultFQBN", func(t *testing.T) { + _, stderr, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-b", "arduino:avr:uno", "--raw", sketchWithFQBN) + require.Error(t, err) + require.Contains(t, string(stderr), "No monitor available for the port protocol default") + }) + + t.Run("WithDefaultPortAndQBN", func(t *testing.T) { + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-b", "arduino:avr:uno", "--raw", sketchWithPortAndFQBN) + require.NoError(t, err) + require.Contains(t, string(stdout), "Opened port: /dev/ttyDEF") + require.Contains(t, string(stdout), "Configuration rts = off") + require.Contains(t, string(stdout), "Configuration dtr = on") + }) + }) + + t.Run("WithPortAndFQBNFlags", func(t *testing.T) { + t.Run("NoDefaultPortNoDefaultFQBN", func(t *testing.T) { + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARGS", "-b", "arduino:avr:uno", "--raw", sketch) + require.NoError(t, err) + require.Contains(t, string(stdout), "Opened port: /dev/ttyARGS") + require.Contains(t, string(stdout), "Configuration rts = off") + require.Contains(t, string(stdout), "Configuration dtr = on") + }) + + t.Run("WithDefaultPort", func(t *testing.T) { + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARGS", "-b", "arduino:avr:uno", "--raw", sketchWithPort) + require.NoError(t, err) + require.Contains(t, string(stdout), "Opened port: /dev/ttyARGS") + require.Contains(t, string(stdout), "Configuration rts = off") + require.Contains(t, string(stdout), "Configuration dtr = on") + }) + + t.Run("WithDefaultFQBN", func(t *testing.T) { + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARGS", "-b", "arduino:avr:uno", "--raw", sketchWithFQBN) + require.NoError(t, err) + require.Contains(t, string(stdout), "Opened port: /dev/ttyARGS") + require.Contains(t, string(stdout), "Configuration rts = off") + require.Contains(t, string(stdout), "Configuration dtr = on") + }) + + t.Run("WithDefaultPortAndQBN", func(t *testing.T) { + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARGS", "-b", "arduino:avr:uno", "--raw", sketchWithPortAndFQBN) + require.NoError(t, err) + require.Contains(t, string(stdout), "Opened port: /dev/ttyARGS") + require.Contains(t, string(stdout), "Configuration rts = off") + require.Contains(t, string(stdout), "Configuration dtr = on") + }) + }) +} diff --git a/internal/integrationtest/monitor/testdata/SketchWithDefaultFQBN/SketchWithDefaultFQBN.ino b/internal/integrationtest/monitor/testdata/SketchWithDefaultFQBN/SketchWithDefaultFQBN.ino new file mode 100644 index 00000000000..5054c040393 --- /dev/null +++ b/internal/integrationtest/monitor/testdata/SketchWithDefaultFQBN/SketchWithDefaultFQBN.ino @@ -0,0 +1,3 @@ + +void setup() {} +void loop() {} diff --git a/internal/integrationtest/monitor/testdata/SketchWithDefaultFQBN/sketch.yaml b/internal/integrationtest/monitor/testdata/SketchWithDefaultFQBN/sketch.yaml new file mode 100644 index 00000000000..19e9541979f --- /dev/null +++ b/internal/integrationtest/monitor/testdata/SketchWithDefaultFQBN/sketch.yaml @@ -0,0 +1 @@ +default_fqbn: arduino:avr:yun diff --git a/internal/integrationtest/monitor/testdata/SketchWithDefaultPort/SketchWithDefaultPort.ino b/internal/integrationtest/monitor/testdata/SketchWithDefaultPort/SketchWithDefaultPort.ino new file mode 100644 index 00000000000..5054c040393 --- /dev/null +++ b/internal/integrationtest/monitor/testdata/SketchWithDefaultPort/SketchWithDefaultPort.ino @@ -0,0 +1,3 @@ + +void setup() {} +void loop() {} diff --git a/internal/integrationtest/monitor/testdata/SketchWithDefaultPort/sketch.yaml b/internal/integrationtest/monitor/testdata/SketchWithDefaultPort/sketch.yaml new file mode 100644 index 00000000000..5670096124c --- /dev/null +++ b/internal/integrationtest/monitor/testdata/SketchWithDefaultPort/sketch.yaml @@ -0,0 +1 @@ +default_port: /dev/ttyDEF diff --git a/internal/integrationtest/monitor/testdata/SketchWithDefaultPortAndFQBN/SketchWithDefaultPortAndFQBN.ino b/internal/integrationtest/monitor/testdata/SketchWithDefaultPortAndFQBN/SketchWithDefaultPortAndFQBN.ino new file mode 100644 index 00000000000..5054c040393 --- /dev/null +++ b/internal/integrationtest/monitor/testdata/SketchWithDefaultPortAndFQBN/SketchWithDefaultPortAndFQBN.ino @@ -0,0 +1,3 @@ + +void setup() {} +void loop() {} diff --git a/internal/integrationtest/monitor/testdata/SketchWithDefaultPortAndFQBN/sketch.yaml b/internal/integrationtest/monitor/testdata/SketchWithDefaultPortAndFQBN/sketch.yaml new file mode 100644 index 00000000000..676d30000e4 --- /dev/null +++ b/internal/integrationtest/monitor/testdata/SketchWithDefaultPortAndFQBN/sketch.yaml @@ -0,0 +1,2 @@ +default_port: /dev/ttyDEF +default_fqbn: arduino:avr:yun diff --git a/internal/integrationtest/monitor/testdata/SketchWithNoProfiles/SketchWithNoProfiles.ino b/internal/integrationtest/monitor/testdata/SketchWithNoProfiles/SketchWithNoProfiles.ino new file mode 100644 index 00000000000..5054c040393 --- /dev/null +++ b/internal/integrationtest/monitor/testdata/SketchWithNoProfiles/SketchWithNoProfiles.ino @@ -0,0 +1,3 @@ + +void setup() {} +void loop() {} From 3f50c4a8fcca37bb8c0c62f05d1b131ae7163ede Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Tue, 24 Oct 2023 12:52:36 +0200 Subject: [PATCH 5/7] always load the sketch if present --- internal/cli/monitor/monitor.go | 58 ++++++++++++++++----------------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/internal/cli/monitor/monitor.go b/internal/cli/monitor/monitor.go index 0d203c5aed4..81330b79361 100644 --- a/internal/cli/monitor/monitor.go +++ b/internal/cli/monitor/monitor.go @@ -93,48 +93,46 @@ func runMonitorCmd( var ( inst *rpc.Instance + profile *rpc.Profile fqbn string defaultPort, defaultProtocol string ) - if !portArgs.IsPortFlagSet() { - sketchPath := arguments.InitSketchPath(sketchPathArg) - sketch, err := sk.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath.String()}) - if err != nil { - feedback.Fatal( - tr("Error getting default port from `sketch.yaml`. Check if you're in the correct sketch folder or provide the --port flag: %s", err), - feedback.ErrGeneric, - ) - } - defaultPort = sketch.GetDefaultPort() - defaultProtocol = sketch.GetDefaultProtocol() - - var profile *rpc.Profile + sketchPath := arguments.InitSketchPath(sketchPathArg) + sketch, err := sk.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath.String()}) + if err != nil && !portArgs.IsPortFlagSet() { + feedback.Fatal( + tr("Error getting default port from `sketch.yaml`. Check if you're in the correct sketch folder or provide the --port flag: %s", err), + feedback.ErrGeneric, + ) + } + if sketch != nil { + defaultPort, defaultProtocol = sketch.GetDefaultPort(), sketch.GetDefaultProtocol() if profileArg.Get() == "" { inst, profile = instance.CreateAndInitWithProfile(sketch.GetDefaultProfile().GetName(), sketchPath) } else { inst, profile = instance.CreateAndInitWithProfile(profileArg.Get(), sketchPath) } - - // Priority on how to retrieve the fqbn - // 1. from flag - // 2. from profile - // 3. from default_fqbn specified in the sketch.yaml - // 4. try to detect from the port - switch { - case fqbnArg.String() != "": - fqbn = fqbnArg.String() - case profile.GetFqbn() != "": - fqbn = profile.GetFqbn() - case sketch.GetDefaultFqbn() != "": - fqbn = sketch.GetDefaultFqbn() - default: - fqbn, _ = portArgs.DetectFQBN(inst) - } - } else { + } + if inst == nil { inst = instance.CreateAndInit() + } + // Priority on how to retrieve the fqbn + // 1. from flag + // 2. from profile + // 3. from default_fqbn specified in the sketch.yaml + // 4. try to detect from the port + switch { + case fqbnArg.String() != "": fqbn = fqbnArg.String() + case profile.GetFqbn() != "": + fqbn = profile.GetFqbn() + case sketch.GetDefaultFqbn() != "": + fqbn = sketch.GetDefaultFqbn() + default: + fqbn, _ = portArgs.DetectFQBN(inst) } + portAddress, portProtocol, err := portArgs.GetPortAddressAndProtocol(inst, defaultPort, defaultProtocol) if err != nil { feedback.FatalError(err, feedback.ErrGeneric) From 00d89407cd594f560c3e65cadc8e81202cc86297 Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Tue, 24 Oct 2023 15:56:42 +0200 Subject: [PATCH 6/7] better handle some edge cases --- internal/cli/monitor/monitor.go | 7 +++++ .../integrationtest/monitor/monitor_test.go | 30 +++++++++++++++++++ .../SketchWithDefaultPortAndFQBN/sketch.yaml | 3 ++ 3 files changed, 40 insertions(+) diff --git a/internal/cli/monitor/monitor.go b/internal/cli/monitor/monitor.go index 81330b79361..3d0c62ad268 100644 --- a/internal/cli/monitor/monitor.go +++ b/internal/cli/monitor/monitor.go @@ -98,6 +98,11 @@ func runMonitorCmd( defaultPort, defaultProtocol string ) + // Flags takes maximum precedence over sketch.yaml + // If {--port --fqbn --profile} are set we ignore the profile. + // If both {--port --profile} are set we read the fqbn in the following order: profile -> default_fqbn -> discovery + // If only --port is set we read the fqbn in the following order: default_fqbn -> discovery + // If only --fqbn is set we read the port in the following order: default_port sketchPath := arguments.InitSketchPath(sketchPathArg) sketch, err := sk.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath.String()}) if err != nil && !portArgs.IsPortFlagSet() { @@ -108,6 +113,8 @@ func runMonitorCmd( } if sketch != nil { defaultPort, defaultProtocol = sketch.GetDefaultPort(), sketch.GetDefaultProtocol() + } + if fqbnArg.String() == "" { if profileArg.Get() == "" { inst, profile = instance.CreateAndInitWithProfile(sketch.GetDefaultProfile().GetName(), sketchPath) } else { diff --git a/internal/integrationtest/monitor/monitor_test.go b/internal/integrationtest/monitor/monitor_test.go index 0521281769f..256d8e0f8cb 100644 --- a/internal/integrationtest/monitor/monitor_test.go +++ b/internal/integrationtest/monitor/monitor_test.go @@ -154,6 +154,13 @@ yun.serial.disableDTR=true require.Contains(t, string(stdout), "Configuration rts = off") require.Contains(t, string(stdout), "Configuration dtr = off") }) + + t.Run("FQBNFromSpecificProfile", func(t *testing.T) { + // The only way to assert we're picking up the fqbn specified from the profile is to provide a wrong value + _, stderr, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "--raw", "--profile", "profile1", sketchWithPortAndFQBN) + require.Error(t, err) + require.Contains(t, string(stderr), "not an FQBN: broken_fqbn") + }) }) t.Run("WithPortFlag", func(t *testing.T) { @@ -188,6 +195,13 @@ yun.serial.disableDTR=true require.Contains(t, string(stdout), "Configuration rts = off") require.Contains(t, string(stdout), "Configuration dtr = off") }) + + t.Run("FQBNFromSpecificProfile", func(t *testing.T) { + // The only way to assert we're picking up the fqbn specified from the profile is to provide a wrong value + _, stderr, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARGS", "--raw", "--profile", "profile1", sketchWithPortAndFQBN) + require.Error(t, err) + require.Contains(t, string(stderr), "not an FQBN: broken_fqbn") + }) }) t.Run("WithFQBNFlag", func(t *testing.T) { @@ -218,6 +232,14 @@ yun.serial.disableDTR=true require.Contains(t, string(stdout), "Configuration rts = off") require.Contains(t, string(stdout), "Configuration dtr = on") }) + + t.Run("IgnoreProfile", func(t *testing.T) { + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-b", "arduino:avr:uno", "--raw", "--profile", "profile1", sketchWithPortAndFQBN) + require.NoError(t, err) + require.Contains(t, string(stdout), "Opened port: /dev/ttyDEF") + require.Contains(t, string(stdout), "Configuration rts = off") + require.Contains(t, string(stdout), "Configuration dtr = on") + }) }) t.Run("WithPortAndFQBNFlags", func(t *testing.T) { @@ -252,5 +274,13 @@ yun.serial.disableDTR=true require.Contains(t, string(stdout), "Configuration rts = off") require.Contains(t, string(stdout), "Configuration dtr = on") }) + + t.Run("IgnoreProfile", func(t *testing.T) { + stdout, _, err := cli.RunWithCustomInput(quitMonitor(), "monitor", "-p", "/dev/ttyARGS", "-b", "arduino:avr:uno", "--raw", "--profile", "profile1", sketchWithPortAndFQBN) + require.NoError(t, err) + require.Contains(t, string(stdout), "Opened port: /dev/ttyARGS") + require.Contains(t, string(stdout), "Configuration rts = off") + require.Contains(t, string(stdout), "Configuration dtr = on") + }) }) } diff --git a/internal/integrationtest/monitor/testdata/SketchWithDefaultPortAndFQBN/sketch.yaml b/internal/integrationtest/monitor/testdata/SketchWithDefaultPortAndFQBN/sketch.yaml index 676d30000e4..c8549c21b99 100644 --- a/internal/integrationtest/monitor/testdata/SketchWithDefaultPortAndFQBN/sketch.yaml +++ b/internal/integrationtest/monitor/testdata/SketchWithDefaultPortAndFQBN/sketch.yaml @@ -1,2 +1,5 @@ default_port: /dev/ttyDEF default_fqbn: arduino:avr:yun +profiles: + profile1: + fqbn: "broken_fqbn" From 6c0a0f304ca582cee000c6ec9fd75a100adc4280 Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Tue, 24 Oct 2023 16:09:32 +0200 Subject: [PATCH 7/7] InitSketchPath make warnings configurable --- internal/cli/arguments/sketch.go | 8 +++++--- internal/cli/board/attach.go | 2 +- internal/cli/compile/compile.go | 2 +- internal/cli/debug/debug.go | 2 +- internal/cli/monitor/monitor.go | 2 +- internal/cli/upload/upload.go | 2 +- 6 files changed, 10 insertions(+), 8 deletions(-) diff --git a/internal/cli/arguments/sketch.go b/internal/cli/arguments/sketch.go index 4e670bab6ac..58df45e3367 100644 --- a/internal/cli/arguments/sketch.go +++ b/internal/cli/arguments/sketch.go @@ -25,7 +25,7 @@ import ( // InitSketchPath returns an instance of paths.Path pointing to sketchPath. // If sketchPath is an empty string returns the current working directory. // In both cases it warns the user if he's using deprecated files -func InitSketchPath(path string) (sketchPath *paths.Path) { +func InitSketchPath(path string, printWarnings bool) (sketchPath *paths.Path) { if path != "" { sketchPath = paths.New(path) } else { @@ -36,8 +36,10 @@ func InitSketchPath(path string) (sketchPath *paths.Path) { logrus.Infof("Reading sketch from dir: %s", wd) sketchPath = wd } - if msg := sk.WarnDeprecatedFiles(sketchPath); msg != "" { - feedback.Warning(msg) + if printWarnings { + if msg := sk.WarnDeprecatedFiles(sketchPath); msg != "" { + feedback.Warning(msg) + } } return sketchPath } diff --git a/internal/cli/board/attach.go b/internal/cli/board/attach.go index f6536991ab7..ed252b019ac 100644 --- a/internal/cli/board/attach.go +++ b/internal/cli/board/attach.go @@ -53,7 +53,7 @@ func initAttachCommand() *cobra.Command { } func runAttachCommand(path string, port *arguments.Port, fqbn string) { - sketchPath := arguments.InitSketchPath(path) + sketchPath := arguments.InitSketchPath(path, true) portAddress, portProtocol, _ := port.GetPortAddressAndProtocol(nil, "", "") newDefaults, err := sketch.SetSketchDefaults(context.Background(), &rpc.SetSketchDefaultsRequest{ diff --git a/internal/cli/compile/compile.go b/internal/cli/compile/compile.go index 4ed8ab6638e..a26a90eceb6 100644 --- a/internal/cli/compile/compile.go +++ b/internal/cli/compile/compile.go @@ -157,7 +157,7 @@ func runCompileCommand(cmd *cobra.Command, args []string) { path = args[0] } - sketchPath := arguments.InitSketchPath(path) + sketchPath := arguments.InitSketchPath(path, true) sk, err := sketch.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath.String()}) if err != nil { diff --git a/internal/cli/debug/debug.go b/internal/cli/debug/debug.go index 69d9ddd716c..e7001a2a8d3 100644 --- a/internal/cli/debug/debug.go +++ b/internal/cli/debug/debug.go @@ -74,7 +74,7 @@ func runDebugCommand(command *cobra.Command, args []string) { path = args[0] } - sketchPath := arguments.InitSketchPath(path) + sketchPath := arguments.InitSketchPath(path, true) sk, err := sketch.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath.String()}) if err != nil { feedback.FatalError(err, feedback.ErrGeneric) diff --git a/internal/cli/monitor/monitor.go b/internal/cli/monitor/monitor.go index 3d0c62ad268..009897e8bdb 100644 --- a/internal/cli/monitor/monitor.go +++ b/internal/cli/monitor/monitor.go @@ -103,7 +103,7 @@ func runMonitorCmd( // If both {--port --profile} are set we read the fqbn in the following order: profile -> default_fqbn -> discovery // If only --port is set we read the fqbn in the following order: default_fqbn -> discovery // If only --fqbn is set we read the port in the following order: default_port - sketchPath := arguments.InitSketchPath(sketchPathArg) + sketchPath := arguments.InitSketchPath(sketchPathArg, false) sketch, err := sk.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath.String()}) if err != nil && !portArgs.IsPortFlagSet() { feedback.Fatal( diff --git a/internal/cli/upload/upload.go b/internal/cli/upload/upload.go index b17f323dc8c..4444cf1e792 100644 --- a/internal/cli/upload/upload.go +++ b/internal/cli/upload/upload.go @@ -89,7 +89,7 @@ func runUploadCommand(args []string, uploadFieldsArgs map[string]string) { if len(args) > 0 { path = args[0] } - sketchPath := arguments.InitSketchPath(path) + sketchPath := arguments.InitSketchPath(path, true) if msg := sk.WarnDeprecatedFiles(sketchPath); importDir == "" && importFile == "" && msg != "" { feedback.Warning(msg)