From d366807102bce97e494e83570d757ebcb6a33b3c Mon Sep 17 00:00:00 2001 From: Jack He Date: Thu, 14 Sep 2017 17:13:19 -0700 Subject: [PATCH] Clean-up BTIF profile queue on profile shutdown * Add btif_profile_cleanup(uuid) method to remove pending connection requests for individual UUIDs * Call the above method in each profile's clean-up method * Add unit tests for btif_profile_queue Bug: 63790458 Test: make, unit tests, pair and connect car kits Change-Id: I28288c295b7ca0259b2112c11b4e5a81d6f2e33c --- btif/Android.bp | 22 ++++ btif/include/btif_profile_queue.h | 1 + btif/src/btif_av.cc | 2 + btif/src/btif_hf.cc | 1 + btif/src/btif_hf_client.cc | 1 + btif/src/btif_profile_queue.cc | 47 ++++++++ btif/test/btif_profile_queue_test.cc | 203 +++++++++++++++++++++++++++++++++++ test/run_unit_tests.sh | 1 + 8 files changed, 278 insertions(+) create mode 100644 btif/test/btif_profile_queue_test.cc diff --git a/btif/Android.bp b/btif/Android.bp index 6cd84059a..c5ff0765f 100644 --- a/btif/Android.bp +++ b/btif/Android.bp @@ -120,3 +120,25 @@ cc_test { ], cflags: ["-DBUILDCFG"], } + +// btif profile queue unit tests for target +// ======================================================== +cc_test { + name: "net_test_btif_profile_queue", + defaults: ["fluoride_defaults"], + include_dirs: btifCommonIncludes, + srcs: [ + "src/btif_profile_queue.cc", + "test/btif_profile_queue_test.cc" + ], + shared_libs: [ + "liblog", + "libhardware", + "libcutils", + ], + static_libs: [ + "libbluetooth-types", + "libosi", + ], + cflags: ["-DBUILDCFG"], +} diff --git a/btif/include/btif_profile_queue.h b/btif/include/btif_profile_queue.h index be3ff0888..df48c7733 100644 --- a/btif/include/btif_profile_queue.h +++ b/btif/include/btif_profile_queue.h @@ -33,6 +33,7 @@ typedef bt_status_t (*btif_connect_cb_t)(RawAddress* bda, uint16_t uuid); bt_status_t btif_queue_connect(uint16_t uuid, const RawAddress* bda, btif_connect_cb_t connect_cb); +void btif_queue_cleanup(uint16_t uuid); void btif_queue_advance(); bt_status_t btif_queue_connect_next(void); void btif_queue_release(); diff --git a/btif/src/btif_av.cc b/btif/src/btif_av.cc index 95a4d4b77..a69f0e5ca 100644 --- a/btif/src/btif_av.cc +++ b/btif/src/btif_av.cc @@ -1410,6 +1410,7 @@ static void cleanup(int service_uuid) { static void cleanup_src(void) { BTIF_TRACE_EVENT("%s", __func__); + btif_queue_cleanup(UUID_SERVCLASS_AUDIO_SOURCE); if (bt_av_src_callbacks) { bt_av_src_callbacks = NULL; if (bt_av_sink_callbacks == NULL) cleanup(BTA_A2DP_SOURCE_SERVICE_ID); @@ -1419,6 +1420,7 @@ static void cleanup_src(void) { static void cleanup_sink(void) { BTIF_TRACE_EVENT("%s", __func__); + btif_queue_cleanup(UUID_SERVCLASS_AUDIO_SINK); if (bt_av_sink_callbacks) { bt_av_sink_callbacks = NULL; if (bt_av_src_callbacks == NULL) cleanup(BTA_A2DP_SINK_SERVICE_ID); diff --git a/btif/src/btif_hf.cc b/btif/src/btif_hf.cc index 538d7d923..6fe571e41 100644 --- a/btif/src/btif_hf.cc +++ b/btif/src/btif_hf.cc @@ -1494,6 +1494,7 @@ bool btif_hf_call_terminated_recently() { static void cleanup(void) { BTIF_TRACE_EVENT("%s", __func__); + btif_queue_cleanup(UUID_SERVCLASS_AG_HANDSFREE); if (bt_hf_callbacks) { #if (defined(BTIF_HF_SERVICES) && (BTIF_HF_SERVICES & BTA_HFP_SERVICE_MASK)) btif_disable_service(BTA_HFP_SERVICE_ID); diff --git a/btif/src/btif_hf_client.cc b/btif/src/btif_hf_client.cc index 16af6ff7e..c1f3c8b09 100644 --- a/btif/src/btif_hf_client.cc +++ b/btif/src/btif_hf_client.cc @@ -704,6 +704,7 @@ static bt_status_t request_last_voice_tag_number(const RawAddress* bd_addr) { static void cleanup(void) { BTIF_TRACE_EVENT("%s", __func__); + btif_queue_cleanup(UUID_SERVCLASS_HF_HANDSFREE); if (bt_hf_client_callbacks) { btif_disable_service(BTA_HFP_HS_SERVICE_ID); bt_hf_client_callbacks = NULL; diff --git a/btif/src/btif_profile_queue.cc b/btif/src/btif_profile_queue.cc index 591178f2b..152f11dde 100644 --- a/btif/src/btif_profile_queue.cc +++ b/btif/src/btif_profile_queue.cc @@ -44,6 +44,7 @@ typedef enum { BTIF_QUEUE_CONNECT_EVT, BTIF_QUEUE_ADVANCE_EVT, + BTIF_QUEUE_CLEANUP_EVT } btif_queue_event_t; typedef struct { @@ -67,6 +68,7 @@ static const size_t MAX_REASONABLE_REQUESTS = 10; static void queue_int_add(connect_node_t* p_param) { if (!connect_queue) { + LOG_INFO(LOG_TAG, "%s: allocating profile queue", __func__); connect_queue = list_new(osi_free); CHECK(connect_queue != NULL); } @@ -105,6 +107,32 @@ static void queue_int_advance() { } } +static void queue_int_cleanup(uint16_t* p_uuid) { + if (!p_uuid) { + LOG_ERROR(LOG_TAG, "%s: UUID is null", __func__); + return; + } + uint16_t uuid = *p_uuid; + LOG_INFO(LOG_TAG, "%s: UUID=%04X", __func__, uuid); + if (!connect_queue) { + return; + } + connect_node_t* connection_request; + const list_node_t* node = list_begin(connect_queue); + while (node && node != list_end(connect_queue)) { + connection_request = (connect_node_t*)list_node(node); + node = list_next(node); + if (connection_request->uuid == uuid) { + LOG_INFO(LOG_TAG, + "%s: removing connection request UUID=%04X, bd_addr=%s, busy=%d", + __func__, connection_request->uuid, + connection_request->bda.ToString().c_str(), + connection_request->busy); + list_remove(connect_queue, connection_request); + } + } +} + static void queue_int_handle_evt(uint16_t event, char* p_param) { switch (event) { case BTIF_QUEUE_CONNECT_EVT: @@ -114,6 +142,10 @@ static void queue_int_handle_evt(uint16_t event, char* p_param) { case BTIF_QUEUE_ADVANCE_EVT: queue_int_advance(); break; + + case BTIF_QUEUE_CLEANUP_EVT: + queue_int_cleanup((uint16_t*)(p_param)); + return; } if (stack_manager_get_interface()->get_stack_is_running()) @@ -144,6 +176,20 @@ bt_status_t btif_queue_connect(uint16_t uuid, const RawAddress* bda, /******************************************************************************* * + * Function btif_queue_cleanup + * + * Description Clean up existing connection requests for a UUID + * + * Returns void, always succeed + * + ******************************************************************************/ +void btif_queue_cleanup(uint16_t uuid) { + btif_transfer_context(queue_int_handle_evt, BTIF_QUEUE_CLEANUP_EVT, + (char*)&uuid, sizeof(uint16_t), NULL); +} + +/******************************************************************************* + * * Function btif_queue_advance * * Description Clear the queue's busy status and advance to the next @@ -186,6 +232,7 @@ bt_status_t btif_queue_connect_next(void) { * ******************************************************************************/ void btif_queue_release() { + LOG_INFO(LOG_TAG, "%s", __func__); list_free(connect_queue); connect_queue = NULL; } diff --git a/btif/test/btif_profile_queue_test.cc b/btif/test/btif_profile_queue_test.cc new file mode 100644 index 000000000..79a283fba --- /dev/null +++ b/btif/test/btif_profile_queue_test.cc @@ -0,0 +1,203 @@ +/****************************************************************************** + * + * Copyright (C) 2017 Google, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + ******************************************************************************/ +#include + +#include "btif/include/btif_profile_queue.h" +#include "stack_manager.h" +#include "types/raw_address.h" + +static bool sStackRunning; + +bool get_stack_is_running(void) { return sStackRunning; } + +static stack_manager_t sStackManager = {nullptr, nullptr, nullptr, nullptr, + get_stack_is_running}; + +const stack_manager_t* stack_manager_get_interface() { return &sStackManager; } + +typedef void(tBTIF_CBACK)(uint16_t event, char* p_param); +typedef void(tBTIF_COPY_CBACK)(uint16_t event, char* p_dest, char* p_src); +bt_status_t btif_transfer_context(tBTIF_CBACK* p_cback, uint16_t event, + char* p_params, int param_len, + tBTIF_COPY_CBACK* p_copy_cback) { + p_cback(event, p_params); + return BT_STATUS_SUCCESS; +} + +enum ResultType { + NOT_SET = 0, + UNKNOWN, + UUID1_ADDR1, + UUID1_ADDR2, + UUID2_ADDR1, + UUID2_ADDR2 +}; + +static ResultType sResult; + +class BtifProfileQueueTest : public ::testing::Test { + public: + static const uint16_t kTestUuid1 = 0x9527; + static const uint16_t kTestUuid2 = 0x819F; + static const RawAddress kTestAddr1; + static const RawAddress kTestAddr2; + + protected: + void SetUp() override { + sStackRunning = true; + sResult = NOT_SET; + }; + void TearDown() override { btif_queue_release(); }; +}; + +const RawAddress BtifProfileQueueTest::kTestAddr1{ + {0x11, 0x22, 0x33, 0x44, 0x55, 0x66}}; +const RawAddress BtifProfileQueueTest::kTestAddr2{ + {0xAB, 0xCD, 0xEF, 0x12, 0x34, 0x56}}; + +static bt_status_t test_connect_cb(RawAddress* bda, uint16_t uuid) { + sResult = UNKNOWN; + if (*bda == BtifProfileQueueTest::kTestAddr1) { + if (uuid == BtifProfileQueueTest::kTestUuid1) { + sResult = UUID1_ADDR1; + } else if (uuid == BtifProfileQueueTest::kTestUuid2) { + sResult = UUID2_ADDR1; + } + } else if (*bda == BtifProfileQueueTest::kTestAddr2) { + if (uuid == BtifProfileQueueTest::kTestUuid1) { + sResult = UUID1_ADDR2; + } else if (uuid == BtifProfileQueueTest::kTestUuid2) { + sResult = UUID2_ADDR2; + } + } + return BT_STATUS_SUCCESS; +} + +TEST_F(BtifProfileQueueTest, test_connect) { + sResult = NOT_SET; + btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb); + EXPECT_EQ(sResult, UUID1_ADDR1); +} + +TEST_F(BtifProfileQueueTest, test_connect_same_uuid_do_not_repeat) { + sResult = NOT_SET; + btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb); + EXPECT_EQ(sResult, UUID1_ADDR1); + // Second connection request on the same UUID do not repeat + sResult = NOT_SET; + btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb); + EXPECT_EQ(sResult, NOT_SET); + // Not even after we advance the queue + sResult = NOT_SET; + btif_queue_advance(); + btif_queue_connect_next(); + EXPECT_EQ(sResult, NOT_SET); +} + +TEST_F(BtifProfileQueueTest, test_multiple_connects) { + // First item is executed + sResult = NOT_SET; + btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb); + EXPECT_EQ(sResult, UUID1_ADDR1); + // Second item with advance is executed + sResult = NOT_SET; + btif_queue_advance(); + btif_queue_connect(kTestUuid2, &kTestAddr1, test_connect_cb); + EXPECT_EQ(sResult, UUID2_ADDR1); +} + +TEST_F(BtifProfileQueueTest, test_multiple_connects_without_advance) { + // First item is executed + sResult = NOT_SET; + btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb); + EXPECT_EQ(sResult, UUID1_ADDR1); + // Second item without advance is not executed + sResult = NOT_SET; + btif_queue_connect(kTestUuid2, &kTestAddr1, test_connect_cb); + EXPECT_EQ(sResult, NOT_SET); + sResult = NOT_SET; + // Connect next doesn't work + btif_queue_connect_next(); + EXPECT_EQ(sResult, NOT_SET); + // Advance moves queue to execute next item + sResult = NOT_SET; + btif_queue_advance(); + EXPECT_EQ(sResult, UUID2_ADDR1); +} + +TEST_F(BtifProfileQueueTest, test_cleanup_first_allow_second) { + // First item is executed + sResult = NOT_SET; + btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb); + EXPECT_EQ(sResult, UUID1_ADDR1); + // Second item without advance is not executed + sResult = NOT_SET; + btif_queue_connect(kTestUuid2, &kTestAddr1, test_connect_cb); + EXPECT_EQ(sResult, NOT_SET); + // Connect next doesn't work + sResult = NOT_SET; + btif_queue_connect_next(); + EXPECT_EQ(sResult, NOT_SET); + // Cleanup UUID1 allows the next profile connection to be executed + sResult = NOT_SET; + btif_queue_cleanup(kTestUuid1); + btif_queue_connect_next(); + EXPECT_EQ(sResult, UUID2_ADDR1); +} + +TEST_F(BtifProfileQueueTest, test_cleanup_both) { + // First item is executed + sResult = NOT_SET; + btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb); + EXPECT_EQ(sResult, UUID1_ADDR1); + // Second item without advance is not executed + sResult = NOT_SET; + btif_queue_connect(kTestUuid2, &kTestAddr1, test_connect_cb); + EXPECT_EQ(sResult, NOT_SET); + // Connect next doesn't work + sResult = NOT_SET; + btif_queue_connect_next(); + EXPECT_EQ(sResult, NOT_SET); + // Cleanup both leaves nothing to execute + sResult = NOT_SET; + btif_queue_cleanup(kTestUuid1); + btif_queue_cleanup(kTestUuid2); + btif_queue_connect_next(); + EXPECT_EQ(sResult, NOT_SET); +} + +TEST_F(BtifProfileQueueTest, test_cleanup_both_reverse_order) { + // First item is executed + sResult = NOT_SET; + btif_queue_connect(kTestUuid1, &kTestAddr1, test_connect_cb); + EXPECT_EQ(sResult, UUID1_ADDR1); + // Second item without advance is not executed + sResult = NOT_SET; + btif_queue_connect(kTestUuid2, &kTestAddr1, test_connect_cb); + EXPECT_EQ(sResult, NOT_SET); + // Connect next doesn't work + sResult = NOT_SET; + btif_queue_connect_next(); + EXPECT_EQ(sResult, NOT_SET); + // Cleanup both in reverse order leaves nothing to execute + sResult = NOT_SET; + btif_queue_cleanup(kTestUuid2); + btif_queue_cleanup(kTestUuid1); + btif_queue_connect_next(); + EXPECT_EQ(sResult, NOT_SET); +} diff --git a/test/run_unit_tests.sh b/test/run_unit_tests.sh index 9198cbba9..ab9ecc2a7 100755 --- a/test/run_unit_tests.sh +++ b/test/run_unit_tests.sh @@ -7,6 +7,7 @@ known_tests=( net_test_btcore net_test_bta net_test_btif + net_test_btif_profile_queue net_test_device net_test_hci net_test_stack -- 2.11.0