Fix boards listing #1520

AlbyIanna · 2022-10-04T09:15:50Z

Motivation

The IDE needs to sort the menu for each platform in Tools > Board (e.g., Tools > Board > Arduino AVR Boards) according to the order of occurrence of the name property for each board definitions in the platform's boards.txt configuration file.

Change description

use listall instead of search command in the arduino-cli
add a proper order to the menu item when registering a board
update the arduino-cli to use the sorting fix

Other information

Fixes #802.

This needed a fix in the CLI to get the correct order: arduino/arduino-cli#1903

This PR is in draft because there isn't a release for the arduino-cli yet, but it can already be tested.

Arduino IDE 2.x before this change:

Arduino IDE 2.x after this change:

Arduino IDE 1.x:

Reviewer checklist

PR addresses a single concern.
The PR has no duplicates (please search among the Pull Requests before creating one)
PR title and description are properly filled.
Docs have been added / updated (for bug fixes / features)

kittaakos

This won't work. If IDE2 uses board list instead of board search, then only boards from installed platforms will be searched.

Steps to reproduce:

Delete ⚠️ (or rename) the directories/data location,
Start IDE2 from this PR,
Use the board search.

board-search.mp4

AlbyIanna · 2022-10-04T13:15:08Z

This won't work. If IDE2 uses board list instead of board search, then only boards from installed platforms will be searched.

Right. I've fixed it by adding a separate method to get only the installed boards.

kittaakos

Please reuse the code instead of copy/pasting it.

https://github.com/arduino/arduino-ide/pull/1520/files#diff-c0fc7c4dd9afbf95ea0168ace3b7e01e84078a50ba6cd54896f8a095f83f30ffL202-R225

AlbyIanna · 2022-10-04T14:54:34Z

Please reuse the code instead of copy/pasting it.

Done

kittaakos · 2022-10-04T15:14:15Z

Done

Thank you! I will do the verification soon. Which issue does this PR suppose to close? I could not find it in the commits or PR description. The closest was #801.

AlbyIanna · 2022-10-04T15:17:58Z

The closest was #801.

Pretty close, it fixes #802. Added to the PR's description.

per1234

The order of the boards is not correct for me:

Arduino IDE version

2.0.1-snapshot-3f7230e (tester build for 56f2ac3)

Operating system

Windows 10, Ubuntu 20.04

Additional context

The order is as expected when using Arduino CLI:

$ arduino-cli version
arduino-cli.exe  Version: git-snapshot Commit: 3d5a87e1 Date: 2022-10-04T18:01:41Z

$ arduino-cli board listall arduino:avr --format json | jq '.boards[] | .name'
"Arduino Yún"
"Arduino Uno"
"Arduino Uno Mini"
"Arduino Duemilanove or Diecimila"
"Arduino Nano"
"Arduino Mega or Mega 2560"
"Arduino Mega ADK"
"Arduino Leonardo"
"Arduino Leonardo ETH"
"Arduino Micro"
"Arduino Esplora"
"Arduino Mini"
"Arduino Ethernet"
"Arduino Fio"
"Arduino BT"
"LilyPad Arduino USB"
"LilyPad Arduino"
"Arduino Pro or Pro Mini"
"Arduino NG or older"
"Arduino Robot Control"
"Arduino Robot Motor"
"Arduino Gemma"
"Adafruit Circuit Playground"
"Arduino Yún Mini"
"Arduino Industrial 101"
"Linino One"
"Arduino Uno WiFi"

kittaakos

I have noticed that sometimes the order of installed boards does not match with IDE 1.x Sometimes it's correct.

IDE2 SAMD:

IDE 1.x SAMD:

IDE2 AVR:

IDE 1.x AVR:

kittaakos · 2022-10-05T14:44:30Z

arduino-ide-extension/src/node/boards-service-impl.ts

