Skip to content

[microTVM] Arduino retry on flash failure#12114

Merged
areusch merged 3 commits intoapache:mainfrom
guberti:arduino-retry-on-flash-failure
Jul 19, 2022
Merged

[microTVM] Arduino retry on flash failure#12114
areusch merged 3 commits intoapache:mainfrom
guberti:arduino-retry-on-flash-failure

Conversation

@guberti
Copy link
Member

@guberti guberti commented Jul 16, 2022

Previously, the flash function would sometimes lock up indefinitely without failing, stalling for hours or more. This pull request changes it to have a 60-second timeout (more than enough time to upload a sketch), and adds the ability to retry the command if it fails.

This pull request also adds a unit test for this functionality, and also makes a few changes to microtvm_api_server.py to better comply with pylint.

cc @alanmacd @gromero @mehrdadh

@areusch areusch merged commit 175f362 into apache:main Jul 19, 2022
@gromero
Copy link
Contributor

gromero commented Jul 19, 2022

@guberti Thanks for the enhancement, I remember facing this issue sometimes in my setup in the past. The changes LGTM (although already merged by @areusch -- he merged right when I was about to finish the review lol, but no worries since you tagged me 4 days ago so fair enough! heh). The only thing I think that needs to get fixed on a follow-on PR are these two f-strings here:

diff --git a/apps/microtvm/arduino/template_project/microtvm_api_server.py b/apps/microtvm/arduino/template_project/microtvm_api_server.py
index 053acefa3..047164eff 100644
--- a/apps/microtvm/arduino/template_project/microtvm_api_server.py
+++ b/apps/microtvm/arduino/template_project/microtvm_api_server.py
@@ -495,12 +495,12 @@ class Handler(server.ProjectAPIHandler):
             # be caught.
             except subprocess.TimeoutExpired:
                 _LOG.warning(
-                    "Upload attempt to port {port} timed out after {self.FLASH_TIMEOUT_SEC} seconds"
+                    f"Upload attempt to port {port} timed out after {self.FLASH_TIMEOUT_SEC} seconds"
                 )
 
         else:
             raise RuntimeError(
-                "Unable to flash Arduino board after {self.FLASH_MAX_RETRIES} attempts"
+                f"Unable to flash Arduino board after {self.FLASH_MAX_RETRIES} attempts"
             )
 
     def open_transport(self, options):

Cheers.

@guberti
Copy link
Member Author

guberti commented Jul 20, 2022

Oops - nice catch @gromero! I'll send a follow up to fix that.

@gromero
Copy link
Contributor

gromero commented Jul 20, 2022

Oops - nice catch @gromero! I'll send a follow up to fix that.

@guberti ok! tag me on the PR and this time I promise I'll be quicker on reviewing it :)

@gromero
Copy link
Contributor

gromero commented Jul 25, 2022

@guberti FYI I've just created #12175 fixing the f-strings since I'm using it for some tests regarding the tvm-bot on merge.

@guberti
Copy link
Member Author

guberti commented Jul 25, 2022

Oops, this totally slipped my mind. Thanks for the change @gromero !

xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
* Retry on flash failure

* Add unit test

* Style improvements for Arduino api server
mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023
* Retry on flash failure

* Add unit test

* Style improvements for Arduino api server
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants