BLE: Improve generic gatt client tests documentation.

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<uint16_t, uint16_t, vector<vector<pair<uint16_t, UUID>>>> test_param_t;
 
+/**
+ * Parametric fixture used for descriptor discovery testing.
+ */
 class TestGattClientDescriptorDiscoveryP :
 	public TestGattClientDescriptorDiscovery,
 	public ::testing::WithParamInterface<test_param_t> {
@@ -224,16 +237,20 @@ struct MockFindInformationResponse : public AttFindInformationResponse {
 	vector<pair<uint16_t, UUID>> _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.
- */
-
-
-

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<bool, bool>: 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<std::tuple<bool, bool>, 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_description_t>& 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_description_t>& 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<characteristic_description_t>& 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<std::vector<uint8_t>> 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<service_description_t>& 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<std::vector<service_description_t>> get_services_transactions() {
 		std::vector<service_description_t> working_set = get_services_discovered();
 
@@ -440,6 +491,9 @@ protected:
 		return result;
 	}
 
+	/*
+	 * Compute the list of services discovered during service discovery.
+	 */
 	std::vector<service_description_t> get_services_discovered() {
 		std::vector<service_description_t> working_set;
 
@@ -456,6 +510,9 @@ protected:
 		return working_set;
 	}
 
+	/*
+	 * Compute the transactions present during the characteristic discovery process.
+	 */
 	std::vector<std::vector<characteristic_description_t>> get_characteristics_transactions(const service_description_t& service) {
 		std::vector<std::vector<characteristic_description_t>> 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<bool, bool>(true, false),
 			std::tuple<bool, bool>(false, true),
 			std::tuple<bool, bool>(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 {

features/FEATURE_BLE/tests/generic/GattClient/TestNoCb.cpp

@@ -38,9 +38,13 @@ using ble::pal::vendor::mock::MockPalGattClient;
 
 using ::testing::_;
 
-
+/*
-class LaunchDiscoveryNoCb :
+ * Parametric fixture used to test service discovery when no callback is registered.
-	public ::testing::TestWithParam<std::tuple<UUID, UUID>> {
+ * Parameters are:
+ * - [0]: service UUID filter
+ * - [1]: characteristic UUID filter
+ */
+class LaunchDiscoveryNoCb : public ::testing::TestWithParam<std::tuple<UUID, UUID>> {
 protected:
 	typedef std::tuple<UUID, UUID> 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"))
 	)
 );

features/FEATURE_BLE/tests/generic/GattClient/TestRead.cpp

@@ -63,6 +63,9 @@ static vector<uint8_t> 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<tuple<AttErrorResponse::AttributeErrorCode, ble_error_t>> {
@@ -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,

features/FEATURE_BLE/tests/generic/GattClient/TestServerEvent.cpp

@@ -56,6 +56,9 @@ static vector<uint8_t> 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);
 

features/FEATURE_BLE/tests/generic/GattClient/TestWrite.cpp

@@ -65,6 +65,9 @@ static vector<uint8_t> 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<tuple<AttErrorResponse::AttributeErrorCode, ble_error_t>> {
@@ -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,