+    return this.handleListBoards(client.boardListAll.bind(client), req);
+  }
+
+  async handleListBoards(


The visibility should be private if it's not called from outside of the module.

kittaakos · 2022-10-05T20:44:46Z

arduino-ide-extension/src/browser/contributions/board-selection.ts

+      const menuAction = {
+        commandId: id,
+        label: name,
+        order: `${index}`,


We can check them together, but I think this is the classic alphanumeric ordering issue.

% node Welcome to Node.js v14.19.0. Type ".help" for more information. > ['1', '2', '3', '11', '12', '111'].sort() [ '1', '11', '111', '12', '2', '3' ] > .exit

With some zero padding, it works for me locally:

diff --git a/arduino-ide-extension/src/browser/contributions/board-selection.ts b/arduino-ide-extension/src/browser/contributions/board-selection.ts index 2241c925..466e4f63 100644 --- a/arduino-ide-extension/src/browser/contributions/board-selection.ts +++ b/arduino-ide-extension/src/browser/contributions/board-selection.ts @@ -243,7 +243,7 @@ PID: ${PID}`; const menuAction = { commandId: id, label: name, - order: `${index}`, + order: String(index).padStart(4), // pads with leading zeros for alphanumeric sort where order is 1, 2, 11, and NOT 1, 11, 2 }; this.commandRegistry.registerCommand(command, handler); this.toDisposeBeforeMenuRebuild.push(

Great that your changes revealed the flaw in the codebase. These are probably all incorrect:

arduino-ide/arduino-ide-extension/src/browser/boards/boards-data-menu-updater.ts

Line 114 in 61a11a0

order: `${i}`,

arduino-ide/arduino-ide-extension/src/browser/contributions/sketch-control.ts

Line 179 in 61a11a0

order: `${i}`,

👇 This probably works only because the open recent is limited to ten items.

arduino-ide/arduino-ide-extension/src/browser/contributions/open-recent-sketch.ts

Line 96 in 61a11a0

order: String(order),

Nice one! Thank you for the suggestion!

I've pushed the suggestion, could you please verify it now?

kittaakos · 2022-10-07T08:33:38Z

@AlbyIanna, I have restarted the failed job. If you mark the PR ready, I will do the final verification. The code looks great. Thanks!

per1234

It is working perfectly for me now.

Thanks Alberto!

Fix boards listing

3546f20

AlbyIanna requested review from davegarthsimpson, francescospissu and per1234 October 4, 2022 09:15

kittaakos self-requested a review October 4, 2022 11:18

kittaakos suggested changes Oct 4, 2022

View reviewed changes

kittaakos added topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project labels Oct 4, 2022

AlbyIanna force-pushed the fix-board-listing branch from 8f9ab6f to 10d2662 Compare October 4, 2022 13:15

AlbyIanna requested a review from kittaakos October 4, 2022 13:15

kittaakos suggested changes Oct 4, 2022

View reviewed changes

AlbyIanna requested a review from kittaakos October 4, 2022 14:50

per1234 requested changes Oct 4, 2022

View reviewed changes

kittaakos suggested changes Oct 5, 2022

View reviewed changes

kittaakos reviewed Oct 5, 2022

View reviewed changes

kittaakos mentioned this pull request Oct 6, 2022

Please make the output of the board listall --format json deterministic arduino/arduino-cli#1909

Closed

3 tasks

Alberto Iannaccone added 2 commits October 6, 2022 09:37

use arduio-cli sorting fix

dd0205c

re-use code to handle board list response

2d9b072

AlbyIanna force-pushed the fix-board-listing branch from 56f2ac3 to 2d9b072 Compare October 6, 2022 07:38

Alberto Iannaccone added 2 commits October 6, 2022 10:55

change handleListBoards visibility to private

36593e9

pad menu items order with leading zeros to fix alphanumeric order

7d4a977

per1234 approved these changes Oct 7, 2022

View reviewed changes

AlbyIanna marked this pull request as ready for review October 7, 2022 12:11

AlbyIanna requested a review from kittaakos October 10, 2022 07:31

kittaakos approved these changes Oct 17, 2022

View reviewed changes

AlbyIanna merged commit 960a2d0 into main Oct 17, 2022

AlbyIanna deleted the fix-board-listing branch October 17, 2022 08:03

nmzaheer mentioned this pull request Oct 20, 2022

Fix #1566 : Port submenu section heading show at top #1569

Merged

4 tasks

Uh oh!

Fix boards listing #1520

Fix boards listing #1520

Uh oh!

Conversation

AlbyIanna commented Oct 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Change description

Other information

Reviewer checklist

Uh oh!

kittaakos left a comment

Choose a reason for hiding this comment

Uh oh!

AlbyIanna commented Oct 4, 2022

Uh oh!

kittaakos left a comment

Choose a reason for hiding this comment

Uh oh!

AlbyIanna commented Oct 4, 2022

Uh oh!

kittaakos commented Oct 4, 2022

Uh oh!

AlbyIanna commented Oct 4, 2022

Uh oh!

per1234 left a comment

Choose a reason for hiding this comment

Arduino IDE version

Operating system

Additional context

Uh oh!

kittaakos left a comment

Choose a reason for hiding this comment

Uh oh!

kittaakos Oct 5, 2022

Choose a reason for hiding this comment

Uh oh!

kittaakos Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlbyIanna Oct 6, 2022

Choose a reason for hiding this comment

Uh oh!

AlbyIanna Oct 6, 2022

Choose a reason for hiding this comment

Uh oh!

kittaakos commented Oct 7, 2022

Uh oh!

per1234 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AlbyIanna commented Oct 4, 2022 •

edited

Loading

kittaakos Oct 5, 2022 •

edited

Loading