Skip to content

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

Merged
merged 2 commits into from
May 5, 2021
Merged

Remove overwrite of run_reason in main #4708

merged 2 commits into from
May 5, 2021

Conversation

hierophect
Copy link
Collaborator

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 the reload_requested trigger. autoreload.c now sets the run reason as AUTO_RELOAD itself.

Resolves #4510
Resolves #3858

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.
@hierophect hierophect requested a review from anecdata May 4, 2021 21:15
@anecdata
Copy link
Member

anecdata commented May 4, 2021

SAMD51 (PyPortal)

PR artifact yields expected results for AUTO_RELOAD, SUPERVISOR_RELOAD, and REPL_RELOAD. STARTUP still absent on:
Adafruit CircuitPython 7.0.0-alpha.1-1006-g964f368ed on 2021-05-04; Adafruit PyPortal with samd51j20

Test code:

import time
import supervisor

time.sleep(5)
print(supervisor.runtime.run_reason)
supervisor.reload()

Result:

soft reboot

Auto-reload is on. Simply save files over USB to run them or enter REPL to disable.

code.py output:
supervisor.RunReason.AUTO_RELOAD  # code.py saved to CIRCUITPY

Code stopped by auto-reload.
soft reboot

Auto-reload is on. Simply save files over USB to run them or enter REPL to disable.

code.py output:
supervisor.RunReason.SUPERVISOR_RELOAD  # code executed supervisor.reload()

Code stopped by auto-reload.
soft reboot

Auto-reload is on. Simply save files over USB to run them or enter REPL to disable.

code.py output:
Traceback (most recent call last):
  File "code.py", line 4, in <module>
KeyboardInterrupt: 

Code done running.

Press any key to enter the REPL. Use CTRL-D to reload.
soft reboot

Auto-reload is on. Simply save files over USB to run them or enter REPL to disable.

code.py output:
supervisor.RunReason.REPL_RELOAD  # Control-D in REPL

Pressing reset button or microcontroller.reset() results in:

supervisor.RunReason.AUTO_RELOAD

(expected: STARTUP)

ESP32-S2 (MagTag)

ESP32-S2 yields all expected RunReason values under expected circumstances: STARTUP, AUTO_RELOAD, SUPERVISOR_RELOAD, and REPL_RELOAD

@anecdata
Copy link
Member

anecdata commented May 4, 2021

This may be expected, but REPL_RELOAD only arises with Control-D immediately after:
Press any key to enter the REPL. Use CTRL-D to reload.

Control-D from the >>> prompt gives:

>>> # Control-D
>>> 
soft reboot

Auto-reload is on. Simply save files over USB to run them or enter REPL to disable.

code.py output:
supervisor.RunReason.SUPERVISOR_RELOAD

(same on PyPortal & MagTag)

@tannewt
Copy link
Member

tannewt commented May 5, 2021

This may be expected, but REPL_RELOAD only arises with Control-D immediately after:
Press any key to enter the REPL. Use CTRL-D to reload.

Control-D from the >>> prompt gives:

>>> # Control-D
>>> 
soft reboot

Auto-reload is on. Simply save files over USB to run them or enter REPL to disable.

code.py output:
supervisor.RunReason.SUPERVISOR_RELOAD

(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 run_code_py.)

@hierophect
Copy link
Collaborator Author

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.

@hierophect
Copy link
Collaborator Author

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 microcontroller.reset(). My M0 and M4 express boards both return this value after restarting. I don't have a PyPortal, but there are exactly 2 places where AUTO_RELOAD can actually be set - when the board is saved in usb_msc_flash.c, and for certain BLE restarts with BLEIO. One of them would need to occur between the point that main.c sets RUN_REASON_STARTUP (right after the reset_board) and when it starts to execute your code. Is it possible some file got saved to your drive as you were testing, or something?

@anecdata
Copy link
Member

anecdata commented May 5, 2021

re: STARTUP, I'm not sure, could be a problem with the test method. I commented out the reload to remove that variable, longer initial delay, and added in a time indicator:

import time
import supervisor

time.sleep(10)
print(time.monotonic(), end=" ")
print(supervisor.runtime.run_reason)
#supervisor.reload()

After a hard reset, I wait about 5 seconds until the volume appears, then start screen. Result:

14.928 supervisor.RunReason.AUTO_RELOAD

Code done running.
Auto-reload is on. Simply save files over USB to run them or enter REPL to disable.

Press any key to enter the REPL. Use CTRL-D to reload.

@hierophect
Copy link
Collaborator Author

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?

@anecdata
Copy link
Member

anecdata commented May 5, 2021

macOS, anything is possible in the interval before I get output... I'll try another method... I'm open to suggestions, but:

import time
import supervisor
import microcontroller

reas = str(supervisor.runtime.run_reason).encode()
microcontroller.nvm[0:len(reas)] = reas
time.sleep(10)
while True:
    print(microcontroller.nvm[0:len(reas)])
    print(time.monotonic(), end=" ")
    print(supervisor.runtime.run_reason)
    time.sleep(5)

after waiting about 20 sceonds to start screen yields:

bytearray(b'supervisor.RunReason.AUTO_RELOAD')
30.259 supervisor.RunReason.AUTO_RELOAD

This works on MagTag, like the simple test:

bytearray(b'supervisor.RunReason.STARTUP')
27.86 supervisor.RunReason.STARTUP

@hierophect
Copy link
Collaborator Author

hierophect commented May 5, 2021

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.

@anecdata
Copy link
Member

anecdata commented May 5, 2021

I'll try that. Odd though that SAMD51 behaves differently than ESP32-S2.

@hierophect
Copy link
Collaborator Author

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?

@anecdata
Copy link
Member

anecdata commented May 5, 2021

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:

>>> supervisor.runtime.run_reason
supervisor.RunReason.STARTUP

Same on PyPortal on 6.2.0:

>>> supervisor.runtime.run_reason
supervisor.RunReason.AUTO_RELOAD

I tested PyPortal in December, and something seemed to change in 6.1.0-beta.2 between 12/3-12/18 (see issue #3858)

@anecdata
Copy link
Member

anecdata commented May 5, 2021

Latest artifacts:

Adafruit CircuitPython 7.0.0-alpha.1-1042-gb9843f15b on 2021-05-05; Adafruit Feather M4 Express with samd51j19
>>> import supervisor
>>> supervisor.runtime.run_reason
supervisor.RunReason.STARTUP
Adafruit CircuitPython 7.0.0-alpha.1-1042-gb9843f15b on 2021-05-05; Adafruit PyPortal with samd51j20
>>> import supervisor
>>> supervisor.runtime.run_reason
supervisor.RunReason.AUTO_RELOAD

@anecdata
Copy link
Member

anecdata commented May 5, 2021

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 STARTUP to NVM, it happens at about 1.25s after hard reset. Previously it was 5-6 seconds in that NVM was written with AUTO-RELOAD.

So I think mystery (mostly) revealed.

Copy link
Member

@anecdata anecdata left a 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.

@hierophect hierophect merged commit 22feda1 into adafruit:main May 5, 2021
@hierophect hierophect deleted the supervisor-run-reason branch May 5, 2021 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants