-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Remove overwrite of run_reason in main #4708
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
Conversation
When `reload_requested` is detected, the run reason will no longer be automatically overwritten as an AUTO_RELOAD, making SUPERVISOR_RELOAD a detectable reload reason. Autoreload now sets the reload reason itself.
SAMD51 (PyPortal) PR artifact yields expected results for Test code:
Result:
Pressing reset button or
(expected: ESP32-S2 (MagTag) ESP32-S2 yields all expected |
This may be expected, but Control-D from the
(same on PyPortal & MagTag) |
Thanks for the testing @anecdata! @hierophect want to fix this too? I think you'll want to set it after the REPL runs (in the loop that calls |
Yes, thank you @anecdata. I am also unable to replicate the issues with STARTUP on my STM32F405, funny that that issue is for SAMD only. I'll check out both problems. |
I've fixed the REPL to correctly set the reload reason when it encounters a CTRL-D during the repl process, as opposed to the triggering character, as Scott suggested. Works for me in testing. However, @anecdata I can't replicate your problem with the SAMD not returning STARTUP after hitting the reset or using |
re:
After a hard reset, I wait about 5 seconds until the volume appears, then start
|
Weird. The SAMD boards have a longer time to connect to the serial connection, in my experience, so it isn't trivial to tell without introducing new variables whether it is restarting for some reason under the hood. What kind of computer are you testing on? Is it possible that Screen is triggering the MSC autoreload somehow? |
macOS, anything is possible in the interval before I get output... I'll try another method... I'm open to suggestions, but:
after waiting about 20 sceonds to start
This works on MagTag, like the simple test:
|
Won't that sketch overwrite the nvm message if reloaded? Or am I just being an idiot and missing something? You could add a counter to the first byte of NVM to see how much it increments by the time you look at it. Again, I'm just shooting in the dark since I can't reproduce this on my feathers. Maybe we can get someone with a PyPortal to help verify. |
I'll try that. Odd though that SAMD51 behaves differently than ESP32-S2. |
I think it's specifically the PyPortal you have, my SAMD21 and 51 work fine. Do you have another SAMD51 board you can try it on? |
You're right, weirder than I thought (board rather than port difference??)... Feather M4 with 6.2.0, with no special preparation, just plug in, start screen, and control-C:
Same on PyPortal on 6.2.0:
I tested PyPortal in December, and something seemed to change in 6.1.0-beta.2 between 12/3-12/18 (see issue #3858) |
Latest artifacts:
|
I didn't notice this earlier, but watching the PyPortal built-in display, the PyPortal does reload after a hard reset, with no other action on my part. That would explain it. No idea why its init would do that when other boards don't. And sure enough, if I conditionally only save So I think mystery (mostly) revealed. |
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.
Tested changes for REPL_RELOAD
from the >>>
prompt, and it now works as expected.
This PR fixes an issue in main that would cause any pre-existing run reason, such as SUPERVISOR_RELOAD set in
supervisor.reload()
, to be overwritten after thereload_requested
trigger.autoreload.c
now sets the run reason as AUTO_RELOAD itself.Resolves #4510
Resolves #3858