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] SystemExit and KeyboardInterrupt appear swapped on REPL #347

Closed
laurensvalk opened this issue May 26, 2021 · 6 comments
Closed

[Bug] SystemExit and KeyboardInterrupt appear swapped on REPL #347

laurensvalk opened this issue May 26, 2021 · 6 comments
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

  • Pressing CTRL+C on the first line of the REPL exits the REPL (but it shouldn't).
  • Running raise SystemExit does not exit the REPL (but it should).

To reproduce
Connect to the hub and start the REPL with the >>> button in Pybricks Code.

Click on the terminal pane and press CTRL+C. The run animation on the hub stops. If you run a few other commands first or wait a while, then CTRL+C does keep you in the REPL as expected.

For the second issue, start REPL, paste and execute raise SystemExit, and observe that it keeps running.

Tested on PrimeHub.

Expected behavior

  • CTRL+C to just go to a new empty line on the REPL
  • SystemExit to exit as if pressing the stop button.

Screenshots

CTRL+C on the first line triggers an exit:

image

If you don't do it right away, it does not:

image

@laurensvalk laurensvalk added triage Issues that have not been triaged yet software: pybricks-micropython Issues with Pybricks MicroPython firmware (or EV3 runtime) bug Something isn't working and removed triage Issues that have not been triaged yet labels May 26, 2021
@laurensvalk
Copy link
Member Author

When running scripts everything appears to be working as expected.

@dlech
Copy link
Member

dlech commented Jun 3, 2021

  • Running raise SystemExit does not exit the REPL (but it should).

AFAIK, not exiting the REPL on plain SystemExit is standard MicroPython behavior. There is an internal PYEXEC_FORCED_EXIT flag that is only set when machine.soft_reset() is called that modifies the behavior of SystemExit. (We also set PYEXEC_FORCED_EXIT with our stop buttons.)

dlech added a commit to pybricks/pybricks-micropython that referenced this issue Jun 3, 2021
This makes the run_user_program() function more like the MicroPython
parse_compile_execute() function. pyexec_friendly_repl() already takes
care of CTRL+C handling, so we only need it on our custom runner.

Issue: pybricks/support#347
@dlech
Copy link
Member

dlech commented Jun 3, 2021

  • Pressing CTRL+C on the first line of the REPL exits the REPL (but it shouldn't).

This is fixed now.

@laurensvalk
Copy link
Member Author

  • Running raise SystemExit does not exit the REPL (but it should).

AFAIK, not exiting the REPL on plain SystemExit is standard MicroPython behavior. There is an internal PYEXEC_FORCED_EXIT flag that is only set when machine.soft_reset() is called that modifies the behavior of SystemExit. (We also set PYEXEC_FORCED_EXIT with our stop buttons.)

OK - I suppose it doesn't exit in MicroPython because usually there is nothing to exit to. I guess I was extrapolating from how it works in scripts. It seems nice to make it the same. This isn't critical to me though.

@dlech
Copy link
Member

dlech commented Jun 3, 2021

The change would need to be made in upstream MicroPython.

@laurensvalk
Copy link
Member Author

Then let's keep it simple and not change it :)

The CTRL+C was the main bug here, and it looks like you fixed it already. The system exit is just something that seemed odd when I dug deeper, but it's not really an issue.

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

2 participants