Skip to content

Fix builtin discovery tools not being loaded if no platform is installed #1428

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 5 commits into from
Sep 1, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
51 changes: 35 additions & 16 deletions arduino/cores/packagemanager/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,39 @@ func (pm *PackageManager) LoadDiscoveries() []*status.Status {
for _, platform := range pm.InstalledPlatformReleases() {
statuses = append(statuses, pm.loadDiscoveries(platform)...)
}
if st := pm.loadBuiltinDiscoveries(); len(st) > 0 {
statuses = append(statuses, st...)
}
return statuses
}

// loadDiscovery loads the discovery tool with id, if it cannot be found a non-nil status is returned
func (pm *PackageManager) loadDiscovery(id string) *status.Status {
tool := pm.GetTool(id)
if tool == nil {
return status.Newf(codes.FailedPrecondition, tr("discovery not found: %s"), id)
}
toolRelease := tool.GetLatestInstalled()
if toolRelease == nil {
return status.Newf(codes.FailedPrecondition, tr("discovery not installed: %s"), id)
}
discoveryPath := toolRelease.InstallDir.Join(tool.Name).String()
d, err := discovery.New(id, discoveryPath)
if err != nil {
return status.Newf(codes.FailedPrecondition, tr("creating discovery: %s"), err)
}
pm.discoveryManager.Add(d)
return nil
}

// loadBuiltinDiscoveries loads the discovery tools that are part of the builtin package
func (pm *PackageManager) loadBuiltinDiscoveries() []*status.Status {
statuses := []*status.Status{}
for _, id := range []string{"builtin:serial-discovery", "builtin:mdns-discovery"} {
if st := pm.loadDiscovery(id); st != nil {
statuses = append(statuses, st)
}
}
return statuses
}

Expand All @@ -705,23 +738,9 @@ func (pm *PackageManager) loadDiscoveries(release *cores.PlatformRelease) []*sta
//
// If both indexed and unindexed properties are found the unindexed are ignored
for _, id := range discoveryProperties.ExtractSubIndexLists("required") {
tool := pm.GetTool(id)
if tool == nil {
statuses = append(statuses, status.Newf(codes.FailedPrecondition, tr("discovery not found: %s"), id))
continue
}
toolRelease := tool.GetLatestInstalled()
if toolRelease == nil {
statuses = append(statuses, status.Newf(codes.FailedPrecondition, tr("discovery not installed: %s"), id))
continue
}
discoveryPath := toolRelease.InstallDir.Join(tool.Name).String()
d, err := discovery.New(id, discoveryPath)
if err != nil {
statuses = append(statuses, status.Newf(codes.FailedPrecondition, tr("creating discovery: %s"), err))
continue
if st := pm.loadDiscovery(id); st != nil {
statuses = append(statuses, st)
}
pm.discoveryManager.Add(d)
}

discoveryIDs := discoveryProperties.FirstLevelOf()
Expand Down
16 changes: 12 additions & 4 deletions arduino/cores/packagemanager/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,9 @@ func TestLoadDiscoveries(t *testing.T) {
})

errs := packageManager.LoadDiscoveries()
require.Len(t, errs, 0)
require.Len(t, errs, 2)
require.Equal(t, errs[0].Message(), "discovery not found: builtin:serial-discovery")
require.Equal(t, errs[1].Message(), "discovery not found: builtin:mdns-discovery")
discoveries := packageManager.DiscoveryManager().IDs()
require.Len(t, discoveries, 1)
require.Contains(t, discoveries, "arduino:ble-discovery")
Expand All @@ -154,7 +156,9 @@ func TestLoadDiscoveries(t *testing.T) {
})

errs = packageManager.LoadDiscoveries()
require.Len(t, errs, 0)
require.Len(t, errs, 2)
require.Equal(t, errs[0].Message(), "discovery not found: builtin:serial-discovery")
require.Equal(t, errs[1].Message(), "discovery not found: builtin:mdns-discovery")
discoveries = packageManager.DiscoveryManager().IDs()
require.Len(t, discoveries, 2)
require.Contains(t, discoveries, "arduino:ble-discovery")
Expand All @@ -169,7 +173,9 @@ func TestLoadDiscoveries(t *testing.T) {
})

errs = packageManager.LoadDiscoveries()
require.Len(t, errs, 0)
require.Len(t, errs, 2)
require.Equal(t, errs[0].Message(), "discovery not found: builtin:serial-discovery")
require.Equal(t, errs[1].Message(), "discovery not found: builtin:mdns-discovery")
discoveries = packageManager.DiscoveryManager().IDs()
require.Len(t, discoveries, 3)
require.Contains(t, discoveries, "arduino:ble-discovery")
Expand All @@ -186,7 +192,9 @@ func TestLoadDiscoveries(t *testing.T) {
})

