From bd3651d97d22beb6ae0769552297e0831989200f Mon Sep 17 00:00:00 2001 From: Phil Howard Date: Mon, 13 Mar 2023 18:20:50 +0000 Subject: [PATCH] Pico RGB Keypad: Refactor to class. Because `mp_tracked_calloc` does not survive a soft reset but the memory region will, resulting in half-initialised frankenclasses that behave unpredictably. Using the class pattern fixes this since it's always guaranteed to be initialised when a user instantiates it, and __del__ can handle cleanup. --- libraries/pico_rgb_keypad/pico_rgb_keypad.cpp | 5 + libraries/pico_rgb_keypad/pico_rgb_keypad.hpp | 1 + micropython/examples/pico_rgb_keypad/demo.py | 4 +- .../modules/pico_rgb_keypad/pico_rgb_keypad.c | 56 ++++-- .../pico_rgb_keypad/pico_rgb_keypad.cpp | 169 +++++++++--------- .../modules/pico_rgb_keypad/pico_rgb_keypad.h | 19 +- 6 files changed, 141 insertions(+), 113 deletions(-) diff --git a/libraries/pico_rgb_keypad/pico_rgb_keypad.cpp b/libraries/pico_rgb_keypad/pico_rgb_keypad.cpp index ccc0f907b..3bc24b7a4 100644 --- a/libraries/pico_rgb_keypad/pico_rgb_keypad.cpp +++ b/libraries/pico_rgb_keypad/pico_rgb_keypad.cpp @@ -17,6 +17,11 @@ enum pin { namespace pimoroni { + PicoRGBKeypad::~PicoRGBKeypad() { + clear(); + update(); + } + void PicoRGBKeypad::init() { memset(buffer, 0, sizeof(buffer)); led_data = buffer + 4; diff --git a/libraries/pico_rgb_keypad/pico_rgb_keypad.hpp b/libraries/pico_rgb_keypad/pico_rgb_keypad.hpp index 0f7ec356e..05b0d5499 100644 --- a/libraries/pico_rgb_keypad/pico_rgb_keypad.hpp +++ b/libraries/pico_rgb_keypad/pico_rgb_keypad.hpp @@ -16,6 +16,7 @@ namespace pimoroni { uint8_t *led_data; public: + ~PicoRGBKeypad(); void init(); void update(); void set_brightness(float brightness); diff --git a/micropython/examples/pico_rgb_keypad/demo.py b/micropython/examples/pico_rgb_keypad/demo.py index ac7927347..40eadb4d5 100644 --- a/micropython/examples/pico_rgb_keypad/demo.py +++ b/micropython/examples/pico_rgb_keypad/demo.py @@ -1,7 +1,7 @@ import time -import picokeypad as keypad +import picokeypad -keypad.init() +keypad = picokeypad.PicoKeypad() keypad.set_brightness(1.0) lit = 0 diff --git a/micropython/modules/pico_rgb_keypad/pico_rgb_keypad.c b/micropython/modules/pico_rgb_keypad/pico_rgb_keypad.c index dfe3967ae..4c14e79a9 100644 --- a/micropython/modules/pico_rgb_keypad/pico_rgb_keypad.c +++ b/micropython/modules/pico_rgb_keypad/pico_rgb_keypad.c @@ -5,21 +5,20 @@ //////////////////////////////////////////////////////////////////////////////////////////////////// /***** Module Functions *****/ -STATIC MP_DEFINE_CONST_FUN_OBJ_0(picokeypad_init_obj, picokeypad_init); -STATIC MP_DEFINE_CONST_FUN_OBJ_0(picokeypad_get_width_obj, picokeypad_get_width); -STATIC MP_DEFINE_CONST_FUN_OBJ_0(picokeypad_get_height_obj, picokeypad_get_height); -STATIC MP_DEFINE_CONST_FUN_OBJ_0(picokeypad_get_num_pads_obj, picokeypad_get_num_pads); -STATIC MP_DEFINE_CONST_FUN_OBJ_0(picokeypad_update_obj, picokeypad_update); -STATIC MP_DEFINE_CONST_FUN_OBJ_1(picokeypad_set_brightness_obj, picokeypad_set_brightness); -STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(picokeypad_illuminate_xy_obj, 5, 5, picokeypad_illuminate_xy); -STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(picokeypad_illuminate_obj, 4, 4, picokeypad_illuminate); -STATIC MP_DEFINE_CONST_FUN_OBJ_0(picokeypad_clear_obj, picokeypad_clear); -STATIC MP_DEFINE_CONST_FUN_OBJ_0(picokeypad_get_button_states_obj, picokeypad_get_button_states); +STATIC MP_DEFINE_CONST_FUN_OBJ_1(picokeypad___del___obj, picokeypad___del__); +STATIC MP_DEFINE_CONST_FUN_OBJ_1(picokeypad_get_width_obj, picokeypad_get_width); +STATIC MP_DEFINE_CONST_FUN_OBJ_1(picokeypad_get_height_obj, picokeypad_get_height); +STATIC MP_DEFINE_CONST_FUN_OBJ_1(picokeypad_get_num_pads_obj, picokeypad_get_num_pads); +STATIC MP_DEFINE_CONST_FUN_OBJ_1(picokeypad_update_obj, picokeypad_update); +STATIC MP_DEFINE_CONST_FUN_OBJ_2(picokeypad_set_brightness_obj, picokeypad_set_brightness); +STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(picokeypad_illuminate_xy_obj, 6, 6, picokeypad_illuminate_xy); +STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(picokeypad_illuminate_obj, 5, 5, picokeypad_illuminate); +STATIC MP_DEFINE_CONST_FUN_OBJ_1(picokeypad_clear_obj, picokeypad_clear); +STATIC MP_DEFINE_CONST_FUN_OBJ_1(picokeypad_get_button_states_obj, picokeypad_get_button_states); -/***** Globals Table *****/ -STATIC const mp_map_elem_t picokeypad_globals_table[] = { - { MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_picokeypad) }, - { MP_ROM_QSTR(MP_QSTR_init), MP_ROM_PTR(&picokeypad_init_obj) }, +/* Class Methods */ +STATIC const mp_rom_map_elem_t picokeypad_locals[] = { + { MP_ROM_QSTR(MP_QSTR___del__), MP_ROM_PTR(&picokeypad___del___obj) }, { MP_ROM_QSTR(MP_QSTR_get_width), MP_ROM_PTR(&picokeypad_get_width_obj) }, { MP_ROM_QSTR(MP_QSTR_get_height), MP_ROM_PTR(&picokeypad_get_height_obj) }, { MP_ROM_QSTR(MP_QSTR_get_num_pads), MP_ROM_PTR(&picokeypad_get_num_pads_obj) }, @@ -30,9 +29,34 @@ STATIC const mp_map_elem_t picokeypad_globals_table[] = { { MP_ROM_QSTR(MP_QSTR_clear), MP_ROM_PTR(&picokeypad_clear_obj) }, { MP_ROM_QSTR(MP_QSTR_get_button_states), MP_ROM_PTR(&picokeypad_get_button_states_obj) }, }; -STATIC MP_DEFINE_CONST_DICT(mp_module_picokeypad_globals, picokeypad_globals_table); +STATIC MP_DEFINE_CONST_DICT(mp_module_picokeypad_locals, picokeypad_locals); + +#ifdef MP_DEFINE_CONST_OBJ_TYPE +MP_DEFINE_CONST_OBJ_TYPE( + PicoKeypad_type, + MP_QSTR_PicoKeypad, + MP_TYPE_FLAG_NONE, + make_new, picokeypad_make_new, + locals_dict, (mp_obj_dict_t*)&mp_module_picokeypad_locals +); +#else +const mp_obj_type_t PicoKeypad_type = { + { &mp_type_type }, + .name = MP_QSTR_PicoKeypad, + .make_new = picokeypad_make_new, + .locals_dict = (mp_obj_dict_t*)&mp_module_picokeypad_locals, +}; +#endif + +/* Module Globals */ +STATIC const mp_map_elem_t picokeypad_globals[] = { + { MP_OBJ_NEW_QSTR(MP_QSTR___name__), MP_OBJ_NEW_QSTR(MP_QSTR_picokeypad) }, + { MP_OBJ_NEW_QSTR(MP_QSTR_PicoKeypad), (mp_obj_t)&PicoKeypad_type }, + { MP_ROM_QSTR(MP_QSTR_WIDTH), MP_ROM_INT(4) }, + { MP_ROM_QSTR(MP_QSTR_HEIGHT), MP_ROM_INT(4) }, +}; +STATIC MP_DEFINE_CONST_DICT(mp_module_picokeypad_globals, picokeypad_globals); -/***** Module Definition *****/ const mp_obj_module_t picokeypad_user_cmodule = { .base = { &mp_type_module }, .globals = (mp_obj_dict_t*)&mp_module_picokeypad_globals, diff --git a/micropython/modules/pico_rgb_keypad/pico_rgb_keypad.cpp b/micropython/modules/pico_rgb_keypad/pico_rgb_keypad.cpp index e71fbb5dd..7e95916fb 100644 --- a/micropython/modules/pico_rgb_keypad/pico_rgb_keypad.cpp +++ b/micropython/modules/pico_rgb_keypad/pico_rgb_keypad.cpp @@ -7,130 +7,125 @@ using namespace pimoroni; -PicoRGBKeypad *keypad = nullptr; - - extern "C" { #include "pico_rgb_keypad.h" -#define NOT_INITIALISED_MSG "Cannot call this function, as picokeypad is not initialised. Call picokeypad.init() first." +typedef struct _PicoKeypad_obj_t { + mp_obj_base_t base; + PicoRGBKeypad* keypad; +} PicoKeypad_obj_t; + +mp_obj_t picokeypad_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) { + _PicoKeypad_obj_t *self = nullptr; + + self = m_new_obj_with_finaliser(PicoKeypad_obj_t); + self->base.type = &PicoKeypad_type; + + self->keypad = m_new_class(PicoRGBKeypad); + self->keypad->init(); + + return MP_OBJ_FROM_PTR(self); +} -mp_obj_t picokeypad_init() { - if(keypad == nullptr) { - keypad = m_tracked_alloc_class(PicoRGBKeypad); - keypad->init(); - } +mp_obj_t picokeypad___del__(mp_obj_t self_in) { + PicoKeypad_obj_t *self = MP_OBJ_TO_PTR2(self_in, PicoKeypad_obj_t); + m_del_class(PicoRGBKeypad, self->keypad); return mp_const_none; } -mp_obj_t picokeypad_get_width() { +mp_obj_t picokeypad_get_width(mp_obj_t self_in) { + (void)self_in; return mp_obj_new_int(PicoRGBKeypad::WIDTH); } -mp_obj_t picokeypad_get_height() { +mp_obj_t picokeypad_get_height(mp_obj_t self_in) { + (void)self_in; return mp_obj_new_int(PicoRGBKeypad::HEIGHT); } -mp_obj_t picokeypad_get_num_pads() { +mp_obj_t picokeypad_get_num_pads(mp_obj_t self_in) { + (void)self_in; return mp_obj_new_int(PicoRGBKeypad::NUM_PADS); } -mp_obj_t picokeypad_update() { - if(keypad != nullptr) - keypad->update(); - else - mp_raise_msg(&mp_type_RuntimeError, NOT_INITIALISED_MSG); +mp_obj_t picokeypad_update(mp_obj_t self_in) { + PicoKeypad_obj_t *self = MP_OBJ_TO_PTR2(self_in, PicoKeypad_obj_t); + self->keypad->update(); return mp_const_none; } -mp_obj_t picokeypad_set_brightness(mp_obj_t brightness_obj) { - if(keypad != nullptr) { - float brightness = mp_obj_get_float(brightness_obj); +mp_obj_t picokeypad_set_brightness(mp_obj_t self_in, mp_obj_t brightness_obj) { + PicoKeypad_obj_t *self = MP_OBJ_TO_PTR2(self_in, PicoKeypad_obj_t); - if(brightness < 0 || brightness > 1.0f) - mp_raise_ValueError("brightness out of range. Expected 0.0 to 1.0"); - else - keypad->set_brightness(brightness); - } - else - mp_raise_msg(&mp_type_RuntimeError, NOT_INITIALISED_MSG); + float brightness = mp_obj_get_float(brightness_obj); + + if(brightness < 0 || brightness > 1.0f) + mp_raise_ValueError("brightness out of range. Expected 0.0 to 1.0"); + + self->keypad->set_brightness(brightness); return mp_const_none; } mp_obj_t picokeypad_illuminate_xy(mp_uint_t n_args, const mp_obj_t *args) { - (void)n_args; //Unused input parameter, we know it's 5 - - if(keypad != nullptr) { - int x = mp_obj_get_int(args[0]); - int y = mp_obj_get_int(args[1]); - int r = mp_obj_get_int(args[2]); - int g = mp_obj_get_int(args[3]); - int b = mp_obj_get_int(args[4]); - - if(x < 0 || x >= PicoRGBKeypad::WIDTH || y < 0 || y >= PicoRGBKeypad::HEIGHT) - mp_raise_ValueError("x or y out of range."); - else { - if(r < 0 || r > 255) - mp_raise_ValueError("r out of range. Expected 0 to 255"); - else if(g < 0 || g > 255) - mp_raise_ValueError("g out of range. Expected 0 to 255"); - else if(b < 0 || b > 255) - mp_raise_ValueError("b out of range. Expected 0 to 255"); - else - keypad->illuminate(x, y, r, g, b); - } - } - else - mp_raise_msg(&mp_type_RuntimeError, NOT_INITIALISED_MSG); - + (void)n_args; //Unused input parameter, we know it's self + 5 + + PicoKeypad_obj_t *self = MP_OBJ_TO_PTR2(args[0], PicoKeypad_obj_t); + + int x = mp_obj_get_int(args[1]); + int y = mp_obj_get_int(args[2]); + int r = mp_obj_get_int(args[3]); + int g = mp_obj_get_int(args[4]); + int b = mp_obj_get_int(args[5]); + + if(x < 0 || x >= PicoRGBKeypad::WIDTH || y < 0 || y >= PicoRGBKeypad::HEIGHT) + mp_raise_ValueError("x or y out of range."); + if(r < 0 || r > 255) + mp_raise_ValueError("r out of range. Expected 0 to 255"); + if(g < 0 || g > 255) + mp_raise_ValueError("g out of range. Expected 0 to 255"); + if(b < 0 || b > 255) + mp_raise_ValueError("b out of range. Expected 0 to 255"); + + self->keypad->illuminate(x, y, r, g, b); + return mp_const_none; } mp_obj_t picokeypad_illuminate(mp_uint_t n_args, const mp_obj_t *args) { - (void)n_args; //Unused input parameter, we know it's 5 - - if(keypad != nullptr) { - int i = mp_obj_get_int(args[0]); - int r = mp_obj_get_int(args[1]); - int g = mp_obj_get_int(args[2]); - int b = mp_obj_get_int(args[3]); - - if(i < 0 || i >= PicoRGBKeypad::NUM_PADS) - mp_raise_ValueError("x or y out of range."); - else { - if(r < 0 || r > 255) - mp_raise_ValueError("r out of range. Expected 0 to 255"); - else if(g < 0 || g > 255) - mp_raise_ValueError("g out of range. Expected 0 to 255"); - else if(b < 0 || b > 255) - mp_raise_ValueError("b out of range. Expected 0 to 255"); - else - keypad->illuminate(i, r, g, b); - } - } - else - mp_raise_msg(&mp_type_RuntimeError, NOT_INITIALISED_MSG); + (void)n_args; //Unused input parameter, we know it's self + 5 + + PicoKeypad_obj_t *self = MP_OBJ_TO_PTR2(args[0], PicoKeypad_obj_t); + + int i = mp_obj_get_int(args[1]); + int r = mp_obj_get_int(args[2]); + int g = mp_obj_get_int(args[3]); + int b = mp_obj_get_int(args[4]); + + if(i < 0 || i >= PicoRGBKeypad::NUM_PADS) + mp_raise_ValueError("x or y out of range."); + if(r < 0 || r > 255) + mp_raise_ValueError("r out of range. Expected 0 to 255"); + if(g < 0 || g > 255) + mp_raise_ValueError("g out of range. Expected 0 to 255"); + if(b < 0 || b > 255) + mp_raise_ValueError("b out of range. Expected 0 to 255"); + + self->keypad->illuminate(i, r, g, b); return mp_const_none; } -mp_obj_t picokeypad_clear() { - if(keypad != nullptr) - keypad->clear(); - else - mp_raise_msg(&mp_type_RuntimeError, NOT_INITIALISED_MSG); - +mp_obj_t picokeypad_clear(mp_obj_t self_in) { + PicoKeypad_obj_t *self = MP_OBJ_TO_PTR2(self_in, PicoKeypad_obj_t); + self->keypad->clear(); return mp_const_none; } -mp_obj_t picokeypad_get_button_states() { +mp_obj_t picokeypad_get_button_states(mp_obj_t self_in) { + PicoKeypad_obj_t *self = MP_OBJ_TO_PTR2(self_in, PicoKeypad_obj_t); uint16_t states = 0; - if(keypad != nullptr) - states = keypad->get_button_states(); - else - mp_raise_msg(&mp_type_RuntimeError, NOT_INITIALISED_MSG); - + states = self->keypad->get_button_states(); return mp_obj_new_int(states); } } \ No newline at end of file diff --git a/micropython/modules/pico_rgb_keypad/pico_rgb_keypad.h b/micropython/modules/pico_rgb_keypad/pico_rgb_keypad.h index 0d9e083cf..b5ee60e4a 100644 --- a/micropython/modules/pico_rgb_keypad/pico_rgb_keypad.h +++ b/micropython/modules/pico_rgb_keypad/pico_rgb_keypad.h @@ -1,14 +1,17 @@ // Include MicroPython API. #include "py/runtime.h" +extern const mp_obj_type_t PicoKeypad_type; + // Declare the functions we'll make available in Python -extern mp_obj_t picokeypad_init(); -extern mp_obj_t picokeypad_get_width(); -extern mp_obj_t picokeypad_get_height(); -extern mp_obj_t picokeypad_get_num_pads(); -extern mp_obj_t picokeypad_update(); -extern mp_obj_t picokeypad_set_brightness(mp_obj_t brightness_obj); +extern mp_obj_t picokeypad_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args); +extern mp_obj_t picokeypad___del__(mp_obj_t self_in); +extern mp_obj_t picokeypad_get_width(mp_obj_t self_in); +extern mp_obj_t picokeypad_get_height(mp_obj_t self_in); +extern mp_obj_t picokeypad_get_num_pads(mp_obj_t self_in); +extern mp_obj_t picokeypad_update(mp_obj_t self_in); +extern mp_obj_t picokeypad_set_brightness(mp_obj_t self_in, mp_obj_t brightness_obj); extern mp_obj_t picokeypad_illuminate_xy(mp_uint_t n_args, const mp_obj_t *args); extern mp_obj_t picokeypad_illuminate(mp_uint_t n_args, const mp_obj_t *args); -extern mp_obj_t picokeypad_clear(); -extern mp_obj_t picokeypad_get_button_states(); \ No newline at end of file +extern mp_obj_t picokeypad_clear(mp_obj_t self_in); +extern mp_obj_t picokeypad_get_button_states(mp_obj_t self_in); \ No newline at end of file