microbit-foundation / micropython-microbit-v2

Temporary home for MicroPython for micro:bit v2 as we stablise it before pushing upstream
MIT License
41 stars 22 forks source link

Ctrl-C during scroll continues to subsequent raised exception #134

Closed microbit-matt-hillsdon closed 1 year ago

microbit-matt-hillsdon commented 1 year ago

Run this script:

from microbit import *

display.scroll('Takes a while')
raise ValueError(123)

Press Ctrl-C during the scrolling text.

Unexpectedly you see the error message from the ValueError scrolled on the micro:bit.

If the raise line is replaced with print("Here") then the print output doesn't show. If the scroll is replaced by a sleep then the ValueError does not show.

This behaviour is a bit awkward for the simulator as our stop button sends a Ctrl-C to interrupt the program, but given this behaviour it can be 10s of seconds before it actually stops.

The original code example was more realistic with the ValueError thrown by micro:bit MicroPython API parameter validation.

dpgeorge commented 1 year ago

Thanks for the report. This is a strange bug!

The following should fix it, but I need to test it more before merging this fix:

--- a/src/codal_port/drv_display.c
+++ b/src/codal_port/drv_display.c
@@ -67,6 +67,7 @@ STATIC void wait_for_event() {
         // allow CTRL-C to stop the animation
         if (MP_STATE_THREAD(mp_pending_exception) != MP_OBJ_NULL) {
             async_stop();
+            mp_handle_pending(true);
             return;
         }
         microbit_hal_idle();
microbit-matt-hillsdon commented 1 year ago

@microbit-robert to try this out and update the issue.

microbit-robert commented 1 year ago

The suggested change above works well and fixes the issue described.

dpgeorge commented 1 year ago

I tested this and checked that all callers of this function can accept it raising an exception.

Implemented in cfe75b2dcd978a1831119b90cd536b5aaf760724