From acbbae904a45c154537cd3a8650f3f490006048d Mon Sep 17 00:00:00 2001 From: Laurens Valk Date: Mon, 22 Sep 2025 13:56:29 +0200 Subject: [PATCH 1/5] pbio/main: Wait for Bluetooth end at pbio. Other than safely freeing allocated memory, application code shouldn't have to worry about deinitializing the Bluetooth driver, so move it towards the pbio layer. Disconnecting from peripherals at the end is not unlike stopping motors etc. --- bricks/_common/micropython.c | 7 +- lib/pbio/drv/bluetooth/bluetooth.c | 7 +- lib/pbio/include/pbdrv/bluetooth.h | 12 ++++ lib/pbio/include/pbio/main.h | 3 +- lib/pbio/include/pbsys/main.h | 9 +++ lib/pbio/src/main.c | 69 +++++++++++++++---- lib/pbio/sys/light_matrix.c | 21 ++---- lib/pbio/sys/light_matrix.h | 4 +- lib/pbio/sys/main.c | 10 ++- pybricks/common/pb_type_ble.c | 18 ----- .../iodevices/pb_type_iodevices_lwp3device.c | 18 ----- pybricks/pybricks.c | 9 --- 12 files changed, 103 insertions(+), 84 deletions(-) diff --git a/bricks/_common/micropython.c b/bricks/_common/micropython.c index 5de3be0dc..eb8d2ceb5 100644 --- a/bricks/_common/micropython.c +++ b/bricks/_common/micropython.c @@ -314,7 +314,7 @@ static void run_user_program(void) { // The global scope is preserved to facilitate debugging, but we // stop active resources like motors and sounds. They are stopped // but not reset so the user can restart them in the REPL. - pbio_stop_all(false); + pbio_main_soft_stop(); // Enter REPL. run_repl(); @@ -420,9 +420,10 @@ void pbsys_main_run_program(pbsys_main_program_t *program) { run_user_program(); break; } +} + +void pbsys_main_run_program_cleanup(void) { - // De-init bluetooth resources (including flushing stdout) that may use - // memory allocated by MicroPython before we wipe it. pb_package_pybricks_deinit(); gc_sweep_all(); diff --git a/lib/pbio/drv/bluetooth/bluetooth.c b/lib/pbio/drv/bluetooth/bluetooth.c index 650ec7f75..cf96653be 100644 --- a/lib/pbio/drv/bluetooth/bluetooth.c +++ b/lib/pbio/drv/bluetooth/bluetooth.c @@ -296,7 +296,12 @@ pbio_error_t pbdrv_bluetooth_await_peripheral_command(pbio_os_state_t *state, vo // of interest and will be cancelled if the active function supports it. pbio_os_timer_set(&peri->watchdog, 10); - return peripheral_singleton.func ? peripheral_singleton.err : PBIO_SUCCESS; + return peri->func ? peri->err : PBIO_SUCCESS; +} + +void pbdrv_bluetooth_cancel_operation_request(void) { + // Only some peripheral actions support cancellation. + peripheral_singleton.cancel = true; } // diff --git a/lib/pbio/include/pbdrv/bluetooth.h b/lib/pbio/include/pbdrv/bluetooth.h index 329279dea..2ecb3fb79 100644 --- a/lib/pbio/include/pbdrv/bluetooth.h +++ b/lib/pbio/include/pbdrv/bluetooth.h @@ -421,6 +421,15 @@ pbio_error_t pbdrv_bluetooth_peripheral_write_characteristic(uint16_t handle, co */ pbio_error_t pbdrv_bluetooth_await_peripheral_command(pbio_os_state_t *state, void *context); +/** + * Requests active Bluetooth tasks to be cancelled. It is up to the task + * implementation to respect or ignore it. The task should still be awaited + * with ::pbdrv_bluetooth_await_advertise_or_scan_command or + * with ::pbdrv_bluetooth_await_peripheral_command. Cancelling just allows + * some commands to exit earlier. + */ +void pbdrv_bluetooth_cancel_operation_request(void); + // // Functions related to advertising and scanning. // @@ -565,6 +574,9 @@ static inline pbio_error_t pbdrv_bluetooth_await_peripheral_command(pbio_os_stat return PBIO_ERROR_NOT_SUPPORTED; } +static inline void pbdrv_bluetooth_cancel_operation_request(void) { +} + static inline pbio_error_t pbdrv_bluetooth_start_advertising(bool start) { return PBIO_ERROR_NOT_SUPPORTED; } diff --git a/lib/pbio/include/pbio/main.h b/lib/pbio/include/pbio/main.h index 7f067f4d5..c1e7509ac 100644 --- a/lib/pbio/include/pbio/main.h +++ b/lib/pbio/include/pbio/main.h @@ -10,6 +10,7 @@ void pbio_init(bool start_processes); void pbio_deinit(void); -void pbio_stop_all(bool reset); +void pbio_main_stop_application_resources(); +void pbio_main_soft_stop(void); #endif // _PBIO_MAIN_H_ diff --git a/lib/pbio/include/pbsys/main.h b/lib/pbio/include/pbsys/main.h index cc1687715..dc2b39574 100644 --- a/lib/pbio/include/pbsys/main.h +++ b/lib/pbio/include/pbsys/main.h @@ -110,6 +110,15 @@ pbio_error_t pbsys_main_program_validate(pbsys_main_program_t *program); */ void pbsys_main_run_program(pbsys_main_program_t *program); +/** + * Cleans up after running main application program, such as wiping application + * heap. + * + * This is separate from ::pbsys_main_run_program so that the system can + * safely close resources and unset callbacks before this cleanup runs. + */ +void pbsys_main_run_program_cleanup(void); + /** * Stops (cancels) the main application program. * diff --git a/lib/pbio/src/main.c b/lib/pbio/src/main.c index 25e9a2e33..ece5b2b18 100644 --- a/lib/pbio/src/main.c +++ b/lib/pbio/src/main.c @@ -8,9 +8,12 @@ #include +#include +#include #include #include +#include #include #include #include @@ -51,26 +54,62 @@ void pbio_deinit(void) { } /** - * Stops all user-level background processes. Called when the user application - * completes to get these modules back into their default state. Drivers and - * OS-level processes continue running. + * Stops resources like motors or sounds or peripheral procedures that take a + * long time. * - * @param [in] reset Whether to reset all user-level processes to a clean - * state (true), or whether to only stop active outputs - * like sound or motors (false). The latter is useful to - * preserve the state for debugging, without sound or - * movement getting in the way, or out of control. + * Useful to get the system in a safe state for the user without doing a full + * reset. Applications can all this to enter a user debug mode like the + * MicroPython REPL. */ -void pbio_stop_all(bool reset) { - #if PBIO_CONFIG_LIGHT - if (reset) { - pbio_light_animation_stop_all(); - } - #endif +void pbio_main_soft_stop(void) { - pbio_port_stop_user_actions(reset); + pbio_port_stop_user_actions(false); pbdrv_sound_stop(); + + pbdrv_bluetooth_cancel_operation_request(); +} + +static void wait_for_bluetooth(void) { + pbio_os_state_t unused; + while (pbdrv_bluetooth_await_advertise_or_scan_command(&unused, NULL) == PBIO_ERROR_AGAIN || + pbdrv_bluetooth_await_peripheral_command(&unused, NULL) == PBIO_ERROR_AGAIN) { + + // Run event loop until Bluetooth is idle. + pbio_os_run_processes_and_wait_for_event(); + } +} + +/** + * Stops all application-level background processes. Called when the user + * application completes to get these modules back into their default state. + * Drivers and OS-level processes continue running. + */ +void pbio_main_stop_application_resources() { + + pbio_main_soft_stop(); + + // Let ongoing task finish first. + wait_for_bluetooth(); + + // Stop broadcasting, observing and disconnect peripheral. + pbdrv_bluetooth_start_broadcasting(NULL, 0); + wait_for_bluetooth(); + + pbdrv_bluetooth_start_observing(NULL); + wait_for_bluetooth(); + + pbdrv_bluetooth_peripheral_disconnect(); + wait_for_bluetooth(); + + #if PBIO_CONFIG_LIGHT + pbio_light_animation_stop_all(); + #endif + + #if PBDRV_CONFIG_DISPLAY + pbio_image_fill(pbdrv_display_get_image(), 0); + pbdrv_display_update(); + #endif } /** @} */ diff --git a/lib/pbio/sys/light_matrix.c b/lib/pbio/sys/light_matrix.c index c125864c2..6c5e62139 100644 --- a/lib/pbio/sys/light_matrix.c +++ b/lib/pbio/sys/light_matrix.c @@ -153,11 +153,9 @@ static uint32_t pbsys_hub_light_matrix_user_program_animation_next(pbio_light_an } /** - * Updates light matrix behavior when program is started or stopped. - * - * @param start @c true for start or @c false for stop. + * Updates light matrix behavior when program is started. */ -void pbsys_hub_light_matrix_handle_user_program_start(bool start) { +void pbsys_hub_light_matrix_handle_user_program_start(void) { #if PBSYS_CONFIG_HUB_LIGHT_MATRIX_DISPLAY pbio_image_fill(pbdrv_display_get_image(), 0); @@ -165,16 +163,11 @@ void pbsys_hub_light_matrix_handle_user_program_start(bool start) { return; #endif - if (start) { - // The user animation updates only a subset of pixels to save time, - // so the rest must be cleared before it starts. - pbsys_hub_light_matrix_user_program_animation_clear(); - pbio_light_animation_init(&pbsys_hub_light_matrix->animation, pbsys_hub_light_matrix_user_program_animation_next); - pbio_light_animation_start(&pbsys_hub_light_matrix->animation); - } else { - // If the user program has ended, show stop sign and selected slot. - pbsys_hub_light_matrix_show_idle_ui(100); - } + // The user animation updates only a subset of pixels to save time, + // so the rest must be cleared before it starts. + pbsys_hub_light_matrix_user_program_animation_clear(); + pbio_light_animation_init(&pbsys_hub_light_matrix->animation, pbsys_hub_light_matrix_user_program_animation_next); + pbio_light_animation_start(&pbsys_hub_light_matrix->animation); } #endif // PBSYS_CONFIG_HUB_LIGHT_MATRIX diff --git a/lib/pbio/sys/light_matrix.h b/lib/pbio/sys/light_matrix.h index 41edd1548..679ae11d7 100644 --- a/lib/pbio/sys/light_matrix.h +++ b/lib/pbio/sys/light_matrix.h @@ -11,12 +11,12 @@ #if PBSYS_CONFIG_HUB_LIGHT_MATRIX void pbsys_hub_light_matrix_init(void); void pbsys_hub_light_matrix_deinit(void); -void pbsys_hub_light_matrix_handle_user_program_start(bool start); +void pbsys_hub_light_matrix_handle_user_program_start(void); void pbsys_hub_light_matrix_update_program_slot(void); #else #define pbsys_hub_light_matrix_init() #define pbsys_hub_light_matrix_deinit() -#define pbsys_hub_light_matrix_handle_user_program_start(start) +#define pbsys_hub_light_matrix_handle_user_program_start() #define pbsys_hub_light_matrix_update_program_slot() #endif diff --git a/lib/pbio/sys/main.c b/lib/pbio/sys/main.c index 1611be3f5..5a95a6f66 100644 --- a/lib/pbio/sys/main.c +++ b/lib/pbio/sys/main.c @@ -102,7 +102,7 @@ int main(int argc, char **argv) { pbsys_status_set_program_id(program.id); pbsys_status_set(PBIO_PYBRICKS_STATUS_USER_PROGRAM_RUNNING); pbsys_host_stdin_set_callback(pbsys_main_stdin_event); - pbsys_hub_light_matrix_handle_user_program_start(true); + pbsys_hub_light_matrix_handle_user_program_start(); #if PBSYS_CONFIG_STATUS_LIGHT #if PBSYS_CONFIG_STATUS_LIGHT_STATE_ANIMATIONS @@ -123,18 +123,22 @@ int main(int argc, char **argv) { // Run the main application. pbsys_main_run_program(&program); + // Stop motors, user animations, user bluetooth activity, etc. + pbio_main_stop_application_resources(); + // Get system back in idle state. pbsys_status_clear(PBIO_PYBRICKS_STATUS_USER_PROGRAM_RUNNING); pbsys_host_stdin_set_callback(NULL); pbsys_program_stop_set_buttons(PBIO_BUTTON_CENTER); - pbsys_hub_light_matrix_handle_user_program_start(false); - pbio_stop_all(true); program.start_request_type = PBSYS_MAIN_PROGRAM_START_REQUEST_TYPE_NONE; // Handle pending events triggered by the status change, such as // stopping status light animation. while (pbio_os_run_processes_once()) { } + + // Finalize application now that system resources are safely closed. + pbsys_main_run_program_cleanup(); } // Stop system processes and selected drivers in reverse order. This will diff --git a/pybricks/common/pb_type_ble.c b/pybricks/common/pb_type_ble.c index d1e6218d5..f824c967e 100644 --- a/pybricks/common/pb_type_ble.c +++ b/pybricks/common/pb_type_ble.c @@ -577,26 +577,8 @@ mp_obj_t pb_type_BLE_new(mp_obj_t broadcast_channel_in, mp_obj_t observe_channel return MP_OBJ_FROM_PTR(self); } -static void wait_for_bluetooth(void) { - pbio_os_state_t state; - while (pbdrv_bluetooth_await_advertise_or_scan_command(&state, NULL) == PBIO_ERROR_AGAIN) { - MICROPY_VM_HOOK_LOOP; - - // Stop waiting (and potentially blocking) in case of forced shutdown. - if (pbsys_status_test(PBIO_PYBRICKS_STATUS_SHUTDOWN_REQUEST)) { - break; - } - } -} - void pb_type_ble_start_cleanup(void) { - wait_for_bluetooth(); - pbdrv_bluetooth_start_broadcasting(NULL, 0); - wait_for_bluetooth(); - pbdrv_bluetooth_start_observing(NULL); - wait_for_bluetooth(); - observed_data = NULL; num_observed_data = 0; } diff --git a/pybricks/iodevices/pb_type_iodevices_lwp3device.c b/pybricks/iodevices/pb_type_iodevices_lwp3device.c index a58e1a469..128053048 100644 --- a/pybricks/iodevices/pb_type_iodevices_lwp3device.c +++ b/pybricks/iodevices/pb_type_iodevices_lwp3device.c @@ -396,29 +396,11 @@ 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); } - -static void wait_for_bluetooth(void) { - pbio_os_state_t state; - while (pbdrv_bluetooth_await_peripheral_command(&state, NULL) == PBIO_ERROR_AGAIN) { - MICROPY_VM_HOOK_LOOP; - // REVISIT: CANCEL SCAN - - // Stop waiting (and potentially blocking) in case of forced shutdown. - if (pbsys_status_test(PBIO_PYBRICKS_STATUS_SHUTDOWN_REQUEST)) { - break; - } - } -} - void pb_type_lwp3device_start_cleanup(void) { #if PYBRICKS_PY_IODEVICES MP_STATE_PORT(notification_buffer) = NULL; #endif - - wait_for_bluetooth(); - pbdrv_bluetooth_peripheral_disconnect(); - wait_for_bluetooth(); } mp_obj_t pb_type_remote_button_pressed(void) { diff --git a/pybricks/pybricks.c b/pybricks/pybricks.c index a91ab3bbe..c7d23ed9d 100644 --- a/pybricks/pybricks.c +++ b/pybricks/pybricks.c @@ -201,13 +201,4 @@ void pb_package_pybricks_deinit(void) { #if PYBRICKS_PY_PUPDEVICES_REMOTE pb_type_lwp3device_start_cleanup(); #endif - - while (!pbsys_host_tx_is_idle()) { - MICROPY_VM_HOOK_LOOP - - // Stop waiting (and potentially blocking) in case of forced shutdown. - if (pbsys_status_test(PBIO_PYBRICKS_STATUS_SHUTDOWN_REQUEST)) { - break; - } - } } From 5294ec82b1238967c21e75966dd89b74ca6ec575 Mon Sep 17 00:00:00 2001 From: Laurens Valk Date: Mon, 22 Sep 2025 14:48:25 +0200 Subject: [PATCH 2/5] pybricks.pupdevices.lwp3device: Use finalizers. Finish an old todo about using finalizers to clean up observe data and lwp3 notification buffer. --- bricks/_common/micropython.c | 3 -- bricks/virtualhub/mp_port.c | 2 - pybricks/common.h | 1 - pybricks/common/pb_type_ble.c | 22 +++++++---- .../iodevices/pb_type_iodevices_lwp3device.c | 38 ++++++++++++------- pybricks/pupdevices.h | 1 - pybricks/pybricks.c | 12 ------ 7 files changed, 38 insertions(+), 41 deletions(-) diff --git a/bricks/_common/micropython.c b/bricks/_common/micropython.c index eb8d2ceb5..6dab17042 100644 --- a/bricks/_common/micropython.c +++ b/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(); } diff --git a/bricks/virtualhub/mp_port.c b/bricks/virtualhub/mp_port.c index cecb484bf..3360ed0d4 100644 --- a/bricks/virtualhub/mp_port.c +++ b/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 diff --git a/pybricks/common.h b/pybricks/common.h index 59d7b177a..44c56ab48 100644 --- a/pybricks/common.h +++ b/pybricks/common.h @@ -26,7 +26,6 @@ #include 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); diff --git a/pybricks/common/pb_type_ble.c b/pybricks/common/pb_type_ble.c index f824c967e..77557f505 100644 --- a/pybricks/common/pb_type_ble.c +++ b/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 diff --git a/pybricks/iodevices/pb_type_iodevices_lwp3device.c b/pybricks/iodevices/pb_type_iodevices_lwp3device.c index 128053048..c2b95b22a 100644 --- a/pybricks/iodevices/pb_type_iodevices_lwp3device.c +++ b/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(¬ification_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, ¬ification_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) }, diff --git a/pybricks/pupdevices.h b/pybricks/pupdevices.h index 0f047d65d..df29f0840 100644 --- a/pybricks/pupdevices.h +++ b/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 diff --git a/pybricks/pybricks.c b/pybricks/pybricks.c index c7d23ed9d..6323fe188 100644 --- a/pybricks/pybricks.c +++ b/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 -} From 5f45ddaa676985aebc6025cc6397aa31777dbbc4 Mon Sep 17 00:00:00 2001 From: Laurens Valk Date: Mon, 22 Sep 2025 19:59:04 +0200 Subject: [PATCH 3/5] pybricks.common: Pass parent object to keypad and light. There may be more than one in general, so pass relevant reference. --- pybricks/common.h | 2 +- pybricks/common/pb_type_colorlight_external.c | 10 +++++----- pybricks/common/pb_type_keypad.c | 6 ++++-- pybricks/hubs/pb_type_cityhub.c | 2 +- pybricks/hubs/pb_type_essentialhub.c | 2 +- pybricks/hubs/pb_type_ev3brick.c | 4 ++-- pybricks/hubs/pb_type_movehub.c | 2 +- pybricks/hubs/pb_type_nxtbrick.c | 4 ++-- pybricks/hubs/pb_type_primehub.c | 4 ++-- pybricks/hubs/pb_type_technichub.c | 2 +- pybricks/hubs/pb_type_virtualhub.c | 2 +- pybricks/iodevices/pb_type_iodevices_lwp3device.c | 8 ++++---- pybricks/iodevices/pb_type_iodevices_xbox_controller.c | 4 ++-- pybricks/parameters/pb_type_button.c | 2 +- pybricks/parameters/pb_type_button.h | 4 ++-- .../pb_type_pupdevices_colordistancesensor.c | 9 +++++---- 16 files changed, 35 insertions(+), 32 deletions(-) diff --git a/pybricks/common.h b/pybricks/common.h index 44c56ab48..751c5e8b3 100644 --- a/pybricks/common.h +++ b/pybricks/common.h @@ -68,7 +68,7 @@ void pb_type_LightMatrix_display_char(pbio_light_matrix_t *light_matrix, mp_obj_ #if PYBRICKS_PY_COMMON_KEYPAD // pybricks._common.KeyPad() -mp_obj_t pb_type_Keypad_obj_new(pb_type_button_get_pressed_t get_pressed); +mp_obj_t pb_type_Keypad_obj_new(mp_obj_t parent_obj, pb_type_button_get_pressed_t get_pressed); #endif // pybricks._common.Battery() diff --git a/pybricks/common/pb_type_colorlight_external.c b/pybricks/common/pb_type_colorlight_external.c index 795a87af9..679104474 100644 --- a/pybricks/common/pb_type_colorlight_external.c +++ b/pybricks/common/pb_type_colorlight_external.c @@ -20,7 +20,7 @@ // pybricks._common.ColorLight class object typedef struct { mp_obj_base_t base; - void *context; + mp_obj_t parent_obj; pb_type_ColorLight_on_t on; } common_ColorLight_external_obj_t; @@ -32,14 +32,14 @@ static mp_obj_t common_ColorLight_external_on(size_t n_args, const mp_obj_t *pos common_ColorLight_external_obj_t, self, PB_ARG_REQUIRED(color)); - return self->on(self->context, pb_type_Color_get_hsv(color_in)); + return self->on(self->parent_obj, pb_type_Color_get_hsv(color_in)); } static MP_DEFINE_CONST_FUN_OBJ_KW(common_ColorLight_external_on_obj, 1, common_ColorLight_external_on); // pybricks._common.ColorLight.off static mp_obj_t common_ColorLight_external_off(mp_obj_t self_in) { common_ColorLight_external_obj_t *self = MP_OBJ_TO_PTR(self_in); - return self->on(self->context, &pb_Color_NONE_obj.hsv); + return self->on(self->parent_obj, &pb_Color_NONE_obj.hsv); } static MP_DEFINE_CONST_FUN_OBJ_1(common_ColorLight_external_off_obj, common_ColorLight_external_off); @@ -57,9 +57,9 @@ static MP_DEFINE_CONST_OBJ_TYPE(pb_type_ColorLight_external, locals_dict, &common_ColorLight_external_locals_dict); // pybricks._common.ColorLight.__init__ -mp_obj_t pb_type_ColorLight_external_obj_new(void *context, pb_type_ColorLight_on_t on) { +mp_obj_t pb_type_ColorLight_external_obj_new(mp_obj_t parent_obj, pb_type_ColorLight_on_t on) { common_ColorLight_external_obj_t *light = mp_obj_malloc(common_ColorLight_external_obj_t, &pb_type_ColorLight_external); - light->context = context; + light->parent_obj = parent_obj; light->on = on; return MP_OBJ_FROM_PTR(light); } diff --git a/pybricks/common/pb_type_keypad.c b/pybricks/common/pb_type_keypad.c index caf8f413a..80cda6025 100644 --- a/pybricks/common/pb_type_keypad.c +++ b/pybricks/common/pb_type_keypad.c @@ -17,13 +17,14 @@ // pybricks._common.Keypad class object typedef struct _common_Keypad_obj_t { mp_obj_base_t base; + mp_obj_t parent_obj; pb_type_button_get_pressed_t get_pressed; } common_Keypad_obj_t; // pybricks._common.Keypad.pressed static mp_obj_t common_Keypad_pressed(mp_obj_t self_in) { common_Keypad_obj_t *self = MP_OBJ_TO_PTR(self_in); - return self->get_pressed(); + return self->get_pressed(self->parent_obj); } MP_DEFINE_CONST_FUN_OBJ_1(common_Keypad_pressed_obj, common_Keypad_pressed); @@ -40,9 +41,10 @@ static MP_DEFINE_CONST_OBJ_TYPE(pb_type_Keypad, locals_dict, &common_Keypad_locals_dict); // pybricks._common.Keypad.__init__ -mp_obj_t pb_type_Keypad_obj_new(pb_type_button_get_pressed_t get_pressed) { +mp_obj_t pb_type_Keypad_obj_new(mp_obj_t parent_obj, pb_type_button_get_pressed_t get_pressed) { common_Keypad_obj_t *self = mp_obj_malloc(common_Keypad_obj_t, &pb_type_Keypad); self->get_pressed = get_pressed; + self->parent_obj = parent_obj; return MP_OBJ_FROM_PTR(self); } diff --git a/pybricks/hubs/pb_type_cityhub.c b/pybricks/hubs/pb_type_cityhub.c index 2d78d4c61..5d13fa435 100644 --- a/pybricks/hubs/pb_type_cityhub.c +++ b/pybricks/hubs/pb_type_cityhub.c @@ -37,7 +37,7 @@ static mp_obj_t hubs_CityHub_make_new(const mp_obj_type_t *type, size_t n_args, #if PYBRICKS_PY_COMMON_BLE self->ble = pb_type_BLE_new(broadcast_channel_in, observe_channels_in); #endif - self->button = pb_type_Keypad_obj_new(pb_type_button_pressed_hub_single_button); + self->button = pb_type_Keypad_obj_new(MP_OBJ_FROM_PTR(self), pb_type_button_pressed_hub_single_button); self->light = common_ColorLight_internal_obj_new(pbsys_status_light_main); self->system = MP_OBJ_FROM_PTR(&pb_type_System); return MP_OBJ_FROM_PTR(self); diff --git a/pybricks/hubs/pb_type_essentialhub.c b/pybricks/hubs/pb_type_essentialhub.c index 610307df5..809b8d0ab 100644 --- a/pybricks/hubs/pb_type_essentialhub.c +++ b/pybricks/hubs/pb_type_essentialhub.c @@ -52,7 +52,7 @@ static mp_obj_t hubs_EssentialHub_make_new(const mp_obj_type_t *type, size_t n_a #if PYBRICKS_PY_COMMON_BLE self->ble = pb_type_BLE_new(broadcast_channel_in, observe_channels_in); #endif - self->buttons = pb_type_Keypad_obj_new(pb_type_button_pressed_hub_single_button); + self->buttons = pb_type_Keypad_obj_new(MP_OBJ_FROM_PTR(self), pb_type_button_pressed_hub_single_button); self->charger = pb_type_Charger_obj_new(); self->imu = pb_type_IMU_obj_new(MP_OBJ_FROM_PTR(self), top_side_in, front_side_in); self->light = common_ColorLight_internal_obj_new(pbsys_status_light_main); diff --git a/pybricks/hubs/pb_type_ev3brick.c b/pybricks/hubs/pb_type_ev3brick.c index 63824ca20..bf2e44d96 100644 --- a/pybricks/hubs/pb_type_ev3brick.c +++ b/pybricks/hubs/pb_type_ev3brick.c @@ -24,7 +24,7 @@ typedef struct _hubs_EV3Brick_obj_t { mp_obj_t system; } hubs_EV3Brick_obj_t; -static mp_obj_t pb_type_ev3brick_button_pressed(void) { +static mp_obj_t pb_type_ev3brick_button_pressed(mp_obj_t parent_obj) { pbio_button_flags_t flags = pbdrv_button_get_pressed(); mp_obj_t pressed[5]; size_t num = 0; @@ -50,7 +50,7 @@ static mp_obj_t hubs_EV3Brick_make_new(const mp_obj_type_t *type, size_t n_args, hubs_EV3Brick_obj_t *self = mp_obj_malloc(hubs_EV3Brick_obj_t, type); self->battery = MP_OBJ_FROM_PTR(&pb_module_battery); - self->buttons = pb_type_Keypad_obj_new(pb_type_ev3brick_button_pressed); + self->buttons = pb_type_Keypad_obj_new(MP_OBJ_FROM_PTR(self), pb_type_ev3brick_button_pressed); self->light = common_ColorLight_internal_obj_new(pbsys_status_light_main); self->screen = pb_type_Image_display_obj_new(); self->speaker = mp_call_function_0(MP_OBJ_FROM_PTR(&pb_type_Speaker)); diff --git a/pybricks/hubs/pb_type_movehub.c b/pybricks/hubs/pb_type_movehub.c index 23e1ee975..568382023 100644 --- a/pybricks/hubs/pb_type_movehub.c +++ b/pybricks/hubs/pb_type_movehub.c @@ -329,7 +329,7 @@ static mp_obj_t hubs_MoveHub_make_new(const mp_obj_type_t *type, size_t n_args, #if PYBRICKS_PY_COMMON_BLE self->ble = pb_type_BLE_new(broadcast_channel_in, observe_channels_in); #endif - self->button = pb_type_Keypad_obj_new(pb_type_button_pressed_hub_single_button); + self->button = pb_type_Keypad_obj_new(MP_OBJ_FROM_PTR(self), pb_type_button_pressed_hub_single_button); self->imu = hubs_MoveHub_IMU_make_new(top_side_in, front_side_in); self->light = common_ColorLight_internal_obj_new(pbsys_status_light_main); self->system = MP_OBJ_FROM_PTR(&pb_type_System); diff --git a/pybricks/hubs/pb_type_nxtbrick.c b/pybricks/hubs/pb_type_nxtbrick.c index 91c4de163..df5bb5278 100644 --- a/pybricks/hubs/pb_type_nxtbrick.c +++ b/pybricks/hubs/pb_type_nxtbrick.c @@ -25,7 +25,7 @@ typedef struct _hubs_NXTBrick_obj_t { mp_obj_t system; } hubs_NXTBrick_obj_t; -static mp_obj_t pb_type_nxtbrick_button_pressed(void) { +static mp_obj_t pb_type_nxtbrick_button_pressed(mp_obj_t parent_obj) { pbio_button_flags_t flags = pbdrv_button_get_pressed(); mp_obj_t pressed[4]; size_t num = 0; @@ -47,7 +47,7 @@ static mp_obj_t pb_type_nxtbrick_button_pressed(void) { static mp_obj_t hubs_NXTBrick_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args) { hubs_NXTBrick_obj_t *self = mp_obj_malloc(hubs_NXTBrick_obj_t, type); self->battery = MP_OBJ_FROM_PTR(&pb_module_battery); - self->buttons = pb_type_Keypad_obj_new(pb_type_nxtbrick_button_pressed); + self->buttons = pb_type_Keypad_obj_new(MP_OBJ_FROM_PTR(self), pb_type_nxtbrick_button_pressed); self->speaker = mp_call_function_0(MP_OBJ_FROM_PTR(&pb_type_Speaker)); self->system = MP_OBJ_FROM_PTR(&pb_type_System); return MP_OBJ_FROM_PTR(self); diff --git a/pybricks/hubs/pb_type_primehub.c b/pybricks/hubs/pb_type_primehub.c index ef7aba703..7931a865b 100644 --- a/pybricks/hubs/pb_type_primehub.c +++ b/pybricks/hubs/pb_type_primehub.c @@ -41,7 +41,7 @@ typedef struct _hubs_PrimeHub_obj_t { mp_obj_t system; } hubs_PrimeHub_obj_t; -static mp_obj_t pb_type_primehub_button_pressed(void) { +static mp_obj_t pb_type_primehub_button_pressed(mp_obj_t parent_obj) { pbio_button_flags_t flags = pbdrv_button_get_pressed(); mp_obj_t pressed[4]; size_t num = 0; @@ -75,7 +75,7 @@ static mp_obj_t hubs_PrimeHub_make_new(const mp_obj_type_t *type, size_t n_args, #if PYBRICKS_PY_COMMON_BLE self->ble = pb_type_BLE_new(broadcast_channel_in, observe_channels_in); #endif - self->buttons = pb_type_Keypad_obj_new(pb_type_primehub_button_pressed); + self->buttons = pb_type_Keypad_obj_new(MP_OBJ_FROM_PTR(self), pb_type_primehub_button_pressed); self->charger = pb_type_Charger_obj_new(); self->display = pb_type_LightMatrix_obj_new(pbsys_hub_light_matrix); self->imu = pb_type_IMU_obj_new(MP_OBJ_FROM_PTR(self), top_side_in, front_side_in); diff --git a/pybricks/hubs/pb_type_technichub.c b/pybricks/hubs/pb_type_technichub.c index f5ed6f08d..ee852eefc 100644 --- a/pybricks/hubs/pb_type_technichub.c +++ b/pybricks/hubs/pb_type_technichub.c @@ -42,7 +42,7 @@ static mp_obj_t hubs_TechnicHub_make_new(const mp_obj_type_t *type, size_t n_arg #if PYBRICKS_PY_COMMON_BLE self->ble = pb_type_BLE_new(broadcast_channel_in, observe_channels_in); #endif - self->button = pb_type_Keypad_obj_new(pb_type_button_pressed_hub_single_button); + self->button = pb_type_Keypad_obj_new(MP_OBJ_FROM_PTR(self), pb_type_button_pressed_hub_single_button); self->imu = pb_type_IMU_obj_new(MP_OBJ_FROM_PTR(self), top_side_in, front_side_in); self->light = common_ColorLight_internal_obj_new(pbsys_status_light_main); self->system = MP_OBJ_FROM_PTR(&pb_type_System); diff --git a/pybricks/hubs/pb_type_virtualhub.c b/pybricks/hubs/pb_type_virtualhub.c index d4a490edc..edd311080 100644 --- a/pybricks/hubs/pb_type_virtualhub.c +++ b/pybricks/hubs/pb_type_virtualhub.c @@ -29,7 +29,7 @@ typedef struct _hubs_VirtualHub_obj_t { static mp_obj_t hubs_VirtualHub_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args) { hubs_VirtualHub_obj_t *self = mp_obj_malloc(hubs_VirtualHub_obj_t, type); self->battery = MP_OBJ_FROM_PTR(&pb_module_battery); - self->buttons = pb_type_Keypad_obj_new(pb_type_button_pressed_hub_single_button); + self->buttons = pb_type_Keypad_obj_new(MP_OBJ_FROM_PTR(self), pb_type_button_pressed_hub_single_button); // FIXME: Implement lights. // self->light = common_ColorLight_internal_obj_new(pbsys_status_light_main); self->system = MP_OBJ_FROM_PTR(&pb_type_System); diff --git a/pybricks/iodevices/pb_type_iodevices_lwp3device.c b/pybricks/iodevices/pb_type_iodevices_lwp3device.c index c2b95b22a..c0c3f71c9 100644 --- a/pybricks/iodevices/pb_type_iodevices_lwp3device.c +++ b/pybricks/iodevices/pb_type_iodevices_lwp3device.c @@ -234,7 +234,7 @@ static mp_obj_t wait_or_await_operation(void) { return pb_type_async_wait_or_await(&config, &lwp3device->iter, true); } -static mp_obj_t pb_type_pupdevices_Remote_light_on(void *context, const pbio_color_hsv_t *hsv) { +static mp_obj_t pb_type_pupdevices_Remote_light_on(mp_obj_t self_in, const pbio_color_hsv_t *hsv) { pb_assert(pb_type_pupdevices_Remote_write_light_msg(hsv)); return wait_or_await_operation(); } @@ -397,7 +397,7 @@ static mp_obj_t pb_lwp3device_connect(mp_obj_t self_in, mp_obj_t name_in, mp_obj } -mp_obj_t pb_type_remote_button_pressed(void) { +mp_obj_t pb_type_remote_button_pressed(mp_obj_t self_in) { pb_lwp3device_t *remote = &pb_lwp3device_singleton; pb_lwp3device_assert_connected(); @@ -448,8 +448,8 @@ static mp_obj_t pb_type_pupdevices_Remote_make_new(const mp_obj_type_t *type, si pb_module_tools_assert_blocking(); pb_type_pupdevices_Remote_obj_t *self = mp_obj_malloc(pb_type_pupdevices_Remote_obj_t, type); - self->buttons = pb_type_Keypad_obj_new(pb_type_remote_button_pressed); - self->light = pb_type_ColorLight_external_obj_new(NULL, pb_type_pupdevices_Remote_light_on); + self->buttons = pb_type_Keypad_obj_new(MP_OBJ_FROM_PTR(self), pb_type_remote_button_pressed); + self->light = pb_type_ColorLight_external_obj_new(MP_OBJ_FROM_PTR(self), pb_type_pupdevices_Remote_light_on); pb_lwp3device_connect(MP_OBJ_FROM_PTR(self), name_in, timeout_in, LWP3_HUB_KIND_HANDSET, handle_remote_notification, false); diff --git a/pybricks/iodevices/pb_type_iodevices_xbox_controller.c b/pybricks/iodevices/pb_type_iodevices_xbox_controller.c index 12513d9b1..5f737dcf0 100644 --- a/pybricks/iodevices/pb_type_iodevices_xbox_controller.c +++ b/pybricks/iodevices/pb_type_iodevices_xbox_controller.c @@ -179,7 +179,7 @@ static xbox_input_map_t *pb_xbox_get_buttons(void) { return buttons; } -static mp_obj_t pb_xbox_button_pressed(void) { +static mp_obj_t pb_xbox_button_pressed(mp_obj_t self_in) { xbox_input_map_t *buttons = pb_xbox_get_buttons(); // At most 16 simultaneous button presses, plus up to two dpad directions. @@ -385,7 +385,7 @@ static mp_obj_t pb_type_xbox_make_new(const mp_obj_type_t *type, size_t n_args, self->iter = NULL; pb_xbox_t *xbox = &pb_xbox_singleton; - self->buttons = pb_type_Keypad_obj_new(pb_xbox_button_pressed); + self->buttons = pb_type_Keypad_obj_new(MP_OBJ_FROM_PTR(self), pb_xbox_button_pressed); // needed to ensure that no buttons are "pressed" after reconnecting since // we are using static memory diff --git a/pybricks/parameters/pb_type_button.c b/pybricks/parameters/pb_type_button.c index 5aa31b0a3..a547ed18b 100644 --- a/pybricks/parameters/pb_type_button.c +++ b/pybricks/parameters/pb_type_button.c @@ -153,7 +153,7 @@ pbio_button_flags_t pb_type_button_get_button_flag(mp_obj_t obj) { /** * Common button pressed function for single button hubs. */ -mp_obj_t pb_type_button_pressed_hub_single_button(void) { +mp_obj_t pb_type_button_pressed_hub_single_button(mp_obj_t parent_obj) { pbio_button_flags_t flags = pbdrv_button_get_pressed(); mp_obj_t buttons[] = { pb_type_button_new(MP_QSTR_CENTER) }; diff --git a/pybricks/parameters/pb_type_button.h b/pybricks/parameters/pb_type_button.h index 8995523a2..a7e9eb5a0 100644 --- a/pybricks/parameters/pb_type_button.h +++ b/pybricks/parameters/pb_type_button.h @@ -22,8 +22,8 @@ extern const pb_obj_button_t pb_type_button; mp_obj_t pb_type_button_new(qstr name); pbio_button_flags_t pb_type_button_get_button_flag(mp_obj_t obj); -typedef mp_obj_t (*pb_type_button_get_pressed_t)(void); -mp_obj_t pb_type_button_pressed_hub_single_button(void); +typedef mp_obj_t (*pb_type_button_get_pressed_t)(mp_obj_t parent_obj); +mp_obj_t pb_type_button_pressed_hub_single_button(mp_obj_t parent_obj); #endif // PYBRICKS_PY_PARAMETERS_BUTTON diff --git a/pybricks/pupdevices/pb_type_pupdevices_colordistancesensor.c b/pybricks/pupdevices/pb_type_pupdevices_colordistancesensor.c index 2f7ddf85b..95b84ea2b 100644 --- a/pybricks/pupdevices/pb_type_pupdevices_colordistancesensor.c +++ b/pybricks/pupdevices/pb_type_pupdevices_colordistancesensor.c @@ -42,8 +42,9 @@ pb_type_device_obj_base_t *pupdevices_ColorDistanceSensor__get_device(mp_obj_t o * @param [in] context Sensor base object. * @param [in] hsv Requested color, will be rounded to nearest color. */ -static mp_obj_t pupdevices_ColorDistanceSensor_light_on(void *context, const pbio_color_hsv_t *hsv) { - pb_type_device_obj_base_t *sensor = context; +static mp_obj_t pupdevices_ColorDistanceSensor_light_on(mp_obj_t parent_obj, const pbio_color_hsv_t *hsv) { + + pupdevices_ColorDistanceSensor_obj_t *self = MP_OBJ_TO_PTR(parent_obj); // Even though the mode takes a 0-10 value for color, only red, green and blue // actually turn on the light. So we just pick the closest of these 3 to the @@ -59,7 +60,7 @@ static mp_obj_t pupdevices_ColorDistanceSensor_light_on(void *context, const pbi color = 9; // red } - return pb_type_device_set_data(sensor, LEGO_DEVICE_MODE_PUP_COLOR_DISTANCE_SENSOR__COL_O, &color, sizeof(color)); + return pb_type_device_set_data(&self->device_base, LEGO_DEVICE_MODE_PUP_COLOR_DISTANCE_SENSOR__COL_O, &color, sizeof(color)); } // pybricks.pupdevices.ColorDistanceSensor.__init__ @@ -71,7 +72,7 @@ static mp_obj_t pupdevices_ColorDistanceSensor_make_new(const mp_obj_type_t *typ pb_type_device_init_class(&self->device_base, port_in, LEGO_DEVICE_TYPE_ID_COLOR_DIST_SENSOR); // Create an instance of the Light class - self->light = pb_type_ColorLight_external_obj_new(&self->device_base, pupdevices_ColorDistanceSensor_light_on); + self->light = pb_type_ColorLight_external_obj_new(MP_OBJ_FROM_PTR(self), pupdevices_ColorDistanceSensor_light_on); // Save default color settings pb_color_map_save_default(&self->color_map); From 05aec1237e881254f024a954ed4ed2c83a69821b Mon Sep 17 00:00:00 2001 From: Laurens Valk Date: Tue, 23 Sep 2025 09:16:05 +0200 Subject: [PATCH 4/5] pybricks.pupdevices.Remote: Clean up self references. For technical and historical reasons, these classes were a bit of a mess, with various different singletons and object types that were almost but not quite working like MicroPython objects. On top of this, the object types were partially shared between the Remote and LWP3Device class. This refactors the module to use real MicroPython objects with the normal self_in convention wherever we can. We can make a reference to the global instance so that the driver callbacks know where to put the data, and use the finalizer to unset it for safety. Then we don't need separate logic for the LWP3Device notification buffer either, which is just allocated as part of the object. Now that each method has a self reference, we can relatively easily upgrade this code to use more than one peripheral in the future. --- .../iodevices/pb_type_iodevices_lwp3device.c | 222 ++++++++++-------- 1 file changed, 127 insertions(+), 95 deletions(-) diff --git a/pybricks/iodevices/pb_type_iodevices_lwp3device.c b/pybricks/iodevices/pb_type_iodevices_lwp3device.c index c0c3f71c9..fb9989661 100644 --- a/pybricks/iodevices/pb_type_iodevices_lwp3device.c +++ b/pybricks/iodevices/pb_type_iodevices_lwp3device.c @@ -85,9 +85,24 @@ static pbdrv_bluetooth_peripheral_char_t pb_lwp3device_char = { .request_notification = true, }; +// Needed for global notification callback. This is cleared when the finalizer +// runs. +static mp_obj_t self_obj; + typedef struct { + mp_obj_base_t base; pb_type_async_t *iter; pbio_os_state_t sub; + mp_obj_t buttons; + mp_obj_t light; + uint8_t left[3]; + uint8_t right[3]; + uint8_t center; + lwp3_hub_kind_t hub_kind; + // Null-terminated name used to filter advertisements and responses. + // Also used as the name of the device when setting the name, since this + // is not updated in the driver until the next time it connects. + char name[LWP3_MAX_HUB_PROPERTY_NAME_SIZE + 1]; #if PYBRICKS_PY_IODEVICES /** * Maximum number of stored notifications. @@ -105,32 +120,32 @@ typedef struct { * The buffer is full, next write will override oldest data. */ bool noti_data_full; + /** + * Variable length buffer holding multiple LWP3 notifications. + */ + uint8_t notification_buffer[]; #endif // PYBRICKS_PY_IODEVICES - uint8_t left[3]; - uint8_t right[3]; - uint8_t center; - lwp3_hub_kind_t hub_kind; - // Null-terminated name used to filter advertisements and responses. - // Also used as the name of the device when setting the name, since this - // is not updated in the driver until the next time it connects. - char name[LWP3_MAX_HUB_PROPERTY_NAME_SIZE + 1]; -} pb_lwp3device_t; - -static pb_lwp3device_t pb_lwp3device_singleton; +} pb_lwp3device_obj_t; // Handles LEGO Wireless protocol messages from the Powered Up Remote. static pbio_pybricks_error_t handle_remote_notification(const uint8_t *value, uint32_t size) { - pb_lwp3device_t *lwp3device = &pb_lwp3device_singleton; + + if (self_obj == MP_OBJ_NULL) { + // Silently ignore incoming notifications when we aren't expecting any. + return PBIO_PYBRICKS_ERROR_OK; + } + + pb_lwp3device_obj_t *self = MP_OBJ_TO_PTR(self_obj); if (value[0] == 5 && value[2] == LWP3_MSG_TYPE_HW_NET_CMDS && value[3] == LWP3_HW_NET_CMD_CONNECTION_REQ) { // This message is meant for something else, but contains the center button state - lwp3device->center = value[4]; + self->center = value[4]; } else if (value[0] == 7 && value[2] == LWP3_MSG_TYPE_PORT_VALUE) { // This assumes that the handset button ports have already been set to mode KEYSD if (value[3] == REMOTE_PORT_LEFT_BUTTONS) { - memcpy(lwp3device->left, &value[4], 3); + memcpy(self->left, &value[4], 3); } else if (value[3] == REMOTE_PORT_RIGHT_BUTTONS) { - memcpy(lwp3device->right, &value[4], 3); + memcpy(self->right, &value[4], 3); } } @@ -138,15 +153,22 @@ static pbio_pybricks_error_t handle_remote_notification(const uint8_t *value, ui } static pbdrv_bluetooth_ad_match_result_flags_t lwp3_advertisement_matches(uint8_t event_type, const uint8_t *data, const char *name, const uint8_t *addr, const uint8_t *match_addr) { + pbdrv_bluetooth_ad_match_result_flags_t flags = PBDRV_BLUETOOTH_AD_MATCH_NONE; + if (self_obj == MP_OBJ_NULL) { + return flags; + } + + pb_lwp3device_obj_t *self = MP_OBJ_TO_PTR(self_obj); + // Whether this looks like a LWP3 advertisement of the correct hub kind. if (event_type == PBDRV_BLUETOOTH_AD_TYPE_ADV_IND && data[3] == 17 /* length */ && (data[4] == PBDRV_BLUETOOTH_AD_DATA_TYPE_128_BIT_SERV_UUID_COMPLETE_LIST || data[4] == PBDRV_BLUETOOTH_AD_DATA_TYPE_128_BIT_SERV_UUID_INCOMPLETE_LIST) && pbio_uuid128_reverse_compare(&data[5], pbio_lwp3_hub_service_uuid) - && data[26] == pb_lwp3device_singleton.hub_kind) { + && data[26] == self->hub_kind) { flags |= PBDRV_BLUETOOTH_AD_MATCH_VALUE; } @@ -159,10 +181,14 @@ static pbdrv_bluetooth_ad_match_result_flags_t lwp3_advertisement_matches(uint8_ static pbdrv_bluetooth_ad_match_result_flags_t lwp3_advertisement_response_matches(uint8_t event_type, const uint8_t *data, const char *name, const uint8_t *addr, const uint8_t *match_addr) { - pb_lwp3device_t *lwp3device = &pb_lwp3device_singleton; - pbdrv_bluetooth_ad_match_result_flags_t flags = PBDRV_BLUETOOTH_AD_MATCH_NONE; + if (self_obj == MP_OBJ_NULL) { + return flags; + } + + pb_lwp3device_obj_t *self = MP_OBJ_TO_PTR(self_obj); + // This is the only value check we do on LWP3 response messages. if (event_type == PBDRV_BLUETOOTH_AD_TYPE_SCAN_RSP) { flags |= PBDRV_BLUETOOTH_AD_MATCH_VALUE; @@ -175,7 +201,7 @@ static pbdrv_bluetooth_ad_match_result_flags_t lwp3_advertisement_response_match // Compare name to user-provided name if given, checking only up to the // user provided name length. - if (lwp3device->name[0] != '\0' && strncmp(name, lwp3device->name, strlen(lwp3device->name)) != 0) { + if (self->name[0] != '\0' && strncmp(name, self->name, strlen(self->name)) != 0) { flags |= PBDRV_BLUETOOTH_AD_MATCH_NAME_FAILED; } @@ -194,7 +220,7 @@ static pbdrv_bluetooth_peripheral_connect_config_t scan_config = { // other options are variable. }; -static pbio_error_t pb_type_pupdevices_Remote_write_light_msg(const pbio_color_hsv_t *hsv) { +static pbio_error_t pb_type_pupdevices_Remote_write_light_msg(mp_obj_t self_in, const pbio_color_hsv_t *hsv) { struct { uint8_t length; @@ -225,18 +251,18 @@ static pbio_error_t pb_type_pupdevices_Remote_write_light_msg(const pbio_color_h return pbdrv_bluetooth_peripheral_write_characteristic(pb_lwp3device_char.handle, (const uint8_t *)&msg, sizeof(msg)); } -static mp_obj_t wait_or_await_operation(void) { - pb_lwp3device_t *lwp3device = &pb_lwp3device_singleton; +static mp_obj_t wait_or_await_operation(mp_obj_t self_in) { pb_type_async_t config = { .iter_once = pbdrv_bluetooth_await_peripheral_command, - .parent_obj = MP_OBJ_FROM_PTR(lwp3device), + .parent_obj = self_in, }; - return pb_type_async_wait_or_await(&config, &lwp3device->iter, true); + pb_lwp3device_obj_t *self = MP_OBJ_TO_PTR(self_in); + return pb_type_async_wait_or_await(&config, &self->iter, true); } static mp_obj_t pb_type_pupdevices_Remote_light_on(mp_obj_t self_in, const pbio_color_hsv_t *hsv) { - pb_assert(pb_type_pupdevices_Remote_write_light_msg(hsv)); - return wait_or_await_operation(); + pb_assert(pb_type_pupdevices_Remote_write_light_msg(self_in, hsv)); + return wait_or_await_operation(self_in); } static pbio_error_t pb_lwp3device_connect_thread(pbio_os_state_t *state, mp_obj_t parent_obj) { @@ -245,7 +271,7 @@ static pbio_error_t pb_lwp3device_connect_thread(pbio_os_state_t *state, mp_obj_ pbio_error_t err; - pb_lwp3device_t *lwp3device = &pb_lwp3device_singleton; + pb_lwp3device_obj_t *self = MP_OBJ_TO_PTR(parent_obj); PBIO_OS_ASYNC_BEGIN(state); @@ -260,7 +286,7 @@ static pbio_error_t pb_lwp3device_connect_thread(pbio_os_state_t *state, mp_obj_ } // Copy the name so we can read it back later, and override locally. - memcpy(lwp3device->name, pbdrv_bluetooth_peripheral_get_name(), sizeof(lwp3device->name)); + memcpy(self->name, pbdrv_bluetooth_peripheral_get_name(), sizeof(self->name)); // Discover common lwp3 characteristic. pb_assert(pbdrv_bluetooth_peripheral_discover_characteristic(&pb_lwp3device_char)); @@ -332,7 +358,7 @@ static pbio_error_t pb_lwp3device_connect_thread(pbio_os_state_t *state, mp_obj_ // set status light to blue. pbio_color_hsv_t hsv; pbio_color_to_hsv(PBIO_COLOR_BLUE, &hsv); - err = pb_type_pupdevices_Remote_write_light_msg(&hsv); + err = pb_type_pupdevices_Remote_write_light_msg(parent_obj, &hsv); if (err != PBIO_SUCCESS) { goto disconnect; } @@ -358,28 +384,28 @@ static mp_obj_t pb_lwp3device_connect(mp_obj_t self_in, mp_obj_t name_in, mp_obj } #endif // PBSYS_CONFIG_BLUETOOTH_TOGGLE - pb_lwp3device_t *lwp3device = &pb_lwp3device_singleton; + pb_lwp3device_obj_t *self = MP_OBJ_TO_PTR(self_in); - lwp3device->iter = NULL; + self->iter = NULL; // needed to ensure that no buttons are "pressed" after reconnecting since // we are using static memory - memset(&lwp3device->left, 0, sizeof(lwp3device->left)); - memset(&lwp3device->right, 0, sizeof(lwp3device->left)); - lwp3device->center = 0; + memset(&self->left, 0, sizeof(self->left)); + memset(&self->right, 0, sizeof(self->right)); + self->center = 0; // Hub kind and name are set to filter advertisements and responses. - lwp3device->hub_kind = hub_kind; + self->hub_kind = hub_kind; if (name_in == mp_const_none) { - lwp3device->name[0] = '\0'; + self->name[0] = '\0'; } else { // Guaranteed to be zero-terminated when using this getter. const char *name = mp_obj_str_get_str(name_in); size_t len = strlen(name); - if (len > sizeof(lwp3device->name) - 1) { + if (len > sizeof(self->name) - 1) { mp_raise_ValueError(MP_ERROR_TEXT("Name too long")); } - strncpy(lwp3device->name, name, sizeof(lwp3device->name)); + strncpy(self->name, name, sizeof(self->name)); } scan_config.notification_handler = notification_handler; @@ -393,37 +419,37 @@ static mp_obj_t pb_lwp3device_connect(mp_obj_t self_in, mp_obj_t name_in, mp_obj .iter_once = pb_lwp3device_connect_thread, .parent_obj = self_in, }; - return pb_type_async_wait_or_await(&config, &lwp3device->iter, true); + return pb_type_async_wait_or_await(&config, &self->iter, true); } mp_obj_t pb_type_remote_button_pressed(mp_obj_t self_in) { - pb_lwp3device_t *remote = &pb_lwp3device_singleton; + pb_lwp3device_obj_t *self = MP_OBJ_TO_PTR(self_in); pb_lwp3device_assert_connected(); mp_obj_t pressed[7]; size_t num = 0; - if (remote->left[0]) { + if (self->left[0]) { pressed[num++] = pb_type_button_new(MP_QSTR_LEFT_PLUS); } - if (remote->left[1]) { + if (self->left[1]) { pressed[num++] = pb_type_button_new(MP_QSTR_LEFT); } - if (remote->left[2]) { + if (self->left[2]) { pressed[num++] = pb_type_button_new(MP_QSTR_LEFT_MINUS); } - if (remote->right[0]) { + if (self->right[0]) { pressed[num++] = pb_type_button_new(MP_QSTR_RIGHT_PLUS); } - if (remote->right[1]) { + if (self->right[1]) { pressed[num++] = pb_type_button_new(MP_QSTR_RIGHT); } - if (remote->right[2]) { + if (self->right[2]) { pressed[num++] = pb_type_button_new(MP_QSTR_RIGHT_MINUS); } - if (remote->center) { + if (self->center) { pressed[num++] = pb_type_button_new(MP_QSTR_CENTER); } @@ -434,11 +460,11 @@ mp_obj_t pb_type_remote_button_pressed(mp_obj_t self_in) { #endif } -typedef struct _pb_type_pupdevices_Remote_obj_t { - mp_obj_base_t base; - mp_obj_t buttons; - mp_obj_t light; -} pb_type_pupdevices_Remote_obj_t; +mp_obj_t pb_lwp3device_close(mp_obj_t self_in) { + self_obj = MP_OBJ_NULL; + return mp_const_none; +} +MP_DEFINE_CONST_FUN_OBJ_1(pb_lwp3device_close_obj, pb_lwp3device_close); static mp_obj_t pb_type_pupdevices_Remote_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, @@ -447,9 +473,18 @@ static mp_obj_t pb_type_pupdevices_Remote_make_new(const mp_obj_type_t *type, si pb_module_tools_assert_blocking(); - pb_type_pupdevices_Remote_obj_t *self = mp_obj_malloc(pb_type_pupdevices_Remote_obj_t, type); + if (self_obj) { + mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("Can use only one Remote")); + } + + pb_lwp3device_obj_t *self = mp_obj_malloc_with_finaliser(pb_lwp3device_obj_t, type); + self_obj = MP_OBJ_FROM_PTR(self); + self->buttons = pb_type_Keypad_obj_new(MP_OBJ_FROM_PTR(self), pb_type_remote_button_pressed); self->light = pb_type_ColorLight_external_obj_new(MP_OBJ_FROM_PTR(self), pb_type_pupdevices_Remote_light_on); + #if PYBRICKS_PY_IODEVICES + self->noti_num = 0; + #endif pb_lwp3device_connect(MP_OBJ_FROM_PTR(self), name_in, timeout_in, LWP3_HUB_KIND_HANDSET, handle_remote_notification, false); @@ -457,7 +492,9 @@ static mp_obj_t pb_type_pupdevices_Remote_make_new(const mp_obj_type_t *type, si } static mp_obj_t pb_lwp3device_name(size_t n_args, const mp_obj_t *args) { - pb_lwp3device_t *lwp3device = &pb_lwp3device_singleton; + + mp_obj_t self_in = args[0]; + pb_lwp3device_obj_t *self = MP_OBJ_TO_PTR(self_in); if (n_args == 2) { size_t len; @@ -483,32 +520,32 @@ static mp_obj_t pb_lwp3device_name(size_t n_args, const mp_obj_t *args) { memcpy(msg.payload, name, len); // assuming write was successful instead of reading back from the handset - memcpy(lwp3device->name, name, len); - lwp3device->name[len] = 0; + memcpy(self->name, name, len); + self->name[len] = 0; pb_assert(pbdrv_bluetooth_peripheral_write_characteristic(pb_lwp3device_char.handle, (const uint8_t *)&msg, sizeof(msg) - sizeof(msg.payload) + len)); - return wait_or_await_operation(); + return wait_or_await_operation(self_in); } pb_lwp3device_assert_connected(); - return mp_obj_new_str(lwp3device->name, strlen(lwp3device->name)); + return mp_obj_new_str(self->name, strlen(self->name)); } static MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(pb_lwp3device_name_obj, 1, 2, pb_lwp3device_name); - static mp_obj_t pb_lwp3device_disconnect(mp_obj_t self_in) { pb_assert(pbdrv_bluetooth_peripheral_disconnect()); - return wait_or_await_operation(); + return wait_or_await_operation(self_in); } static MP_DEFINE_CONST_FUN_OBJ_1(pb_lwp3device_disconnect_obj, pb_lwp3device_disconnect); static const pb_attr_dict_entry_t pb_type_pupdevices_Remote_attr_dict[] = { - PB_DEFINE_CONST_ATTR_RO(MP_QSTR_buttons, pb_type_pupdevices_Remote_obj_t, buttons), - PB_DEFINE_CONST_ATTR_RO(MP_QSTR_light, pb_type_pupdevices_Remote_obj_t, light), + PB_DEFINE_CONST_ATTR_RO(MP_QSTR_buttons, pb_lwp3device_obj_t, buttons), + PB_DEFINE_CONST_ATTR_RO(MP_QSTR_light, pb_lwp3device_obj_t, light), PB_ATTR_DICT_SENTINEL }; static const mp_rom_map_elem_t pb_type_pupdevices_Remote_locals_dict_table[] = { + { MP_ROM_QSTR(MP_QSTR___del__), MP_ROM_PTR(&pb_lwp3device_close_obj) }, { 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) }, }; @@ -524,13 +561,16 @@ 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 (!notification_buffer || !self->noti_num) { + if (self_obj == MP_OBJ_NULL) { + // Silently ignore incoming notifications when we aren't expecting any. + return PBIO_PYBRICKS_ERROR_OK; + } + + pb_lwp3device_obj_t *self = MP_OBJ_TO_PTR(self_obj); + + if (!self->noti_num) { // Allocated data not ready, but no error. return PBIO_PYBRICKS_ERROR_OK; } @@ -540,7 +580,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(¬ification_buffer[self->noti_idx_write * LWP3_MAX_MESSAGE_SIZE], &value[0], (size < LWP3_MAX_MESSAGE_SIZE) ? size : LWP3_MAX_MESSAGE_SIZE); + memcpy(&self->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 @@ -550,10 +590,6 @@ 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, @@ -563,31 +599,32 @@ 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)); - - pb_lwp3device_t *self = &pb_lwp3device_singleton; + if (self_obj) { + mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("Can use only one LWP3Device")); + } uint8_t hub_kind = pb_obj_get_positive_int(hub_kind_in); bool pair = mp_obj_is_true(pair_in); - self->noti_num = mp_obj_get_int(num_notifications_in); - self->noti_num = self->noti_num > 0 ? self->noti_num : 1; - - if (notification_buffer) { - mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("Can use only one LWP3Device")); + size_t noti_num = mp_obj_get_int(num_notifications_in); + if (!noti_num) { + noti_num = 1; } - 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); + pb_lwp3device_obj_t *self = mp_obj_malloc_var_with_finaliser(pb_lwp3device_obj_t, uint8_t, LWP3_MAX_MESSAGE_SIZE * noti_num, type); + self_obj = MP_OBJ_FROM_PTR(self); + + memset(self->notification_buffer, 0, LWP3_MAX_MESSAGE_SIZE * noti_num); + self->noti_num = noti_num; self->noti_idx_read = 0; self->noti_idx_write = 0; self->noti_data_full = false; pb_module_tools_assert_blocking(); - pb_lwp3device_connect(MP_OBJ_FROM_PTR(obj), name_in, timeout_in, hub_kind, handle_lwp3_generic_notification, pair); + pb_lwp3device_connect(MP_OBJ_FROM_PTR(self), name_in, timeout_in, hub_kind, handle_lwp3_generic_notification, pair); - return MP_OBJ_FROM_PTR(obj); + return MP_OBJ_FROM_PTR(self); } static mp_obj_t lwp3device_write(mp_obj_t self_in, mp_obj_t buf_in) { @@ -603,16 +640,16 @@ static mp_obj_t lwp3device_write(mp_obj_t self_in, mp_obj_t buf_in) { } pb_assert(pbdrv_bluetooth_peripheral_write_characteristic(pb_lwp3device_char.handle, (const uint8_t *)bufinfo.buf, bufinfo.len)); - return wait_or_await_operation(); + return wait_or_await_operation(self_in); } static MP_DEFINE_CONST_FUN_OBJ_2(lwp3device_write_obj, lwp3device_write); static mp_obj_t lwp3device_read(mp_obj_t self_in) { - pb_lwp3device_t *self = &pb_lwp3device_singleton; + pb_lwp3device_obj_t *self = MP_OBJ_TO_PTR(self_in); pb_lwp3device_assert_connected(); - if (!self->noti_num || !notification_buffer) { + if (!self->noti_num) { pb_assert(PBIO_ERROR_FAILED); } @@ -626,7 +663,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 = notification_buffer[index * LWP3_MAX_MESSAGE_SIZE]; + uint8_t len = self->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. @@ -636,18 +673,13 @@ 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, ¬ification_buffer[index * LWP3_MAX_MESSAGE_SIZE], len); + memcpy(message, &self->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___del__), MP_ROM_PTR(&pb_lwp3device_close_obj) }, { 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) }, { MP_ROM_QSTR(MP_QSTR_write), MP_ROM_PTR(&lwp3device_write_obj) }, From 66ae6c41936c56478e8a0286012bb5cef1b8c9cd Mon Sep 17 00:00:00 2001 From: Laurens Valk Date: Thu, 25 Sep 2025 16:45:40 +0200 Subject: [PATCH 5/5] pbio/drv/bluetooth: Shut down gracefully. Before the Bluetooth overhaul, we were just powering down. In the recent commits, this was not yet implemented, so the hub would stay connected or keep advertising once turned off and charging. Poweroff is now back, but we also gracefully disconnect first to be nice to Pybricks Code. --- lib/pbio/drv/bluetooth/bluetooth.c | 37 ++++++++++++++++--- lib/pbio/drv/bluetooth/bluetooth_btstack.c | 6 ++- .../drv/bluetooth/bluetooth_stm32_bluenrg.c | 9 +++++ .../drv/bluetooth/bluetooth_stm32_cc2640.c | 7 ++++ lib/pbio/drv/core.c | 1 + lib/pbio/include/pbdrv/bluetooth.h | 8 ++++ 6 files changed, 61 insertions(+), 7 deletions(-) diff --git a/lib/pbio/drv/bluetooth/bluetooth.c b/lib/pbio/drv/bluetooth/bluetooth.c index cf96653be..639d0490d 100644 --- a/lib/pbio/drv/bluetooth/bluetooth.c +++ b/lib/pbio/drv/bluetooth/bluetooth.c @@ -11,6 +11,7 @@ #include +#include #include #include #include @@ -411,6 +412,8 @@ pbio_error_t pbdrv_bluetooth_await_advertise_or_scan_command(pbio_os_state_t *st return advertising_or_scan_func ? advertising_or_scan_err : PBIO_SUCCESS; } +static bool shutting_down; + /** * This is the main high level pbdrv/bluetooth thread. It is driven forward by * the platform-specific HCI process whenever there is new data to process or @@ -434,7 +437,9 @@ pbio_error_t pbdrv_bluetooth_process_thread(pbio_os_state_t *state, void *contex PBIO_OS_ASYNC_BEGIN(state); - for (;;) { + while (!shutting_down) { + + DEBUG_PRINT("Bluetooth disable requested.\n"); pbdrv_bluetooth_is_broadcasting = false; pbdrv_bluetooth_is_observing = false; @@ -447,7 +452,10 @@ pbio_error_t pbdrv_bluetooth_process_thread(pbio_os_state_t *state, void *contex pbio_os_request_poll(); // Bluetooth is now disabled. Await system processes to ask for enable. - PBIO_OS_AWAIT_UNTIL(state, power_on_requested); + PBIO_OS_AWAIT_UNTIL(state, power_on_requested || shutting_down); + if (shutting_down) { + break; + } DEBUG_PRINT("Bluetooth enable requested.\n"); PBIO_OS_AWAIT(state, &sub, err = pbdrv_bluetooth_controller_initialize(&sub, &timer)); @@ -463,9 +471,11 @@ pbio_error_t pbdrv_bluetooth_process_thread(pbio_os_state_t *state, void *contex pbio_os_timer_set(&status_timer, PBDRV_BLUETOOTH_STATUS_UPDATE_INTERVAL); // Service scheduled tasks as long as Bluetooth is enabled. - while (power_on_requested) { + while (power_on_requested && !shutting_down) { - // REVISIT: Only needed if there is nothing to do + // In principle, this wait is only needed if there is nothing to do. + // In practice, leaving it here helps rather than hurts since it + // allows short stdout messages to be queued rather than sent separately. PBIO_OS_AWAIT_MS(state, &timer, 1); // Handle pending status update, if any. @@ -534,11 +544,26 @@ pbio_error_t pbdrv_bluetooth_process_thread(pbio_os_state_t *state, void *contex observe_restart_requested = false; } } - - DEBUG_PRINT("Bluetooth disable requested.\n"); } + DEBUG_PRINT("Shutdown requested.\n"); + + // Power down the chip. This will disconnect from the host first. + // The peripheral has already been disconnected in the cleanup that runs after + // every program. If we change that behavior, we can do the disconnect here. + + PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_controller_reset(&sub, &timer)); + + pbio_busy_count_down(); + PBIO_OS_ASYNC_END(PBIO_SUCCESS); } +void pbdrv_bluetooth_deinit(void) { + pbio_busy_count_up(); + pbdrv_bluetooth_cancel_operation_request(); + shutting_down = true; + pbio_os_request_poll(); +} + #endif // PBDRV_CONFIG_BLUETOOTH_STM32_CC2640 diff --git a/lib/pbio/drv/bluetooth/bluetooth_btstack.c b/lib/pbio/drv/bluetooth/bluetooth_btstack.c index f78795d08..f35242c0a 100644 --- a/lib/pbio/drv/bluetooth/bluetooth_btstack.c +++ b/lib/pbio/drv/bluetooth/bluetooth_btstack.c @@ -858,7 +858,11 @@ pbio_error_t pbdrv_bluetooth_controller_reset(pbio_os_state_t *state, pbio_os_ti PBIO_OS_ASYNC_BEGIN(state); - // TODO: Reset state + // Disconnect gracefully if connected to host. + if (le_con_handle != HCI_CON_HANDLE_INVALID) { + gap_disconnect(le_con_handle); + PBIO_OS_AWAIT_UNTIL(state, le_con_handle == HCI_CON_HANDLE_INVALID); + } hci_power_control(HCI_POWER_OFF); PBIO_OS_AWAIT_UNTIL(state, hci_get_state() == HCI_STATE_OFF); diff --git a/lib/pbio/drv/bluetooth/bluetooth_stm32_bluenrg.c b/lib/pbio/drv/bluetooth/bluetooth_stm32_bluenrg.c index 38c3cc87e..dc2853cae 100644 --- a/lib/pbio/drv/bluetooth/bluetooth_stm32_bluenrg.c +++ b/lib/pbio/drv/bluetooth/bluetooth_stm32_bluenrg.c @@ -1240,6 +1240,15 @@ static pbio_error_t init_uart_service(pbio_os_state_t *state, void *context) { pbio_error_t pbdrv_bluetooth_controller_reset(pbio_os_state_t *state, pbio_os_timer_t *timer) { PBIO_OS_ASYNC_BEGIN(state); + // Disconnect gracefully if connected to host. + if (conn_handle) { + PBIO_OS_AWAIT_WHILE(state, write_xfer_size); + aci_gap_terminate_begin(conn_handle, HCI_OE_USER_ENDED_CONNECTION); + PBIO_OS_AWAIT_UNTIL(state, hci_command_status); + aci_gap_terminate_end(); + PBIO_OS_AWAIT_UNTIL(state, conn_handle == 0); + } + pybricks_notify_en = uart_tx_notify_en = false; conn_handle = peripheral_singleton.con_handle = 0; diff --git a/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c b/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c index e70968aeb..2159cf8f7 100644 --- a/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c +++ b/lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c @@ -1840,6 +1840,13 @@ static pbio_error_t init_uart_service(pbio_os_state_t *state, void *context) { pbio_error_t pbdrv_bluetooth_controller_reset(pbio_os_state_t *state, pbio_os_timer_t *timer) { PBIO_OS_ASYNC_BEGIN(state); + // Disconnect gracefully if connected to host. + if (conn_handle != NO_CONNECTION) { + PBIO_OS_AWAIT_WHILE(state, write_xfer_size); + GAP_TerminateLinkReq(conn_handle, 0x13); + PBIO_OS_AWAIT_UNTIL(state, conn_handle == NO_CONNECTION); + } + pybricks_notify_en = uart_tx_notify_en = false; conn_handle = peripheral_singleton.con_handle = NO_CONNECTION; diff --git a/lib/pbio/drv/core.c b/lib/pbio/drv/core.c index ca4cb4907..bd594d98b 100644 --- a/lib/pbio/drv/core.c +++ b/lib/pbio/drv/core.c @@ -101,6 +101,7 @@ void pbdrv_init(void) { void pbdrv_deinit(void) { pbdrv_imu_deinit(); + pbdrv_bluetooth_deinit(); while (pbio_busy_count_busy()) { pbio_os_run_processes_once(); diff --git a/lib/pbio/include/pbdrv/bluetooth.h b/lib/pbio/include/pbdrv/bluetooth.h index 2ecb3fb79..f55994c9d 100644 --- a/lib/pbio/include/pbdrv/bluetooth.h +++ b/lib/pbio/include/pbdrv/bluetooth.h @@ -232,6 +232,11 @@ typedef void (*pbdrv_bluetooth_start_observing_callback_t)(pbdrv_bluetooth_ad_ty */ void pbdrv_bluetooth_init(void); +/** + * Deinitializes the Bluetooth driver. + */ +void pbdrv_bluetooth_deinit(void); + /** * Turns the Bluetooth chip power on or off. Await the operation. * @@ -503,6 +508,9 @@ pbio_error_t pbdrv_bluetooth_await_advertise_or_scan_command(pbio_os_state_t *st static inline void pbdrv_bluetooth_init(void) { } +static inline void pbdrv_bluetooth_deinit(void) { +} + static inline pbio_error_t pbdrv_bluetooth_power_on(pbio_os_state_t *state, bool on) { return PBIO_ERROR_NOT_SUPPORTED; }