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

[Bug] Successfully exiting program with stop button immediately restarts it #1863

Closed
laurensvalk opened this issue Oct 2, 2024 · 2 comments
Assignees
Labels
bug Something isn't working software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime)

Comments

@laurensvalk
Copy link
Member

Describe the bug
This program restarts itself when you stop it with the stop button.

To reproduce

from pybricks.hubs import PrimeHub

hub = PrimeHub()

while True:
    if hub.buttons.pressed():
        break

Expected behavior
Stop normally, no restart.

Additional context
This was already happening with V3.5.0 so it wasn't introduced with the recent changes to program starts.

@laurensvalk laurensvalk added bug Something isn't working software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) labels Oct 2, 2024
@laurensvalk laurensvalk self-assigned this Oct 2, 2024
@laurensvalk
Copy link
Member Author

/**
 * Protothread to monitor the button state to trigger starting the user program.
 * @param [in]  button_pressed      The current button state.
 */
static PT_THREAD(update_program_run_button_wait_state(bool button_pressed)) {
    struct pt *pt = &update_program_run_button_wait_state_pt;
    // Creative use of protothread to reduce code size. This is the same
    // as checking if the user program is running after each PT_WAIT.
    if (pbsys_status_test(PBIO_PYBRICKS_STATUS_USER_PROGRAM_RUNNING)) {
        goto start;
    }

    PT_BEGIN(pt);

    for (;;) {
    start:
        // button may still be pressed from power on or user program stop
        PT_WAIT_UNTIL(pt, !button_pressed);
        printf("R1\n");
        PT_WAIT_UNTIL(pt, button_pressed);
        printf("P\n");
        PT_WAIT_UNTIL(pt, !button_pressed);
        printf("R2\n");

        // If we made it through a full press and release, without the user
        // program running, then start the currently selected user program.
        pbsys_main_program_request_start(selected_slot);
    }

    PT_END(pt);
}
R1
R1
R1    # program still running
R1    # program still running && button pressed, PROGRAM_RUNNING unsets
P      # no goto start, && still pressed
R2    # so program restarts
R1
R1
R1
R1
R1
R1

@laurensvalk
Copy link
Member Author

I think we can do the following. PT_EXIT will also PT_INIT.

Was there a reason the goto approach was used or possibly preferred?

diff --git a/lib/pbio/sys/hmi.c b/lib/pbio/sys/hmi.c
index 41b5db23a..fb32a52da 100644
--- a/lib/pbio/sys/hmi.c
+++ b/lib/pbio/sys/hmi.c
@@ -40,16 +40,15 @@ static uint8_t selected_slot = 0;
  */
 static PT_THREAD(update_program_run_button_wait_state(bool button_pressed)) {
     struct pt *pt = &update_program_run_button_wait_state_pt;
-    // Creative use of protothread to reduce code size. This is the same
-    // as checking if the user program is running after each PT_WAIT.
+
+    // This should not be active while a program is running.
     if (pbsys_status_test(PBIO_PYBRICKS_STATUS_USER_PROGRAM_RUNNING)) {
-        goto start;
+        PT_EXIT(pt);
     }
 
     PT_BEGIN(pt);
 
     for (;;) {
-    start:
         // button may still be pressed from power on or user program stop
         PT_WAIT_UNTIL(pt, !button_pressed);
         PT_WAIT_UNTIL(pt, button_pressed);

laurensvalk added a commit to pybricks/pybricks-micropython that referenced this issue Oct 2, 2024
The previous approach would go to the start but still run until the next yield. This meant that if the stop button was pressed while a program gracefully exists, it would already be in the "second stage" of the following

        PT_WAIT_UNTIL(pt, !button_pressed);
        PT_WAIT_UNTIL(pt, button_pressed);
        PT_WAIT_UNTIL(pt, !button_pressed);
        pbsys_main_program_request_start(selected_slot);

Then on release, the program would immediately restart.

PT_EXIT has the same effect of resetting the state to the start, but existing immediately after that.

Fixes pybricks/support#1863
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime)
Projects
None yet
Development

No branches or pull requests

1 participant