pbio/drv/bluetooth: Keep Bluetooth always on.

It turns out that we don't need to reset Bluetooth ever, so simplify this further by taking this out. The Bluetooth button will just toggle the advertising state.

Author: laurensvalk

lib/pbio/drv/bluetooth/bluetooth.c

@@ -27,38 +27,6 @@
 #define DEBUG_PRINT(...)
 #endif
 
-/**
- * Bluetooth power state. True means on (or on requested) False means off (or off requested).
- */
-static bool power_on_requested;
-
-/**
- * Bluetooth power enable busy state. True means that the operation is busy.
- *
- * Starts off in the reset state, so starts busy.
- */
-static bool power_change_busy = true;
-
-pbio_error_t pbdrv_bluetooth_power_on(pbio_os_state_t *state, bool on) {
-    PBIO_OS_ASYNC_BEGIN(state);
-
-    // Await ongoing state change if any.
-    PBIO_OS_AWAIT_WHILE(state, power_change_busy);
-
-    // Already in requested state.
-    if (power_on_requested == on) {
-        return PBIO_SUCCESS;
-    }
-
-    // Kick the Bluetooth process and await Bluetooth to become ready again.
-    power_change_busy = true;
-    power_on_requested = on;
-    pbio_os_request_poll();
-    PBIO_OS_AWAIT_WHILE(state, power_change_busy);
-
-    PBIO_OS_ASYNC_END(PBIO_SUCCESS);
-}
-
 pbdrv_bluetooth_receive_handler_t pbdrv_bluetooth_receive_handler;
 
 void pbdrv_bluetooth_set_receive_handler(pbdrv_bluetooth_receive_handler_t handler) {
@@ -98,6 +66,7 @@ void pbdrv_bluetooth_init(void) {
     // as soon as the previous one completes + 1 byte for ring buf pointer
     static uint8_t stdout_buf[PBDRV_BLUETOOTH_MAX_CHAR_SIZE * 2 + 1];
     lwrb_init(&stdout_ring_buf, stdout_buf, PBIO_ARRAY_SIZE(stdout_buf));
+    pbio_busy_count_up();
 
     pbdrv_bluetooth_init_hci();
 }
@@ -450,112 +419,98 @@ pbio_error_t pbdrv_bluetooth_process_thread(pbio_os_state_t *state, void *contex
 
     PBIO_OS_ASYNC_BEGIN(state);
 
-    while (!shutting_down) {
-
-        DEBUG_PRINT("Bluetooth disable requested.\n");
-
-        pbdrv_bluetooth_advertising_state = PBDRV_BLUETOOTH_ADVERTISING_STATE_NONE;
-        pbdrv_bluetooth_is_observing = false;
-        observe_restart_requested = false;
+init:
 
-        PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_controller_reset(&sub, &timer));
+    DEBUG_PRINT("Bluetooth reset.\n");
 
-        DEBUG_PRINT("Bluetooth is now off.\n");
-        power_change_busy = false;
-        pbio_os_request_poll();
+    PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_controller_reset(&sub, &timer));
 
-        // Bluetooth is now disabled. Await system processes to ask for enable.
-        PBIO_OS_AWAIT_UNTIL(state, power_on_requested || shutting_down);
-        if (shutting_down) {
-            break;
-        }
-        DEBUG_PRINT("Bluetooth enable requested.\n");
+    DEBUG_PRINT("Bluetooth enable.\n");
 
-        PBIO_OS_AWAIT(state, &sub, err = pbdrv_bluetooth_controller_initialize(&sub, &timer));
-        if (err != PBIO_SUCCESS) {
-            DEBUG_PRINT("Initialization failed. Reset and retry.\n");
-            continue;
-        }
+    PBIO_OS_AWAIT(state, &sub, err = pbdrv_bluetooth_controller_initialize(&sub, &timer));
+    if (err != PBIO_SUCCESS) {
+        DEBUG_PRINT("Initialization failed. Reset and retry.\n");
+        goto init;
+    }
 
-        DEBUG_PRINT("Bluetooth is now on and initialized.\n");
-        power_change_busy = false;
-        pbio_os_request_poll();
+    DEBUG_PRINT("Bluetooth is now on and initialized.\n");
 
-        pbio_os_timer_set(&status_timer, PBDRV_BLUETOOTH_STATUS_UPDATE_INTERVAL);
+    pbio_busy_count_down();
 
-        // Service scheduled tasks as long as Bluetooth is enabled.
-        while (power_on_requested && !shutting_down) {
+    pbio_os_timer_set(&status_timer, PBDRV_BLUETOOTH_STATUS_UPDATE_INTERVAL);
 
-            // 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);
+    // Service scheduled tasks as long as Bluetooth is enabled.
+    while (!shutting_down) {
 
-            // Handle pending status update, if any.
-            if (can_send && (status_data_pending || pbio_os_timer_is_expired(&status_timer))) {
+        // 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);
 
-                // When a status is pending, cache it here while we write it out,
-                // so a new status can be set in the mean time without losing it.
-                static uint8_t status_buf[PBIO_PYBRICKS_EVENT_STATUS_REPORT_SIZE];
-                memcpy(status_buf, status_data, PBIO_PYBRICKS_EVENT_STATUS_REPORT_SIZE);
-                status_data_pending = false;
+        // Handle pending status update, if any.
+        if (can_send && (status_data_pending || pbio_os_timer_is_expired(&status_timer))) {
 
-                PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_send_pybricks_value_notification(&sub, status_buf, sizeof(status_buf)));
+            // When a status is pending, cache it here while we write it out,
+            // so a new status can be set in the mean time without losing it.
+            static uint8_t status_buf[PBIO_PYBRICKS_EVENT_STATUS_REPORT_SIZE];
+            memcpy(status_buf, status_data, PBIO_PYBRICKS_EVENT_STATUS_REPORT_SIZE);
+            status_data_pending = false;
 
-                pbio_os_timer_set(&status_timer, status_timer.duration);
-            }
+            PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_send_pybricks_value_notification(&sub, status_buf, sizeof(status_buf)));
 
-            // Handle pending stdout, if any.
-            if (can_send && lwrb_get_full(&stdout_ring_buf) != 0) {
-                stdout_send_busy = true;
+            pbio_os_timer_set(&status_timer, status_timer.duration);
+        }
 
-                static uint8_t stdout_buf[PBDRV_BLUETOOTH_MAX_CHAR_SIZE] = { PBIO_PYBRICKS_EVENT_WRITE_STDOUT };
-                static uint16_t stdout_len;
+        // Handle pending stdout, if any.
+        if (can_send && lwrb_get_full(&stdout_ring_buf) != 0) {
+            stdout_send_busy = true;
 
-                stdout_len = lwrb_read(&stdout_ring_buf, &stdout_buf[1], PBDRV_BLUETOOTH_MAX_CHAR_SIZE - 1) + 1;
-                PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_send_pybricks_value_notification(&sub, stdout_buf, stdout_len));
+            static uint8_t stdout_buf[PBDRV_BLUETOOTH_MAX_CHAR_SIZE] = { PBIO_PYBRICKS_EVENT_WRITE_STDOUT };
+            static uint16_t stdout_len;
 
-                stdout_send_busy = false;
-            }
+            stdout_len = lwrb_read(&stdout_ring_buf, &stdout_buf[1], PBDRV_BLUETOOTH_MAX_CHAR_SIZE - 1) + 1;
+            PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_send_pybricks_value_notification(&sub, stdout_buf, stdout_len));
 
