pybricks.pupdevices.lwp3device: Use finalizers.

Finish an old todo about using finalizers to clean up observe data and lwp3 notification buffer.

Author: laurensvalk

bricks/_common/micropython.c

@@ -423,9 +423,6 @@ void pbsys_main_run_program(pbsys_main_program_t *program) {
 }
 
 void pbsys_main_run_program_cleanup(void) {
-
-    pb_package_pybricks_deinit();
-
     gc_sweep_all();
     mp_deinit();
 }

bricks/virtualhub/mp_port.c

@@ -86,8 +86,6 @@ void pb_virtualhub_port_init(void) {
 
 // MICROPY_PORT_DEINIT_FUNC
 void pb_virtualhub_port_deinit(void) {
-
-    pb_package_pybricks_deinit();
 }
 
 // Implementation for MICROPY_EVENT_POLL_HOOK

pybricks/common.h

@@ -26,7 +26,6 @@
 #include <pybricks/common/pb_type_device.h>
 
 void pb_package_pybricks_init(bool import_all);
-void pb_package_pybricks_deinit(void);
 
 #if PYBRICKS_PY_COMMON_BLE
 mp_obj_t pb_type_BLE_new(mp_obj_t broadcast_channel_in, mp_obj_t observe_channels_in);

pybricks/common/pb_type_ble.c

@@ -91,6 +91,11 @@ typedef enum {
  *                          is not allocated in the table.
  */
 static observed_data_t *lookup_observed_data(uint8_t channel) {
+
+    if (!observed_data) {
+        return NULL;
+    }
+
     for (size_t i = 0; i < num_observed_data; i++) {
         observed_data_t *data = &observed_data[i];
 
@@ -502,8 +507,16 @@ static mp_obj_t pb_module_ble_version(mp_obj_t self_in) {
 }
 static MP_DEFINE_CONST_FUN_OBJ_1(pb_module_ble_version_obj, pb_module_ble_version);
 
+mp_obj_t pb_module_ble_data_close(mp_obj_t self_in) {
+    observed_data = NULL;
+    num_observed_data = 0;
+    return mp_const_none;
+}
+MP_DEFINE_CONST_FUN_OBJ_1(pb_module_ble_data_close_obj, pb_module_ble_data_close);
+
 static const mp_rom_map_elem_t common_BLE_locals_dict_table[] = {
     { MP_ROM_QSTR(MP_QSTR_broadcast), MP_ROM_PTR(&pb_module_ble_broadcast_obj) },
+    { MP_ROM_QSTR(MP_QSTR___del__), MP_ROM_PTR(&pb_module_ble_data_close_obj) },
     { MP_ROM_QSTR(MP_QSTR_observe), MP_ROM_PTR(&pb_module_ble_observe_obj) },
     { MP_ROM_QSTR(MP_QSTR_signal_strength), MP_ROM_PTR(&pb_module_ble_signal_strength_obj) },
     { MP_ROM_QSTR(MP_QSTR_version), MP_ROM_PTR(&pb_module_ble_version_obj) },
@@ -544,8 +557,7 @@ mp_obj_t pb_type_BLE_new(mp_obj_t broadcast_channel_in, mp_obj_t observe_channel
         mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("Bluetooth not enabled"));
     }
     #endif // PBSYS_CONFIG_BLUETOOTH_TOGGLE
-
-    pb_obj_BLE_t *self = mp_obj_malloc_var(pb_obj_BLE_t, observed_data, observed_data_t, num_observe_channels, &pb_type_BLE);
+    pb_obj_BLE_t *self = mp_obj_malloc_var_with_finaliser(pb_obj_BLE_t, observed_data_t, num_observe_channels, &pb_type_BLE);
     self->broadcast_channel = broadcast_channel_in;
 
     for (mp_int_t i = 0; i < num_observe_channels; i++) {
@@ -577,10 +589,4 @@ mp_obj_t pb_type_BLE_new(mp_obj_t broadcast_channel_in, mp_obj_t observe_channel
     return MP_OBJ_FROM_PTR(self);
 }
 
-void pb_type_ble_start_cleanup(void) {
-
-    observed_data = NULL;
-    num_observed_data = 0;
-}
-
 #endif // PYBRICKS_PY_COMMON_BLE

pybricks/iodevices/pb_type_iodevices_lwp3device.c

@@ -396,12 +396,6 @@ static mp_obj_t pb_lwp3device_connect(mp_obj_t self_in, mp_obj_t name_in, mp_obj
     return pb_type_async_wait_or_await(&config, &lwp3device->iter, true);
 }
 
-void pb_type_lwp3device_start_cleanup(void) {
-
-    #if PYBRICKS_PY_IODEVICES
-    MP_STATE_PORT(notification_buffer) = NULL;
-    #endif
-}
 
 mp_obj_t pb_type_remote_button_pressed(void) {
     pb_lwp3device_t *remote = &pb_lwp3device_singleton;
@@ -530,10 +524,13 @@ MP_DEFINE_CONST_OBJ_TYPE(pb_type_pupdevices_Remote,
 
 #if PYBRICKS_PY_IODEVICES
 
+// pointer to dynamically allocated memory - needed for driver callback
+static uint8_t *notification_buffer;
+
 static pbio_pybricks_error_t handle_lwp3_generic_notification(const uint8_t *value, uint32_t size) {
     pb_lwp3device_t *self = &pb_lwp3device_singleton;
 
-    if (!MP_STATE_PORT(notification_buffer) || !self->noti_num) {
+    if (!notification_buffer || !self->noti_num) {
         // Allocated data not ready, but no error.
         return PBIO_PYBRICKS_ERROR_OK;
     }
@@ -543,7 +540,7 @@ static pbio_pybricks_error_t handle_lwp3_generic_notification(const uint8_t *val
         self->noti_idx_read = (self->noti_idx_read + 1) % self->noti_num;
     }
 
-    memcpy(&MP_STATE_PORT(notification_buffer)[self->noti_idx_write * LWP3_MAX_MESSAGE_SIZE], &value[0], (size < LWP3_MAX_MESSAGE_SIZE) ? size : LWP3_MAX_MESSAGE_SIZE);
+    memcpy(&notification_buffer[self->noti_idx_write * LWP3_MAX_MESSAGE_SIZE], &value[0], (size < LWP3_MAX_MESSAGE_SIZE) ? size : LWP3_MAX_MESSAGE_SIZE);
     self->noti_idx_write = (self->noti_idx_write + 1) % self->noti_num;
 
     // After writing it is full if the _next_ write will override the
@@ -553,6 +550,11 @@ static pbio_pybricks_error_t handle_lwp3_generic_notification(const uint8_t *val
     return PBIO_PYBRICKS_ERROR_OK;
 }
 
+typedef struct {
+    mp_obj_base_t base;
+    uint8_t notification_buffer[];
+} lwp3_device_obj_t;
+
 static mp_obj_t pb_type_iodevices_LWP3Device_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args) {
     PB_PARSE_ARGS_CLASS(n_args, n_kw, args,
         PB_ARG_REQUIRED(hub_kind),
@@ -561,7 +563,7 @@ static mp_obj_t pb_type_iodevices_LWP3Device_make_new(const mp_obj_type_t *type,
         PB_ARG_DEFAULT_FALSE(pair),
         PB_ARG_DEFAULT_INT(num_notifications, 8));
 
-    mp_obj_base_t *obj = mp_obj_malloc(mp_obj_base_t, type);
+
     pb_lwp3device_t *self = &pb_lwp3device_singleton;
 
     uint8_t hub_kind = pb_obj_get_positive_int(hub_kind_in);
@@ -570,11 +572,13 @@ static mp_obj_t pb_type_iodevices_LWP3Device_make_new(const mp_obj_type_t *type,
     self->noti_num = mp_obj_get_int(num_notifications_in);
     self->noti_num = self->noti_num > 0 ? self->noti_num : 1;
 
-    if (MP_STATE_PORT(notification_buffer)) {
+    if (notification_buffer) {
         mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("Can use only one LWP3Device"));
     }
 
-    MP_STATE_PORT(notification_buffer) = m_malloc0(LWP3_MAX_MESSAGE_SIZE * self->noti_num);
+    lwp3_device_obj_t *obj = mp_obj_malloc_var_with_finaliser(lwp3_device_obj_t, uint8_t, LWP3_MAX_MESSAGE_SIZE * self->noti_num, type);
+    notification_buffer = obj->notification_buffer;
+    memset(notification_buffer, 0, LWP3_MAX_MESSAGE_SIZE * self->noti_num);
     self->noti_idx_read = 0;
     self->noti_idx_write = 0;
     self->noti_data_full = false;
@@ -608,7 +612,7 @@ static mp_obj_t lwp3device_read(mp_obj_t self_in) {
 
     pb_lwp3device_assert_connected();
 
-    if (!self->noti_num || !MP_STATE_PORT(notification_buffer)) {
+    if (!self->noti_num || !notification_buffer) {
         pb_assert(PBIO_ERROR_FAILED);
     }
 
@@ -622,7 +626,7 @@ static mp_obj_t lwp3device_read(mp_obj_t self_in) {
     self->noti_idx_read = (self->noti_idx_read + 1) % self->noti_num;
 
     // First byte is the size.
-    uint8_t len = MP_STATE_PORT(notification_buffer)[index * LWP3_MAX_MESSAGE_SIZE];
+    uint8_t len = notification_buffer[index * LWP3_MAX_MESSAGE_SIZE];
     if (len < LWP3_HEADER_SIZE || len > LWP3_MAX_MESSAGE_SIZE) {
         // This is rare but it can happen sometimes. It is better to just
         // ignore it rather than raise and crash the user application.
@@ -632,11 +636,17 @@ static mp_obj_t lwp3device_read(mp_obj_t self_in) {
     // Allocation of the return object may drive the runloop and process
     // new incoming messages, so copy data atomically before that happens.
     uint8_t message[LWP3_MAX_MESSAGE_SIZE];
-    memcpy(message, &MP_STATE_PORT(notification_buffer)[index * LWP3_MAX_MESSAGE_SIZE], len);
+    memcpy(message, &notification_buffer[index * LWP3_MAX_MESSAGE_SIZE], len);
     return mp_obj_new_bytes(message, len);
 }
 static MP_DEFINE_CONST_FUN_OBJ_1(lwp3device_read_obj, lwp3device_read);
 
+mp_obj_t lwp3device_data_close(mp_obj_t self_in) {
+    notification_buffer = NULL;
+    return mp_const_none;
+}
+MP_DEFINE_CONST_FUN_OBJ_1(lwp3device_data_close_obj, lwp3device_data_close);
+
 static const mp_rom_map_elem_t pb_type_iodevices_LWP3Device_locals_dict_table[] = {
     { MP_ROM_QSTR(MP_QSTR_disconnect), MP_ROM_PTR(&pb_lwp3device_disconnect_obj) },
     { MP_ROM_QSTR(MP_QSTR_name), MP_ROM_PTR(&pb_lwp3device_name_obj) },

pybricks/pupdevices.h

@@ -25,7 +25,6 @@ extern const mp_obj_type_t pb_type_pupdevices_TiltSensor;
 extern const mp_obj_type_t pb_type_pupdevices_UltrasonicSensor;
 
 pb_type_device_obj_base_t *pupdevices_ColorDistanceSensor__get_device(mp_obj_t obj);
-void pb_type_lwp3device_start_cleanup(void);
 
 #endif // PYBRICKS_PY_PUPDEVICES
 

pybricks/pybricks.c

@@ -190,15 +190,3 @@ void pb_package_pybricks_init(bool import_all) {
     pb_module_tools_init();
 }
 #endif // PYBRICKS_OPT_COMPILER
-
-// REVISIT: move these to object finalizers if we enable finalizers in the GC
-void pb_package_pybricks_deinit(void) {
-
-    #if PYBRICKS_PY_COMMON_BLE
-    pb_type_ble_start_cleanup();
-    #endif
-
-    #if PYBRICKS_PY_PUPDEVICES_REMOTE
-    pb_type_lwp3device_start_cleanup();
-    #endif
-}