fix #1383: missing port labels #1412
Conversation
| </div> | ||
| <div className="arduino-boards-dropdown-item--port-label noWrapInfo noselect"> | ||
| {port.addressLabel} | ||
| {port.addressLabel || port.address} |
There was a problem hiding this comment.
maybe this is not appropriate, I imagine it depends on whether or not .addressLabel was intended to be a "catch all" or not, for the sake of efficiency I've plugged this quick fix with a question:
@kittaakos
are you able to confirm if a fallback is appropriate here, or should port.addressLabel be our only reference as asserted in this PR, UPDATE: in that case we could go with the approach below (or change the boardsConfig object to store .addressLabel) Thanks!
There was a problem hiding this comment.
Were you able to determine why port.addressLabel is empty after opening a new window @davegarthsimpson?
There was a problem hiding this comment.
Were you able to determine why port.addressLabel is empty after opening a new window
Yes, thanks for the important question, it's due to a chunk of code in our reconcileAvailableBoards method of the boards-service-provider here where we set the port in question by copying the boardsConfig.selectedPort object which does not contain .addressLabel only .address, this comment explains the reason why the author wrote this logic generally.
There was a problem hiding this comment.
The other alternative would be something like this in the "culprit code":
before:
581 ...
const found = availableBoards.findIndex(
(board) => board.port?.address === boardsConfig.selectedPort?.address
);
if (found >= 0) {
availableBoards.splice(found, 1);
}
availableBoards.push({
...boardsConfig.selectedBoard,
port: boardsConfig.selectedPort,
selected: true,
state: AvailableBoard.State.incomplete,
});
after:
581 let port = boardsConfig.selectedPort
...
const found = availableBoards.findIndex(
(board) => board.port?.address === boardsConfig.selectedPort?.address
);
if (found >= 0) {
port = availableBoards[found].port // get the "Unknown board port" that we will substitute
availableBoards.splice(found, 1);
}
availableBoards.push({
...boardsConfig.selectedBoard,
port, // "copy" the port info here where we'll have an "addressLabel" too
selected: true,
state: AvailableBoard.State.incomplete,
});
There was a problem hiding this comment.
That looks like the correct approach to me.
There was a problem hiding this comment.
Nice catch; the fix looks good to me. The breaking change was probably here when new props were added to the Port but on board#port.
Motivation
Reported that upon opening a new sketch port labels can go missing for "Unknown board ports"
Change description
Ensures that we do not lose
addressLabel(s) in "availableBoard objects" when reconciling our available boards.Issue Identified
Described below.
Closes #1383
Reviewer checklist