-            // Handle pending user value notification, if any.
-            if (can_send && user_notification_size) {
+            stdout_send_busy = false;
+        }
 
-                PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_send_pybricks_value_notification(&sub,
-                    user_notification_send_buf,
-                    user_notification_size));
+        // Handle pending user value notification, if any.
+        if (can_send && user_notification_size) {
 
-                user_notification_size = 0;
-            }
+            PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_send_pybricks_value_notification(&sub,
+                user_notification_send_buf,
+                user_notification_size));
 
-            // Handle pending advertising/scan enable/disable task, if any.
-            if (advertising_or_scan_func) {
-                PBIO_OS_AWAIT(state, &sub, advertising_or_scan_err = advertising_or_scan_func(&sub, NULL));
-                advertising_or_scan_func = NULL;
-            }
-
-            // Handle pending peripheral task, if any.
-            if (peri->func) {
+            user_notification_size = 0;
+        }
 
-                // If currently observing, stop if we need to scan for a peripheral.
-                if (pbdrv_bluetooth_is_observing && peri->func == pbdrv_bluetooth_peripheral_scan_and_connect_func) {
-                    PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_stop_observing_func(&sub, NULL));
-                    observe_restart_requested = true;
-                }
+        // Handle pending advertising/scan enable/disable task, if any.
+        if (advertising_or_scan_func) {
+            PBIO_OS_AWAIT(state, &sub, advertising_or_scan_err = advertising_or_scan_func(&sub, NULL));
+            advertising_or_scan_func = NULL;
+        }
 
-                PBIO_OS_AWAIT(state, &sub, peri->err = peri->func(&sub, peri));
-                peri->func = NULL;
-                peri->cancel = false;
-            }
+        // Handle pending peripheral task, if any.
+        if (peri->func) {
 
-            // Restart if we stopped it temporarily to scan for a peripheral or
-            // when externaly requested.
-            if (observe_restart_requested) {
-                DEBUG_PRINT("Restart observe.\n");
+            // If currently observing, stop if we need to scan for a peripheral.
+            if (pbdrv_bluetooth_is_observing && peri->func == pbdrv_bluetooth_peripheral_scan_and_connect_func) {
                 PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_stop_observing_func(&sub, NULL));
-                PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_start_observing_func(&sub, NULL));
-                observe_restart_requested = false;
+                observe_restart_requested = true;
             }