errs = packageManager.LoadDiscoveries()
require.Len(t, errs, 0)
require.Len(t, errs, 2)
require.Equal(t, errs[0].Message(), "discovery not found: builtin:serial-discovery")
require.Equal(t, errs[1].Message(), "discovery not found: builtin:mdns-discovery")
discoveries = packageManager.DiscoveryManager().IDs()
require.Len(t, discoveries, 3)
require.Contains(t, discoveries, "arduino:ble-discovery")
Expand Down
56 changes: 56 additions & 0 deletions docs/UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,62 @@ Here you can find a list of migration guides to handle breaking changes between

## 0.19.0

### `board list` command JSON output change

The `board list` command JSON output has been changed quite a bit, from:

```
$ arduino-cli board list --format json
[
{
"address": "/dev/ttyACM1",
"protocol": "serial",
"protocol_label": "Serial Port (USB)",
"boards": [
{
"name": "Arduino Uno",
"fqbn": "arduino:avr:uno",
"vid": "0x2341",
"pid": "0x0043"
}
],
"serial_number": "954323132383515092E1"
}
]
```

to:

```
$ arduino-cli board list --format json
[
{
"matching_boards": [
{
"name": "Arduino Uno",
"fqbn": "arduino:avr:uno"
}
],
"port": {
"address": "/dev/ttyACM1",
"label": "/dev/ttyACM1",
"protocol": "serial",
"protocol_label": "Serial Port (USB)",
"properties": {
"pid": "0x0043",
"serialNumber": "954323132383515092E1",
"vid": "0x2341"
}
}
}
]
```

The `boards` array has been renamed `matching_boards`, each contained object will now contain only `name` and `fqbn`.
Properties that can be used to identify a board are now moved to the new `properties` object, it can contain any key
name. `pid`, `vid` and `serialNumber` have been moved to `properties`. The new `label` field is the name of the `port`
if it should be displayed in a GUI.

### gRPC interface `DebugConfigRequest`, `UploadRequest`, `UploadUsingProgrammerRequest`, `BurnBootloaderRequest`, `DetectedPort` field changes

`DebugConfigRequest`, `UploadRequest`, `UploadUsingProgrammerRequest` and `BurnBootloaderRequest` had their `port` field
Expand Down
8 changes: 4 additions & 4 deletions i18n/data/en.po
Original file line number Diff line number Diff line change
Expand Up @@ -2447,7 +2447,7 @@ msgstr "can't find latest release of %s"
msgid "can't find main Sketch file in %s"
msgstr "can't find main Sketch file in %s"

#: arduino/cores/packagemanager/loader.go:749
#: arduino/cores/packagemanager/loader.go:768
msgid "can't find pattern for discovery with id %s"
msgstr "can't find pattern for discovery with id %s"

Expand Down Expand Up @@ -2518,7 +2518,7 @@ msgstr "computing hash: %s"
msgid "could not find a valid build artifact"
msgstr "could not find a valid build artifact"

#: arduino/cores/packagemanager/loader.go:721
#: arduino/cores/packagemanager/loader.go:705
msgid "creating discovery: %s"
msgstr "creating discovery: %s"

Expand Down Expand Up @@ -2555,11 +2555,11 @@ msgstr "destination dir %s already exists, cannot install"
msgid "discovery %[1]s process not started: %[2]w"
msgstr "discovery %[1]s process not started: %[2]w"

#: arduino/cores/packagemanager/loader.go:710
#: arduino/cores/packagemanager/loader.go:696
msgid "discovery not found: %s"
msgstr "discovery not found: %s"

#: arduino/cores/packagemanager/loader.go:715
#: arduino/cores/packagemanager/loader.go:700
msgid "discovery not installed: %s"
msgstr "discovery not installed: %s"

Expand Down
8 changes: 4 additions & 4 deletions i18n/rice-box.go

Large diffs are not rendered by default.

8 changes: 3 additions & 5 deletions test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,14 +203,12 @@ def detected_boards(run_command):

detected_boards = []
for port in json.loads(result.stdout):
for board in port.get("boards", []):
fqbn = board.get("FQBN")
if not fqbn:
continue
for board in port.get("matching_boards", []):
fqbn = board.get("fqbn")
package, architecture, _id = fqbn.split(":")
detected_boards.append(
Board(
address=port.get("address"),
address=port["port"]["address"],
fqbn=fqbn,
package=package,
architecture=architecture,
Expand Down
8 changes: 6 additions & 2 deletions test/test_board.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
from git import Repo
import simplejson as json
import semver
import pytest

from .common import running_on_ci


gold_board = """
Expand Down Expand Up @@ -389,6 +392,7 @@
""" # noqa: E501


@pytest.mark.skipif(running_on_ci(), reason="VMs have no serial ports")
def test_board_list(run_command):
run_command("core update-index")
result = run_command("board list --format json")
Expand All @@ -397,8 +401,8 @@ def test_board_list(run_command):
ports = json.loads(result.stdout)
assert isinstance(ports, list)
for port in ports:
assert "protocol" in port
assert "protocol_label" in port
assert "protocol" in port["port"]
assert "protocol_label" in port["port"]


def test_board_listall(run_command):
Expand Down