-
-
Notifications
You must be signed in to change notification settings - Fork 406
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
fix: flawed config handling in the FQBN #2113
Conversation
Also relaxed the LS assertions. As turned out, the CLI can detect FQBNs with already appended board config values. Closes #1588 Signed-off-by: Akos Kitta <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UPDATE: fixed by 2473435
- Select File > New Sketch from the Arduino IDE menus.
- Select Tools > Board > Arduino AVR Boards > Arduino Uno from the Arduino IDE menus.
- Select Sketch > Verify/Compile from the Arduino IDE menus.
🐛 Compilation fails:
Invalid FQBN: invalid config option:
Compilation error: Invalid FQBN: invalid config option:
With the arduino.cli.daemon.debug
setting enabled, I can see the FQBN used in the compile requests is arduino:avr:uno:
(note the trailing colon) instead of the correct arduino:avr:uno
:
2023-06-27T05:36:00.949Z daemon INFO 49 CALLED: /cc.arduino.cli.commands.v1.ArduinoCoreService/Compile STREAM_RESP
49 | REQ: {
49 | "instance": {
49 | "id": 1
49 | },
49 | "fqbn": "arduino:avr:uno:",
49 | "sketch_path": "c:\\Users\\per\\AppData\\Local\\Temp\\.arduinoIDE-unsaved2023526-26024-1m7bb86.6w07\\sketch_jun26b",
49 | "warnings": "none"
49 | }
2023-06-27T05:36:00.949Z daemon INFO 49 | ERROR: rpc error: code = InvalidArgument desc = Invalid FQBN: invalid config option:
49 STREAM CLOSED
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UPDATE: fixed by 0caf0ca
Custom board options may have a significant impact on the compilation results so the full FQBN is always passed to Arduino Language Server and it must be restarted whenever the user changes the selection in one of the custom board option menus.
🐛 Changing a custom board option menu selection does not trigger a restart of Arduino Language Server if the pluggable discovery discovery specifies custom board options for the currently selected port.
To reproduce
- Start Arduino IDE from the terminal.
- Select Tools > Board > Arduino AVR Boards > Arduino Mega or Mega 2560 from the Arduino IDE menus.
🙂 The terminal logs show something like:2023-06-27T05:40:49.229Z root INFO Starting language server: arduino:avr:mega:cpu=atmega2560
- Select Tools > Processor > ATmega1280Arduino AVR Boards > Arduino Mega or Mega 2560 from the Arduino IDE menus.
🙂 The terminal logs show that the language server is restarted after the custom board option change. Something like:2023-06-27T05:40:58.046Z root INFO Starting language server: arduino:avr:mega:cpu=atmega1280
- Install the "Teensy" boards platform from https://www.pjrc.com/teensy/package_issue1588_index.json
- Connect a Teensy board to your computer with the USB cable.
- Select the Teensy board from the board selector in Arduino IDE.
🙂 The terminal logs show something like:2023-06-27T05:42:18.753Z root INFO Starting language server: teensy:avr:teensy41:usb=rawhid,speed=600,opt=o2std,keys=en-us
- Make a change to one of the custom board options under the Tools menu (e.g., select Tools > Keyboard Layout > Canadian French
🐛 There is no "Starting language server
" log entry in response to the change.
With the arduino.cli.daemon.debug setting enabled, I can see that when I edit the sketch the compile requests that are triggered to build the sketch to update the language server are still using the original FQBN rather than the one that is appropriate for the current menu selections in Arduino IDE.
Expected behavior
The FQBN used with the language server always reflects the current custom board options selections in Arduino IDE.
Arduino IDE version
e47befc (tester build for b0422a7)
Operating system
Windows 11
Signed-off-by: Akos Kitta <[email protected]>
FQBNs can be detected with custom board configs. Make sure to sanitize all FQBNs before comparing them. Signed-off-by: Akos Kitta <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PR does accomplish the goal of fixing the serious bug #1588 and I am fine if that is considered the entire scope. However, I think it is at least worth mentioning here that I don't think the feature is working as specified in the original implementation request:
the IDE could utilize the more specific FQBN to initialize those menu options when a user clicks the port+board from the toolbar's drop-down list.
Describe the problem
Board options are not set according to data from pluggable discovery if the option has previously been set by the user.
To reproduce
Set Up
- Install the "Teensy" boards platform from https://www.pjrc.com/teensy/package_issue1588_index.json if not already installed.
- Connect a Teensy board to your computer with the USB cable.
- Select the Teensy board from the board selector menu in Arduino IDE.
- Select Tools > USB Type > Dual Serial from the Arduino IDE menus.
ⓘ There is nothing special about this specific option setting. It is chosen arbitrarily to establish a known configuration of the board. - Select Sketch > Upload from the Arduino IDE menus.
- Wait for the upload to finish successfully.
- Select File > Quit from the Arduino IDE menus.
- Rename or delete (:warning: cautiously) the "User Data" folder to clear all saved data about user custom board option menu selections:
- Windows:
C:\Users\<user name>\AppData\Roaming\arduino-ide
- Linux:
~/.config/arduino-ide
- macOS:
~/Library/Application Support/arduino-ide
- Windows:
Demo
- Start Arduino IDE.
- Select the Teensy board from the board selector menu in Arduino IDE.
- Open the Tools menu.
🙂 The "USB Type" menu is set to Dual Serial according to the data from the discovery (following the behavior specified in Provide a way to identify board options via pluggable discovery arduino-cli#1660). - Select Tools > USB Type > Triple Serial from the Arduino IDE menus.
ⓘ There is nothing special about this specific option setting. It is chosen arbitrarily to establish a known user selection from the custom board option menu.
❗ Do not upload to the board after making this change to the menu. This step is only intended to change the configuration of Arduino IDE, without affecting the data the board will provide to the discovery. - Connect any other Arduino board to your computer.
- Select the other Arduino board from the board selector menu in Arduino IDE.
- Select File > New Sketch from the Arduino IDE menus.
ⓘ This and the previous step were chosen arbitrarily in order to clearly demonstrate that a distinct board+port selection event is occurring in the next step. - Select the Teensy board from the board selector menu in Arduino IDE.
- Open the Tools menu.
🐛 The "USB Type" menu is set to Triple Serial instead of the Dual Serial setting indicated by the data from the discovery (not following the behavior specified in Provide a way to identify board options via pluggable discovery arduino-cli#1660).
Expected behavior
The custom board options menus in Arduino IDE are configured according to the data from the discovery whenever a port is selected that provides board option data.
Arduino IDE version
f4e766a (tester build for 0caf0ca)
Operating system
Windows 11
Thank you! Let's not merge this PR if it's not fully functional. |
Those are very helpful instructions. Thank you!
Does it mean when the CLI detects a board and board configs are part of the FQBN (in this case, |
I think that behavior is the only way the feature can be useful. My expected behavior is that at step (9) of the demo instructions I provided in my review that when I open the Tools menu after selecting the board+port from the Board Selector menu, the IDE should have changed the "USB Type" menu selection to "Dual Serial". The user should then be able to adjust the custom board option menus manually however they like, overriding any default settings that were provided for the selected port by the discovery. I think there might be some corner cases to consider: Port changes during uploadIn some cases the port may change during the course of an upload (either changing address or disappearing and reappearing), with the IDE following the port change. This automatic port selection should not cause a change in the IDE's custom board option settings according to data from the discovery as it would when the user made that port selection manually. Manual port selection via the Tools > Port menuSo far we only mentioned the Board Selector menu. I'm feeling ambivalent about whether or not the custom board option menus should be set according to the information from the discovery when a user selects a port via this menu. I don't see any technical reason why it couldn't be done (yes, it is different from the board selector in that you aren't selecting a board simultaneously, but that could be handled), but I'm currently leaning toward the opinion that the Tools > Port menu should be exclusively a way to select a port, without producing any side effects. |
Motivation
Fixes the incorrect config options handling in the FQBN.
Change description
This PR also relaxes the LS assertions at the start. As it turned out, the CLI can detect FQBNs with already appended board config values.
Other information
To set up:
https://www.pjrc.com/teensy/package_teensy_index.json
added, remove it.https://www.pjrc.com/teensy/package_issue1588_index.json
.To verify:
If you had Teensy installed before the verification, you probably know how to revert to the original setup 😊 but in case you don't
https://www.pjrc.com/teensy/package_issue1588_index.json
withhttps://www.pjrc.com/teensy/package_teensy_index.json
in the 3rd party URLs dialog.Closes #1588
Reviewer checklist