+
+            PBIO_OS_AWAIT(state, &sub, peri->err = peri->func(&sub, peri));
+            peri->func = NULL;
+            peri->cancel = false;
+        }
+
+        // Restart if we stopped it temporarily to scan for a peripheral or
+        // when externaly requested.
+        if (observe_restart_requested) {
+            DEBUG_PRINT("Restart observe.\n");
+            PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_stop_observing_func(&sub, NULL));
+            PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_start_observing_func(&sub, NULL));
+            observe_restart_requested = false;
         }
     }
 

lib/pbio/include/pbdrv/bluetooth.h

@@ -237,19 +237,6 @@ void pbdrv_bluetooth_init(void);
  */
 void pbdrv_bluetooth_deinit(void);
 
-/**
- * Turns the Bluetooth chip power on or off. Await the operation.
- *
- * If it is already in the requested power state, this completes successfully
- * right away.
- *
- * @param [in]  state   The protothread state.
- * @param [in]  on      If true, turns power on, otherwise turns power off.
- * @return              ::PBIO_SUCCESS on completion,
- *                      ::PBIO_ERROR_AGAIN while awaiting.
- */
-pbio_error_t pbdrv_bluetooth_power_on(pbio_os_state_t *state, bool on);
-
 /**
  * Gets the bluetooth hub name.
  */
@@ -524,10 +511,6 @@ 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;
-}
-
 static inline const char *pbdrv_bluetooth_get_hub_name(void) {
     return "Pybricks Hub";
 }

lib/pbio/sys/hmi_pup.c

@@ -157,15 +157,6 @@ static pbio_error_t run_ui(pbio_os_state_t *state, pbio_os_timer_t *timer) {
 
     static pbio_os_state_t sub;
 
-    /**
-     * Persistent state indicating whether we were connected the last time the
-     * HMI ran. If it was connected before but not now, we know we became
-     * disconnected. This is when we restart Bluetooth to get a new address and
-     * avoid reconnection issues. We also want to do that on boot, so we start
-     * this in true.
-     */
-    static bool previously_connected = true;
-
     PBIO_OS_ASYNC_BEGIN(state);
 
     for (;;) {
@@ -182,28 +173,15 @@ static pbio_error_t run_ui(pbio_os_state_t *state, pbio_os_timer_t *timer) {
             DEBUG_PRINT("Connected: yes\n");
             pbsys_status_set(PBIO_PYBRICKS_STATUS_BLE_HOST_CONNECTED);
             pbsys_status_clear(PBIO_PYBRICKS_STATUS_BLE_ADVERTISING);
-            previously_connected = true;
             // No need to stop advertising since this is automatic.
         } else {
             // Not connected right now.
             DEBUG_PRINT("Connected: No\n");
-            if (previously_connected) {
-                // Became disconnected just now or some time throughout the
-                // last user program run. Reset Bluetooth to get a new address.
-                // Also used the very first time we power on.
-                DEBUG_PRINT("Reset Bluetooth.\n");
-                PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_power_on(&sub, false));
-            }
             pbsys_status_clear(PBIO_PYBRICKS_STATUS_BLE_HOST_CONNECTED);
-            previously_connected = false;
 
-            // Enable or disable Bluetooth depending on user setting. This is
-            // a safe no-op if this was already set.
-            DEBUG_PRINT("Bluetooth is configured to be: %s. \n", pbsys_storage_settings_bluetooth_enabled_get() ? "on" : "off");
-            PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_power_on(&sub, pbsys_storage_settings_bluetooth_enabled_get()));
-
-            // Update advertising state.
+            // Enable or disable advertising depending on user setting.
             bool do_advertise = pbsys_storage_settings_bluetooth_enabled_get();
+            DEBUG_PRINT("Advertising is configured to be: %s. \n", do_advertise ? "on" : "off");
             if (do_advertise) {
                 pbsys_status_set(PBIO_PYBRICKS_STATUS_BLE_ADVERTISING);
             } else {
@@ -243,11 +221,11 @@ static pbio_error_t run_ui(pbio_os_state_t *state, pbio_os_timer_t *timer) {
             // Wait for button press, external program start, or connection change.
             pbdrv_button_get_pressed() ||
             pbsys_main_program_start_is_requested() ||
-            pbdrv_bluetooth_is_connected(PBDRV_BLUETOOTH_CONNECTION_LE) != previously_connected;
+            pbdrv_bluetooth_is_connected(PBDRV_BLUETOOTH_CONNECTION_LE) != pbsys_status_test(PBIO_PYBRICKS_STATUS_BLE_HOST_CONNECTED);
         }));
 
         // Became connected or disconnected, so go back to handle it.
