Skip to content

Extended Pluggable Monitor specification to allow board-specific settings to be applied #1855

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

Merged
merged 6 commits into from
Sep 2, 2022
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
6 changes: 6 additions & 0 deletions arduino/cores/board.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,3 +173,9 @@ func (b *Board) IsBoardMatchingIDProperties(query *properties.Map) bool {
}
return false
}

// GetMonitorSettings returns the settings for the pluggable monitor of the given protocol
// and set of board properties.
func GetMonitorSettings(protocol string, boardProperties *properties.Map) *properties.Map {
return boardProperties.SubTree("monitor_port." + protocol)
}
38 changes: 37 additions & 1 deletion arduino/cores/packagemanager/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@ func (pm *Builder) loadBoards(platform *cores.PlatformRelease) error {
convertVidPidIdentificationPropertiesToPluggableDiscovery(boardProperties)
convertUploadToolsToPluggableDiscovery(boardProperties)
}
convertLegacySerialPortRTSDTRSettingsToPluggableMonitor(boardProperties)

// The board's ID must be available in a board's properties since it can
// be used in all configuration files for several reasons, like setting compilation
Expand All @@ -523,6 +524,42 @@ func (pm *Builder) loadBoards(platform *cores.PlatformRelease) error {
return nil
}

// Converts the old:
//
// - xxx.serial.disableRTS=true
// - xxx.serial.disableDTR=true
//
// properties into pluggable monitor compatible:
//
// - xxx.monitor_port.serial.rts=off
// - xxx.monitor_port.serial.dtr=off
func convertLegacySerialPortRTSDTRSettingsToPluggableMonitor(boardProperties *properties.Map) {
disabledToOnOff := func(k string) string {
if boardProperties.GetBoolean(k) {
return "off" // Disabled
}
return "on" // Not disabled
}
if boardProperties.ContainsKey("serial.disableDTR") {
boardProperties.Set("monitor_port.serial.dtr", disabledToOnOff("serial.disableDTR"))
}
if boardProperties.ContainsKey("serial.disableRTS") {
boardProperties.Set("monitor_port.serial.rts", disabledToOnOff("serial.disableRTS"))
}
for _, k := range boardProperties.Keys() {
if strings.HasSuffix(k, ".serial.disableDTR") {
boardProperties.Set(
strings.TrimSuffix(k, ".serial.disableDTR")+".monitor_port.serial.dtr",
disabledToOnOff(k))
}
if strings.HasSuffix(k, ".serial.disableRTS") {
boardProperties.Set(
strings.TrimSuffix(k, ".serial.disableRTS")+".monitor_port.serial.rts",
disabledToOnOff(k))
}
}
}

