feat(esptool): Add --retry-open-serial flag, config file entry and envar (ESPTOOL-890)

[2opremio]

esptool frequently fails when trying to open the serial port of a device which deep-sleeps often:

$ esptool.py --chip esp32s3 -p /dev/cu.usbmodem6101 [...] write_flash foo.bin Serial port /dev/cu.usbmodem6101

A fatal error occurred: Could not open /dev/cu.usbmodem6101, the port is busy or doesn't exist.
([Errno 35] Could not exclusively lock port [...]: [Errno 35] Resource temporarily unavailable)

This makes developers add unnecessarily long sleepswhen the main CPU is awake, in order to give esptool the chance to find the serial port.

This PR adds a new flag --retry-open-serial (with corresponding env variable and cfg file entry) which retries opening the port indefinitely until the device shows up:

$ esptool.py --chip esp32s3 -p /dev/cu.usbmodem6101 [...] write_flash --retry-open-serial foo.bin Serial port /dev/cu.usbmodem6101
[Errno 35] Could not exclusively lock port [...]: [Errno 35] Resource temporarily unavailable Retrying to open port .........................
Connecting....
Chip is ESP32-S3 (QFN56) (revision v0.2)
[...]

I have tested this change with the following hardware & software combinations:

ESP32S3 + esptool master branch

I have run the esptool.py automated integration tests with this change and the above hardware:

"NO TESTING" (I can do so if considered necessary in order to merge)

I did test the flag, the env variable and the config file entry though.

[2opremio]

This is a counterpart to espressif/esp-idf-monitor#15

[2opremio]

CC @sudeep-mohanty who I am sure will appreciate the feaure when developing ULP code :)

[github-actions]

	Messages
📖	🎉 Good Job! All checks are passing!

👋 Hello 2opremio, we appreciate your contribution to this project!

---

📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

---

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests via this public GitHub repository.

This GitHub project is public mirror of our internal git repository

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved, we synchronize it into our internal git repository.
4. In the internal git repository we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
5. If the change is approved and passes the tests it is merged into the default branch.
5. On next sync from the internal git repository merged change will appear in this public GitHub repository.

Generated by 🚫 dangerJS against 940e1dc

[sudeep-mohanty]

CC @sudeep-mohanty who I am sure will appreciate the feaure when developing ULP code :)

Thanks for the contribution @2opremio! Seems like a useful feature! But, I'd let our colleagues from the esptool team to further evaluate the changes. Thanks! :)

[2opremio]

I am not sure I understand the linux-arm32 error on CI but it seems it's also failing in master

[peterdragun]

Hi, first of all, thank you for your contribution, and I am sorry for the late response.

We have discussed this with the team and we think that this would be a nice addition to esptool. Overall we like the feature, but I have left some comments regarding the implementation.

Please let me know if you want to address them, if not we will do it by ourselves.

Thank you!

[2opremio]

I am away this week, so, feel free to address the comments yourselves. Otherwise I will do so next week once I am back.

On August 5, 2024, GitHub ***@***.***> wrote: @peterdragun commented on this pull request. Hi, first of all, thank you for your contribution, and I am sorry for the late response. We have discussed this with the team and we think that this would be a nice addition to esptool. Overall we like the feature, but I have left some comments regarding the implementation. Please let me know if you want to address them, if not we will do it by ourselves. Thank you! In docs/en/esptool/configuration-file.rst <#995 (comment)>: > @@ -109,6 +109,8 @@ Complete list configurable options: +------------------------------ +----------------------------------------------------------- +----------+ | custom_reset_sequence | Custom reset sequence for resetting into the bootloader | | +------------------------------ +----------------------------------------------------------- +----------+ +| retry_open_serial | Retry opening the serial port indefinitely | False | Please fix the indentation to match the rest of the table In esptool/__init__.py <#995 (comment)>: > + parser.add_argument( + "--retry-open-serial", + help=( + "Retry opening the serial port indefinitely. " + "Default: %s" % DEFAULT_RETRY_OPEN_SERIAL + ), + default=os.environ.get("ESPTOOL_RETRY_OPEN_SERIAL", DEFAULT_RETRY_OPEN_SERIAL), + action="store_true", + ) We are trying to keep the number of arguments for esptool as slim as possible. It should be sufficient to configure this function using ENV and config variables. In esptool/__init__.py <#995 (comment)>: > + _esp = get_default_specific_connected_device( + chip, + each_port, + initial_baud, + trace, + before, + connect_attempts, + retry_open_serial, + ) I think it only makes sense to run the retry open if the user has specified the port. In this case, it will also trigger on the auto- detected port. In esptool/__init__.py <#995 (comment)>: > + ) as e: + if not retry_open_serial: + raise + if _esp and _esp._port: + _esp._port.close() + _esp = None + if retry_attempts == 0: + print(e) + print("Retrying failed connection ", end="", flush=True) + else: + if retry_attempts % 9 == 0: + # print a dot every second + print(".", end="", flush=True) + time.sleep(0.1) + retry_attempts += 1 + continue this continue is not necessary

— Reply to this email directly, view it on GitHub <#995 (review)- 2218676394>, or unsubscribe <https:/notifications/unsubscribe- auth/AASA4JDD7Q35K52GJ7XXKEDZP5NADAVCNFSM6AAAAABK7TMW2SVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMJYGY3TMMZZGQ>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[2opremio]

@peterdragun PTAL.

I've rebased the pre-existing code into a single commit and addressed all the comments (into a separate commit to differentiate it).