From 1bff698b4ea099f8af4ce926e3972158419119f6 Mon Sep 17 00:00:00 2001 From: Myles Watson Date: Fri, 22 Mar 2019 16:51:39 -0700 Subject: [PATCH] GATT: Use a list for services in a GATT database Iterators to list elements are not invalidated by the addition and removal of other elements. Bug: 128938477 Test: net_test_bta Change-Id: I45d74a63fc6b55ece3a4af02e0cc3e1de757cc4d --- bta/gatt/bta_gattc_api.cc | 2 +- bta/gatt/bta_gattc_cache.cc | 13 ++- bta/gatt/bta_gattc_int.h | 3 +- bta/gatt/database.cc | 5 +- bta/gatt/database.h | 9 ++- bta/gatt/database_builder.h | 1 - bta/hearing_aid/hearing_aid.cc | 2 +- bta/hh/bta_hh_le.cc | 2 +- bta/include/bta_gatt_api.h | 3 +- .../gatt/database_builder_sample_device_test.cc | 94 ++++++++++------------ bta/test/gatt/database_builder_test.cc | 87 +++++++++++--------- 11 files changed, 108 insertions(+), 113 deletions(-) diff --git a/bta/gatt/bta_gattc_api.cc b/bta/gatt/bta_gattc_api.cc index 9b0bd9e24..829084c5b 100644 --- a/bta/gatt/bta_gattc_api.cc +++ b/bta/gatt/bta_gattc_api.cc @@ -269,7 +269,7 @@ void BTA_GATTC_DiscoverServiceByUuid(uint16_t conn_id, const Uuid& srvc_uuid) { * Returns returns list of gatt::Service or NULL. * ******************************************************************************/ -const std::vector* BTA_GATTC_GetServices(uint16_t conn_id) { +const std::list* BTA_GATTC_GetServices(uint16_t conn_id) { return bta_gattc_get_services(conn_id); } diff --git a/bta/gatt/bta_gattc_cache.cc b/bta/gatt/bta_gattc_cache.cc index 6c793f18e..f80b5e0d8 100644 --- a/bta/gatt/bta_gattc_cache.cc +++ b/bta/gatt/bta_gattc_cache.cc @@ -120,7 +120,7 @@ void bta_gattc_init_cache(tBTA_GATTC_SERV* p_srvc_cb) { } const Service* bta_gattc_find_matching_service( - const std::vector& services, uint16_t handle) { + const std::list& services, uint16_t handle) { for (const Service& service : services) { if (handle >= service.handle && handle <= service.end_handle) return &service; @@ -419,14 +419,13 @@ void bta_gattc_search_service(tBTA_GATTC_CLCB* p_clcb, Uuid* p_uuid) { } } -const std::vector* bta_gattc_get_services_srcb( - tBTA_GATTC_SERV* p_srcb) { +const std::list* bta_gattc_get_services_srcb(tBTA_GATTC_SERV* p_srcb) { if (!p_srcb || p_srcb->gatt_database.IsEmpty()) return NULL; return &p_srcb->gatt_database.Services(); } -const std::vector* bta_gattc_get_services(uint16_t conn_id) { +const std::list* bta_gattc_get_services(uint16_t conn_id) { tBTA_GATTC_CLCB* p_clcb = bta_gattc_find_clcb_by_conn_id(conn_id); if (p_clcb == NULL) return NULL; @@ -438,14 +437,14 @@ const std::vector* bta_gattc_get_services(uint16_t conn_id) { const Service* bta_gattc_get_service_for_handle_srcb(tBTA_GATTC_SERV* p_srcb, uint16_t handle) { - const std::vector* services = bta_gattc_get_services_srcb(p_srcb); + const std::list* services = bta_gattc_get_services_srcb(p_srcb); if (services == NULL) return NULL; return bta_gattc_find_matching_service(*services, handle); } const Service* bta_gattc_get_service_for_handle(uint16_t conn_id, uint16_t handle) { - const std::vector* services = bta_gattc_get_services(conn_id); + const std::list* services = bta_gattc_get_services(conn_id); if (services == NULL) return NULL; return bta_gattc_find_matching_service(*services, handle); @@ -556,7 +555,7 @@ void bta_gattc_fill_gatt_db_el(btgatt_db_element_t* p_attr, /******************************************************************************* * Returns number of elements inside db from start_handle to end_handle ******************************************************************************/ -static size_t bta_gattc_get_db_size(const std::vector& services, +static size_t bta_gattc_get_db_size(const std::list& services, uint16_t start_handle, uint16_t end_handle) { if (services.empty()) return 0; diff --git a/bta/gatt/bta_gattc_int.h b/bta/gatt/bta_gattc_int.h index 2b60ac720..e1d8cf5d8 100644 --- a/bta/gatt/bta_gattc_int.h +++ b/bta/gatt/bta_gattc_int.h @@ -426,8 +426,7 @@ extern tGATT_STATUS bta_gattc_discover_pri_service(uint16_t conn_id, uint8_t disc_type); extern void bta_gattc_search_service(tBTA_GATTC_CLCB* p_clcb, bluetooth::Uuid* p_uuid); -extern const std::vector* bta_gattc_get_services( - uint16_t conn_id); +extern const std::list* bta_gattc_get_services(uint16_t conn_id); extern const gatt::Service* bta_gattc_get_service_for_handle(uint16_t conn_id, uint16_t handle); const gatt::Characteristic* bta_gattc_get_characteristic_srcb( diff --git a/bta/gatt/database.cc b/bta/gatt/database.cc index 5f99d559e..1486b0827 100644 --- a/bta/gatt/database.cc +++ b/bta/gatt/database.cc @@ -21,6 +21,7 @@ #include "stack/include/gattdefs.h" #include +#include #include #include @@ -39,7 +40,7 @@ bool HandleInRange(const Service& svc, uint16_t handle) { } } // namespace -Service* FindService(std::vector& services, uint16_t handle) { +Service* FindService(std::list& services, uint16_t handle) { for (Service& service : services) { if (handle >= service.handle && handle <= service.end_handle) return &service; @@ -182,4 +183,4 @@ Database Database::Deserialize(const std::vector& nv_attr, return result; } -} // namespace gatt \ No newline at end of file +} // namespace gatt diff --git a/bta/gatt/database.h b/bta/gatt/database.h index 1e74809ef..d1ba836e5 100644 --- a/bta/gatt/database.h +++ b/bta/gatt/database.h @@ -18,6 +18,7 @@ #pragma once +#include #include #include #include @@ -101,10 +102,10 @@ class Database { /* Clear the GATT database. This method forces relocation to ensure no extra * space is used unnecesarly */ - void Clear() { std::vector().swap(services); } + void Clear() { std::list().swap(services); } /* Return list of services available in this database */ - const std::vector& Services() const { return services; } + const std::list& Services() const { return services; } std::string ToString() const; @@ -116,11 +117,11 @@ class Database { friend class DatabaseBuilder; private: - std::vector services; + std::list services; }; /* Find a service that should contain handle. Helper method for internal use * inside gatt namespace.*/ -Service* FindService(std::vector& services, uint16_t handle); +Service* FindService(std::list& services, uint16_t handle); } // namespace gatt diff --git a/bta/gatt/database_builder.h b/bta/gatt/database_builder.h index 0913100fd..d2cc72054 100644 --- a/bta/gatt/database_builder.h +++ b/bta/gatt/database_builder.h @@ -21,7 +21,6 @@ #include "gatt/database.h" #include -#include namespace gatt { diff --git a/bta/hearing_aid/hearing_aid.cc b/bta/hearing_aid/hearing_aid.cc index e07b3286d..a9d567b57 100644 --- a/bta/hearing_aid/hearing_aid.cc +++ b/bta/hearing_aid/hearing_aid.cc @@ -602,7 +602,7 @@ class HearingAidImpl : public HearingAid { return; } - const std::vector* services = BTA_GATTC_GetServices(conn_id); + const std::list* services = BTA_GATTC_GetServices(conn_id); const gatt::Service* service = nullptr; for (const gatt::Service& tmp : *services) { diff --git a/bta/hh/bta_hh_le.cc b/bta/hh/bta_hh_le.cc index eb87cdff0..510e3a483 100644 --- a/bta/hh/bta_hh_le.cc +++ b/bta/hh/bta_hh_le.cc @@ -1490,7 +1490,7 @@ void bta_hh_le_srvc_search_cmpl(tBTA_GATTC_SEARCH_CMPL* p_data) { return; } - const std::vector* services = + const std::list* services = BTA_GATTC_GetServices(p_data->conn_id); bool have_hid = false; diff --git a/bta/include/bta_gatt_api.h b/bta/include/bta_gatt_api.h index 917650154..41151f813 100644 --- a/bta/include/bta_gatt_api.h +++ b/bta/include/bta_gatt_api.h @@ -491,8 +491,7 @@ extern void BTA_GATTC_DiscoverServiceByUuid(uint16_t conn_id, * Returns returns list of gatt::Service or NULL. * ******************************************************************************/ -extern const std::vector* BTA_GATTC_GetServices( - uint16_t conn_id); +extern const std::list* BTA_GATTC_GetServices(uint16_t conn_id); /******************************************************************************* * diff --git a/bta/test/gatt/database_builder_sample_device_test.cc b/bta/test/gatt/database_builder_sample_device_test.cc index bd4dabc71..2bfaa0425 100644 --- a/bta/test/gatt/database_builder_sample_device_test.cc +++ b/bta/test/gatt/database_builder_sample_device_test.cc @@ -19,6 +19,7 @@ #include #include +#include #include #include "gatt/database_builder.h" @@ -223,64 +224,51 @@ TEST(DatabaseBuilderSampleDeviceTest, DoDiscovery) { EXPECT_FALSE(builder.InProgress()); // verify that the returned database matches what was discovered - EXPECT_EQ(result.Services()[0].handle, 0x0001); - EXPECT_EQ(result.Services()[0].uuid, SERVICE_1_UUID); - EXPECT_EQ(result.Services()[1].uuid, SERVICE_2_UUID); - EXPECT_EQ(result.Services()[2].uuid, SERVICE_3_UUID); - EXPECT_EQ(result.Services()[3].uuid, SERVICE_4_UUID); - EXPECT_EQ(result.Services()[4].uuid, SERVICE_5_UUID); - EXPECT_EQ(result.Services()[5].uuid, SERVICE_6_UUID); - - EXPECT_EQ(result.Services()[0].characteristics[0].uuid, - SERVICE_1_CHAR_1_UUID); - EXPECT_EQ(result.Services()[0].characteristics[1].uuid, - SERVICE_1_CHAR_2_UUID); - EXPECT_EQ(result.Services()[0].characteristics[2].uuid, - SERVICE_1_CHAR_3_UUID); - - EXPECT_EQ(result.Services()[2].characteristics[0].uuid, - SERVICE_3_CHAR_1_UUID); - EXPECT_EQ(result.Services()[2].characteristics[0].descriptors[0].uuid, + auto service = result.Services().begin(); + EXPECT_EQ(service->handle, 0x0001); + EXPECT_EQ(service->uuid, SERVICE_1_UUID); + + EXPECT_EQ(service->characteristics[0].uuid, SERVICE_1_CHAR_1_UUID); + EXPECT_EQ(service->characteristics[1].uuid, SERVICE_1_CHAR_2_UUID); + EXPECT_EQ(service->characteristics[2].uuid, SERVICE_1_CHAR_3_UUID); + + service++; + EXPECT_EQ(service->uuid, SERVICE_2_UUID); + + service++; + EXPECT_EQ(service->uuid, SERVICE_3_UUID); + EXPECT_EQ(service->characteristics[0].uuid, SERVICE_3_CHAR_1_UUID); + EXPECT_EQ(service->characteristics[0].descriptors[0].uuid, SERVICE_3_CHAR_1_DESC_1_UUID); - EXPECT_EQ(result.Services()[3].characteristics[0].uuid, - SERVICE_4_CHAR_1_UUID); - EXPECT_EQ(result.Services()[3].characteristics[1].uuid, - SERVICE_4_CHAR_2_UUID); - EXPECT_EQ(result.Services()[3].characteristics[2].uuid, - SERVICE_4_CHAR_3_UUID); - EXPECT_EQ(result.Services()[3].characteristics[3].uuid, - SERVICE_4_CHAR_4_UUID); - EXPECT_EQ(result.Services()[3].characteristics[4].uuid, - SERVICE_4_CHAR_5_UUID); - EXPECT_EQ(result.Services()[3].characteristics[5].uuid, - SERVICE_4_CHAR_6_UUID); - EXPECT_EQ(result.Services()[3].characteristics[5].descriptors[0].uuid, + service++; + EXPECT_EQ(service->uuid, SERVICE_4_UUID); + EXPECT_EQ(service->characteristics[0].uuid, SERVICE_4_CHAR_1_UUID); + EXPECT_EQ(service->characteristics[1].uuid, SERVICE_4_CHAR_2_UUID); + EXPECT_EQ(service->characteristics[2].uuid, SERVICE_4_CHAR_3_UUID); + EXPECT_EQ(service->characteristics[3].uuid, SERVICE_4_CHAR_4_UUID); + EXPECT_EQ(service->characteristics[4].uuid, SERVICE_4_CHAR_5_UUID); + EXPECT_EQ(service->characteristics[5].uuid, SERVICE_4_CHAR_6_UUID); + EXPECT_EQ(service->characteristics[5].descriptors[0].uuid, SERVICE_4_CHAR_6_DESC_1_UUID); - EXPECT_EQ(result.Services()[4].characteristics[0].uuid, - SERVICE_5_CHAR_1_UUID); - EXPECT_EQ(result.Services()[4].characteristics[1].uuid, - SERVICE_5_CHAR_2_UUID); - EXPECT_EQ(result.Services()[4].characteristics[2].uuid, - SERVICE_5_CHAR_3_UUID); - EXPECT_EQ(result.Services()[4].characteristics[3].uuid, - SERVICE_5_CHAR_4_UUID); - EXPECT_EQ(result.Services()[4].characteristics[4].uuid, - SERVICE_5_CHAR_5_UUID); - EXPECT_EQ(result.Services()[4].characteristics[5].uuid, - SERVICE_5_CHAR_6_UUID); - EXPECT_EQ(result.Services()[4].characteristics[6].uuid, - SERVICE_5_CHAR_7_UUID); - - EXPECT_EQ(result.Services()[5].characteristics[0].uuid, - SERVICE_6_CHAR_1_UUID); - EXPECT_EQ(result.Services()[5].characteristics[0].descriptors[0].uuid, + service++; + EXPECT_EQ(service->uuid, SERVICE_5_UUID); + EXPECT_EQ(service->characteristics[0].uuid, SERVICE_5_CHAR_1_UUID); + EXPECT_EQ(service->characteristics[1].uuid, SERVICE_5_CHAR_2_UUID); + EXPECT_EQ(service->characteristics[2].uuid, SERVICE_5_CHAR_3_UUID); + EXPECT_EQ(service->characteristics[3].uuid, SERVICE_5_CHAR_4_UUID); + EXPECT_EQ(service->characteristics[4].uuid, SERVICE_5_CHAR_5_UUID); + EXPECT_EQ(service->characteristics[5].uuid, SERVICE_5_CHAR_6_UUID); + EXPECT_EQ(service->characteristics[6].uuid, SERVICE_5_CHAR_7_UUID); + + service++; + EXPECT_EQ(service->uuid, SERVICE_6_UUID); + EXPECT_EQ(service->characteristics[0].uuid, SERVICE_6_CHAR_1_UUID); + EXPECT_EQ(service->characteristics[0].descriptors[0].uuid, SERVICE_6_CHAR_1_DESC_1_UUID); - EXPECT_EQ(result.Services()[5].characteristics[1].uuid, - SERVICE_6_CHAR_2_UUID); - EXPECT_EQ(result.Services()[5].characteristics[2].uuid, - SERVICE_6_CHAR_3_UUID); + EXPECT_EQ(service->characteristics[1].uuid, SERVICE_6_CHAR_2_UUID); + EXPECT_EQ(service->characteristics[2].uuid, SERVICE_6_CHAR_3_UUID); } } // namespace gatt \ No newline at end of file diff --git a/bta/test/gatt/database_builder_test.cc b/bta/test/gatt/database_builder_test.cc index fcafb49e4..0a7e9280d 100644 --- a/bta/test/gatt/database_builder_test.cc +++ b/bta/test/gatt/database_builder_test.cc @@ -19,6 +19,7 @@ #include #include +#include #include #include "gatt/database_builder.h" @@ -59,10 +60,11 @@ TEST(DatabaseBuilderTest, EmptyServiceAddTest) { Database result = builder.Build(); // verify that the returned database matches what was discovered - EXPECT_EQ(result.Services()[0].handle, 0x0001); - EXPECT_EQ(result.Services()[0].end_handle, 0x0001); - EXPECT_EQ(result.Services()[0].is_primary, true); - EXPECT_EQ(result.Services()[0].uuid, SERVICE_1_UUID); + auto service = result.Services().begin(); + EXPECT_EQ(service->handle, 0x0001); + EXPECT_EQ(service->end_handle, 0x0001); + EXPECT_EQ(service->is_primary, true); + EXPECT_EQ(service->uuid, SERVICE_1_UUID); } /* Verify adding service, characteristic and descriptor work */ @@ -79,21 +81,20 @@ TEST(DatabaseBuilderTest, DescriptorAddTest) { Database result = builder.Build(); // verify that the returned database matches what was discovered - EXPECT_EQ(result.Services()[0].handle, 0x0001); - EXPECT_EQ(result.Services()[0].end_handle, 0x000f); - EXPECT_EQ(result.Services()[0].is_primary, true); - EXPECT_EQ(result.Services()[0].uuid, SERVICE_1_UUID); - - EXPECT_EQ(result.Services()[0].characteristics[0].uuid, - SERVICE_1_CHAR_1_UUID); - EXPECT_EQ(result.Services()[0].characteristics[0].declaration_handle, 0x0002); - EXPECT_EQ(result.Services()[0].characteristics[0].value_handle, 0x0003); - EXPECT_EQ(result.Services()[0].characteristics[0].properties, 0x02); - - EXPECT_EQ(result.Services()[0].characteristics[0].descriptors[0].uuid, + auto service = result.Services().begin(); + EXPECT_EQ(service->handle, 0x0001); + EXPECT_EQ(service->end_handle, 0x000f); + EXPECT_EQ(service->is_primary, true); + EXPECT_EQ(service->uuid, SERVICE_1_UUID); + + EXPECT_EQ(service->characteristics[0].uuid, SERVICE_1_CHAR_1_UUID); + EXPECT_EQ(service->characteristics[0].declaration_handle, 0x0002); + EXPECT_EQ(service->characteristics[0].value_handle, 0x0003); + EXPECT_EQ(service->characteristics[0].properties, 0x02); + + EXPECT_EQ(service->characteristics[0].descriptors[0].uuid, SERVICE_1_CHAR_1_DESC_1_UUID); - EXPECT_EQ(result.Services()[0].characteristics[0].descriptors[0].handle, - 0x0004); + EXPECT_EQ(service->characteristics[0].descriptors[0].handle, 0x0004); } /* This test verifies that DatabaseBuilder properly handle discovery of @@ -139,27 +140,35 @@ TEST(DatabaseBuilderTest, SecondaryServiceOutOfOrderTest) { Database result = builder.Build(); // verify that the returned database matches what was discovered - EXPECT_EQ(result.Services()[0].handle, 0x0001); - EXPECT_EQ(result.Services()[0].is_primary, true); - EXPECT_EQ(result.Services()[0].uuid, SERVICE_1_UUID); - - EXPECT_EQ(result.Services()[1].handle, 0x0020); - EXPECT_EQ(result.Services()[1].end_handle, 0x002f); - EXPECT_EQ(result.Services()[1].uuid, SERVICE_2_UUID); - EXPECT_EQ(result.Services()[1].is_primary, false); - - EXPECT_EQ(result.Services()[2].handle, 0x0030); - EXPECT_EQ(result.Services()[2].end_handle, 0x003f); - EXPECT_EQ(result.Services()[2].uuid, SERVICE_3_UUID); - EXPECT_EQ(result.Services()[2].is_primary, true); - - EXPECT_EQ(result.Services()[3].handle, 0x0040); - EXPECT_EQ(result.Services()[3].uuid, SERVICE_4_UUID); - EXPECT_EQ(result.Services()[3].is_primary, false); - - EXPECT_EQ(result.Services()[4].handle, 0x0050); - EXPECT_EQ(result.Services()[4].uuid, SERVICE_5_UUID); - EXPECT_EQ(result.Services()[4].is_primary, true); + auto service = result.Services().begin(); + EXPECT_EQ(service->handle, 0x0001); + EXPECT_EQ(service->is_primary, true); + EXPECT_EQ(service->uuid, SERVICE_1_UUID); + + service++; + EXPECT_EQ(service->handle, 0x0020); + EXPECT_EQ(service->end_handle, 0x002f); + EXPECT_EQ(service->uuid, SERVICE_2_UUID); + EXPECT_EQ(service->is_primary, false); + + service++; + EXPECT_EQ(service->handle, 0x0030); + EXPECT_EQ(service->end_handle, 0x003f); + EXPECT_EQ(service->uuid, SERVICE_3_UUID); + EXPECT_EQ(service->is_primary, true); + + service++; + EXPECT_EQ(service->handle, 0x0040); + EXPECT_EQ(service->uuid, SERVICE_4_UUID); + EXPECT_EQ(service->is_primary, false); + + service++; + EXPECT_EQ(service->handle, 0x0050); + EXPECT_EQ(service->uuid, SERVICE_5_UUID); + EXPECT_EQ(service->is_primary, true); + + service++; + ASSERT_EQ(service, result.Services().end()); } } // namespace gatt \ No newline at end of file -- 2.11.0