From 1ce95de6aee00f0433c45bb785807ce1dfe7ec4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Delawarde?= Date: Fri, 4 Oct 2019 14:29:11 +0200 Subject: [PATCH] bluetooth: host: Persist Service Changed data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add support for persisted Service Changed data, to fix the case of a paired device not reconnecting before a reboot and thus not receiving SC indication. It also enables support for GATT database being changed during a firmware update. Move Service Changed data outside of the CCC struct and make it persistent by adding support for a bt/sc/... setting. Signed-off-by: François Delawarde --- include/bluetooth/gatt.h | 1 - subsys/bluetooth/host/gatt.c | 412 +++++++++++++++++++++++++++++------ 2 files changed, 344 insertions(+), 69 deletions(-) diff --git a/include/bluetooth/gatt.h b/include/bluetooth/gatt.h index 3d759a94215..1cdf9b77061 100644 --- a/include/bluetooth/gatt.h +++ b/include/bluetooth/gatt.h @@ -563,7 +563,6 @@ struct bt_gatt_ccc_cfg { u8_t id; bt_addr_le_t peer; u16_t value; - u8_t data[4] __aligned(4); }; /* Internal representation of CCC value */ diff --git a/subsys/bluetooth/host/gatt.c b/subsys/bluetooth/host/gatt.c index 7674dee330f..9801d040701 100644 --- a/subsys/bluetooth/host/gatt.c +++ b/subsys/bluetooth/host/gatt.c @@ -183,13 +183,123 @@ BT_GATT_SERVICE_DEFINE(_2_gap_svc, #endif ); -#if defined(CONFIG_BT_GATT_SERVICE_CHANGED) -static void sc_ccc_cfg_changed(const struct bt_gatt_attr *attr, - u16_t value) +struct sc_data { + u16_t start; + u16_t end; +} __packed; + +struct gatt_sc_cfg { + u8_t id; + bt_addr_le_t peer; + struct sc_data data; +}; + +#define SC_CFG_MAX (CONFIG_BT_MAX_PAIRED + CONFIG_BT_MAX_CONN) +static struct gatt_sc_cfg sc_cfg[SC_CFG_MAX]; + +static struct gatt_sc_cfg *find_sc_cfg(u8_t id, bt_addr_le_t *addr) +{ + BT_DBG("id: %u, addr: %s", id, bt_addr_le_str(addr)); + + for (size_t i = 0; i < ARRAY_SIZE(sc_cfg); i++) { + if (id == sc_cfg[i].id && + !bt_addr_le_cmp(&sc_cfg[i].peer, addr)) { + return &sc_cfg[i]; + } + } + + return NULL; +} + +static void sc_store(struct gatt_sc_cfg *cfg) +{ + char key[BT_SETTINGS_KEY_MAX]; + int err; + + if (cfg->id) { + char id_str[4]; + + u8_to_dec(id_str, sizeof(id_str), cfg->id); + bt_settings_encode_key(key, sizeof(key), "sc", + &cfg->peer, id_str); + } else { + bt_settings_encode_key(key, sizeof(key), "sc", + &cfg->peer, NULL); + } + + err = settings_save_one(key, (char *)&cfg->data, sizeof(cfg->data)); + if (err) { + BT_ERR("failed to store SC (err %d)", err); + return; + } + + BT_DBG("stored SC for %s (%s, 0x%04x-0x%04x)", + bt_addr_le_str(&cfg->peer), log_strdup(key), cfg->data.start, + cfg->data.end); +} + +static void sc_clear(struct gatt_sc_cfg *cfg) +{ + BT_DBG("peer %s", bt_addr_le_str(&cfg->peer)); + + if (IS_ENABLED(CONFIG_BT_SETTINGS)) { + bool modified = false; + + if (cfg->data.start || cfg->data.end) { + modified = true; + } + + if (modified && bt_addr_le_is_bonded(cfg->id, &cfg->peer)) { + char key[BT_SETTINGS_KEY_MAX]; + int err; + + if (cfg->id) { + char id_str[4]; + + u8_to_dec(id_str, sizeof(id_str), cfg->id); + bt_settings_encode_key(key, sizeof(key), "sc", + &cfg->peer, id_str); + } else { + bt_settings_encode_key(key, sizeof(key), "sc", + &cfg->peer, NULL); + } + + err = settings_delete(key); + if (err) { + BT_ERR("failed to delete SC (err %d)", err); + } else { + BT_DBG("deleted SC for %s (%s)", + bt_addr_le_str(&cfg->peer), + log_strdup(key)); + } + } + } + + memset(cfg, 0, sizeof(*cfg)); +} + +static bool sc_ccc_cfg_write(struct bt_conn *conn, + const struct bt_gatt_attr *attr, + u16_t value) { BT_DBG("value 0x%04x", value); + + if (value != BT_GATT_CCC_INDICATE) { + struct gatt_sc_cfg *cfg; + + /* Clear SC configuration if unsubscribed */ + cfg = find_sc_cfg(conn->id, &conn->le.dst); + if (cfg) { + sc_clear(cfg); + } + } + + return true; } -#endif /* CONFIG_BT_GATT_SERVICE_CHANGED */ + +static struct _bt_gatt_ccc sc_ccc = BT_GATT_CCC_INITIALIZER(NULL, + sc_ccc_cfg_write, + NULL); #if defined(CONFIG_BT_GATT_CACHING) enum { @@ -508,7 +618,7 @@ BT_GATT_SERVICE_DEFINE(_1_gatt_svc, */ BT_GATT_CHARACTERISTIC(BT_UUID_GATT_SC, BT_GATT_CHRC_INDICATE, BT_GATT_PERM_NONE, NULL, NULL, NULL), - BT_GATT_CCC(sc_ccc_cfg_changed, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), + BT_GATT_CCC_MANAGED(&sc_ccc, BT_GATT_PERM_READ | BT_GATT_PERM_WRITE), #if defined(CONFIG_BT_GATT_CACHING) BT_GATT_CHARACTERISTIC(BT_UUID_GATT_CLIENT_FEATURES, BT_GATT_CHRC_READ | BT_GATT_CHRC_WRITE, @@ -608,7 +718,6 @@ populate: } #endif /* CONFIG_BT_GATT_DYNAMIC_DB */ -#if defined(CONFIG_BT_GATT_SERVICE_CHANGED) enum { SC_RANGE_CHANGED, /* SC range changed */ SC_INDICATE_PENDING, /* SC indicate pending */ @@ -685,7 +794,6 @@ static void sc_process(struct k_work *work) atomic_set_bit(sc->flags, SC_INDICATE_PENDING); } -#endif /* CONFIG_BT_GATT_SERVICE_CHANGED */ #if defined(CONFIG_BT_SETTINGS_CCC_STORE_ON_WRITE) static struct gatt_ccc_store { @@ -758,15 +866,22 @@ void bt_gatt_init(void) */ k_delayed_work_submit(&db_hash_work, DB_HASH_TIMEOUT); #endif /* CONFIG_BT_GATT_CACHING */ -#if defined(CONFIG_BT_GATT_SERVICE_CHANGED) - k_delayed_work_init(&gatt_sc.work, sc_process); -#endif /* CONFIG_BT_GATT_SERVICE_CHANGED */ + + if (IS_ENABLED(CONFIG_BT_GATT_SERVICE_CHANGED)) { + k_delayed_work_init(&gatt_sc.work, sc_process); + if (IS_ENABLED(CONFIG_BT_SETTINGS)) { + /* Make sure to not send SC indications until SC + * settings are loaded + */ + atomic_set_bit(gatt_sc.flags, SC_INDICATE_PENDING); + } + } + #if defined(CONFIG_BT_SETTINGS_CCC_STORE_ON_WRITE) k_delayed_work_init(&gatt_ccc_store.work, ccc_delayed_store); #endif } -#if defined(CONFIG_BT_GATT_SERVICE_CHANGED) static bool update_range(u16_t *start, u16_t *end, u16_t new_start, u16_t new_end) { @@ -790,28 +905,32 @@ static bool update_range(u16_t *start, u16_t *end, u16_t new_start, return true; } -static void sc_indicate(struct gatt_sc *sc, u16_t start, u16_t end) +#if defined(CONFIG_BT_GATT_DYNAMIC_DB) || \ + (defined(CONFIG_BT_GATT_CACHING) && defined(CONFIG_BT_SETTINGS)) +static void sc_indicate(u16_t start, u16_t end) { - if (!atomic_test_and_set_bit(sc->flags, SC_RANGE_CHANGED)) { - sc->start = start; - sc->end = end; + BT_DBG("start 0x%04x end 0x%04x", start, end); + + if (!atomic_test_and_set_bit(gatt_sc.flags, SC_RANGE_CHANGED)) { + gatt_sc.start = start; + gatt_sc.end = end; goto submit; } - if (!update_range(&sc->start, &sc->end, start, end)) { + if (!update_range(&gatt_sc.start, &gatt_sc.end, start, end)) { return; } submit: - if (atomic_test_bit(sc->flags, SC_INDICATE_PENDING)) { + if (atomic_test_bit(gatt_sc.flags, SC_INDICATE_PENDING)) { BT_DBG("indicate pending, waiting until complete..."); return; } /* Reschedule since the range has changed */ - k_delayed_work_submit(&sc->work, SC_TIMEOUT); + k_delayed_work_submit(&gatt_sc.work, SC_TIMEOUT); } -#endif /* CONFIG_BT_GATT_SERVICE_CHANGED */ +#endif /* BT_GATT_DYNAMIC_DB || (BT_GATT_CACHING && BT_SETTINGS) */ #if defined(CONFIG_BT_GATT_DYNAMIC_DB) static void db_changed(void) @@ -867,7 +986,7 @@ int bt_gatt_service_register(struct bt_gatt_service *svc) return err; } - sc_indicate(&gatt_sc, svc->attrs[0].handle, + sc_indicate(svc->attrs[0].handle, svc->attrs[svc->attr_count - 1].handle); db_changed(); @@ -883,7 +1002,7 @@ int bt_gatt_service_unregister(struct bt_gatt_service *svc) return -ENOENT; } - sc_indicate(&gatt_sc, svc->attrs[0].handle, + sc_indicate(svc->attrs[0].handle, svc->attrs[svc->attr_count - 1].handle); db_changed(); @@ -1117,12 +1236,12 @@ static void foreach_attr_type_dyndb(u16_t start_handle, u16_t end_handle, struct bt_gatt_attr *attr = &svc->attrs[i]; if (gatt_foreach_iter(attr, - start_handle, - end_handle, - uuid, attr_data, - &num_matches, - func, user_data) == - BT_GATT_ITER_STOP) { + start_handle, + end_handle, + uuid, attr_data, + &num_matches, + func, user_data) == + BT_GATT_ITER_STOP) { return; } } @@ -1199,7 +1318,6 @@ static void clear_ccc_cfg(struct bt_gatt_ccc_cfg *cfg) bt_addr_le_copy(&cfg->peer, BT_ADDR_LE_ANY); cfg->id = 0U; cfg->value = 0U; - memset(cfg->data, 0, sizeof(cfg->data)); } static struct bt_gatt_ccc_cfg *find_ccc_cfg(const struct bt_conn *conn, @@ -1459,7 +1577,8 @@ static int gatt_indicate(struct bt_conn *conn, u16_t handle, * notifications and indications to such a client until it becomes * change-aware. */ - if (!(params->func && params->func == sc_indicate_rsp) && + if (!(params->func && (params->func == sc_indicate_rsp || + params->func == sc_restore_rsp)) && !bt_gatt_change_aware(conn, false)) { return -EAGAIN; } @@ -1487,40 +1606,43 @@ static int gatt_indicate(struct bt_conn *conn, u16_t handle, return gatt_send(conn, buf, gatt_indicate_rsp, params, NULL); } -#if defined(CONFIG_BT_GATT_SERVICE_CHANGED) -struct sc_data { - u16_t start; - u16_t end; -}; - -static void sc_save(struct bt_gatt_ccc_cfg *cfg, - struct bt_gatt_indicate_params *params) +static void sc_save(u8_t id, bt_addr_le_t *peer, u16_t start, u16_t end) { - struct sc_data data; - struct sc_data *stored; + struct gatt_sc_cfg *cfg; + bool modified = false; - memcpy(&data, params->data, params->len); + BT_DBG("peer %s start 0x%04x end 0x%04x", bt_addr_le_str(peer), start, + end); - data.start = sys_le16_to_cpu(data.start); - data.end = sys_le16_to_cpu(data.end); + cfg = find_sc_cfg(id, peer); + if (!cfg) { + /* Find and initialize a free sc_cfg entry */ + cfg = find_sc_cfg(BT_ID_DEFAULT, BT_ADDR_LE_ANY); + if (!cfg) { + BT_ERR("unable to save SC: no cfg left"); + return; + } - /* Load data stored */ - stored = (struct sc_data *)cfg->data; + cfg->id = id; + bt_addr_le_copy(&cfg->peer, peer); + } /* Check if there is any change stored */ - if (!stored->start && !stored->end) { - *stored = data; + if (!(cfg->data.start || cfg->data.end)) { + cfg->data.start = start; + cfg->data.end = end; + modified = true; goto done; } - update_range(&stored->start, &stored->end, - data.start, data.end); + modified = update_range(&cfg->data.start, &cfg->data.end, start, end); done: - BT_DBG("peer %s start 0x%04x end 0x%04x", bt_addr_le_str(&cfg->peer), - stored->start, stored->end); + if (IS_ENABLED(CONFIG_BT_SETTINGS) && + modified && bt_addr_le_is_bonded(cfg->id, &cfg->peer)) { + sc_store(cfg); + } } -#endif /* CONFIG_BT_GATT_SERVICE_CHANGED */ static u8_t notify_cb(const struct bt_gatt_attr *attr, void *user_data) { @@ -1550,10 +1672,14 @@ static u8_t notify_cb(const struct bt_gatt_attr *attr, void *user_data) conn = bt_conn_lookup_addr_le(cfg->id, &cfg->peer); if (!conn) { - if (IS_ENABLED(CONFIG_BT_GATT_SERVICE_CHANGED)) { - if (ccc->cfg_changed == sc_ccc_cfg_changed) { - sc_save(cfg, data->ind_params); - } + if (IS_ENABLED(CONFIG_BT_GATT_SERVICE_CHANGED) && + ccc == &sc_ccc) { + struct sc_data *sc; + + sc = (struct sc_data *)data->ind_params->data; + sc_save(cfg->id, &cfg->peer, + sys_le16_to_cpu(sc->start), + sys_le16_to_cpu(sc->end)); } continue; } @@ -1763,24 +1889,66 @@ u8_t bt_gatt_check_perm(struct bt_conn *conn, const struct bt_gatt_attr *attr, return 0; } -#if defined(CONFIG_BT_GATT_SERVICE_CHANGED) -static void sc_restore(struct bt_gatt_ccc_cfg *cfg) +static void sc_restore_rsp(struct bt_conn *conn, + const struct bt_gatt_attr *attr, u8_t err) { - struct sc_data *data = (struct sc_data *)cfg->data; +#if defined(CONFIG_BT_GATT_CACHING) + struct gatt_cf_cfg *cfg; +#endif - if (!data->start && !data->end) { + BT_DBG("err 0x%02x", err); + +#if defined(CONFIG_BT_GATT_CACHING) + /* BLUETOOTH CORE SPECIFICATION Version 5.1 | Vol 3, Part G page 2347: + * 2.5.2.1 Robust Caching + * A connected client becomes change-aware when... + * The client receives and confirms a Service Changed indication. + */ + cfg = find_cf_cfg(conn); + if (cfg && CF_ROBUST_CACHING(cfg)) { + atomic_set_bit(cfg->flags, CF_CHANGE_AWARE); + BT_DBG("%s change-aware", bt_addr_le_str(&cfg->peer)); + } +#endif +} + +static struct bt_gatt_indicate_params sc_restore_params[CONFIG_BT_MAX_CONN]; + +static void sc_restore(struct bt_conn *conn) +{ + struct gatt_sc_cfg *cfg; + u16_t sc_range[2]; + u8_t index; + + cfg = find_sc_cfg(conn->id, &conn->le.dst); + if (!cfg) { + BT_DBG("no SC data found"); + return; + } + + if (!(cfg->data.start || cfg->data.end)) { return; } BT_DBG("peer %s start 0x%04x end 0x%04x", bt_addr_le_str(&cfg->peer), - data->start, data->end); + cfg->data.start, cfg->data.end); - sc_indicate(&gatt_sc, data->start, data->end); + sc_range[0] = sys_cpu_to_le16(cfg->data.start); + sc_range[1] = sys_cpu_to_le16(cfg->data.end); + + index = bt_conn_index(conn); + sc_restore_params[index].attr = &_1_gatt_svc.attrs[2]; + sc_restore_params[index].func = sc_restore_rsp; + sc_restore_params[index].data = &sc_range[0]; + sc_restore_params[index].len = sizeof(sc_range); + + if (bt_gatt_indicate(conn, &sc_restore_params[index])) { + BT_ERR("SC restore indication failed"); + } /* Reset config data */ - (void)memset(cfg->data, 0, sizeof(cfg->data)); + sc_clear(cfg); } -#endif /* CONFIG_BT_GATT_SERVICE_CHANGED */ struct conn_data { struct bt_conn *conn; @@ -1836,10 +2004,9 @@ static u8_t update_ccc(const struct bt_gatt_attr *attr, void *user_data) if (ccc->cfg[i].value) { gatt_ccc_changed(attr, ccc); - if (IS_ENABLED(CONFIG_BT_GATT_SERVICE_CHANGED)) { - if (ccc->cfg_changed == sc_ccc_cfg_changed) { - sc_restore(&ccc->cfg[i]); - } + if (IS_ENABLED(CONFIG_BT_GATT_SERVICE_CHANGED) && + ccc == &sc_ccc) { + sc_restore(conn); } return BT_GATT_ITER_CONTINUE; } @@ -3694,6 +3861,20 @@ static int bt_gatt_clear_cf(u8_t id, const bt_addr_le_t *addr) return settings_delete(key); #endif /* CONFIG_BT_GATT_CACHING */ return 0; + +} + +static int sc_clear_by_addr(u8_t id, const bt_addr_le_t *addr) +{ + if (IS_ENABLED(CONFIG_BT_GATT_SERVICE_CHANGED)) { + struct gatt_sc_cfg *cfg; + + cfg = find_sc_cfg(id, (bt_addr_le_t *)addr); + if (cfg) { + sc_clear(cfg); + } + } + return 0; } int bt_gatt_clear(u8_t id, const bt_addr_le_t *addr) @@ -3705,7 +3886,17 @@ int bt_gatt_clear(u8_t id, const bt_addr_le_t *addr) return err; } - return bt_gatt_clear_cf(id, addr); + err = sc_clear_by_addr(id, addr); + if (err < 0) { + return err; + } + + err = bt_gatt_clear_cf(id, addr); + if (err < 0) { + return err; + } + + return 0; } static void ccc_clear(struct _bt_gatt_ccc *ccc, @@ -3845,6 +4036,82 @@ static int ccc_set(const char *name, size_t len_rd, settings_read_cb read_cb, SETTINGS_STATIC_HANDLER_DEFINE(bt_ccc, "bt/ccc", NULL, ccc_set, NULL, NULL); +#if defined(CONFIG_BT_GATT_SERVICE_CHANGED) +static int sc_set(const char *name, size_t len_rd, settings_read_cb read_cb, + void *cb_arg) +{ + struct gatt_sc_cfg *cfg; + u8_t id; + bt_addr_le_t addr; + int len, err; + const char *next; + + if (!name) { + BT_ERR("Insufficient number of arguments"); + return -EINVAL; + } + + err = bt_settings_decode_key(name, &addr); + if (err) { + BT_ERR("Unable to decode address %s", log_strdup(name)); + return -EINVAL; + } + + settings_name_next(name, &next); + + if (!next) { + id = BT_ID_DEFAULT; + } else { + id = strtol(next, NULL, 10); + } + + cfg = find_sc_cfg(id, &addr); + if (!cfg && len_rd) { + /* Find and initialize a free sc_cfg entry */ + cfg = find_sc_cfg(BT_ID_DEFAULT, BT_ADDR_LE_ANY); + if (!cfg) { + BT_ERR("Unable to restore SC: no cfg left"); + return -ENOMEM; + } + + cfg->id = id; + bt_addr_le_copy(&cfg->peer, &addr); + } + + if (len_rd) { + len = read_cb(cb_arg, &cfg->data, sizeof(cfg->data)); + if (len < 0) { + BT_ERR("Failed to decode value (err %d)", len); + return len; + } + BT_DBG("Read SC: len %d", len); + + BT_DBG("Restored SC for %s", bt_addr_le_str(&addr)); + } else if (cfg) { + /* Clear configuration */ + memset(cfg, 0, sizeof(*cfg)); + + BT_DBG("Removed SC for %s", bt_addr_le_str(&addr)); + } + + return 0; +} + +static int sc_commit(void) +{ + atomic_clear_bit(gatt_sc.flags, SC_INDICATE_PENDING); + + if (atomic_test_bit(gatt_sc.flags, SC_RANGE_CHANGED)) { + /* Schedule SC indication since the range has changed */ + k_delayed_work_submit(&gatt_sc.work, SC_TIMEOUT); + } + + return 0; +} + +SETTINGS_STATIC_HANDLER_DEFINE(bt_sc, "bt/sc", NULL, sc_set, sc_commit, NULL); +#endif /* CONFIG_BT_GATT_SERVICE_CHANGED */ + #if defined(CONFIG_BT_GATT_CACHING) static int cf_set(const char *name, size_t len_rd, settings_read_cb read_cb, void *cb_arg) @@ -3927,6 +4194,15 @@ static int db_hash_commit(void) BT_HEXDUMP_DBG(db_hash, sizeof(db_hash), "New Hash: "); + /** + * GATT database has been modified since last boot, likely due to + * a firmware update or a dynamic service that was not re-registered on + * boot. Indicate Service Changed to all bonded devices for the full + * database range to invalidate client-side cache and force discovery on + * reconnect. + */ + sc_indicate(0x0001, 0xffff); + /* Hash did not match overwrite with current hash */ db_hash_store();