Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

mpremote: Introduce timeout_overall to prevent endless loops#16616

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

hmaerki
Copy link
Contributor

@hmaerkihmaerki commentedJan 21, 2025•
edited
Loading

Summary

This is a follow up of#16531.

Testing

In this situation, mpremote blocks forever:

  • Precondition: Connect a brand new ESP32-C3-DevKitC. The factory installed firmware spits outTurning the LED ON! every second.
  • Stimuli: Callmpremote u0 eval 42
  • Error:mpremote blocks forever.
  • Why: The character timeout inread_until() is reset every second.
  • Desired behaviour:read_unit() should return after 10s.

It might not be a problem when mpremote hangs forever on the command line. However it is a severe problem whenread_until() is called in octoprobe andread_until() hangs forever. This PR fixes both situations.

Compile/flash the firmware to reproduce above situation

Connect a esp32s3 and flash using below command:

docker run --rm -v /dev:/dev --privileged -t docker.io/espressif/idf:v5.2.2 bash -c"git clone --depth 1 https://github.com/espressif/esp-idf.git && cd esp-idf/examples/get-started/blink && idf.py set-target esp32s3 && idf.py flash -p /dev/ttyUSB0"

Fix

  • Introducetimeout_overall inread_until().

This is a new optional parameter and defaults to NONE (infinite). It does not break existing code.

@hmaerkihmaerkiforce-pushed thehmaerki/mpremote_add_timeoutoverall branch from9b6c4a7 to4ab2df9CompareJanuary 21, 2025 10:55
@github-actionsGitHub Actions
Copy link

Code size report:

   bare-arm:    +0 +0.000% minimal x86:    +0 +0.000%    unix x64:    +0 +0.000% standard      stm32:    +0 +0.000% PYBV10     mimxrt:    +0 +0.000% TEENSY40        rp2:    +0 +0.000% RPI_PICO_W       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS  qemu rv32:    +0 +0.000% VIRT_RV32

@hmaerkihmaerkiforce-pushed thehmaerki/mpremote_add_timeoutoverall branch from4ab2df9 toa190608CompareJanuary 21, 2025 11:07
@codecovCodecov
Copy link

codecovbot commentedJan 21, 2025•
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.58%. Comparing base(0d46e45) to head(03fe9c5).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@##           master   #16616   +/-   ##=======================================  Coverage   98.58%   98.58%           =======================================  Files         167      167             Lines       21590    21590           =======================================  Hits        21285    21285             Misses        305      305

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report?Share it here.

@hmaerkihmaerkiforce-pushed thehmaerki/mpremote_add_timeoutoverall branch 6 times, most recently from1d812e7 tobaf69d1CompareJanuary 27, 2025 13:04
@hmaerkihmaerki marked this pull request as ready for reviewJanuary 27, 2025 13:18
@dpgeorgedpgeorge added the toolsRelates to tools/ directory in source, or other tooling labelJan 28, 2025
@@ -142,7 +151,7 @@ def enter_raw_repl(self, soft_reset=True):
self.serial.write(b"\r\x01") # ctrl-A: enter raw REPL

if soft_reset:
data = self.read_until(1, b"raw REPL; CTRL-B to exit\r\n>")
data = self.read_until(1, b"raw REPL; CTRL-B to exit\r\n>", timeout_overall=10)
Copy link
Member

Choose a reason for hiding this comment

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

I thinktimeout_overall=10 should be passed to both of the other calls toread_until below in thisenter_raw_repl function. That will prevent waiting forever in the case (1) triggering the soft reset does not work correctly but does print out data continuously (rare but could happen); and (2) boot.py forever prints out data and never exits.

Also, I suggest making a global constant for this timeout, eg at the top of this file just after the imports:

_ENTER_RAW_REPL_TIMEOUT=10

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

I added a commit but did it slightly different: I added a default parameter 'timeout_overall' to 'enter_raw_repl()' - it is consistent with the rest of the code. But I am also open for your proposal:-)

Copy link
Member

Choose a reason for hiding this comment

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

Looks good!

@hmaerkihmaerkiforce-pushed thehmaerki/mpremote_add_timeoutoverall branch 2 times, most recently from070b6dd to266d2c3CompareJanuary 28, 2025 06:57
And use it in `enter_raw_repl()`.  This prevents waiting forever for aserial device that does not respond to the Ctrl-C/Ctrl-D/etc commands andis constantly outputting data.Signed-off-by: Hans Maerki <buhtig.hans.maerki@ergoinfo.ch>
@dpgeorgedpgeorgeforce-pushed thehmaerki/mpremote_add_timeoutoverall branch from266d2c3 to03fe9c5CompareJanuary 28, 2025 23:45
@dpgeorgedpgeorge merged commit03fe9c5 intomicropython:masterJan 28, 2025
63 checks passed
@hmaerkihmaerki deleted the hmaerki/mpremote_add_timeoutoverall branchMay 12, 2025 08:05
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@dpgeorgedpgeorgedpgeorge approved these changes

Assignees
No one assigned
Labels
toolsRelates to tools/ directory in source, or other tooling
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@hmaerki@dpgeorge
[8]ページ先頭
©2009-2025 Movatter.jp