pybricks.common.BLE: Implement observe_enable()

CHANGELOG.md

@@ -9,11 +9,14 @@
 - Allow color objects to be iterated as h, s, v = color_object or indexed
   as color_object[0]. This allows access to these properties in block
   coding ([support#1661]).
+- Added `observe_enable` to the hub `BLE` class to selectively turn observing
+  on and off, just like you can with broadcasting ([support#1806]).
 
 ### Changed
 
 - Relaxed speed limit from 1000 deg/s to 1200 deg/s for external Boost
   motor ([support#1623]).
+- Make `broadcast_channel` optional instead of defaulting to `0`.
 
 ### Fixed
 - Fixed persistent data not being deleted when swapping
@@ -31,6 +34,7 @@
 [support#1623]: https://github.com/pybricks/support/issues/1623
 [support#1661]: https://github.com/pybricks/support/issues/1661
 [support#1668]: https://github.com/pybricks/support/issues/1668
+[support#1806]: https://github.com/pybricks/support/issues/1806
 [support#1846]: https://github.com/pybricks/support/issues/1846
 [support#1858]: https://github.com/pybricks/support/issues/1858
 [support#1863]: https://github.com/pybricks/support/issues/1863

pybricks/common/pb_type_ble.c

@@ -49,11 +49,11 @@ static observed_data_t *observed_data;
 static uint8_t num_observed_data;
 
 static pbio_task_t broadcast_task;
+static pbio_task_t toggle_observe_task;
 
 typedef struct {
     mp_obj_base_t base;
-    uint8_t broadcast_channel;
-    pbio_task_t *broadcast_task;
+    mp_obj_t broadcast_channel;
     observed_data_t observed_data[];
 } pb_obj_BLE_t;
 
@@ -263,14 +263,9 @@ static mp_obj_t pb_module_ble_broadcast(size_t n_args, const mp_obj_t *pos_args,
     // move hub is connected to Pybricks Code. Also, broadcasting interferes
     // with observing even when not connected to Pybricks Code.
 
-    // FIXME: This check is (and should only be) done in the BLE constructor,
-    // but it may still pass there since 0 is a valid broadcast channel. That
-    // should be fixed by defaulting to None if no broadcast channel is provided.
-    #if PBSYS_CONFIG_BLUETOOTH_TOGGLE
-    if (!pbsys_storage_settings_bluetooth_enabled()) {
-        mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("Bluetooth not enabled"));
+    if (self->broadcast_channel == mp_const_none) {
+        mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("no broadcast channel selected"));
     }
-    #endif // PBSYS_CONFIG_BLUETOOTH_TOGGLE
 
     // Stop broadcasting if data is None.
     if (data_in == mp_const_none) {
@@ -309,10 +304,10 @@ static mp_obj_t pb_module_ble_broadcast(size_t n_args, const mp_obj_t *pos_args,
     value.v.data[0] = index + 4; // length
     value.v.data[1] = MFG_SPECIFIC;
     pbio_set_uint16_le(&value.v.data[2], LEGO_CID);
-    value.v.data[4] = self->broadcast_channel;
+    value.v.data[4] = mp_obj_get_int(self->broadcast_channel);
 
-    pbdrv_bluetooth_start_broadcasting(self->broadcast_task, &value.v);
-    return pb_module_tools_pbio_task_wait_or_await(self->broadcast_task);
+    pbdrv_bluetooth_start_broadcasting(&broadcast_task, &value.v);
+    return pb_module_tools_pbio_task_wait_or_await(&broadcast_task);
 }
 static MP_DEFINE_CONST_FUN_OBJ_KW(pb_module_ble_broadcast_obj, 1, pb_module_ble_broadcast);
 
@@ -411,7 +406,7 @@ static const observed_data_t *pb_module_ble_get_channel_data(mp_obj_t channel_in
     observed_data_t *ch_data = lookup_observed_data(channel);
 
     if (!ch_data) {
-        mp_raise_ValueError(MP_ERROR_TEXT("channel not allocated"));
+        mp_raise_ValueError(MP_ERROR_TEXT("channel not configured"));
     }
 
     // Reset the data if it is too old.
@@ -472,6 +467,28 @@ static mp_obj_t pb_module_ble_observe(mp_obj_t self_in, mp_obj_t channel_in) {
 }
 static MP_DEFINE_CONST_FUN_OBJ_2(pb_module_ble_observe_obj, pb_module_ble_observe);
 
+/**
+ * Enables or disable observing
+ *
+ * @param [in]  self_in     The BLE object.
+ * @param [in]  enable_in   Thruthy to enable, falsy to disable.
+ * @returns                 Awaitable.
+ */
+static mp_obj_t pb_module_ble_observe_enable(mp_obj_t self_in, mp_obj_t enable_in) {
+
+    if (num_observed_data == 0) {
+        mp_raise_msg(&mp_type_RuntimeError, MP_ERROR_TEXT("channel not configured"));
+    }
+
+    if (mp_obj_is_true(enable_in)) {
+        pbdrv_bluetooth_start_observing(&toggle_observe_task, handle_observe_event);
+    } else {
+        pbdrv_bluetooth_stop_observing(&toggle_observe_task);
+    }
+    return pb_module_tools_pbio_task_wait_or_await(&toggle_observe_task);
+}
+static MP_DEFINE_CONST_FUN_OBJ_2(pb_module_ble_observe_enable_obj, pb_module_ble_observe_enable);
+
 /**
  * Retrieves the filtered RSSI signal strength of the given channel.
  *
@@ -501,6 +518,7 @@ static MP_DEFINE_CONST_FUN_OBJ_1(pb_module_ble_version_obj, pb_module_ble_versio
 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_observe), MP_ROM_PTR(&pb_module_ble_observe_obj) },
+    { MP_ROM_QSTR(MP_QSTR_observe_enable), MP_ROM_PTR(&pb_module_ble_observe_enable_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) },
 };
@@ -516,38 +534,36 @@ static MP_DEFINE_CONST_OBJ_TYPE(pb_type_BLE,
  *
  * Do not call this function more than once unless pb_type_ble_start_cleanup() is called first.
  *
- * @param [in]  broadcast_channel_in    (int) The channel number to use for broadcasting.
+ * @param [in]  broadcast_channel_in    (int) The channel number to use for broadcasting or None for no broadcasting.
  * @param [in]  observe_channels_in     (list[int]) A list of channels numbers to observe.
  * @returns                             A newly allocated object.
  * @throws ValueError                   If either parameter contains an out of range channel number.
  */
 mp_obj_t pb_type_BLE_new(mp_obj_t broadcast_channel_in, mp_obj_t observe_channels_in) {
     // making the assumption that this is only called once before each pb_type_ble_start_cleanup()
     assert(observed_data == NULL);
+    pb_module_tools_assert_blocking();
 
-    mp_int_t broadcast_channel = mp_obj_get_int(broadcast_channel_in);
-
-    if (broadcast_channel < 0 || broadcast_channel > UINT8_MAX) {
-        mp_raise_ValueError(MP_ERROR_TEXT("broadcast channel must be 0 to 255"));
+    // Validate channel arguments.
+    if (broadcast_channel_in != mp_const_none && (mp_obj_get_int(broadcast_channel_in) < 0 || mp_obj_get_int(broadcast_channel_in) > UINT8_MAX)) {
+        mp_raise_ValueError(MP_ERROR_TEXT("Broadcast channel must be 0 to 255 or None"));
+    }
+    mp_int_t num_observe_channels = mp_obj_get_int(mp_obj_len(observe_channels_in));
+    if (num_observe_channels < 0 || num_observe_channels > UINT8_MAX) {
+        mp_raise_ValueError(MP_ERROR_TEXT("Too many observe channels"));
     }
 
-    mp_int_t num_channels = mp_obj_get_int(mp_obj_len(observe_channels_in));
-
+    // Raise if Bluetooth is attempted to be used while not enabled.
     #if PBSYS_CONFIG_BLUETOOTH_TOGGLE
-    if (!pbsys_storage_settings_bluetooth_enabled() && (num_channels > 0 || broadcast_channel)) {
+    if (!pbsys_storage_settings_bluetooth_enabled() && (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_BLUETOOTH_TOGGLE
 
-    if (num_channels < 0 || num_channels > UINT8_MAX) {
-        mp_raise_ValueError(MP_ERROR_TEXT("len observe channels must be 0 to 255"));
-    }
-
-    pb_obj_BLE_t *self = mp_obj_malloc_var(pb_obj_BLE_t, observed_data_t, num_channels, &pb_type_BLE);
-    self->broadcast_task = &broadcast_task;
-    self->broadcast_channel = broadcast_channel;
+    pb_obj_BLE_t *self = mp_obj_malloc_var(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_channels; i++) {
+    for (mp_int_t i = 0; i < num_observe_channels; i++) {
         mp_int_t channel = mp_obj_get_int(mp_obj_subscr(
             observe_channels_in, MP_OBJ_NEW_SMALL_INT(i), MP_OBJ_SENTINEL));
 
@@ -564,13 +580,11 @@ mp_obj_t pb_type_BLE_new(mp_obj_t broadcast_channel_in, mp_obj_t observe_channel
 
     // globals for driver callback
     observed_data = self->observed_data;
-    num_observed_data = num_channels;
+    num_observed_data = num_observe_channels;
 
-    // Start observing.
-    if (num_channels > 0) {
-        pbio_task_t task;
-        pbdrv_bluetooth_start_observing(&task, handle_observe_event);
-        pb_module_tools_pbio_task_do_blocking(&task, -1);
+    // Start observing right away by default.
+    if (num_observe_channels > 0) {
+        pb_module_ble_observe_enable(MP_OBJ_FROM_PTR(self), mp_const_true);
     }
 
     return MP_OBJ_FROM_PTR(self);
@@ -583,7 +597,7 @@ void pb_type_ble_start_cleanup(void) {
     pbdrv_bluetooth_stop_observing(&stop_observing_task);
     observed_data = NULL;
     num_observed_data = 0;
-    // Tasks awaited in pybricks de-init.
+    // The aforementioned tasks started here are awaited in pybricks de-init.
 }
 
 #endif // PYBRICKS_PY_COMMON_BLE

pybricks/hubs/pb_type_cityhub.c

@@ -28,7 +28,7 @@ typedef struct _hubs_CityHub_obj_t {
 static mp_obj_t hubs_CityHub_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args) {
     #if PYBRICKS_PY_COMMON_BLE
     PB_PARSE_ARGS_CLASS(n_args, n_kw, args,
-        PB_ARG_DEFAULT_INT(broadcast_channel, 0),
+        PB_ARG_DEFAULT_NONE(broadcast_channel),
         PB_ARG_DEFAULT_OBJ(observe_channels, mp_const_empty_tuple_obj));
     #endif
 

pybricks/hubs/pb_type_essentialhub.c

@@ -42,7 +42,7 @@ static mp_obj_t hubs_EssentialHub_make_new(const mp_obj_type_t *type, size_t n_a
         PB_ARG_DEFAULT_OBJ(top_side, pb_type_Axis_Z_obj),
         PB_ARG_DEFAULT_OBJ(front_side, pb_type_Axis_X_obj)
         #if PYBRICKS_PY_COMMON_BLE
-        , PB_ARG_DEFAULT_INT(broadcast_channel, 0)
+        , PB_ARG_DEFAULT_NONE(broadcast_channel)
         , PB_ARG_DEFAULT_OBJ(observe_channels, mp_const_empty_tuple_obj)
         #endif
         );

pybricks/hubs/pb_type_movehub.c

@@ -319,7 +319,7 @@ static mp_obj_t hubs_MoveHub_make_new(const mp_obj_type_t *type, size_t n_args,
         PB_ARG_DEFAULT_INT(top_side, pb_type_Axis_Z_int_enum),
         PB_ARG_DEFAULT_INT(front_side, pb_type_Axis_X_int_enum)
         #if PYBRICKS_PY_COMMON_BLE
-        , PB_ARG_DEFAULT_INT(broadcast_channel, 0)
+        , PB_ARG_DEFAULT_NONE(broadcast_channel)
         , PB_ARG_DEFAULT_OBJ(observe_channels, mp_const_empty_tuple_obj)
         #endif
         );

pybricks/hubs/pb_type_primehub.c

@@ -66,7 +66,7 @@ static mp_obj_t hubs_PrimeHub_make_new(const mp_obj_type_t *type, size_t n_args,
         PB_ARG_DEFAULT_OBJ(top_side, pb_type_Axis_Z_obj),
         PB_ARG_DEFAULT_OBJ(front_side, pb_type_Axis_X_obj)
         #if PYBRICKS_PY_COMMON_BLE
-        , PB_ARG_DEFAULT_INT(broadcast_channel, 0)
+        , PB_ARG_DEFAULT_NONE(broadcast_channel)
         , PB_ARG_DEFAULT_OBJ(observe_channels, mp_const_empty_tuple_obj)
         #endif
         );

pybricks/hubs/pb_type_technichub.c

@@ -32,7 +32,7 @@ static mp_obj_t hubs_TechnicHub_make_new(const mp_obj_type_t *type, size_t n_arg
         PB_ARG_DEFAULT_OBJ(top_side, pb_type_Axis_Z_obj),
         PB_ARG_DEFAULT_OBJ(front_side, pb_type_Axis_X_obj)
         #if PYBRICKS_PY_COMMON_BLE
-        , PB_ARG_DEFAULT_INT(broadcast_channel, 0)
+        , PB_ARG_DEFAULT_NONE(broadcast_channel)
         , PB_ARG_DEFAULT_OBJ(observe_channels, mp_const_empty_tuple_obj)
         #endif
         );