ports/rp2: Replace pico-sdk assertion faults with python exceptions. #16991

AJMansfield · 2025-03-22T00:02:28Z

Summary

This change replaces the default pico-sdk assertion fault handlers with new handlers that raise micropython exceptions.

Testing

I've verified that this works on real rp2040 hardware, using a custom module to trigger the pico-sdk assertion mechanism:

#include "py/runtime.h"
#include "pico/assert.h"

#define PARAM_ASSERTIONS_ENABLED_FAIL (1)

static mp_obj_t rp2_param_fault(void) {
    invalid_params_if(FAIL, true);
    return mp_const_none;
}
MP_DEFINE_CONST_FUN_OBJ_0(rp2_param_fault_obj, rp2_param_fault);

static mp_obj_t rp2_hard_fault(void) {
    hard_assert_if(FAIL, true);
    return mp_const_none;
}
MP_DEFINE_CONST_FUN_OBJ_0(rp2_hard_fault_obj, rp2_hard_fault);

static const mp_rom_map_elem_t fail_module_globals_table[] = {
    { MP_ROM_QSTR(MP_QSTR___name__),            MP_ROM_QSTR(MP_QSTR_fail) },
    { MP_ROM_QSTR(MP_QSTR_param),               MP_ROM_PTR(&rp2_param_fault_obj) },
    { MP_ROM_QSTR(MP_QSTR_hard),                MP_ROM_PTR(&rp2_hard_fault_obj) },
};
static MP_DEFINE_CONST_DICT(fail_module_globals, fail_module_globals_table);

const mp_obj_module_t mp_module_fail = {
    .base = { &mp_type_module },
    .globals = (mp_obj_dict_t *)&fail_module_globals,
};

MP_REGISTER_MODULE(MP_QSTR_fail, mp_module_fail);

Which leads to:

MicroPython v1.25.0-preview.400.ge3adb89f8.dirty on 2025-03-21; Raspberry Pi Pico W with RP2040
Type "help()" for more information.
>>> import fail
>>> fail.hard()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AssertionError: assertion failure in pico-sdk FAIL
>>> fail.param()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: invalid params in pico-sdk FAIL

Tradeoffs and Alternatives

It's not entirely clear what the best choice of exception and exception message is, and I invite comments on this topic.

At present I'm using variadic exceptions with a parameter derived from the name of the associated assertion enable gate. invalid_params_if and valid_params_if both map to ValueError("invalid params in pico-sdk %s"), and hard_assert_if failures map to AssertionError("assertion failure in pico-sdk %s"). Should the latter perhaps be a RuntimeError rather than an AssertionError, though?

github-actions · 2025-03-22T00:10:12Z

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

Gadgetoid · 2025-03-24T10:34:14Z

Woof, is the 4k size gain because you've forced all assertions to be enabled here?

This would probably be extremely useful for debugging user C modules, of which we (Pimoroni) have way, way too many, but should definitely be optional. Only a developer should ever see/care about these asserts, and if MicroPython is hitting a condition where an end user can trigger one then I'd argue that's a bug in MicroPython.

But otherwise 👍 knowing how to hook this up might have saved us some real headaches.

AJMansfield · 2025-03-24T13:13:09Z

Woof, is the 4k size gain because you've forced all assertions to be enabled here?

Come to think of it, maybe #define PARAM_ASSERTIONS_ENABLE_ALL 1 is not the correct value to be setting here...

This would probably be extremely useful for debugging user C modules, of which we (Pimoroni) have way, way too many, but should definitely be optional. Only a developer should ever see/care about these asserts, and if MicroPython is hitting a condition where an end user can trigger one then I'd argue that's a bug in MicroPython.

Are there some debug assertion defines that already exist that could be used to gate this?

Gadgetoid · 2025-03-24T13:27:23Z

Are there some debug assertion defines that already exist that could be used to gate this?

Not as far as I'm aware, there's some debug stuff here but I've... surprisingly little experience with it 😬

micropython/py/mpconfig.h

