pbio/drv/bluetooth: Fix peripheral notifications not handled after reconnect.

The finalizer is usually called _after_ a new Remote object is made (when it is reassigned to the same variable), so we would unset the memory reference for storing notifications.

The Powered Up Remote or Xbox Controller would connect but notification would appear to never arrive, even though the message did arrive. They were just not processed.

Author: laurensvalk

CHANGELOG.md

@@ -30,6 +30,9 @@
 - Fixed Powered Up remote raising the wrong exception on timeout ([support#2521]).
 - Fixed Xbox Controller connection waiting indefinitely if it isn't in pairing mode ([support#2521]).
 - Fixed Xbox Controller attempting to pair with host ([support#2522]).
+- Fixed Powered Up remote and Xbox Controller working only after the first
+  connection. All of the above Remote and Xbox Controller were introduced in
+  the previous beta release, so did not affect any stable release ([support#2521]).
 
 [support#1962]: https://github.com/pybricks/support/issues/1962
 [support#2468]: https://github.com/pybricks/support/issues/2468

lib/pbio/drv/bluetooth/bluetooth.c

@@ -149,15 +149,34 @@ pbio_error_t pbdrv_bluetooth_send_event_notification(pbio_os_state_t *state, pbi
 
 pbdrv_bluetooth_peripheral_t peripheral_singleton;
 
-pbio_error_t pbdrv_bluetooth_peripheral_get_available(pbdrv_bluetooth_peripheral_t **peripheral) {
+pbio_error_t pbdrv_bluetooth_peripheral_get_available(pbdrv_bluetooth_peripheral_t **peripheral, void *user) {
     // Only a single peripheral instance supported for now.
-    if (peripheral_singleton.func || pbdrv_bluetooth_is_connected(PBDRV_BLUETOOTH_CONNECTION_PERIPHERAL)) {
+    *peripheral = &peripheral_singleton;
+
+    if (/* Already connected. */
+        pbdrv_bluetooth_is_connected(PBDRV_BLUETOOTH_CONNECTION_PERIPHERAL) ||
+        /* A different user is still claiming this resource. */
+        (*peripheral)->user ||
+        /* Busy. Could be connecting but not connected yet. */
+        (*peripheral)->func) {
         return PBIO_ERROR_BUSY;
     }
-    *peripheral = &peripheral_singleton;
+
+    // Claim this device for new user.
+    (*peripheral)->user = user;
+
     return PBIO_SUCCESS;
 }
 
+void pbdrv_bluetooth_peripheral_release(pbdrv_bluetooth_peripheral_t *peripheral, void *user) {
+    // Only release if the user matches. A new user may have already safely
+    // claimed it, and this call to release may come from an orphaned user.
+    if (peripheral->user != user) {
+        return;
+    }
+    peripheral->user = NULL;
+}
+
 const char *pbdrv_bluetooth_peripheral_get_name(void) {
     pbdrv_bluetooth_peripheral_t *peri = &peripheral_singleton;
     return peri->name;
@@ -176,8 +195,12 @@ pbio_error_t pbdrv_bluetooth_peripheral_scan_and_connect(pbdrv_bluetooth_periphe
         return PBIO_ERROR_BUSY;
     }
 
-    // Initialize operation for handling on the main thread.
+    // Reset peripheral instance but keep user reference.
+    void *user = peri->user;
     memset(peri, 0, sizeof(pbdrv_bluetooth_peripheral_t));
+    peri->user = user;
+
+    // Initialize operation for handling on the main thread.
     peri->config = config;
     peri->func = pbdrv_bluetooth_peripheral_scan_and_connect_func;
     peri->err = PBIO_ERROR_AGAIN;

lib/pbio/drv/bluetooth/bluetooth_btstack.c

@@ -361,7 +361,7 @@ static void packet_handler(uint8_t packet_type, uint16_t channel, uint8_t *packe
             if (peri->config->notification_handler) {
                 uint16_t length = gatt_event_notification_get_value_length(packet);
                 const uint8_t *value = gatt_event_notification_get_value(packet);
-                peri->config->notification_handler(peri, value, length);
+                peri->config->notification_handler(peri->user, value, length);
             }
             break;
         }

lib/pbio/drv/bluetooth/bluetooth_stm32_bluenrg.c

@@ -952,7 +952,7 @@ static void handle_event(hci_event_pckt *event) {
                     evt_gatt_attr_notification *subevt = (void *)evt->data;
                     pbdrv_bluetooth_peripheral_t *peri = &peripheral_singleton;
                     if (peri->config->notification_handler) {
-                        peri->config->notification_handler(peri, subevt->attr_value, subevt->event_data_length - 2);
+                        peri->config->notification_handler(peri->user, subevt->attr_value, subevt->event_data_length - 2);
                     }
                 }
                 break;

lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c

@@ -1310,7 +1310,7 @@ static void handle_event(uint8_t *packet) {
                     // uint8_t attr_handle = (data[7] << 8) | data[6];
                     pbdrv_bluetooth_peripheral_t *peri = &peripheral_singleton;
                     if (peri->config->notification_handler) {
-                        peri->config->notification_handler(peri, &data[8], pdu_len - 2);
+                        peri->config->notification_handler(peri->user, &data[8], pdu_len - 2);
                     }
                 }
                 break;

lib/pbio/include/pbdrv/bluetooth.h

@@ -135,13 +135,13 @@ typedef enum {
 typedef struct _pbdrv_bluetooth_peripheral_t pbdrv_bluetooth_peripheral_t;
 
 /**
- * Callback that is called when peripheral sends a notification.
+ * Callback that is called when a peripheral sends a notification.
  *
- * @param [in]  peripheral   The peripheral that sent the notification.
+ * @param [in]  user        The addressee, usually a high-level object.
  * @param [in]  data        The data that was received.
  * @param [in]  size        The size of @p data in bytes.
  */
-typedef void (*pbdrv_bluetooth_peripheral_notification_handler_t)(pbdrv_bluetooth_peripheral_t *peripheral, const uint8_t *data, uint32_t size);
+typedef void (*pbdrv_bluetooth_peripheral_notification_handler_t)(void *user, const uint8_t *data, uint32_t size);
 
 /** Peripheral scan and connection configuration */
 typedef struct {
@@ -161,6 +161,10 @@ typedef struct {
  * State of a peripheral that the hub may be connected to, such as a remote.
  */
 struct _pbdrv_bluetooth_peripheral_t {
+    /**
+     * Optional reference to higher-level user of this peripheral.
+     */
+    void *user;
     uint16_t con_handle;
     uint8_t status;
     uint8_t bdaddr_type;
@@ -356,11 +360,20 @@ pbio_error_t pbdrv_bluetooth_send_event_notification(pbio_os_state_t *state, pbi
  * Gets an available peripheral instance.
  *
  * @param [out] peripheral   Pointer to the peripheral instance if found.
+ * @param [in]  user         Optional user reference to associate with the peripheral.
  * @return                  ::PBIO_SUCCESS if a peripheral instance is available.
  *                          ::PBIO_ERROR_NO_DEV if no peripheral instance is available.
  *                          ::PBIO_ERROR_BUSY if all peripheral instances are in use.
  */
-pbio_error_t pbdrv_bluetooth_peripheral_get_available(pbdrv_bluetooth_peripheral_t **peripheral);
+pbio_error_t pbdrv_bluetooth_peripheral_get_available(pbdrv_bluetooth_peripheral_t **peripheral, void *user);
+
+/**
+ * Releases a peripheral instance for reuse by another user.
+ *
+ * @param [in]  peripheral   The peripheral instance to free.
+ * @param [in]  user         The user reference that was used to claim the peripheral.
+ */
+void pbdrv_bluetooth_peripheral_release(pbdrv_bluetooth_peripheral_t *peripheral, void *user);
 
 /**
  * Gets the name of the connected peripheral.

pybricks/iodevices/pb_type_iodevices_lwp3device.c

@@ -152,17 +152,10 @@ typedef struct {
 } pb_lwp3device_obj_t;
 
 // Handles LEGO Wireless protocol messages from the Powered Up Remote.
-static void handle_remote_notification(pbdrv_bluetooth_peripheral_t *peripheral, const uint8_t *value, uint32_t size) {
+static void handle_remote_notification(void *user, const uint8_t *value, uint32_t size) {
 
-    // No object registered for processing notifcations.
-    if (self_obj == MP_OBJ_NULL) {
-        return;
-    }
-
-    pb_lwp3device_obj_t *self = MP_OBJ_TO_PTR(self_obj);
-
-    // It's not for us.
-    if (self->peripheral != peripheral) {
+    pb_lwp3device_obj_t *self = user;
+    if (!self) {
         return;
     }
 
@@ -306,7 +299,7 @@ static pbio_error_t pb_lwp3device_connect_thread(pbio_os_state_t *state, mp_obj_
     PBIO_OS_ASYNC_BEGIN(state);
 
     // Get available peripheral instance.
-    pb_assert(pbdrv_bluetooth_peripheral_get_available(&self->peripheral));
+    pb_assert(pbdrv_bluetooth_peripheral_get_available(&self->peripheral, self));
 
     // Scan and connect with timeout.
     pb_assert(pbdrv_bluetooth_peripheral_scan_and_connect(&scan_config));
@@ -484,8 +477,10 @@ mp_obj_t pb_type_remote_button_pressed(mp_obj_t self_in) {
     #endif
 }
 
-mp_obj_t pb_lwp3device_close(mp_obj_t self_in) {
-    self_obj = MP_OBJ_NULL;
+static mp_obj_t pb_lwp3device_close(mp_obj_t self_in) {
+    pb_lwp3device_obj_t *self = MP_OBJ_TO_PTR(self_in);
+    // Disables notification handler from accessing allocated memory.
+    pbdrv_bluetooth_peripheral_release(self->peripheral, self);
     return mp_const_none;
 }
 MP_DEFINE_CONST_FUN_OBJ_1(pb_lwp3device_close_obj, pb_lwp3device_close);
@@ -497,10 +492,6 @@ static mp_obj_t pb_type_pupdevices_Remote_make_new(const mp_obj_type_t *type, si
 
     pb_module_tools_assert_blocking();
 
-    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);
 
@@ -557,6 +548,10 @@ static mp_obj_t pb_lwp3device_name(size_t n_args, const mp_obj_t *args) {
 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) {
+    // Needed to release claim on allocated data so we can make a new
+    // connection later.
+    pb_lwp3device_close(self_in);
+
     pb_assert(pbdrv_bluetooth_peripheral_disconnect());
     return wait_or_await_operation(self_in);
 }
@@ -588,21 +583,11 @@ MP_DEFINE_CONST_OBJ_TYPE(pb_type_pupdevices_Remote,
 /**
  * Handles LEGO Wireless protocol messages from generic LWP3 devices.
  */
-static void handle_lwp3_generic_notification(pbdrv_bluetooth_peripheral_t *peripheral, const uint8_t *value, uint32_t size) {
-
-    // No object registered for processing notifcations.
-    if (self_obj == MP_OBJ_NULL) {
-        return;
-    }
+static void handle_lwp3_generic_notification(void *user, const uint8_t *value, uint32_t size) {
 
-    pb_lwp3device_obj_t *self = MP_OBJ_TO_PTR(self_obj);
+    pb_lwp3device_obj_t *self = user;
 
-    // It's not for us.
-    if (self->peripheral != peripheral) {
-        return;
-    }
-
-    if (!self->noti_num) {
+    if (!self || !self->noti_num) {
         // Allocated data not ready.
         return;
     }
@@ -622,7 +607,6 @@ static void handle_lwp3_generic_notification(pbdrv_bluetooth_peripheral_t *perip
     return;
 }
 
-
 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),
@@ -631,10 +615,6 @@ 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));
 
-    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);
 

pybricks/iodevices/pb_type_iodevices_xbox_controller.c

@@ -82,7 +82,7 @@ typedef struct {
 static pb_xbox_t pb_xbox_singleton;
 
 // Handles LEGO Wireless protocol messages from the XBOX Device.
-static void handle_notification(pbdrv_bluetooth_peripheral_t *peripheral, const uint8_t *value, uint32_t size) {
+static void handle_notification(void *user, const uint8_t *value, uint32_t size) {
     pb_xbox_t *xbox = &pb_xbox_singleton;
     if (size <= sizeof(xbox_input_map_t)) {
         memcpy(&xbox->state, &value[0], size);