-        if (pbdrv_bluetooth_is_connected(PBDRV_BLUETOOTH_CONNECTION_LE) != previously_connected) {
+        if (pbdrv_bluetooth_is_connected(PBDRV_BLUETOOTH_CONNECTION_LE) != pbsys_status_test(PBIO_PYBRICKS_STATUS_BLE_HOST_CONNECTED)) {
             DEBUG_PRINT("Connection changed.\n");
             continue;
         }
@@ -296,11 +274,9 @@ static pbio_error_t run_ui(pbio_os_state_t *state, pbio_os_timer_t *timer) {
     }
 
     // Stop advertising if we are still doing so.
-    if (pbsys_status_test(PBIO_PYBRICKS_STATUS_BLE_ADVERTISING)) {
-        DEBUG_PRINT("Stop advertising on HMI exit.\n");
-        pbdrv_bluetooth_start_advertising(false);
-        PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_await_advertise_or_scan_command(&sub, NULL));
-    }
+    DEBUG_PRINT("Stop advertising on HMI exit.\n");
+    pbdrv_bluetooth_start_advertising(false);
+    PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_await_advertise_or_scan_command(&sub, NULL));
 
     // Wait for all buttons to be released so the user doesn't accidentally
     // push their robot off course.

lib/pbio/test/sys/test_bluetooth.c

@@ -26,12 +26,7 @@ static pbio_error_t test_bluetooth(pbio_os_state_t *state, void *context) {
 
     pbsys_host_init();
 
-    // power should be initialized to off
-    tt_want_uint_op(pbio_test_bluetooth_get_control_state(), ==, PBIO_TEST_BLUETOOTH_STATE_OFF);
-
-    PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_power_on(&sub, false));
-    PBIO_OS_AWAIT(state, &sub, pbdrv_bluetooth_power_on(&sub, true));
-
+    // power should be initialized by the time pbdrv completes.
     tt_want_uint_op(pbio_test_bluetooth_get_control_state(), ==, PBIO_TEST_BLUETOOTH_STATE_ON);
 
     pbdrv_bluetooth_start_advertising(true);

pybricks/common/pb_type_ble.c

@@ -551,12 +551,6 @@ mp_obj_t pb_type_BLE_new(mp_obj_t broadcast_channel_in, mp_obj_t observe_channel
         mp_raise_ValueError(MP_ERROR_TEXT("Too many observe channels"));
     }
 
-    // Raise if Bluetooth is attempted to be used while not enabled.
-    #if PBSYS_CONFIG_HMI_PUP_BLUETOOTH_BUTTON
-    if (!pbsys_storage_settings_bluetooth_enabled_get() && (num_observe_channels > 0 || broadcast_channel_in != mp_const_none)) {
-        mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("Bluetooth not enabled"));
-    }
-    #endif // PBSYS_CONFIG_HMI_PUP_BLUETOOTH_BUTTON
     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;
 

pybricks/iodevices/pb_type_iodevices_lwp3device.c

@@ -378,12 +378,6 @@ static pbio_error_t pb_lwp3device_connect_thread(pbio_os_state_t *state, mp_obj_
 
 static mp_obj_t pb_lwp3device_connect(mp_obj_t self_in, mp_obj_t name_in, mp_obj_t timeout_in, lwp3_hub_kind_t hub_kind, pbdrv_bluetooth_receive_handler_t notification_handler, bool pair) {
 
-    #if PBSYS_CONFIG_HMI_PUP_BLUETOOTH_BUTTON
-    if (!pbsys_storage_settings_bluetooth_enabled_get()) {
-        mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("Bluetooth not enabled"));
-    }
-    #endif // PBSYS_CONFIG_HMI_PUP_BLUETOOTH_BUTTON
-
     pb_lwp3device_obj_t *self = MP_OBJ_TO_PTR(self_in);
 
     self->iter = NULL;

pybricks/iodevices/pb_type_iodevices_xbox_controller.c

@@ -372,12 +372,6 @@ static mp_obj_t pb_type_xbox_make_new(const mp_obj_type_t *type, size_t n_args,
         #endif // PYBRICKS_HUB_TECHNICHUB
         );
 
-    #if PBSYS_CONFIG_HMI_PUP_BLUETOOTH_BUTTON
-    if (!pbsys_storage_settings_bluetooth_enabled_get()) {
-        mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("Bluetooth not enabled"));
-    }
-    #endif // PBSYS_CONFIG_HMI_PUP_BLUETOOTH_BUTTON
-
     pb_module_tools_assert_blocking();
 
     pb_type_xbox_obj_t *self = mp_obj_malloc(pb_type_xbox_obj_t, type);