Skip to content
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

bricks/stm32: Force exit when shutdown requested. #117

Merged
merged 4 commits into from
Nov 15, 2022

Conversation

laurensvalk
Copy link
Member

If a user catches the SystemExit exception indefinitely, there is currently not a way to power off the hub.

This PR makes the following program exit normally so we can still de-init pbsys and do a clean power off.

from pybricks.tools import wait

while True:
    try:
        wait(10)
    except SystemExit:
        print("nope!")

I'm not merging this yet because there may be a cleaner way to exit MicroPython using PYEXEC_FORCED_EXIT and / or various available hooks.

@dlech
Copy link
Member

dlech commented Aug 29, 2022

Does this work if you replace the wait() and print() with pass so the the event poll hook never runs?

@laurensvalk
Copy link
Member Author

Does this work if you replace the wait() and print() with pass so the the event poll hook never runs?

while True:
    try:
        pass
    except:
        pass

Yes and no - this already cleanly exits on a single press because most of the time is not spent in the try part.

@laurensvalk
Copy link
Member Author

laurensvalk commented Aug 30, 2022

This, however, does still get stuck:

# This is the same as time.sleep_ms
from pybricks.tools import wait

while True:
    try:
        while True:
            try:
                wait(10)  
            except SystemExit:
                print("nope!")
    except SystemExit:
        pass

@laurensvalk
Copy link
Member Author

This, however, does still get stuck:

We can detect the situation just fine, nothing truly gets stuck, it's a matter of telling MicroPython to stop.

Raising a specific exception could work, but the user may still use a bare except.

@laurensvalk
Copy link
Member Author

Another possibility: With the recent code refactoring, reintroducing the longjmp can be done now by confining it to just the main application:

image

Along with this to call it:

image

This costs about 200 bytes instead of the 500 bytes from before (?).

This freezes the hub though (watch dog), so this isn't quite right.

@dlech
Copy link
Member

dlech commented Aug 30, 2022

I had a thought as I was falling asleep last night that maybe we could do something like the long jump, but put it in the MicroPython code so that we could make use of the MicroPython "nlr" code to hopefully keep the code size down.

@laurensvalk
Copy link
Member Author

laurensvalk commented Sep 1, 2022

I had a thought as I was falling asleep last night that maybe we could do something like the long jump, but put it in the MicroPython code so that we could make use of the MicroPython "nlr" code to hopefully keep the code size down.

That could be nice too.

pbsys_main_stop_program always gets called correctly, so we could jump from there to near the end of pbsys_main_run_program.

For generality, it would be nice to change:

-void pbsys_main_stop_program(void)
+void pbsys_main_stop_program(bool force_stop)

That way the micropython application code doesn't have to poke at pbsys, it just has to exit when it is told to.

But I'm having some trouble making this work, probably I'm making the same mistake as in the pbsys variant above. It always freezes and reboots.

@laurensvalk
Copy link
Member Author

I think I've figured out how to make this work. When a forced exit is triggered we can pop the nlr to the top level that was used to start the main program or REPL, and then raise the SystemExit to make it jump to the end. Does that make sense? See updated PR.

Separately, I've added a commit to also use this when pressing the stop button in Pybricks Code. Thoughts? Until now the stop button could theoretically be used as an "input", but we have the I/O terminal for that as well (on most hubs). It seems nice if the stop button always makes it stop.

@laurensvalk
Copy link
Member Author

Feel free to merge this in if it looks right, I may be offline for a bit.

@laurensvalk
Copy link
Member Author

Upstream MicroPython is considering addressing this by adding an exception type that can only be caught in C, not with a bare except.

@dlech dlech added the software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) label Nov 8, 2022
This lets us keep all MicroPython code in micropython.c and platform specifics in mphal.c.

This lets us do things like #117 in a platform-agnostic way.

This also renames pb_poll to the more descriptive pb_event_poll_hook.
Since the removal of the longjump in 6d06933,
the user could prevent the hub from stopping the program and shutting down
by catching all exceptions.

This commit ensures we can force a safe exit on shutdown in all cases.
This is a much cleaner approach.

Also restore local nlr buffers instead of a global one used to previously implement the system abort.
@laurensvalk laurensvalk merged commit 2a50b40 into master Nov 15, 2022
laurensvalk added a commit that referenced this pull request Nov 15, 2022
This lets us keep all MicroPython code in micropython.c and platform specifics in mphal.c.

This lets us do things like #117 in a platform-agnostic way.

This also renames pb_poll to the more descriptive pb_event_poll_hook.
@laurensvalk laurensvalk deleted the fix-system-exit branch November 15, 2022 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants