From c4f2f9966d3bd1169fe5e89527ff197693364a7b Mon Sep 17 00:00:00 2001 From: Marek Pieta Date: Thu, 17 Aug 2023 13:43:22 +0200 Subject: [PATCH] applications: nrf_desktop: Add state machine to BLE scan module Change introduces a state machine to BLE scan module. Jira: NCSDK-20843 Signed-off-by: Marek Pieta --- .../nrf_desktop/src/modules/ble_scan.c | 491 +++++++++++++----- 1 file changed, 373 insertions(+), 118 deletions(-) diff --git a/applications/nrf_desktop/src/modules/ble_scan.c b/applications/nrf_desktop/src/modules/ble_scan.c index 766f14a3a71..a19e30555e1 100644 --- a/applications/nrf_desktop/src/modules/ble_scan.c +++ b/applications/nrf_desktop/src/modules/ble_scan.c @@ -24,13 +24,30 @@ #include LOG_MODULE_REGISTER(MODULE, CONFIG_DESKTOP_BLE_SCAN_LOG_LEVEL); -#define SCAN_TRIG_CHECK_MS (1 * MSEC_PER_SEC) -#define SCAN_TRIG_TIMEOUT_MS (CONFIG_DESKTOP_BLE_SCAN_START_TIMEOUT_S * MSEC_PER_SEC) +#define SCAN_START_CHECK_MS (1 * MSEC_PER_SEC) +#define SCAN_START_TIMEOUT_MS (CONFIG_DESKTOP_BLE_SCAN_START_TIMEOUT_S * MSEC_PER_SEC) #define SCAN_DURATION_MS (CONFIG_DESKTOP_BLE_SCAN_DURATION_S * MSEC_PER_SEC) #define SCAN_START_DELAY_MS 15 +BUILD_ASSERT(SCAN_START_CHECK_MS > 0); +BUILD_ASSERT(SCAN_START_TIMEOUT_MS > 0); +BUILD_ASSERT(SCAN_DURATION_MS > 0, ""); +BUILD_ASSERT(SCAN_START_DELAY_MS > 0, ""); + #define SUBSCRIBED_PEERS_STORAGE_NAME "subscribers" +enum state { + STATE_DISABLED, + STATE_DISABLED_OFF, + STATE_INITIALIZED, + STATE_INITIALIZED_OFF, + STATE_ACTIVE, + STATE_DELAYED_ACTIVE, + STATE_INACTIVE, + STATE_OFF, + STATE_ERROR, +}; + struct subscriber_data { uint8_t conn_count; uint8_t peer_count; @@ -45,13 +62,13 @@ struct subscribed_peer { static struct subscribed_peer subscribed_peers[CONFIG_BT_MAX_PAIRED]; static struct bt_conn *discovering_peer_conn; -static unsigned int scan_counter; -static struct k_work_delayable scan_start_trigger; -static struct k_work_delayable scan_stop_trigger; +static unsigned int scan_start_counter; +static struct k_work_delayable update_state_work; static bool peers_only = !IS_ENABLED(CONFIG_DESKTOP_BLE_NEW_PEER_SCAN_ON_BOOT); static bool scanning; -static bool scan_blocked; -static bool ble_bond_ready; + +static enum state state; + static void store_subscribed_peers(void) { @@ -211,34 +228,29 @@ static void broadcast_scan_state(bool active) APP_EVENT_SUBMIT(event); } -static void scan_stop(void) +static int scan_stop(void) { if (!scanning) { - return; + return 0; } int err = bt_scan_stop(); - if (err == -EALREADY) { + if (!err) { + LOG_INF("Scan stopped"); + } else if (err == -EALREADY) { LOG_WRN("Active scan already disabled"); - } else if (err) { - LOG_ERR("Stop LE scan failed (err %d)", err); - module_set_state(MODULE_STATE_ERROR); - return; + err = 0; } else { - LOG_INF("Scan stopped"); + LOG_ERR("Stop LE scan failed (err: %d)", err); } - scanning = false; - broadcast_scan_state(scanning); - - /* Cancel cannot fail if executed from another work's context. */ - (void)k_work_cancel_delayable(&scan_stop_trigger); - - if (count_conn() < CONFIG_BT_MAX_CONN) { - scan_counter = 0; - (void)k_work_reschedule(&scan_start_trigger, K_MSEC(SCAN_TRIG_CHECK_MS)); + if (!err) { + scanning = false; + broadcast_scan_state(scanning); } + + return err; } static int configure_address_filters(uint8_t *filter_mode) @@ -396,27 +408,13 @@ static void update_init_conn_params(bool llpm_peer_connected) bt_scan_update_init_conn_params(&cp); } -static void scan_start(void) +static int scan_start(void) { - size_t conn_count = count_conn(); - size_t bond_count = count_bond(); - int err; + int err = scan_stop(); - scan_stop(); - - if (IS_ENABLED(CONFIG_DESKTOP_BLE_SCAN_PM_EVENTS) && scan_blocked) { - LOG_INF("Power down mode - scanning blocked"); - return; - } else if (IS_ENABLED(CONFIG_DESKTOP_BLE_NEW_PEER_SCAN_REQUEST) && - (conn_count == bond_count) && peers_only) { - LOG_INF("All known peers connected - scanning disabled"); - return; - } else if (conn_count == CONFIG_BT_MAX_CONN) { - LOG_INF("Max number of peers connected - scanning disabled"); - return; - } else if (discovering_peer_conn) { - LOG_INF("Discovery in progress"); - return; + if (err) { + LOG_ERR("Failed to stop scanning before restart (err %d)", err); + return err; } if (IS_ENABLED(CONFIG_CAF_BLE_USE_LLPM) && (CONFIG_BT_MAX_CONN == 2)) { @@ -431,55 +429,23 @@ static void scan_start(void) err = configure_filters(); if (err) { LOG_ERR("Cannot set filters (err %d)", err); - goto error; + return err; } err = bt_scan_start(BT_SCAN_TYPE_SCAN_ACTIVE); if (err) { LOG_ERR("Cannot start scanning (err %d)", err); - goto error; - } else { - LOG_INF("Scan started"); + return err; } + LOG_INF("Scan started"); scanning = true; broadcast_scan_state(scanning); - (void)k_work_reschedule(&scan_stop_trigger, K_MSEC(SCAN_DURATION_MS)); - - /* Cancel cannot fail if executed from another work's context. */ - (void)k_work_cancel_delayable(&scan_start_trigger); - - return; - -error: - module_set_state(MODULE_STATE_ERROR); -} - -static void scan_start_trigger_fn(struct k_work *w) -{ - BUILD_ASSERT(SCAN_START_DELAY_MS > 0, ""); - BUILD_ASSERT(SCAN_TRIG_CHECK_MS > 0, ""); - BUILD_ASSERT(SCAN_TRIG_TIMEOUT_MS > SCAN_TRIG_CHECK_MS, ""); - - scan_counter += SCAN_TRIG_CHECK_MS; - if (scan_counter > SCAN_TRIG_TIMEOUT_MS) { - scan_counter = 0; - scan_start(); - } else { - (void)k_work_reschedule(&scan_start_trigger, K_MSEC(SCAN_TRIG_CHECK_MS)); - } + return err; } -static void scan_stop_trigger_fn(struct k_work *w) -{ - BUILD_ASSERT(SCAN_DURATION_MS > 0, ""); - - if (count_conn() != 0) { - peers_only = true; - scan_stop(); - } -} +static void update_state(enum state new_state); static void scan_filter_match(struct bt_scan_device_info *device_info, struct bt_scan_filter_match *filter_match, @@ -496,9 +462,10 @@ static void scan_filter_match(struct bt_scan_device_info *device_info, static void scan_connecting_error(struct bt_scan_device_info *device_info) { LOG_WRN("Connecting failed"); - scan_counter = SCAN_TRIG_TIMEOUT_MS; - (void)k_work_reschedule(&scan_start_trigger, K_MSEC(SCAN_START_DELAY_MS)); + if (state != STATE_OFF) { + update_state(STATE_DELAYED_ACTIVE); + } } static void scan_connecting(struct bt_scan_device_info *device_info, @@ -511,6 +478,8 @@ static void scan_connecting(struct bt_scan_device_info *device_info, BT_SCAN_CB_INIT(scan_cb, scan_filter_match, NULL, scan_connecting_error, scan_connecting); +static void update_state_work_fn(struct k_work *w); + static void scan_init(void) { reset_subscribers(false); @@ -544,8 +513,235 @@ static void scan_init(void) bt_scan_init(&scan_init); bt_scan_cb_register(&scan_cb); - k_work_init_delayable(&scan_start_trigger, scan_start_trigger_fn); - k_work_init_delayable(&scan_stop_trigger, scan_stop_trigger_fn); + k_work_init_delayable(&update_state_work, update_state_work_fn); +} + +static const char *state2str(enum state s) +{ + switch (s) { + case STATE_DISABLED: + return "DISABLED"; + + case STATE_DISABLED_OFF: + return "DISABLED_OFF"; + + case STATE_INITIALIZED: + return "INITIALIZED"; + + case STATE_INITIALIZED_OFF: + return "INITIALIZED_OFF"; + + case STATE_ACTIVE: + return "ACTIVE"; + + case STATE_DELAYED_ACTIVE: + return "DELAYED_ACTIVE"; + + case STATE_INACTIVE: + return "INACTIVE"; + + case STATE_OFF: + return "OFF"; + + case STATE_ERROR: + return "ERROR"; + + default: + __ASSERT_NO_MSG(false); + return "UNKNOWN"; + } +} + +static enum state update_state_transition(enum state prev_state, enum state new_state, bool verbose) +{ + size_t conn_count = count_conn(); + size_t bond_count = count_bond(); + + if (new_state == STATE_ACTIVE) { + if (IS_ENABLED(CONFIG_DESKTOP_BLE_NEW_PEER_SCAN_REQUEST) && + (conn_count == bond_count) && peers_only) { + if (verbose) { + LOG_INF("All known peers connected - scanning disabled"); + } + return STATE_INACTIVE; + } else if (conn_count == CONFIG_BT_MAX_CONN) { + if (verbose) { + LOG_INF("Max number of peers connected - scanning disabled"); + } + return STATE_INACTIVE; + } else if (discovering_peer_conn) { + if (verbose) { + LOG_INF("Discovery in progress"); + } + return STATE_INACTIVE; + } + } else if (new_state == STATE_INACTIVE) { + __ASSERT_NO_MSG(prev_state != STATE_OFF); + if (conn_count == 0) { + if (verbose) { + LOG_INF("No peer connected - keep scanning"); + } + return STATE_ACTIVE; + } + } + + return new_state; +} + +static bool is_module_init(enum state s) +{ + return (s != STATE_DISABLED) && (s != STATE_DISABLED_OFF); +} + +/* The module only reports: + * - ready state after initialization + * - error state after a fatal error + */ +static void broadcast_module_state(enum state prev_state, enum state new_state) +{ + if (new_state == STATE_ERROR) { + module_set_state(MODULE_STATE_ERROR); + return; + } + + if (!is_module_init(prev_state) && is_module_init(new_state)) { + module_set_state(MODULE_STATE_READY); + } +} + +static int update_scan_control(enum state prev_state, enum state new_state) +{ + __ASSERT_NO_MSG(!is_module_init(prev_state) || is_module_init(new_state)); + + if (!is_module_init(prev_state)) { + if (!is_module_init(new_state) || (new_state == STATE_ERROR)) { + /* Do not perform actions before initialized or in the error state. */ + return 0; + } + + scan_init(); + } + + int err = 0; + + switch (new_state) { + case STATE_ACTIVE: + err = scan_start(); + break; + + case STATE_DELAYED_ACTIVE: + case STATE_INACTIVE: + case STATE_OFF: + case STATE_ERROR: + err = scan_stop(); + break; + + default: + break; + } + + return err; +} + +static void update_work(enum state prev_state, enum state new_state) +{ + switch (new_state) { + case STATE_ACTIVE: + /* Schedule work only if state transition is possible. */ + if (update_state_transition(STATE_ACTIVE, STATE_INACTIVE, false) == + STATE_INACTIVE) { + (void)k_work_reschedule(&update_state_work, K_MSEC(SCAN_DURATION_MS)); + } else { + (void)k_work_cancel_delayable(&update_state_work); + } + break; + + case STATE_DELAYED_ACTIVE: + (void)k_work_reschedule(&update_state_work, K_MSEC(SCAN_START_DELAY_MS)); + break; + + case STATE_INACTIVE: + /* Schedule work only if state transition is possible. */ + if (update_state_transition(STATE_INACTIVE, STATE_ACTIVE, false) == + STATE_ACTIVE) { + scan_start_counter = 0; + (void)k_work_reschedule(&update_state_work, K_MSEC(SCAN_START_CHECK_MS)); + } else { + (void)k_work_cancel_delayable(&update_state_work); + } + break; + + case STATE_OFF: + case STATE_ERROR: + (void)k_work_cancel_delayable(&update_state_work); + break; + + default: + /* Do nothing. */ + break; + } +} + +static int update_state_internal(enum state prev_state, enum state new_state) +{ + update_work(prev_state, new_state); + broadcast_module_state(prev_state, new_state); + return update_scan_control(prev_state, new_state); +} + +static void update_state(enum state new_state) +{ + /* Unrecoverable error. */ + if (state == STATE_ERROR) { + return; + } + + /* Check if requested state transition is possible. Update new state if needed. */ + new_state = update_state_transition(state, new_state, true); + int err = update_state_internal(state, new_state); + + if (err) { + (void)update_state_internal(state, STATE_ERROR); + state = STATE_ERROR; + LOG_ERR("Fatal error (err: %d)", err); + } else { + state = new_state; + LOG_DBG("State: %s, peers_only: %s", state2str(state), + peers_only ? "true" : "false"); + } +} + +static void update_state_work_fn(struct k_work *w) +{ + ARG_UNUSED(w); + + switch (state) { + case STATE_ACTIVE: + if (IS_ENABLED(CONFIG_DESKTOP_BLE_NEW_PEER_SCAN_REQUEST)) { + peers_only = true; + } + update_state(STATE_INACTIVE); + break; + + case STATE_DELAYED_ACTIVE: + update_state(STATE_ACTIVE); + break; + + case STATE_INACTIVE: + scan_start_counter += SCAN_START_CHECK_MS; + if (scan_start_counter >= SCAN_START_TIMEOUT_MS) { + scan_start_counter = 0; + update_state(STATE_ACTIVE); + } else { + (void)k_work_reschedule(&update_state_work, K_MSEC(SCAN_START_CHECK_MS)); + } + break; + + default: + /* Should not happen. */ + __ASSERT_NO_MSG(false); + break; + } } static bool handle_hid_report_event(const struct hid_report_event *event) @@ -555,9 +751,15 @@ static bool handle_hid_report_event(const struct hid_report_event *event) return false; } - /* Do not scan when devices are in use. */ - scan_counter = 0; - scan_stop(); + if (state == STATE_INACTIVE) { + /* Avoid complete state update and only zero scan start counter to block scan start. + * This reduces number of operations done in HID report event handler and helps to + * avoid negatively affecting HID report rate. + */ + scan_start_counter = 0; + } else if ((state == STATE_ACTIVE) || (state == STATE_DELAYED_ACTIVE)) { + update_state(STATE_INACTIVE); + } return false; } @@ -565,23 +767,46 @@ static bool handle_hid_report_event(const struct hid_report_event *event) static bool handle_module_state_event(const struct module_state_event *event) { if (check_state(event, MODULE_ID(ble_state), MODULE_STATE_READY)) { - static bool initialized; - - __ASSERT_NO_MSG(!initialized); - initialized = true; + switch (state) { + case STATE_DISABLED: + update_state(STATE_INITIALIZED); + break; - scan_init(); + case STATE_DISABLED_OFF: + update_state(STATE_INITIALIZED_OFF); + break; - module_set_state(MODULE_STATE_READY); + default: + /* Should not happen. */ + __ASSERT_NO_MSG(false); + update_state(STATE_ERROR); + break; + } } else if (check_state(event, MODULE_ID(ble_bond), MODULE_STATE_READY)) { /* Settings need to be loaded on start before the scan begins. * As the ble_bond module reports its READY state also on wake_up_event, * to avoid multiple scan start triggering on wake-up scan will be started * from here only on start-up. */ - if (!ble_bond_ready) { - ble_bond_ready = true; - scan_start(); + switch (state) { + case STATE_INITIALIZED: + update_state(STATE_ACTIVE); + break; + + case STATE_INITIALIZED_OFF: + update_state(STATE_OFF); + break; + + /* ble_scan module must be initialized before ble_bond module. */ + case STATE_DISABLED: + case STATE_DISABLED_OFF: + __ASSERT_NO_MSG(false); + update_state(STATE_ERROR); + break; + + default: + /* Ignore the event. */ + break; } } @@ -603,11 +828,9 @@ static bool handle_ble_peer_event(const struct ble_peer_event *event) peer_discovery_end(NULL); } - /* ble_state keeps reference to connection object. - * Cannot create new connection now. - */ - scan_counter = SCAN_TRIG_TIMEOUT_MS; - (void)k_work_reschedule(&scan_start_trigger, K_MSEC(SCAN_START_DELAY_MS)); + if (state != STATE_OFF) { + update_state(STATE_DELAYED_ACTIVE); + } break; default: @@ -629,8 +852,13 @@ static bool handle_ble_peer_operation_event(const struct ble_peer_operation_even if (IS_ENABLED(CONFIG_BT_SCAN_CONN_ATTEMPTS_FILTER)) { bt_scan_conn_attempts_filter_clear(); } - peers_only = false; - scan_start(); + if (IS_ENABLED(CONFIG_DESKTOP_BLE_NEW_PEER_SCAN_REQUEST)) { + peers_only = false; + } + + if (state != STATE_OFF) { + update_state(STATE_ACTIVE); + } break; case PEER_OPERATION_ERASE_ADV: @@ -653,36 +881,63 @@ static bool handle_ble_peer_operation_event(const struct ble_peer_operation_even static bool handle_ble_discovery_complete_event(const struct ble_discovery_complete_event *event) { peer_discovery_end(event); - - /* Cannot start scanning right after discovery - problems - * establishing security - using delayed work as workaround. - */ - scan_counter = SCAN_TRIG_TIMEOUT_MS; - (void)k_work_reschedule(&scan_start_trigger, K_MSEC(SCAN_TRIG_TIMEOUT_MS)); - if (IS_ENABLED(CONFIG_BT_SCAN_CONN_ATTEMPTS_FILTER)) { bt_scan_conn_attempts_filter_clear(); } + /* Cannot start scanning right after discovery - problems establishing security. + * Delay scan start to workaround the issue. + */ + if (state != STATE_OFF) { + update_state(STATE_DELAYED_ACTIVE); + } + return false; } static bool handle_power_down_event(const struct power_down_event *event) { - scan_stop(); - scan_blocked = true; - (void)k_work_cancel_delayable(&scan_start_trigger); + switch (state) { + case STATE_DISABLED: + update_state(STATE_DISABLED_OFF); + break; + + case STATE_INITIALIZED: + update_state(STATE_INITIALIZED_OFF); + break; + + case STATE_ACTIVE: + case STATE_DELAYED_ACTIVE: + case STATE_INACTIVE: + update_state(STATE_OFF); + break; + + default: + /* Ignore event. */ + break; + } return false; } static bool handle_wake_up_event(const struct wake_up_event *event) { - if (scan_blocked) { - scan_blocked = false; - if (ble_bond_ready) { - scan_start(); - } + switch (state) { + case STATE_DISABLED_OFF: + update_state(STATE_DISABLED); + break; + + case STATE_INITIALIZED_OFF: + update_state(STATE_INITIALIZED); + break; + + case STATE_OFF: + update_state(STATE_ACTIVE); + break; + + default: + /* Ignore event. */ + break; } return false;