[PATCH BlueZ v3 1/4] profiles/gap: Some code cleanup

[PATCH BlueZ v3 1/4] profiles/gap: Some code cleanup

[Luiz Augusto von Dentz]

From: "Felipe F. Tonello" <eu@felipetonello.com>

Just removing unecessary function and code duplication.
---
 profiles/gap/gas.c | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
index 400818d67591..713b9aaf28f2 100644
--- a/profiles/gap/gas.c
+++ b/profiles/gap/gas.c
@@ -50,10 +50,18 @@ struct gas {
 	struct gatt_db_attribute *attr;
 };
 
+static void gas_reset(struct gas *gas)
+{
+	gas->attr = NULL;
+	gatt_db_unref(gas->db);
+	gas->db = NULL;
+	bt_gatt_client_unref(gas->client);
+	gas->client = NULL;
+}
+
 static void gas_free(struct gas *gas)
 {
-	gatt_db_unref(gas->db);
-	bt_gatt_client_unref(gas->client);
+	gas_reset(gas);
 	btd_device_unref(gas->device);
 	g_free(gas);
 }
@@ -152,7 +160,7 @@ static void handle_appearance(struct gas *gas, uint16_t value_handle)
 		DBG("Failed to send request to read appearance");
 }
 
-static bool uuid_cmp(uint16_t u16, const bt_uuid_t *uuid)
+static inline bool uuid_cmp(uint16_t u16, const bt_uuid_t *uuid)
 {
 	bt_uuid_t lhs;
 
@@ -188,11 +196,6 @@ static void handle_characteristic(struct gatt_db_attribute *attr,
 	}
 }
 
-static void handle_gap_service(struct gas *gas)
-{
-	gatt_db_service_foreach_char(gas->attr, handle_characteristic, gas);
-}
-
 static int gap_probe(struct btd_service *service)
 {
 	struct btd_device *device = btd_service_get_device(service);
@@ -246,16 +249,7 @@ static void foreach_gap_service(struct gatt_db_attribute *attr, void *user_data)
 	}
 
 	gas->attr = attr;
-	handle_gap_service(gas);
-}
-
-static void gas_reset(struct gas *gas)
-{
-	gas->attr = NULL;
-	gatt_db_unref(gas->db);
-	gas->db = NULL;
-	bt_gatt_client_unref(gas->client);
-	gas->client = NULL;
+	gatt_db_service_foreach_char(gas->attr, handle_characteristic, gas);
 }
 
 static int gap_accept(struct btd_service *service)
@@ -266,13 +260,15 @@ static int gap_accept(struct btd_service *service)
 	struct gas *gas = btd_service_get_user_data(service);
 	char addr[18];
 	bt_uuid_t gap_uuid;
+	int err = 0;
 
 	ba2str(device_get_address(device), addr);
 	DBG("GAP profile accept (%s)", addr);
 
 	if (!gas) {
 		error("GAP service not handled by profile");
-		return -1;
+		err = -1;
+		goto _finish;
 	}
 
 	gas->db = gatt_db_ref(db);
@@ -285,12 +281,14 @@ static int gap_accept(struct btd_service *service)
 	if (!gas->attr) {
 		error("GAP attribute not found");
 		gas_reset(gas);
-		return -1;
+		err = -1;
 	}
 
-	btd_service_connecting_complete(service, 0);
+_finish:
 
-	return 0;
+	btd_service_connecting_complete(service, err);
+
+	return err;
 }
 
 static int gap_disconnect(struct btd_service *service)
-- 
2.44.0

[PATCH BlueZ v3 2/4] src/adapter: Added connection parameter load/store functions

[Luiz Augusto von Dentz]

From: "Felipe F. Tonello" <eu@felipetonello.com>

It is interesting to let other parts of bluetoothd to access these
functions since there are few different use-cases where this updating
and loading connection parameters can happen.
---
 src/adapter.c | 34 +++++++++++++++++++++++++++++-----
 src/adapter.h | 10 ++++++++++
 2 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 5505edbb29c1..4943e5e88e70 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -4635,6 +4635,30 @@ static void load_conn_params(struct btd_adapter *adapter, GSList *params)
 		btd_error(adapter->dev_id, "Load connection parameters failed");
 }
 
