diff --git a/CHANGELOG.md b/CHANGELOG.md index af1603c93..fe4848c44 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/pybricks/common/pb_type_ble.c b/pybricks/common/pb_type_ble.c index b84644237..273966240 100644 --- a/pybricks/common/pb_type_ble.c +++ b/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,7 +534,7 @@ 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. @@ -524,30 +542,28 @@ static MP_DEFINE_CONST_OBJ_TYPE(pb_type_BLE, 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 diff --git a/pybricks/hubs/pb_type_cityhub.c b/pybricks/hubs/pb_type_cityhub.c index 64e3521cc..2d78d4c61 100644 --- a/pybricks/hubs/pb_type_cityhub.c +++ b/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 diff --git a/pybricks/hubs/pb_type_essentialhub.c b/pybricks/hubs/pb_type_essentialhub.c index 6066eb689..610307df5 100644 --- a/pybricks/hubs/pb_type_essentialhub.c +++ b/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 ); diff --git a/pybricks/hubs/pb_type_movehub.c b/pybricks/hubs/pb_type_movehub.c index 4e6537078..23e1ee975 100644 --- a/pybricks/hubs/pb_type_movehub.c +++ b/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 ); diff --git a/pybricks/hubs/pb_type_primehub.c b/pybricks/hubs/pb_type_primehub.c index cffe79b66..2032a5544 100644 --- a/pybricks/hubs/pb_type_primehub.c +++ b/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 ); diff --git a/pybricks/hubs/pb_type_technichub.c b/pybricks/hubs/pb_type_technichub.c index 10b0960ec..f5ed6f08d 100644 --- a/pybricks/hubs/pb_type_technichub.c +++ b/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 );