BLE observe fix

CHANGELOG.md

@@ -2,6 +2,13 @@
 
 # Changelog
 
+## [Unreleased]
+
+### Fixes
+- Fix observing stopping on City and Technic hubs after some time ([support#1096]).
+
+[support#1096]: https://github.com/pybricks/support/issues/1096
+
 ## [3.3.0] - 2023-11-24
 
 ### Changed

lib/pbio/drv/bluetooth/bluetooth_btstack.c

@@ -771,11 +771,22 @@ void pbdrv_bluetooth_start_broadcasting(pbio_task_t *task, pbdrv_bluetooth_value
     start_task(task, start_broadcasting_task, value);
 }
 
-void pbdrv_bluetooth_stop_broadcasting(void) {
+static PT_THREAD(stop_broadcasting_task(struct pt *pt, pbio_task_t *task)) {
+    PT_BEGIN(pt);
+
     if (is_broadcasting) {
         gap_advertisements_enable(false);
         is_broadcasting = false;
     }
+
+    // REVISIT: use callback to actually wait for stop?
+    task->status = PBIO_SUCCESS;
+
+    PT_END(pt);
+}
+
+void pbdrv_bluetooth_stop_broadcasting(pbio_task_t *task) {
+    start_task(task, stop_broadcasting_task, NULL);
 }
 
 static PT_THREAD(start_observing_task(struct pt *pt, pbio_task_t *task)) {
@@ -800,13 +811,23 @@ void pbdrv_bluetooth_start_observing(pbio_task_t *task, pbdrv_bluetooth_start_ob
     start_task(task, start_observing_task, callback);
 }
 
-void pbdrv_bluetooth_stop_observing(void) {
-    observe_callback = NULL;
+static PT_THREAD(stop_observing_task(struct pt *pt, pbio_task_t *task)) {
+    PT_BEGIN(pt);
 
     if (is_observing) {
         gap_stop_scan();
         is_observing = false;
     }
+
+    // REVISIT: use callback to actually wait for stop?
+    task->status = PBIO_SUCCESS;
+
+    PT_END(pt);
+}
+
+void pbdrv_bluetooth_stop_observing(pbio_task_t *task) {
+    observe_callback = NULL;
+    start_task(task, stop_observing_task, NULL);
 }
 
 #endif // PBDRV_CONFIG_BLUETOOTH_BTSTACK

lib/pbio/drv/bluetooth/bluetooth_stm32_bluenrg.c

@@ -789,9 +789,8 @@ static PT_THREAD(stop_broadcast_task(struct pt *pt, pbio_task_t *task)) {
     PT_END(pt);
 }
 
-void pbdrv_bluetooth_stop_broadcasting(void) {
-    static pbio_task_t task;
-    start_task(&task, stop_broadcast_task, NULL);
+void pbdrv_bluetooth_stop_broadcasting(pbio_task_t *task) {
+    start_task(task, stop_broadcast_task, NULL);
 }
 
 static PT_THREAD(observe_task(struct pt *pt, pbio_task_t *task)) {
@@ -850,11 +849,10 @@ static PT_THREAD(stop_observe_task(struct pt *pt, pbio_task_t *task)) {
     PT_END(pt);
 }
 
-void pbdrv_bluetooth_stop_observing(void) {
+void pbdrv_bluetooth_stop_observing(pbio_task_t *task) {
     observe_callback = NULL;
 
-    static pbio_task_t task;
-    start_task(&task, stop_observe_task, NULL);
+    start_task(task, stop_observe_task, NULL);
 }
 
 // overrides weak function in start_*.S

lib/pbio/drv/bluetooth/bluetooth_stm32_cc2640.c

@@ -162,6 +162,12 @@ static pbdrv_bluetooth_receive_handler_t receive_handler;
 static pbdrv_bluetooth_receive_handler_t notification_handler;
 static const pbdrv_bluetooth_stm32_cc2640_platform_data_t *pdata = &pbdrv_bluetooth_stm32_cc2640_platform_data;
 
+// HACK: restart observing every 10 seconds to work around a bug in the Bluetooth
+// chip firmware that causes it to stop receiving advertisements after a while.
+#define OBSERVE_RESTART_INTERVAL 10000
+static struct etimer observe_restart_timer;
+static bool observe_restart_enabled;
+
 /**
  * Converts a ble error code to the most appropriate pbio error code.
  * @param [in]  status      The ble error code.
@@ -476,6 +482,8 @@ static PT_THREAD(scan_and_connect_task(struct pt *pt, pbio_task_t *task)) {
 
     // temporarily stop observing so we can active scan
     if (is_observing) {
+        observe_restart_enabled = false;
+
         PT_WAIT_WHILE(pt, write_xfer_size);
         GAP_DeviceDiscoveryCancel();
         PT_WAIT_UNTIL(pt, hci_command_status);
@@ -716,6 +724,11 @@ static PT_THREAD(scan_and_connect_task(struct pt *pt, pbio_task_t *task)) {
         }
 
         device_discovery_done = false;
+
+        PROCESS_CONTEXT_BEGIN(&pbdrv_bluetooth_spi_process);
+        etimer_set(&observe_restart_timer, OBSERVE_RESTART_INTERVAL);
+        PROCESS_CONTEXT_END(&pbdrv_bluetooth_spi_process);
+        observe_restart_enabled = true;
     }
 
     PT_END(pt);
@@ -946,28 +959,13 @@ static PT_THREAD(stop_broadcast_task(struct pt *pt, pbio_task_t *task)) {
     PT_END(pt);
 }
 
-void pbdrv_bluetooth_stop_broadcasting(void) {
-    static pbio_task_t task;
-    start_task(&task, stop_broadcast_task, NULL);
+void pbdrv_bluetooth_stop_broadcasting(pbio_task_t *task) {
+    start_task(task, stop_broadcast_task, NULL);
 }
 
 static PT_THREAD(observe_task(struct pt *pt, pbio_task_t *task)) {
     PT_BEGIN(pt);
 
-    // HACK: Always stop observing first so that this task always does a clean
-    // restart of observing. This clears the size limited buffer of discovered
-    // devices. Should be replaced by a generalized scan process that only
-    // restarts scanning when needed. https://github.com/pybricks/support/issues/1096
-    if (is_observing) {
-        PT_WAIT_WHILE(pt, write_xfer_size);
-        GAP_DeviceDiscoveryCancel();
-        PT_WAIT_UNTIL(pt, hci_command_status);
-        if (read_buf[8] == bleSUCCESS) {
-            PT_WAIT_UNTIL(pt, device_discovery_done);
-        }
-        is_observing = false;
-    }
-
     if (!is_observing) {
         PT_WAIT_WHILE(pt, write_xfer_size);
         GAP_DeviceDiscoveryRequest(GAP_DEVICE_DISCOVERY_MODE_NONDISCOVERABLE, 0, GAP_FILTER_POLICY_SCAN_ANY_CONNECT_ANY);
@@ -980,6 +978,11 @@ static PT_THREAD(observe_task(struct pt *pt, pbio_task_t *task)) {
 
         device_discovery_done = false;
         is_observing = true;
+
+        PROCESS_CONTEXT_BEGIN(&pbdrv_bluetooth_spi_process);
+        etimer_set(&observe_restart_timer, OBSERVE_RESTART_INTERVAL);
+        PROCESS_CONTEXT_END(&pbdrv_bluetooth_spi_process);
+        observe_restart_enabled = true;
     }
 
     task->status = PBIO_SUCCESS;
@@ -1012,11 +1015,12 @@ static PT_THREAD(stop_observe_task(struct pt *pt, pbio_task_t *task)) {
     PT_END(pt);
 }
 
-void pbdrv_bluetooth_stop_observing(void) {
+void pbdrv_bluetooth_stop_observing(pbio_task_t *task) {
     observe_callback = NULL;
+    // avoid restarting observing even if this task get queued
+    observe_restart_enabled = false;
 
-    static pbio_task_t task;
-    start_task(&task, stop_observe_task, NULL);
+    start_task(task, stop_observe_task, NULL);
 }
 
 // Driver interrupt callbacks
@@ -1973,7 +1977,8 @@ PROCESS_THREAD(pbdrv_bluetooth_spi_process, ev, data) {
     PROCESS_EXITHANDLER({
         spi_set_mrdy(false);
         bluetooth_reset(RESET_STATE_OUT_LOW);
-        bluetooth_ready = pybricks_notify_en = uart_tx_notify_en = is_broadcasting = is_observing = false;
+        bluetooth_ready = pybricks_notify_en = uart_tx_notify_en =
+            is_broadcasting = is_observing = observe_restart_enabled = false;
         conn_handle = remote_handle = remote_lwp3_char_handle = NO_CONNECTION;
 
         pbio_task_t *task;
@@ -2032,6 +2037,15 @@ PROCESS_THREAD(pbdrv_bluetooth_spi_process, ev, data) {
     for (;;) {
         PROCESS_WAIT_UNTIL({
             for (;;) {
+                if (observe_restart_enabled && etimer_expired(&observe_restart_timer)) {
+                    static pbio_task_t observe_restart_stop_task;
+                    static pbio_task_t observe_restart_start_task;
+                    pbdrv_bluetooth_start_observing_callback_t callback = observe_callback;
+
+                    pbdrv_bluetooth_stop_observing(&observe_restart_stop_task);
+                    pbdrv_bluetooth_start_observing(&observe_restart_start_task, callback);
+                }
+
                 pbio_task_t *current_task = list_head(task_queue);
 
                 if (!current_task) {

lib/pbio/include/pbdrv/bluetooth.h

@@ -241,8 +241,10 @@ void pbdrv_bluetooth_start_broadcasting(pbio_task_t *task, pbdrv_bluetooth_value
  * Stops broadcasting that was started by pbdrv_bluetooth_start_broadcasting().
  *
  * It is safe to call this function even when broadcasting has not been started.
+ *
+ * @param [out] task    An uninitialized task to be filled in.
  */
-void pbdrv_bluetooth_stop_broadcasting(void);
+void pbdrv_bluetooth_stop_broadcasting(pbio_task_t *task);
 
 /**
  * Starts observing, non-connectable, non-scannable advertisements.
@@ -258,8 +260,10 @@ void pbdrv_bluetooth_start_observing(pbio_task_t *task, pbdrv_bluetooth_start_ob
  * Stops observing/scanning that was started by pbdrv_bluetooth_start_observing().
  *
  * It is safe to call this function even when observing has not been started.
+ *
+ * @param [out] task        An uninitialized task to be filled in.
  */
-void pbdrv_bluetooth_stop_observing(void);
+void pbdrv_bluetooth_stop_observing(pbio_task_t *task);
 
 #else // PBDRV_CONFIG_BLUETOOTH
 
@@ -312,14 +316,16 @@ static inline void pbdrv_bluetooth_start_broadcasting(pbio_task_t *task, pbdrv_b
     task->status = PBIO_ERROR_NOT_SUPPORTED;
 }
 
-static inline void pbdrv_bluetooth_stop_broadcasting(void) {
+static inline void pbdrv_bluetooth_stop_broadcasting(pbio_task_t *task) {
+    task->status = PBIO_ERROR_NOT_SUPPORTED;
 }
 
 static inline void pbdrv_bluetooth_start_observing(pbio_task_t *task, pbdrv_bluetooth_start_observing_callback_t callback) {
     task->status = PBIO_ERROR_NOT_SUPPORTED;
 }
 
-static inline void pbdrv_bluetooth_stop_observing(void) {
+static inline void pbdrv_bluetooth_stop_observing(pbio_task_t *task) {
+    task->status = PBIO_ERROR_NOT_SUPPORTED;
 }
 
 #endif // PBDRV_CONFIG_BLUETOOTH

pybricks/common/pb_type_ble.c

@@ -426,22 +426,6 @@ STATIC mp_obj_t pb_module_ble_observe(mp_obj_t self_in, mp_obj_t channel_in) {
 
     // Have not received data yet or timed out.
     if (ch_data.rssi == INT8_MIN) {
-
-        // HACK: Work around observing eventually stopping on the CC2640 due to
-        // full buffer of discovered devices. Needs to be fixed at the driver
-        // level with a scan process that restarts automatically.
-        // See https://github.com/pybricks/support/issues/1096
-        #if PBDRV_CONFIG_BLUETOOTH_STM32_CC2640
-        static uint32_t time_restart = 0;
-        uint32_t time_now = mp_hal_ticks_ms();
-        if (time_now - time_restart > OBSERVED_DATA_TIMEOUT_MS) {
-            pbio_task_t task;
-            pbdrv_bluetooth_start_observing(&task, handle_observe_event);
-            pb_module_tools_pbio_task_do_blocking(&task, -1);
-            time_restart = time_now;
-        }
-        #endif
-
         return mp_const_none;
     }
 
@@ -567,11 +551,16 @@ mp_obj_t pb_type_BLE_new(mp_obj_t broadcast_channel_in, mp_obj_t observe_channel
 }
 
 void pb_type_BLE_cleanup(void) {
-    pbdrv_bluetooth_stop_broadcasting();
-    pbdrv_bluetooth_stop_observing();
+    static pbio_task_t stop_broadcasting_task;
+    static pbio_task_t stop_observing_task;
+    pbdrv_bluetooth_stop_broadcasting(&stop_broadcasting_task);
+    pbdrv_bluetooth_stop_observing(&stop_observing_task);
     observed_data = NULL;
     num_observed_data = 0;
-    // TODO: wait for stop?
+
+    while (stop_broadcasting_task.status == PBIO_ERROR_AGAIN || stop_observing_task.status == PBIO_ERROR_AGAIN) {
+        MICROPY_VM_HOOK_LOOP
+    }
 }
 
 #endif // PYBRICKS_PY_COMMON_BLE