Lines 501 to 544 in f1018ee

    
           /*****************************************************************************/ 
        
           /* Internal debugging stuff                                                  */ 
        
           // Whether to collect memory allocation stats 
        
           #ifndef MICROPY_MEM_STATS 
        
           #define MICROPY_MEM_STATS (0) 
        
           #endif 
        
           // The mp_print_t printer used for debugging output 
        
           #ifndef MICROPY_DEBUG_PRINTER 
        
           #define MICROPY_DEBUG_PRINTER (&mp_plat_print) 
        
           #endif 
        
           // Whether to build functions that print debugging info: 
        
           //   mp_bytecode_print 
        
           //   mp_parse_node_print 
        
           #ifndef MICROPY_DEBUG_PRINTERS 
        
           #define MICROPY_DEBUG_PRINTERS (0) 
        
           #endif 
        
           // Whether to enable all debugging outputs (it will be extremely verbose) 
        
           #ifndef MICROPY_DEBUG_VERBOSE 
        
           #define MICROPY_DEBUG_VERBOSE (0) 
        
           #endif 
        
           // Whether to enable debugging versions of MP_OBJ_NULL/STOP_ITERATION/SENTINEL 
        
           #ifndef MICROPY_DEBUG_MP_OBJ_SENTINELS 
        
           #define MICROPY_DEBUG_MP_OBJ_SENTINELS (0) 
        
           #endif 
        
           // Whether to print parse rule names (rather than integers) in mp_parse_node_print 
        
           #ifndef MICROPY_DEBUG_PARSE_RULE_NAME 
        
           #define MICROPY_DEBUG_PARSE_RULE_NAME (0) 
        
           #endif 
        
           // Whether to enable a simple VM stack overflow check 
        
           #ifndef MICROPY_DEBUG_VM_STACK_OVERFLOW 
        
           #define MICROPY_DEBUG_VM_STACK_OVERFLOW (0) 
        
           #endif 
        
           // Whether to enable extra instrumentation for valgrind 
        
           #ifndef MICROPY_DEBUG_VALGRIND 
        
           #define MICROPY_DEBUG_VALGRIND (0) 
        
           #endif

The ESP8266 port has an interesting (read: uncomplicated) take on this:

micropython/ports/esp8266/esp_mphal.c

Lines 131 to 134 in f1018ee

    
           void __assert_func(const char *file, int line, const char *func, const char *expr) { 
        
               printf("assert:%s:%d:%s: %s\n", file, line, func, expr); 
        
               mp_raise_msg(&mp_type_AssertionError, MP_ERROR_TEXT("C-level assert")); 
        
           }

There doesn't really seem to be a consensus on dealing with SDK assertions in MicroPython itself, presumably because the various SDKs all have their own way of doing things.

AJMansfield · 2025-03-24T17:35:51Z

It took me way too long messing around with trying to convert the assert flag's name into a qstr instead of just a raw char* (which didn't work), but yes: all of that size increase did indeed come down to that #define PARAM_ASSERTIONS_ENABLE_ALL 1 line.

On the plus side, it looks like with that set to 0 now, the the compiler even ends up even fully eliding the text for pico_param_fault and pico_hard_fault, so there's no size penalty unless you configure your port/variant with some or all of the assertions enabled.

Signed-off-by: Anson Mansfield <[email protected]>

AJMansfield mentioned this pull request Mar 22, 2025

rp2/rp2_pio: Switch to mask64 for pins/pindirs. #16985

Closed

AJMansfield force-pushed the pico-sdk-asserts branch 3 times, most recently from d8fdbc1 to 185f42f Compare March 22, 2025 02:41

AJMansfield force-pushed the pico-sdk-asserts branch 2 times, most recently from 66ee83e to bd6ec3d Compare March 24, 2025 17:31

AJMansfield added 2 commits March 24, 2025 13:44

rp2/pico_assert: Replace pico-sdk assertions with python exceptions.

c8711f5

Signed-off-by: Anson Mansfield <[email protected]>

rp2/pico_assert: Include more debugging info in pico-sdk exceptions.

97a3ab4

Signed-off-by: Anson Mansfield <[email protected]>

AJMansfield force-pushed the pico-sdk-asserts branch from bd6ec3d to 97a3ab4 Compare March 24, 2025 17:47

dpgeorge added the port-rp2 label Mar 27, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

ports/rp2: Replace pico-sdk assertion faults with python exceptions. #16991

ports/rp2: Replace pico-sdk assertion faults with python exceptions. #16991

AJMansfield commented Mar 22, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Mar 22, 2025 •

edited

Loading

Uh oh!

Gadgetoid commented Mar 24, 2025

Uh oh!

AJMansfield commented Mar 24, 2025

Uh oh!

Gadgetoid commented Mar 24, 2025

Uh oh!

AJMansfield commented Mar 24, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

ports/rp2: Replace pico-sdk assertion faults with python exceptions. #16991

Are you sure you want to change the base?

ports/rp2: Replace pico-sdk assertion faults with python exceptions. #16991

Conversation

AJMansfield commented Mar 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Tradeoffs and Alternatives

Uh oh!

github-actions bot commented Mar 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gadgetoid commented Mar 24, 2025

Uh oh!

AJMansfield commented Mar 24, 2025

Uh oh!

Gadgetoid commented Mar 24, 2025

Uh oh!

AJMansfield commented Mar 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AJMansfield commented Mar 22, 2025 •

edited

Loading

github-actions bot commented Mar 22, 2025 •

edited

Loading

AJMansfield commented Mar 24, 2025 •

edited

Loading