Skip to content
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

#740, #1331 Use port properties #1335

Merged
merged 3 commits into from
Aug 25, 2022
Merged

#740, #1331 Use port properties #1335

merged 3 commits into from
Aug 25, 2022

Conversation

kittaakos
Copy link
Contributor

@kittaakos kittaakos commented Aug 18, 2022

Motivation

To show the hostname of the network port in the boards dropdown and Tools > Ports menu (#1331)
To fix the upload caused by missing port properties (#740)

Change description

Other information

Closes #1331
Closes #740

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 kittaakos added topic: code Related to content of the project itself status: on hold Do not proceed at this time type: imperfection Perceived defect in any part of project labels Aug 18, 2022
@kittaakos kittaakos changed the title Use port properties. #1331 Use port properties Aug 18, 2022
@kittaakos kittaakos changed the title #1331 Use port properties #740, #1331 Use port properties Aug 18, 2022
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If possible, please break the changes into two PRs:

My impression is that the first is an essential fix for a problem specific to the Arduino IDE 2.x code base (since the bug does not occur when uploading to these boards via Arduino CLI).

My impression is that the second is a purely cosmetic improvement that is being done in a hacky way with the intention of eventually being removed.

Since the two are so distinct, I would prefer to test and evaluate each separately.

@kittaakos kittaakos removed the status: on hold Do not proceed at this time label Aug 23, 2022
@kittaakos kittaakos marked this pull request as ready for review August 23, 2022 09:36
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, this does not fix #740 for me:

Using 2.0.0-rc9.2.snapshot-7908674 (tester build for 6cae53b) to upload to a network port of an ESP8266 board fails with the same error:

"C:\Users\per\AppData\Local\Arduino15\packages\esp8266\tools\python3\3.7.2-post1/python3" -I "C:\Users\per\AppData\Local\Arduino15\packages\esp8266\hardware\esp8266\3.0.2/tools/espota.py" -i "192.168.254.130" -p "{upload.port.properties.port}" "--auth=adsg" -f "C:\Users\per\AppData\Local\Temp\arduino-sketch-C6FD83427A9EBA5E20435BABAB642981/esp8266.ino.bin"
Usage: espota.py [options]

espota.py: error: option -p: invalid integer value: '{upload.port.properties.port}'
Failed uploading: uploading error: exit status 2

@per1234
Copy link
Contributor

per1234 commented Aug 23, 2022

this does not fix #740 for me

After further investigation, I found that #740 now only occurs when uploading a sketch that was associated with the port while using a previous version of the Arduino IDE.

To reproduce

  1. Start any build of the Arduino IDE that doesn't have the fix made in this PR (e.g., 2.0.0-rc9.2.snapshot-ca47e8a).
  2. Set up your ESP8266 board to produce a network port as usual: https://arduino-esp8266.readthedocs.io/en/latest/ota_updates/readme.html#application-example
  3. Create a sketch named "SomeSketch".
  4. Select your ESP8266 board from the Tools > Board menu in the Arduino IDE.
  5. Select the network port of your ESP8266 board from the Tools > Port menu in the Arduino IDE.
  6. Select Sketch > Upload from the Arduino IDE menus.
  7. Wait for the upload to fail as expected due to espota.py: error: option -p: invalid integer value: '{upload.port.properties.port}' #740
  8. Select File > Quit from the Arduino IDE menus.
  9. Open the "SomeSketch" sketch in the Arduino IDE build from this PR.
    The board and port should already be selected since they have been associated with the sketch.
  10. Select Sketch > Upload from the Arduino IDE menus.
    🐛 The upload fails unexpectedly with the same error as reported in espota.py: error: option -p: invalid integer value: '{upload.port.properties.port}' #740
  11. Select any other port from the Tools > Port menu in the Arduino IDE.
  12. Select the network port of your ESP8266 board from the Tools > Port menu in the Arduino IDE.
  13. Select Sketch > Upload from the Arduino IDE menus.
    🙂 The upload is successful.

Additional context

Instead of the port selection flip flop described in steps 11-12 above, I was also able to resolve the issue by deleting the "User Data Folder" and restarting the IDE:

Windows

%APPDATA%\arduino-ide\

(e.g., C:\Users\<user name>\AppData\Roaming\arduino-ide\)

Linux

~/.config/arduino-ide/

macOS

~/Library/Application Support/arduino-ide/

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Describe the problem

The cc.arduino.cli.commands.v1.Port message of the Arduino CLI gRPC interface contains two fields used for primary identifiers of a port:

  • address - identifier for machines
  • label - identifier for humans

🐛 Unlike the "Board Selector" menu and "Select Other Board and Port" dialog, which use the human identifier label field value, the address field value is used in the Tools > Port menu.

To reproduce

  1. Create a port that has a label field value different from the address field value.
    I believe the two are always the same for serial ports, so this will be most easily accomplished with a network port, where the address value will be an IP address alone like "192.168.254.127", while the label value will typically have a form like "esp32-b4e62dbf693d at 192.168.254.127".
  2. Open the "Board Selector" menu.
    🙂 The label field value is used.
    image
  3. Select "Select other board and port..." from the "Board Selector" menu.
    🙂 The label field value is used in the "PORTS" menu.
    image
  4. Open the Tools > Port menu.

🐛 The address field value is used in the Tools > Port menu.

image

Expected behavior

The label field value is used in the Tools > Port menu.

Arduino IDE version

2.0.0-rc9.2.snapshot-7908674 (tester build for 6cae53b)

Operating system

Windows, Linux

Operating system version

Windows 10, Linux 20.04

Additional context

Arduino IDE 1.x uses the label field value in the Tools > Port menu:

image


The Tools > Port menu is explicitly mentioned in the original issue #567, indicating to me that the expectation was for the label field value to be used in that menu.


Even though the pluggable discovery system does provide a very nice board identification capability, I am not aware of any boards platforms other than "Arduino SAMD (32-bits ARM Cortex-M0+) Boards" (arduino/ArduinoCore-samd#648) that are using this capability currently.

This means that the network ports of those board will not have the "(<board name>)" suffix added to identified ports in the Tools > Port menu, making the prefix provided by the label field value especially important.

Akos Kitta added 3 commits August 24, 2022 12:54
Signed-off-by: Akos Kitta <a.kitta@arduino.cc>

Closes #740
Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
 - for the boards dropdown, and
 - for the `Tools` > `Port` menu.

Closes #1331

Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
@kittaakos
Copy link
Contributor Author

After further investigation, I found that #740 now only occurs when uploading a sketch that was associated with the port while using a previous version of the Arduino IDE.

🐛 The address field value is used in the Tools > Port menu.

Both issues should be fixed now. Please check again. Thank you!

Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified this fixes #567, #1331, and #740 (including under the specific case I reported in my previous review: #1335 (review)).

Thanks for your excellent work on this Akos!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
2 participants