Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression: allow monitor to not require to specify the board if the port cannot be identified. #2647

Merged
merged 5 commits into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/cli/arguments/fqbn.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ func CalculateFQBNAndPort(ctx context.Context, portArgs *Port, fqbnArg *Fqbn, in
if portArgs == nil || portArgs.address == "" {
feedback.FatalError(&cmderrors.MissingFQBNError{}, feedback.ErrGeneric)
}
fqbn, port := portArgs.DetectFQBN(ctx, instance, srv)
if fqbn == "" {
fqbn, port, err := portArgs.DetectFQBN(ctx, instance, srv)
if err != nil {
feedback.FatalError(&cmderrors.MissingFQBNError{}, feedback.ErrGeneric)
}
return fqbn, port
Expand Down
14 changes: 7 additions & 7 deletions internal/cli/arguments/port.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ package arguments
import (
"context"
"errors"
"fmt"
"time"

"github.com/arduino/arduino-cli/commands"
"github.com/arduino/arduino-cli/commands/cmderrors"
f "github.com/arduino/arduino-cli/internal/algorithms"
"github.com/arduino/arduino-cli/internal/cli/feedback"
"github.com/arduino/arduino-cli/internal/i18n"
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -132,13 +132,13 @@ func (p *Port) GetSearchTimeout() time.Duration {
// DetectFQBN tries to identify the board connected to the port and returns the
// discovered Port object together with the FQBN. If the port does not match
// exactly 1 board,
func (p *Port) DetectFQBN(ctx context.Context, inst *rpc.Instance, srv rpc.ArduinoCoreServiceServer) (string, *rpc.Port) {
func (p *Port) DetectFQBN(ctx context.Context, inst *rpc.Instance, srv rpc.ArduinoCoreServiceServer) (string, *rpc.Port, error) {
detectedPorts, err := srv.BoardList(ctx, &rpc.BoardListRequest{
Instance: inst,
Timeout: p.timeout.Get().Milliseconds(),
})
if err != nil {
feedback.Fatal(i18n.Tr("Error during FQBN detection: %v", err), feedback.ErrGeneric)
return "", nil, fmt.Errorf("%s: %w", i18n.Tr("Error during board detection"), err)
}
for _, detectedPort := range detectedPorts.GetPorts() {
port := detectedPort.GetPort()
Expand All @@ -149,14 +149,14 @@ func (p *Port) DetectFQBN(ctx context.Context, inst *rpc.Instance, srv rpc.Ardui
continue
}
if len(detectedPort.GetMatchingBoards()) > 1 {
feedback.FatalError(&cmderrors.MultipleBoardsDetectedError{Port: port}, feedback.ErrBadArgument)
return "", nil, &cmderrors.MultipleBoardsDetectedError{Port: port}
}
if len(detectedPort.GetMatchingBoards()) == 0 {
feedback.FatalError(&cmderrors.NoBoardsDetectedError{Port: port}, feedback.ErrBadArgument)
return "", nil, &cmderrors.NoBoardsDetectedError{Port: port}
}
return detectedPort.GetMatchingBoards()[0].GetFqbn(), port
return detectedPort.GetMatchingBoards()[0].GetFqbn(), port, nil
}
return "", nil
return "", nil, &cmderrors.NoBoardsDetectedError{Port: &rpc.Port{Address: p.address, Protocol: p.protocol}}
}

// IsPortFlagSet returns true if the port address is provided
Expand Down
60 changes: 39 additions & 21 deletions internal/cli/monitor/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ import (
"bytes"
"context"
"errors"
"fmt"
"io"
"os"
"slices"
"sort"
"strings"
"time"
Expand All @@ -34,6 +34,7 @@ import (
"github.com/arduino/arduino-cli/internal/cli/instance"
"github.com/arduino/arduino-cli/internal/i18n"
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
"github.com/arduino/go-properties-orderedmap"
"github.com/fatih/color"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
Expand All @@ -58,6 +59,8 @@ func NewCommand(srv rpc.ArduinoCoreServiceServer) *cobra.Command {
Long: i18n.Tr("Open a communication port with a board."),
Example: "" +
" " + os.Args[0] + " monitor -p /dev/ttyACM0\n" +
" " + os.Args[0] + " monitor -p /dev/ttyACM0 -b arduino:avr:uno\n" +
" " + os.Args[0] + " monitor -p /dev/ttyACM0 --config 115200\n" +
" " + os.Args[0] + " monitor -p /dev/ttyACM0 --describe",
Run: func(cmd *cobra.Command, args []string) {
sketchPath := ""
Expand Down Expand Up @@ -89,13 +92,6 @@ func runMonitorCmd(
quiet = true
}

var (
inst *rpc.Instance
profile *rpc.SketchProfile
fqbn string
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
Expand All @@ -110,9 +106,9 @@ func runMonitorCmd(
)
}
sketch := resp.GetSketch()
if sketch != nil {
defaultPort, defaultProtocol = sketch.GetDefaultPort(), sketch.GetDefaultProtocol()
}

var inst *rpc.Instance
var profile *rpc.SketchProfile
if fqbnArg.String() == "" {
if profileArg.Get() == "" {
inst, profile = instance.CreateAndInitWithProfile(ctx, srv, sketch.GetDefaultProfile().GetName(), sketchPath)
Expand All @@ -123,11 +119,13 @@ func runMonitorCmd(
if inst == nil {
inst = instance.CreateAndInit(ctx, srv)
}

// 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
var fqbn string
switch {
case fqbnArg.String() != "":
fqbn = fqbnArg.String()
Expand All @@ -136,15 +134,19 @@ func runMonitorCmd(
case sketch.GetDefaultFqbn() != "":
fqbn = sketch.GetDefaultFqbn()
default:
fqbn, _ = portArgs.DetectFQBN(ctx, inst, srv)
fqbn, _, _ = portArgs.DetectFQBN(ctx, inst, srv)
}

var defaultPort, defaultProtocol string
if sketch != nil {
defaultPort, defaultProtocol = sketch.GetDefaultPort(), sketch.GetDefaultProtocol()
}
portAddress, portProtocol, err := portArgs.GetPortAddressAndProtocol(ctx, inst, srv, defaultPort, defaultProtocol)
if err != nil {
feedback.FatalError(err, feedback.ErrGeneric)
}

enumerateResp, err := srv.EnumerateMonitorPortSettings(ctx, &rpc.EnumerateMonitorPortSettingsRequest{
defaultSettings, err := srv.EnumerateMonitorPortSettings(ctx, &rpc.EnumerateMonitorPortSettingsRequest{
Instance: inst,
PortProtocol: portProtocol,
Fqbn: fqbn,
Expand All @@ -153,14 +155,19 @@ func runMonitorCmd(
feedback.Fatal(i18n.Tr("Error getting port settings details: %s", err), feedback.ErrGeneric)
}
if describe {
settings := make([]*result.MonitorPortSettingDescriptor, len(enumerateResp.GetSettings()))
for i, v := range enumerateResp.GetSettings() {
settings := make([]*result.MonitorPortSettingDescriptor, len(defaultSettings.GetSettings()))
for i, v := range defaultSettings.GetSettings() {
settings[i] = result.NewMonitorPortSettingDescriptor(v)
}
feedback.PrintResult(&detailsResult{Settings: settings})
return
}

actualConfigurationLabels := properties.NewMap()
for _, setting := range defaultSettings.GetSettings() {
actualConfigurationLabels.Set(setting.GetSettingId(), setting.GetValue())
}

configuration := &rpc.MonitorPortConfiguration{}
if len(configs) > 0 {
for _, config := range configs {
Expand All @@ -173,7 +180,7 @@ func runMonitorCmd(
}

var setting *rpc.MonitorPortSettingDescriptor
for _, s := range enumerateResp.GetSettings() {
for _, s := range defaultSettings.GetSettings() {
if k == "" {
if contains(s.GetEnumValues(), v) {
setting = s
Expand All @@ -196,10 +203,7 @@ func runMonitorCmd(
SettingId: setting.GetSettingId(),
Value: v,
})
if !quiet {
feedback.Print(i18n.Tr("Monitor port settings:"))
feedback.Print(fmt.Sprintf("%s=%s", setting.GetSettingId(), v))
}
actualConfigurationLabels.Set(setting.GetSettingId(), v)
}
}

Expand Down Expand Up @@ -229,7 +233,6 @@ func runMonitorCmd(
}
ttyIn = io.TeeReader(ttyIn, ctrlCDetector)
}

monitorServer, portProxy := commands.MonitorServerToReadWriteCloser(ctx, &rpc.MonitorPortOpenRequest{
Instance: inst,
Port: &rpc.Port{Address: portAddress, Protocol: portProtocol},
Expand All @@ -238,6 +241,21 @@ func runMonitorCmd(
})
go func() {
if !quiet {
if len(configs) == 0 {
if fqbn != "" {
feedback.Print(i18n.Tr("Using default monitor configuration for board: %s", fqbn))
} else if portProtocol == "serial" {
feedback.Print(i18n.Tr("Using generic monitor configuration.\nWARNING: Your board may require different settings to work!\n"))
}
}
feedback.Print(i18n.Tr("Monitor port settings:"))
keys := actualConfigurationLabels.Keys()
slices.Sort(keys)
for _, k := range keys {
feedback.Printf(" %s=%s", k, actualConfigurationLabels.Get(k))
}
feedback.Print("")

feedback.Print(i18n.Tr("Connecting to %s. Press CTRL-C to exit.", portAddress))
}
if err := srv.Monitor(monitorServer); err != nil {
Expand Down
Loading