From c6bca57633e2707c6c637b7e206c85e253cba636 Mon Sep 17 00:00:00 2001 From: Vincent Coubard Date: Tue, 27 Feb 2018 10:56:36 +0000 Subject: [PATCH] BLE: Improve generic gatt client tests documentation. --- .../TestCharacteristicDesctiptorDiscovery.cpp | 55 +++++--------- .../GattClient/TestDiscoverAllServices.cpp | 75 ++++++++++++++++++- .../tests/generic/GattClient/TestNoCb.cpp | 14 +++- .../tests/generic/GattClient/TestRead.cpp | 10 +++ .../generic/GattClient/TestServerEvent.cpp | 11 +++ .../tests/generic/GattClient/TestWrite.cpp | 36 ++++++++- 6 files changed, 155 insertions(+), 46 deletions(-) diff --git a/features/FEATURE_BLE/tests/generic/GattClient/TestCharacteristicDesctiptorDiscovery.cpp b/features/FEATURE_BLE/tests/generic/GattClient/TestCharacteristicDesctiptorDiscovery.cpp index 193d97c55b..b8f25ff78c 100644 --- a/features/FEATURE_BLE/tests/generic/GattClient/TestCharacteristicDesctiptorDiscovery.cpp +++ b/features/FEATURE_BLE/tests/generic/GattClient/TestCharacteristicDesctiptorDiscovery.cpp @@ -84,6 +84,9 @@ struct ConstructibleDiscoveredCharacteristic : public DiscoveredCharacteristic { } }; +/** + * Test fixture used for client descriptor discovery testing. + */ class TestGattClientDescriptorDiscovery : public ::testing::Test { protected: TestGattClientDescriptorDiscovery() : @@ -179,8 +182,18 @@ TEST_F(TestGattClientDescriptorDiscovery, descriptor_discovery_on_characteristic EXPECT_EQ(err, BLE_ERROR_NONE); } +/** + * Test parameter pass into tests using TestGattClientDescriptorDiscoveryP fixture. + * - first element: value handle of the characteristic. + * - second element: last handle of the characteristic + * - third element: expected transactions; each transaction can contain multiple + * pair containing the handle of the descriptor and its UUID. + */ typedef tuple>>> test_param_t; +/** + * Parametric fixture used for descriptor discovery testing. + */ class TestGattClientDescriptorDiscoveryP : public TestGattClientDescriptorDiscovery, public ::testing::WithParamInterface { @@ -224,16 +237,20 @@ struct MockFindInformationResponse : public AttFindInformationResponse { vector> _response; }; - +/** + * Helper returning a DiscoveredCharacteristic from a DiscoveryCallbackParams_t* + */ static const DiscoveredCharacteristic& get_characteristic(const CharacteristicDescriptorDiscovery::DiscoveryCallbackParams_t* p) { return p->characteristic; } +/** + * Helper returning a DiscoveredCharacteristicDescriptor from a DiscoveryCallbackParams_t* + */ static const DiscoveredCharacteristicDescriptor& get_descriptor(const CharacteristicDescriptorDiscovery::DiscoveryCallbackParams_t* p) { return p->descriptor; } - /* * Given a discovered characteristic with the value handle not equal to the * last handle of the characteristic. @@ -333,6 +350,7 @@ TEST_P(TestGattClientDescriptorDiscoveryP, descriptor_discovery) { EXPECT_EQ(err, BLE_ERROR_NONE); } +// Instantiation of the tests cases relying on the parametric fixture INSTANTIATE_TEST_CASE_P( TestGattClientDescriptorDiscoveryP_combination, TestGattClientDescriptorDiscoveryP, @@ -649,36 +667,3 @@ INSTANTIATE_TEST_CASE_P( ) ); -// errors: Invalid handle if: -// starting handle > ending handle -// stating handle == 00 -// ending handle > last handle on the server - -// if no handle will be return => ATTRIBUTE NOT FOUND - -// Complete when ATTRIBUTE NOT FOUND is returned or an attribute handle -// in the response is equal to the ending handle in the request. - - -// Find information response: -// format & [(handle, UUID)] -// format == 1 => 16 bit UUID -// format == 2 => 128 bit UUID - - -/* - * Given a discovered characteristic with the value handle not equal to the - * last handle of the characteristic. - * when the client launch the discovery of the descriptor of the characteristic. - * Then: - * - the client invoke the pal function discover_characteristics_descriptors - * with an handle range starting at characteristic value handle + 1 and ending - * at the last characteristic handle. - * - The pal will reply with a FindInformationResponse containing the - * descriptors discovered. - * When the client call terminateCharacteristicDescriptorDiscovery the termination - * callback is called immediately. - */ - - - diff --git a/features/FEATURE_BLE/tests/generic/GattClient/TestDiscoverAllServices.cpp b/features/FEATURE_BLE/tests/generic/GattClient/TestDiscoverAllServices.cpp index 0bc1b976f2..dadec0694f 100644 --- a/features/FEATURE_BLE/tests/generic/GattClient/TestDiscoverAllServices.cpp +++ b/features/FEATURE_BLE/tests/generic/GattClient/TestDiscoverAllServices.cpp @@ -56,6 +56,16 @@ using ::testing::AllOf; using ::testing::Property; using ::testing::InSequence; +/* + * Parametric fixture used for service discovery. + * + * Parameters are modeled by a tuple where: + * - [0] std::tuple: pair of boolean indicating the presence of the + * service callback and the characteristic callback. + * - [1] UUID: UUID used for service filtering. + * - [2] UUID: UUID used for characteristic filtering. + * - [3] server_description_t: Discovered service layout. + */ class LaunchDiscoveryNoServiceFilter : public ::testing::TestWithParam< std::tuple, UUID, UUID, server_description_t> > { @@ -74,6 +84,7 @@ protected: server_stub() { } + // setup the test environment virtual void SetUp() { gatt_client.onServiceDiscoveryTermination( makeFunctionPointer( @@ -93,6 +104,10 @@ protected: std::tie(has_service_cb, has_characteristic_cb) = cb_combination; } + /* + * Return a closure that sends a read by group type response to a client. + * @param[in] service_transactions The services to include in the response. + */ auto reply_read_by_group_type(const std::vector& service_transactions) { return Invoke([service_transactions, this](connection_handle_t connection, attribute_handle_t handle) -> ble_error_t { //Log::info() << "discover primary service (" << connection << "," << handle << ")" << std::endl; @@ -123,6 +138,11 @@ protected: }); } + /* + * Return a closure that sends a find by type value response to the client. + * @param[in] service_transactions Services to include in the response sent + * to the client. + */ auto reply_find_by_type_value(const std::vector& service_transactions) { return Invoke([service_transactions, this](connection_handle_t connection, attribute_handle_t handle, const UUID& uuid) -> ble_error_t { //Log::info() << "discover primary service by uuid(" << connection << "," << handle << "," << uuid << ")" << std::endl; @@ -146,6 +166,9 @@ protected: }); } + /** + * Convert a DiscoveredCharacteristic::Properties_t into an uint8_t + */ uint8_t properties_to_byte(DiscoveredCharacteristic::Properties_t p) { return ( p.broadcast() << 0 | @@ -159,6 +182,11 @@ protected: ); } + /** + * Return a closure that sends a read by type to a client. + * @param[in] transaction Characteristics to include in the response send to + * the client. + */ auto reply_read_by_type(const std::vector& transaction) { return Invoke([transaction, this](connection_handle_t connection, attribute_handle_range_t range) -> ble_error_t { //Log::info() << "discover characteristic(" << connection << "," << range.begin << "," << range.end << ")" << std::endl; @@ -188,6 +216,11 @@ protected: }); } + /* + * Return a closure that send an error response to the client. + * @param[in] opcode Opcode that caused the error. + * @param[in] error_code Error code. + */ auto reply_error(AttributeOpcode opcode, AttErrorResponse::AttributeErrorCode error_code) { return InvokeWithoutArgs([this, opcode, error_code]() -> ble_error_t { //Log::info() << "reply error: opcode = " << (uint8_t) opcode << ", error_code = " << error_code << std::endl; @@ -199,8 +232,10 @@ protected: }); } - // helper - // note sequence insured by caller + /* + * Set service discovery expectation when the procedure discover all services + * is used. + */ void set_discover_all_services_expectations() { auto services_transactions = get_services_transactions(); @@ -233,6 +268,9 @@ protected: } } + /* + * Set client expectations when the discover services by UUID is used. + */ void set_discover_services_by_uuid_expectations() { std::vector> services_response; auto services_transactions = get_services_transactions(); @@ -276,6 +314,10 @@ protected: } } + /* + * Set an expectation regarding call of the service discovery callback with + * the services @p services. + */ void set_services_callback_expectations(const std::vector& services) { if (!has_service_cb) { return; @@ -286,6 +328,10 @@ protected: } } + /* + * Set an expectation regarding call of the service discovery callback with + * the service @p service. + */ void set_service_callback_expectation(const service_description_t& service) { if (!has_service_cb) { return; @@ -313,7 +359,9 @@ protected: })); } - + /* + * Set expectations for characteristic discovery. + */ void set_discover_characteristics_expectations() { auto services = get_services_discovered(); @@ -411,6 +459,9 @@ protected: })); } + /* + * Compute the transactions involved during the service discovery process. + */ std::vector> get_services_transactions() { std::vector working_set = get_services_discovered(); @@ -440,6 +491,9 @@ protected: return result; } + /* + * Compute the list of services discovered during service discovery. + */ std::vector get_services_discovered() { std::vector working_set; @@ -456,6 +510,9 @@ protected: return working_set; } + /* + * Compute the transactions present during the characteristic discovery process. + */ std::vector> get_characteristics_transactions(const service_description_t& service) { std::vector> transactions; for (const auto& characteristic : service.characteristics) { @@ -495,7 +552,8 @@ protected: }; TEST_P(LaunchDiscoveryNoServiceFilter, regular) { - + ///////////////////////////// + // Install default calls ON_CALL( mock_client, discover_primary_service(_, _) ).WillByDefault(Invoke([](connection_handle_t connection, attribute_handle_t handle) -> ble_error_t { @@ -533,6 +591,8 @@ TEST_P(LaunchDiscoveryNoServiceFilter, regular) { })); } + ///////////////////////////// + // Install expectations { InSequence seq; if (service_filter == UUID()) { @@ -548,6 +608,8 @@ TEST_P(LaunchDiscoveryNoServiceFilter, regular) { EXPECT_CALL(termination_callback, call(connection_handle)); } + ///////////////////////////// + // Launch the discovery process. ble_error_t err = gatt_client.launchServiceDiscovery( connection_handle, has_service_cb ? @@ -568,14 +630,19 @@ TEST_P(LaunchDiscoveryNoServiceFilter, regular) { INSTANTIATE_TEST_CASE_P( GattClient_launch_discovery_no_service_filter, LaunchDiscoveryNoServiceFilter, + // Yield all combination of each generator ::testing::Combine( + // service cb generator ::testing::Values( std::tuple(true, false), std::tuple(false, true), std::tuple(true, true) ), + // service UUID filter generator ::testing::Values(UUID(), UUID(0x1452), UUID("a3d1495f-dba7-4441-99f2-d0a20f663422")), + // characteristic UUID filter generator ::testing::Values(UUID(), UUID(0xBEEF), UUID("1f551ee3-aef4-4719-8c52-8b419fc4ac01")), + // server layout generator. ::testing::Values( server_description_t { }, server_description_t { diff --git a/features/FEATURE_BLE/tests/generic/GattClient/TestNoCb.cpp b/features/FEATURE_BLE/tests/generic/GattClient/TestNoCb.cpp index aad3519ec3..463d21eeae 100644 --- a/features/FEATURE_BLE/tests/generic/GattClient/TestNoCb.cpp +++ b/features/FEATURE_BLE/tests/generic/GattClient/TestNoCb.cpp @@ -38,9 +38,13 @@ using ble::pal::vendor::mock::MockPalGattClient; using ::testing::_; - -class LaunchDiscoveryNoCb : - public ::testing::TestWithParam> { +/* + * Parametric fixture used to test service discovery when no callback is registered. + * Parameters are: + * - [0]: service UUID filter + * - [1]: characteristic UUID filter + */ +class LaunchDiscoveryNoCb : public ::testing::TestWithParam> { protected: typedef std::tuple parameters_t; @@ -160,11 +164,15 @@ TEST_P(LaunchDiscoveryNoCb, shall_not_change_discovery_status) { EXPECT_TRUE(_gatt_client.isServiceDiscoveryActive()); } +// Instantiate tests cases with the given parameters INSTANTIATE_TEST_CASE_P( GattClient_launch_discovery_no_cb, LaunchDiscoveryNoCb, + // Yield combination of each generator value ::testing::Combine( + // service UUID filter ::testing::Values(UUID(), UUID(0x1452), UUID("a3d1495f-dba7-4441-99f2-d0a20f663422")), + // characteristic UUID filter ::testing::Values(UUID(), UUID(0xBEEF), UUID("1f551ee3-aef4-4719-8c52-8b419fc4ac01")) ) ); diff --git a/features/FEATURE_BLE/tests/generic/GattClient/TestRead.cpp b/features/FEATURE_BLE/tests/generic/GattClient/TestRead.cpp index 3cf3ab9167..3e811606c8 100644 --- a/features/FEATURE_BLE/tests/generic/GattClient/TestRead.cpp +++ b/features/FEATURE_BLE/tests/generic/GattClient/TestRead.cpp @@ -63,6 +63,9 @@ static vector make_char_value(uint16_t length) { return characteristic_value; } +/* + * fixture used to test GattClient::read. + */ class TestGattClientRead : public ::testing::Test { protected: TestGattClientRead() : @@ -273,6 +276,12 @@ TEST_F(TestGattClientRead, read_with_out_of_range_offset_shall_fail) { } } +/* + * Parametric fixture used to test error generated during GattClient::read. + * Parameters are: + * - [0] The attribute error code + * - [1] Expected status returned in the read callback. + */ class TestGattClientReadAttributeError : public TestGattClientRead, public ::testing::WithParamInterface> { @@ -384,6 +393,7 @@ TEST_P(TestGattClientReadAttributeError, read_with_offset) { EXPECT_EQ(err, BLE_ERROR_NONE); } +// Instantiate test cases using TestGattClientReadAttributeError INSTANTIATE_TEST_CASE_P( TestGattClientReadAttributeError_combination, TestGattClientReadAttributeError, diff --git a/features/FEATURE_BLE/tests/generic/GattClient/TestServerEvent.cpp b/features/FEATURE_BLE/tests/generic/GattClient/TestServerEvent.cpp index b8cf101819..0f4b1d2347 100644 --- a/features/FEATURE_BLE/tests/generic/GattClient/TestServerEvent.cpp +++ b/features/FEATURE_BLE/tests/generic/GattClient/TestServerEvent.cpp @@ -56,6 +56,9 @@ static vector make_char_value(uint16_t length) { return characteristic_value; } +/* + * Fixture use to test server notification or indication. + */ class TestGattServerEvent : public ::testing::Test { protected: TestGattServerEvent() : @@ -91,6 +94,10 @@ protected: uint16_t _mtu_size; }; +/* + * Ensure the right callback is called with the correct parameters when an + * indication is received. + */ TEST_F(TestGattServerEvent, event_callback_shall_be_called_on_indication) { auto value = make_char_value(5000); @@ -118,6 +125,10 @@ TEST_F(TestGattServerEvent, event_callback_shall_be_called_on_indication) { } } +/* + * Ensure the right callback is called with the correct parameters when a + * notification is received. + */ TEST_F(TestGattServerEvent, event_callback_shall_be_called_on_notification) { auto value = make_char_value(5000); diff --git a/features/FEATURE_BLE/tests/generic/GattClient/TestWrite.cpp b/features/FEATURE_BLE/tests/generic/GattClient/TestWrite.cpp index 6bb2d4af5c..b4386450f3 100644 --- a/features/FEATURE_BLE/tests/generic/GattClient/TestWrite.cpp +++ b/features/FEATURE_BLE/tests/generic/GattClient/TestWrite.cpp @@ -65,6 +65,9 @@ static vector make_char_value(uint16_t length) { return characteristic_value; } +/* + * Fixture used for GattClient::write tests. + */ class TestGattClientWrite : public ::testing::Test { protected: TestGattClientWrite() : @@ -234,6 +237,14 @@ TEST_F(TestGattClientWrite, write_request_short_characteristics) { } } +/* + * Fixture used to test error handling during a characteristic write. + * Parameters are: + * [0] AttErrorResponse::AttributeErrorCode The attribute error code to push in + * the AttErrorresponse sent to the client. + * [1] ble_error_t The expected error status present in the parameters received + * by the callback handling GattClient::write. + */ class TestGattClientWriteAttributeError : public TestGattClientWrite, public ::testing::WithParamInterface> { @@ -314,6 +325,7 @@ TEST_P(TestGattClientWriteAttributeError, write_request_short_characteristics) { } } +// Instantiate test cases using the TestGattClientWriteAttributeError fixture INSTANTIATE_TEST_CASE_P( TestGattClientWriteAttributeError_combination, TestGattClientWriteAttributeError, @@ -416,10 +428,14 @@ TEST_F(TestGattClientWrite, write_request_long_characteristics) { } } +/* + * Fixture used to test long write errors. + * Parameter are similar to the one used in TestGattClientWriteAttributeError + */ class TestGattClientPrepareWriteAttributeError : public TestGattClientWriteAttributeError { }; /** - * TODO doc + * Test errors received by a client at the beginning of a long write procedure. */ TEST_P(TestGattClientPrepareWriteAttributeError, prepare_write_error) { auto value = make_char_value(10000); @@ -482,6 +498,7 @@ TEST_P(TestGattClientPrepareWriteAttributeError, prepare_write_error) { EXPECT_EQ(error, BLE_ERROR_NONE); } +// Instantiate test cases relying on TestGattClientPrepareWriteAttributeError INSTANTIATE_TEST_CASE_P( TestGattClientPrepareWriteAttributeError_combination, TestGattClientPrepareWriteAttributeError, @@ -498,11 +515,15 @@ INSTANTIATE_TEST_CASE_P( ); - +/* + * Fixture used to test errors in the middle of a long write transaction. + * Parameter are similar to the one used in TestGattClientWriteAttributeError + */ class TestGattClientPrepareWriteAttributeErrorInTransaction : public TestGattClientWriteAttributeError { }; /** - * TODO doc + * Test that errors are correctly reported to the client if an error occur in the + * middle of a long write transaction. */ TEST_P(TestGattClientPrepareWriteAttributeErrorInTransaction, prepare_write_error_in_transaction) { auto value = make_char_value(10000); @@ -591,6 +612,7 @@ TEST_P(TestGattClientPrepareWriteAttributeErrorInTransaction, prepare_write_erro EXPECT_EQ(error, BLE_ERROR_NONE); } +// Instantiate test cases relying on TestGattClientPrepareWriteAttributeErrorInTransaction INSTANTIATE_TEST_CASE_P( TestGattClientPrepareWriteAttributeErrorInTransaction_combination, TestGattClientPrepareWriteAttributeErrorInTransaction, @@ -600,10 +622,15 @@ INSTANTIATE_TEST_CASE_P( ) ); +/* + * Fixture used to test errors at the end of a long write transaction. + * Parameter are similar to the one used in TestGattClientWriteAttributeError + */ class TestGattClientExecuteWriteRequestError : public TestGattClientWriteAttributeError { }; /** - * TODO doc + * Test that errors are correctly reported to the client if an error occurs when + * a write queue is executed. */ TEST_P(TestGattClientExecuteWriteRequestError, prepare_write_error_in_transaction) { auto value = make_char_value(10000); @@ -675,6 +702,7 @@ TEST_P(TestGattClientExecuteWriteRequestError, prepare_write_error_in_transactio EXPECT_EQ(error, BLE_ERROR_NONE); } +// Instantiate test cases relying on TestGattClientExecuteWriteRequestError INSTANTIATE_TEST_CASE_P( TestGattClientExecuteWriteRequestError_combination, TestGattClientExecuteWriteRequestError,