// Converts the old:
//
// - xxx.vid.N
Expand All @@ -532,7 +569,6 @@ func (pm *Builder) loadBoards(platform *cores.PlatformRelease) error {
//
// - xxx.upload_port.N.vid
// - xxx.upload_port.N.pid
//
func convertVidPidIdentificationPropertiesToPluggableDiscovery(boardProperties *properties.Map) {
n := 0
outputVidPid := func(vid, pid string) {
Expand Down
61 changes: 61 additions & 0 deletions arduino/cores/packagemanager/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,67 @@ arduino_zero_native.pid.3=0x024d
}`, zero4.Dump())
}

func TestDisableDTRRTSConversionToPluggableMonitor(t *testing.T) {
m, err := properties.LoadFromBytes([]byte(`
menu.rts=RTS
menu.dtr=DTR
foo.menu.rts.on=On
foo.menu.rts.on.serial.disableRTS=false
foo.menu.rts.off=Off
foo.menu.rts.off.serial.disableRTS=true
foo.menu.dtr.on=On
foo.menu.dtr.on.serial.disableDTR=false
foo.menu.dtr.off=Off
foo.menu.dtr.off.serial.disableDTR=true

arduino_zero.serial.disableDTR=true
arduino_zero.serial.disableRTS=true

arduino_zero_edbg.serial.disableDTR=false
arduino_zero_edbg.serial.disableRTS=true
`))
require.NoError(t, err)

{
zero := m.SubTree("arduino_zero")
convertLegacySerialPortRTSDTRSettingsToPluggableMonitor(zero)
require.Equal(t, `properties.Map{
"serial.disableDTR": "true",
"serial.disableRTS": "true",
"monitor_port.serial.dtr": "off",
"monitor_port.serial.rts": "off",
}`, zero.Dump())
}
{
zeroEdbg := m.SubTree("arduino_zero_edbg")
convertLegacySerialPortRTSDTRSettingsToPluggableMonitor(zeroEdbg)
require.Equal(t, `properties.Map{
"serial.disableDTR": "false",
"serial.disableRTS": "true",
"monitor_port.serial.dtr": "on",
"monitor_port.serial.rts": "off",
}`, zeroEdbg.Dump())
}
{
foo := m.SubTree("foo")
convertLegacySerialPortRTSDTRSettingsToPluggableMonitor(foo)
require.Equal(t, `properties.Map{
"menu.rts.on": "On",
"menu.rts.on.serial.disableRTS": "false",
"menu.rts.off": "Off",
"menu.rts.off.serial.disableRTS": "true",
"menu.dtr.on": "On",
"menu.dtr.on.serial.disableDTR": "false",
"menu.dtr.off": "Off",
"menu.dtr.off.serial.disableDTR": "true",
"menu.rts.on.monitor_port.serial.rts": "on",
"menu.rts.off.monitor_port.serial.rts": "off",
"menu.dtr.on.monitor_port.serial.dtr": "on",
"menu.dtr.off.monitor_port.serial.dtr": "off",
}`, foo.Dump())
}
}

func TestLoadDiscoveries(t *testing.T) {
// Create all the necessary data to load discoveries
fakePath := paths.New("fake-path")
Expand Down
40 changes: 25 additions & 15 deletions commands/monitor/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func Monitor(ctx context.Context, req *rpc.MonitorRequest) (*PortProxy, *pluggab
}
defer release()

m, err := findMonitorForProtocolAndBoard(pme, req.GetPort().GetProtocol(), req.GetFqbn())
m, boardSettings, err := findMonitorAndSettingsForProtocolAndBoard(pme, req.GetPort().GetProtocol(), req.GetFqbn())
if err != nil {
return nil, nil, err
}
Expand All @@ -82,18 +82,25 @@ func Monitor(ctx context.Context, req *rpc.MonitorRequest) (*PortProxy, *pluggab
return nil, nil, &arduino.FailedMonitorError{Cause: err}
}

monIO, err := m.Open(req.GetPort().GetAddress(), req.GetPort().GetProtocol())
if err != nil {
m.Quit()
return nil, nil, &arduino.FailedMonitorError{Cause: err}
}
// Apply user-requested settings
if portConfig := req.GetPortConfiguration(); portConfig != nil {
for _, setting := range portConfig.Settings {
boardSettings.Remove(setting.SettingId) // Remove board settings overridden by the user
if err := m.Configure(setting.SettingId, setting.Value); err != nil {
logrus.Errorf("Could not set configuration %s=%s: %s", setting.SettingId, setting.Value, err)
}
}
}
// Apply specific board settings
for setting, value := range boardSettings.AsMap() {
m.Configure(setting, value)
}

monIO, err := m.Open(req.GetPort().GetAddress(), req.GetPort().GetProtocol())
if err != nil {
m.Quit()
return nil, nil, &arduino.FailedMonitorError{Cause: err}
}

logrus.Infof("Port %s successfully opened", req.GetPort().GetAddress())
return &PortProxy{
Expand All @@ -106,36 +113,39 @@ func Monitor(ctx context.Context, req *rpc.MonitorRequest) (*PortProxy, *pluggab
}, descriptor, nil
}

func findMonitorForProtocolAndBoard(pme *packagemanager.Explorer, protocol, fqbn string) (*pluggableMonitor.PluggableMonitor, error) {
func findMonitorAndSettingsForProtocolAndBoard(pme *packagemanager.Explorer, protocol, fqbn string) (*pluggableMonitor.PluggableMonitor, *properties.Map, error) {
if protocol == "" {
return nil, &arduino.MissingPortProtocolError{}
return nil, nil, &arduino.MissingPortProtocolError{}
}

var monitorDepOrRecipe *cores.MonitorDependency
boardSettings := properties.NewMap()

// If a board is specified search the monitor in the board package first
if fqbn != "" {
fqbn, err := cores.ParseFQBN(fqbn)
if err != nil {
return nil, &arduino.InvalidFQBNError{Cause: err}
return nil, nil, &arduino.InvalidFQBNError{Cause: err}
}

_, boardPlatform, _, boardProperties, _, err := pme.ResolveFQBN(fqbn)
if err != nil {
return nil, &arduino.UnknownFQBNError{Cause: err}
return nil, nil, &arduino.UnknownFQBNError{Cause: err}
}

boardSettings = cores.GetMonitorSettings(protocol, boardProperties)

if mon, ok := boardPlatform.Monitors[protocol]; ok {
monitorDepOrRecipe = mon
} else if recipe, ok := boardPlatform.MonitorsDevRecipes[protocol]; ok {
// If we have a recipe we must resolve it
cmdLine := boardProperties.ExpandPropsInString(recipe)
cmdArgs, err := properties.SplitQuotedString(cmdLine, `"'`, false)
if err != nil {
return nil, &arduino.InvalidArgumentError{Message: tr("Invalid recipe in platform.txt"), Cause: err}
return nil, nil, &arduino.InvalidArgumentError{Message: tr("Invalid recipe in platform.txt"), Cause: err}
}
id := fmt.Sprintf("%s-%s", boardPlatform, protocol)
return pluggableMonitor.New(id, cmdArgs...), nil
return pluggableMonitor.New(id, cmdArgs...), boardSettings, nil
}
}

