Skip to content

Commit

Permalink
Pico RGB Keypad: Refactor to class.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@Gadgetoid
Gadgetoid committed Mar 16, 2023
1 parent 9964ed7 commit bd3651d
Show file tree
Hide file tree
Showing 6 changed files with 141 additions and 113 deletions.
5 changes: 5 additions & 0 deletions libraries/pico_rgb_keypad/pico_rgb_keypad.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ enum pin {

namespace pimoroni {

PicoRGBKeypad::~PicoRGBKeypad() {
clear();
update();
}

void PicoRGBKeypad::init() {
memset(buffer, 0, sizeof(buffer));
led_data = buffer + 4;
Expand Down
1 change: 1 addition & 0 deletions libraries/pico_rgb_keypad/pico_rgb_keypad.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ namespace pimoroni {
uint8_t *led_data;

public:
~PicoRGBKeypad();
void init();
void update();
void set_brightness(float brightness);
Expand Down
4 changes: 2 additions & 2 deletions micropython/examples/pico_rgb_keypad/demo.py
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import time
import picokeypad as keypad
import picokeypad

keypad.init()
keypad = picokeypad.PicoKeypad()

This comment has been minimized.

Sign in to view

Copy link
@ysfktn

ysfktn Mar 24, 2023

this version is not running in Thonny

AttributeError: 'module' object has no attribute 'RGBKeypad'

I tried the previous version and it worked well.

This comment has been minimized.

Sign in to view

Copy link
@Gadgetoid

Gadgetoid Mar 24, 2023

Author Member

Build v1.19.17 is incredibly broken with arcane memory corruption bugs.

You can grab a - hopefully working - Pico W firmware from here: https://github.com/pimoroni/pimoroni-pico/actions/runs/4512636578?pr=725

Or Pico from here: https://github.com/pimoroni/pimoroni-pico/actions/runs/4512637445?pr=725
keypad.set_brightness(1.0)

lit = 0
Expand Down
56 changes: 40 additions & 16 deletions micropython/modules/pico_rgb_keypad/pico_rgb_keypad.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) },
Expand All @@ -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,
Expand Down
169 changes: 82 additions & 87 deletions micropython/modules/pico_rgb_keypad/pico_rgb_keypad.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
19 changes: 11 additions & 8 deletions micropython/modules/pico_rgb_keypad/pico_rgb_keypad.h
View file
Original file line number Diff line number Diff line change
@@ -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();
extern mp_obj_t picokeypad_clear(mp_obj_t self_in);
extern mp_obj_t picokeypad_get_button_states(mp_obj_t self_in);

0 comments on commit bd3651d

Please sign in to comment.