+void btd_adapter_load_conn_param(struct btd_adapter *adapter,
+				const bdaddr_t *peer, uint8_t bdaddr_type,
+				uint16_t min_interval, uint16_t max_interval,
+				uint16_t latency, uint16_t timeout)
+{
+	GSList *params = NULL;
+	struct conn_param param;
+
+	/* Only versions >= 1.23 support updating connection parameters */
+	if (MGMT_VERSION(mgmt_version, mgmt_revision) >= MGMT_VERSION(1, 23))
+		return;
+
+	bacpy(&param.bdaddr, peer);
+	param.bdaddr_type = bdaddr_type;
+	param.max_interval = max_interval;
+	param.min_interval = min_interval;
+	param.latency = latency;
+	param.timeout = timeout;
+
+	params = g_slist_append(params, &param);
+	load_conn_params(adapter, params);
+	g_slist_free(params);
+}
+
 static uint8_t get_addr_type(GKeyFile *keyfile)
 {
 	uint8_t addr_type;
@@ -8919,10 +8943,10 @@ static void new_irk_callback(uint16_t index, uint16_t length,
 	btd_device_set_temporary(device, false);
 }
 
-static void store_conn_param(struct btd_adapter *adapter, const bdaddr_t *peer,
-				uint8_t bdaddr_type, uint16_t min_interval,
-				uint16_t max_interval, uint16_t latency,
-				uint16_t timeout)
+void btd_adapter_store_conn_param(struct btd_adapter *adapter,
+				const bdaddr_t *peer, uint8_t bdaddr_type,
+				uint16_t min_interval, uint16_t max_interval,
+				uint16_t latency, uint16_t timeout)
 {
 	char device_addr[18];
 	char filename[PATH_MAX];
@@ -9002,7 +9026,7 @@ static void new_conn_param(uint16_t index, uint16_t length,
 	if (!ev->store_hint)
 		return;
 
-	store_conn_param(adapter, &ev->addr.bdaddr, ev->addr.type,
+	btd_adapter_store_conn_param(adapter, &ev->addr.bdaddr, ev->addr.type,
 					ev->min_interval, ev->max_interval,
 					ev->latency, ev->timeout);
 }
diff --git a/src/adapter.h b/src/adapter.h
index 2ca045539ec0..3534986f5a72 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -289,3 +289,13 @@ bool btd_adapter_set_allowed_uuids(struct btd_adapter *adapter,
 							struct queue *uuids);
 bool btd_adapter_is_uuid_allowed(struct btd_adapter *adapter,
 							const char *uuid_str);
+
+void btd_adapter_load_conn_param(struct btd_adapter *adapter,
+				const bdaddr_t *peer, uint8_t bdaddr_type,
+				uint16_t min_interval, uint16_t max_interval,
+				uint16_t latency, uint16_t timeout);
+
+void btd_adapter_store_conn_param(struct btd_adapter *adapter,
+				const bdaddr_t *peer, uint8_t bdaddr_type,
+				uint16_t min_interval, uint16_t max_interval,
+				uint16_t latency, uint16_t timeout);
-- 
2.44.0

[PATCH BlueZ v3 3/4] src/device: Added function to set connection parameters

[Luiz Augusto von Dentz]

From: "Felipe F. Tonello" <eu@felipetonello.com>

This function allows plugins to set the connection parameters of the
respective btd_device object.

It is useful for GAP Peripheral Preferred Connection Parameters
characteristic for example.
---
 src/device.c | 14 ++++++++++++++
 src/device.h |  3 +++
 2 files changed, 17 insertions(+)

diff --git a/src/device.c b/src/device.c
index 1d4b8ab36720..79d8bb7ebc9b 100644
--- a/src/device.c
+++ b/src/device.c
@@ -7322,3 +7322,17 @@ void btd_device_foreach_ad(struct btd_device *dev, bt_ad_func_t func,
 {
 	bt_ad_foreach_data(dev->ad, func, data);
 }
+
+void btd_device_set_conn_param(struct btd_device *device, uint16_t min_interval,
+					uint16_t max_interval, uint16_t latency,
+					uint16_t timeout)
+{
+	btd_adapter_store_conn_param(device->adapter, &device->bdaddr,
+					device->bdaddr_type, min_interval,
+					max_interval, latency,
+					timeout);
+	btd_adapter_load_conn_param(device->adapter, &device->bdaddr,
+					device->bdaddr_type, min_interval,
+					max_interval, latency,
+					timeout);
+}
diff --git a/src/device.h b/src/device.h
index 5722ca9cafe0..1a9f7e72a00f 100644
--- a/src/device.h
+++ b/src/device.h
@@ -204,3 +204,6 @@ typedef void (*bt_device_ad_func_t)(void *data, void *user_data);
 
 void btd_device_foreach_ad(struct btd_device *dev, bt_device_ad_func_t func,
 							void *data);
+void btd_device_set_conn_param(struct btd_device *device, uint16_t min_interval,
+					uint16_t max_interval, uint16_t latency,
+					uint16_t timeout);
-- 
2.44.0

[PATCH BlueZ v3 4/4] profiles/gap: Added support for PPCP characteristic

[Luiz Augusto von Dentz]

From: "Felipe F. Tonello" <eu@felipetonello.com>

The Peripheral Preferred Connection Parameters (PPCP) characteristic
contains the preferred connection parameters of a peripheral device.

These parameters are stored in the info file and loaded to Kernel using
MGMT's respective command.
---
 profiles/gap/gas.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/profiles/gap/gas.c b/profiles/gap/gas.c
index 713b9aaf28f2..b50a9c03acd1 100644
--- a/profiles/gap/gas.c
+++ b/profiles/gap/gas.c
@@ -160,6 +160,75 @@ static void handle_appearance(struct gas *gas, uint16_t value_handle)
 		DBG("Failed to send request to read appearance");
 }
 
+static void read_ppcp_cb(bool success, uint8_t att_ecode,
+			const uint8_t *value, uint16_t length,
+			void *user_data)
+{
+	struct gas *gas = user_data;
+	uint16_t min_interval, max_interval, latency, timeout, max_latency;
+
+	if (!success) {
+		DBG("Reading PPCP failed with ATT error: %u", att_ecode);
+		return;
+	}
+
+	if (length != 8) {
+		DBG("Malformed PPCP value");
+		return;
+	}
+
+	min_interval = get_le16(&value[0]);
+	max_interval = get_le16(&value[2]);
+	latency = get_le16(&value[4]);
+	timeout = get_le16(&value[6]);
+
+	DBG("GAP Peripheral Preferred Connection Parameters:");
+	DBG("\tMinimum connection interval: %u", min_interval);
+	DBG("\tMaximum connection interval: %u", max_interval);
+	DBG("\tSlave latency: %u", latency);
+	DBG("\tConnection Supervision timeout multiplier: %u", timeout);
+
+	/* 0xffff indicates no specific min/max */
+	if (min_interval == 0xffff)
+		min_interval = 6;
+
+	if (max_interval == 0xffff)
+		max_interval = 3200;
+
+	/* avoid persisting connection parameters that are not valid */
+	if (min_interval > max_interval ||
+	    min_interval < 6 || max_interval > 3200) {
+		warn("GAS PPCP: Invalid Connection Parameters values");
+		return;
+	}
+
+	if (timeout < 10 || timeout > 3200) {
+		warn("GAS PPCP: Invalid Connection Parameters values");
+		return;
+	}
+
+	if (max_interval >= timeout * 8) {
+		warn("GAS PPCP: Invalid Connection Parameters values");
+		return;
+	}
+
+	max_latency = (timeout * 4 / max_interval) - 1;
+	if (latency > 499 || latency > max_latency) {
+		warn("GAS PPCP: Invalid Connection Parameters values");
+		return;
+	}
+
+	btd_device_set_conn_param(gas->device, min_interval, max_interval,
+					latency, timeout);
+}
+
+static void handle_ppcp(struct gas *gas, uint16_t value_handle)
+{
+	if (!bt_gatt_client_read_value(gas->client, value_handle,
+						read_ppcp_cb, gas, NULL))
+		DBG("Failed to send request to read PPCP");
+}
+
 static inline bool uuid_cmp(uint16_t u16, const bt_uuid_t *uuid)
 {
 	bt_uuid_t lhs;
@@ -186,6 +255,8 @@ static void handle_characteristic(struct gatt_db_attribute *attr,
 		handle_device_name(gas, value_handle);
 	else if (uuid_cmp(GATT_CHARAC_APPEARANCE, &uuid))
 		handle_appearance(gas, value_handle);
+	else if (uuid_cmp(GATT_CHARAC_PERIPHERAL_PREF_CONN, &uuid))
+		handle_ppcp(gas, value_handle);
 	else {
 		char uuid_str[MAX_LEN_UUID_STR];
 
-- 
2.44.0

Re: [PATCH BlueZ v3 1/4] profiles/gap: Some code cleanup

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Fri, 10 May 2024 11:09:35 -0400 you wrote:
> From: "Felipe F. Tonello" <eu@felipetonello.com>
> 
> Just removing unecessary function and code duplication.
> ---
>  profiles/gap/gas.c | 42 ++++++++++++++++++++----------------------
>  1 file changed, 20 insertions(+), 22 deletions(-)

Here is the summary with links:
  - [BlueZ,v3,1/4] profiles/gap: Some code cleanup
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=1cb5708fe7cf
  - [BlueZ,v3,2/4] src/adapter: Added connection parameter load/store functions
    (no matching commit)
  - [BlueZ,v3,3/4] src/device: Added function to set connection parameters
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d391bf4cf320
  - [BlueZ,v3,4/4] profiles/gap: Added support for PPCP characteristic
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=d6fe19bccba0

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

RE: [BlueZ,v3,1/4] profiles/gap: Some code cleanup

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1867 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=852343

---Test result---

Test Summary:
CheckPatch                    FAIL      1.57 seconds
GitLint                       PASS      0.81 seconds
BuildEll                      PASS      24.18 seconds
BluezMake                     PASS      1646.05 seconds
MakeCheck                     PASS      13.67 seconds
MakeDistcheck                 PASS      173.75 seconds
CheckValgrind                 PASS      244.10 seconds
CheckSmatch                   PASS      346.75 seconds
bluezmakeextell               PASS      117.90 seconds
IncrementalBuild              PASS      6038.21 seconds
ScanBuild                     PASS      966.73 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,v3,1/4] profiles/gap: Some code cleanup
WARNING:TYPO_SPELLING: 'unecessary' may be misspelled - perhaps 'unnecessary'?
#91: 
Just removing unecessary function and code duplication.
              ^^^^^^^^^^

/github/workspace/src/src/13661723.patch total: 0 errors, 1 warnings, 89 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/src/13661723.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.




---
Regards,
Linux Bluetooth