Expand All @@ -150,17 +160,17 @@ func findMonitorForProtocolAndBoard(pme *packagemanager.Explorer, protocol, fqbn
}

if monitorDepOrRecipe == nil {
return nil, &arduino.NoMonitorAvailableForProtocolError{Protocol: protocol}
return nil, nil, &arduino.NoMonitorAvailableForProtocolError{Protocol: protocol}
}

// If it is a monitor dependency, resolve tool and create a monitor client
tool := pme.FindMonitorDependency(monitorDepOrRecipe)
if tool == nil {
return nil, &arduino.MonitorNotFoundError{Monitor: monitorDepOrRecipe.String()}
return nil, nil, &arduino.MonitorNotFoundError{Monitor: monitorDepOrRecipe.String()}
}

return pluggableMonitor.New(
monitorDepOrRecipe.Name,
tool.InstallDir.Join(monitorDepOrRecipe.Name).String(),
), nil
), boardSettings, nil
}
15 changes: 14 additions & 1 deletion commands/monitor/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func EnumerateMonitorPortSettings(ctx context.Context, req *rpc.EnumerateMonitor
}
defer release()

m, err := findMonitorForProtocolAndBoard(pme, req.GetPortProtocol(), req.GetFqbn())
m, boardSettings, err := findMonitorAndSettingsForProtocolAndBoard(pme, req.GetPortProtocol(), req.GetFqbn())
if err != nil {
return nil, err
}
Expand All @@ -46,6 +46,19 @@ func EnumerateMonitorPortSettings(ctx context.Context, req *rpc.EnumerateMonitor
if err != nil {
return nil, &arduino.FailedMonitorError{Cause: err}
}

// Apply default settings for this board and protocol
for setting, value := range boardSettings.AsMap() {
if param, ok := desc.ConfigurationParameters[setting]; ok {
for _, v := range param.Values {
if v == value {
param.Selected = value
break
}
}
}
}

return &rpc.EnumerateMonitorPortSettingsResponse{Settings: convert(desc)}, nil
}

Expand Down
41 changes: 41 additions & 0 deletions docs/platform-specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,47 @@ will allow all legacy non-pluggable platforms to migrate to pluggable monitor wi

For detailed information, see the [Pluggable Monitor specification](pluggable-monitor-specification.md).

#### Port configuration

Each pluggable monitor has its own default settings that can be overridden using the following board properties:

```
BOARD_ID.monitor_port.PROTOCOL.SETTING_NAME=SETTING_VALUE
```

where:

- `BOARD_ID` is the board identifier
- `PROTOCOL` is the port protocol
- `SETTING_NAME` and `SETTING_VALUE` are the port setting and the desired value

For example, let's suppose that a board needs the `baudrate` setting of the `serial` port to be `9600`, then the
corresponding properties in the `boards.txt` file will be:

```
myboard.monitor_port.serial.baudrate=9600
```

The settings available in a specific pluggable monitor can be
[queried directly from it](pluggable-monitor-specification.md#describe-command).

#### Legacy `serial.disableRTS` and `serial.disableDTR` properties

In the old Arduino IDE (<=1.8.x) we used the properties:

```
BOARD_ID.serial.disableRTS=true
BOARD_ID.serial.disableDTR=true
```

to disable RTS and DTR when opening the serial monitor. To keep backward compatibilty the properties above are
automatically converted to the corresponding pluggable monitor properties:

```
BOARD_ID.monitor_port.serial.rts=off
BOARD_ID.monitor_port.serial.dtr=off
```

### Verbose parameter

It is possible for the user to enable verbosity from the Preferences panel of the IDEs or Arduino CLI's `--verbose`
Expand Down