[PATCH BlueZ v1 0/6] RAP: Move CS default settings

[PATCH BlueZ v1 0/6] RAP: Move CS default settings

[Naga Bhavani Akella]

This series removes CS default settings (role, cs_sync_ant_sel, and
max_tx_power) from RAP initialization and configuration paths where they
do not logically belong.

These parameters are measurement-session specific and are already supplied
through StartMeasurement. Keeping them in main.conf and passing them during
HCI attachment introduces unnecessary duplication and mixes per-session
configuration with device initialization.

The changes in this series:

  1. Remove CS default settings from main.conf parsing and configuration.
  2. Update bt_rap_attach_hci() callers to stop passing these parameters.
  3. Simplify bt_rap_attach_hci() by removing the corresponding arguments.
  4. Remove wrapper API

Naga Bhavani Akella (6):
  shared: remove default settings params from bt_rap_attach_hci
  profiles: update bt_rap_attach_hci call to drop default settings args
  rap: remove CS default settings from main.conf
  shared: replace the wrapper API
  profiles: replace wrapper API
  shared: remove wrapper API

 profiles/ranging/rap.c     |   5 +-
 profiles/ranging/rap_hci.c |  27 +++-----
 src/btd.h                  |   7 --
 src/main.c                 | 127 +------------------------------------
 src/main.conf              |  24 -------
 src/shared/rap.h           |   4 +-
 6 files changed, 13 insertions(+), 181 deletions(-)

-- 

[PATCH BlueZ v1 1/6] shared: remove default settings params from bt_rap_attach_hci

[Naga Bhavani Akella]

role, cs_sync_ant_sel and max_tx_power were passed into bt_rap_attach_hci
but these are CS default settings that belong to the measurement session,
Callers need to set them via bt_rap_set_default_settings_params() in
start_measurement where they belong, alongside the other per-session
CS parameters
---
 src/shared/rap.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/shared/rap.h b/src/shared/rap.h
index 7bc49f03d..45be14dde 100644
--- a/src/shared/rap.h
+++ b/src/shared/rap.h
@@ -206,9 +206,11 @@ void bt_rap_hci_cs_subevent_result_cont_callback(uint16_t length,
 						  const void *param,
 						  void *user_data);
 
+/* Wrapper to be removed */
 void *bt_rap_attach_hci(struct bt_rap *rap, struct bt_hci *hci,
 			uint8_t role, uint8_t cs_sync_ant_sel,
 			int8_t max_tx_power);
+void *bt_rap_attach_hci_v2(struct bt_rap *rap, struct bt_hci *hci);
 void bt_rap_detach_hci(struct bt_rap *rap, void *hci_sm);
 
 /* Connection handle mapping functions */
-- 

[PATCH BlueZ v1 2/6] profiles: update bt_rap_attach_hci call to drop default settings args

[Naga Bhavani Akella]

role, cs_sync_ant_sel and max_tx_power are no longer passed at attach
time. They are applied via bt_rap_set_default_settings_params() in
start_measurement where they belong, alongside the other per-session
CS parameters
---
 profiles/ranging/rap.c     |  7 ++-----
 profiles/ranging/rap_hci.c | 27 ++++++++++-----------------
 2 files changed, 12 insertions(+), 22 deletions(-)

diff --git a/profiles/ranging/rap.c b/profiles/ranging/rap.c
index fa3e27328..da6f1241b 100644
--- a/profiles/ranging/rap.c
+++ b/profiles/ranging/rap.c
@@ -383,11 +383,8 @@ static int rap_accept(struct btd_service *service)
 
 	/* per-device HCI state machine */
 	if (!data->hci_sm) {
-		data->hci_sm = bt_rap_attach_hci(data->rap,
-					data->adapter_data->hci,
-					btd_opts.defaults.bcs.role,
-					btd_opts.defaults.bcs.cs_sync_ant_sel,
-					btd_opts.defaults.bcs.max_tx_power);
+		data->hci_sm = bt_rap_attach_hci_v2(data->rap,
+					data->adapter_data->hci);
 		if (!data->hci_sm) {
 			error("Failed to attach HCI state machine for device");
 			rap_adapter_data_unref(data->adapter_data);
diff --git a/profiles/ranging/rap_hci.c b/profiles/ranging/rap_hci.c
index 13f1365f3..8726ce824 100644
--- a/profiles/ranging/rap_hci.c
+++ b/profiles/ranging/rap_hci.c
@@ -169,9 +169,7 @@ static void remove_conn_mapping(struct cs_state_machine *sm, uint16_t handle)
 
 /*  State Machine Functions */
 static void cs_state_machine_init(struct cs_state_machine *sm,
-				struct bt_rap *rap, struct bt_hci *hci,
-				uint8_t role, uint8_t cs_sync_ant_sel,
-				int8_t max_tx_power)
+				struct bt_rap *rap, struct bt_hci *hci)
 {
 	if (!sm)
 		return;
@@ -183,16 +181,18 @@ static void cs_state_machine_init(struct cs_state_machine *sm,
 	sm->procedure_active = false;
 	sm->conn_mappings = queue_new();
 
-	/* Store role_enable for HCI commands (1, 2, or 3 from config) */
-	sm->role_enable = role;
+	/* Defaults — caller
+	 * should follow up with bt_rap_set_default_settings_params()
+	 */
+	sm->role_enable = 0x03;
 
 	/* Initialize per-instance CS options
 	 * Note: cs_opt.role will be overwritten with actual role (0x00 or 0x01)
 	 * from config complete event, but role_enable preserves the HCI value
 	 */
-	sm->cs_opt.role = role;
-	sm->cs_opt.cs_sync_ant_sel = cs_sync_ant_sel;
-	sm->cs_opt.max_tx_power = max_tx_power;
+	sm->cs_opt.role = 0x03;
+	sm->cs_opt.cs_sync_ant_sel = 0xFF;
+	sm->cs_opt.max_tx_power = 0x14;
 	sm->cs_opt.rtt_type = 0;  /* Will be set from config complete event */
 }
 
@@ -1533,9 +1533,7 @@ static void unregister_event_id(void *data, void *user_data)
 	bt_hci_unregister_subevent(hci, PTR_TO_UINT(data));
 }
 
-void *bt_rap_attach_hci(struct bt_rap *rap, struct bt_hci *hci,
-			uint8_t role, uint8_t cs_sync_ant_sel,
-			int8_t max_tx_power)
+void *bt_rap_attach_hci_v2(struct bt_rap *rap, struct bt_hci *hci)
 {
 	struct cs_state_machine *sm;
 	unsigned int id;
@@ -1552,9 +1550,7 @@ void *bt_rap_attach_hci(struct bt_rap *rap, struct bt_hci *hci,
 		return NULL;
 	}
 
-	/* Initialize state machine with provided CS options */
-	cs_state_machine_init(sm, rap, hci, role, cs_sync_ant_sel,
-				max_tx_power);
+	cs_state_machine_init(sm, rap, hci);
 
 	sm->event_ids = queue_new();
 	if (!sm->event_ids) {
@@ -1621,9 +1617,6 @@ void *bt_rap_attach_hci(struct bt_rap *rap, struct bt_hci *hci,
 
 	queue_push_tail(sm->event_ids, UINT_TO_PTR(id));
 
-	DBG("CS options: role=%u, cs_sync_ant_sel=%u, max_tx_power=%d",
-		role, cs_sync_ant_sel, max_tx_power);
-
 	/* place holder, need DBus API to be called */
 	bt_rap_read_local_supported_capabilities(sm);
 
-- 

[PATCH BlueZ v1 3/6] rap: remove CS default settings from main.conf

[Naga Bhavani Akella]

role, cs_sync_ant_sel and max_tx_power were configurable in main.conf
and parsed in main.c, but these are per-measurement parameters that
callers supply via StartMeasurement
---
 src/btd.h     |   7 ---
 src/main.c    | 127 +-------------------------------------------------
 src/main.conf |  24 ----------
 3 files changed, 1 insertion(+), 157 deletions(-)

diff --git a/src/btd.h b/src/btd.h
index db2e81239..c84a600d1 100644
--- a/src/btd.h
+++ b/src/btd.h
@@ -94,18 +94,11 @@ struct btd_le_defaults {
 	uint8_t		enable_advmon_interleave_scan;
 };
 
-struct btd_le_bcs {
-	uint8_t role;
-	uint8_t cs_sync_ant_sel;
-	int8_t max_tx_power;
-};
-
 struct btd_defaults {
 	uint16_t	num_entries;
 
 	struct btd_br_defaults br;
 	struct btd_le_defaults le;
-	struct btd_le_bcs bcs;
 };
 
 struct btd_csis {
diff --git a/src/main.c b/src/main.c
index 8aa19a3e3..53f3f784f 100644
--- a/src/main.c
+++ b/src/main.c
@@ -156,12 +156,6 @@ static const char *gatt_options[] = {
 	NULL
 };
 
-static const char *const bcs_options[] = {
-	"Role",
-	"CsSyncAntennaSel",
-	"MaxTxPower",
-	NULL
-};
 
 static const char *csip_options[] = {
 	"SIRK",
@@ -200,7 +194,7 @@ static const struct group_table {
 	{ "CSIS",	csip_options },
 	{ "AVDTP",	avdtp_options },
 	{ "AVRCP",	avrcp_options },
-	{ "ChannelSounding",	bcs_options },
+
 	{ "AdvMon",	advmon_options },
 	{ }
 };
@@ -496,46 +490,6 @@ static bool parse_config_int(GKeyFile *config, const char *group,
 	return true;
 }
 
-static bool parse_config_signed_int(GKeyFile *config, const char *group,
-					const char *key, int8_t *val,
-					long min, long max)
-{
-	char *str = NULL;
-	char *endptr = NULL;
-	long tmp;
-	bool result = false;
-
-	str = g_key_file_get_string(config, group, key, NULL);
-	if (!str)
-		return false;
-
-	tmp = strtol(str, &endptr, 0);
-	if (!endptr || *endptr != '\0') {
-		warn("%s.%s = %s is not integer", group, key, str);
-		goto cleanup;
-	}
-
-	if (tmp < min) {
-		warn("%s.%s = %ld is out of range (< %ld)", group, key, tmp,
-			min);
-		goto cleanup;
-	}
-
-	if (tmp > max) {
-		warn("%s.%s = %ld is out of range (> %ld)", group, key, tmp,
-									max);
-		goto cleanup;
-	}
-
-	if (val)
-		*val = (int8_t) tmp;
-	result = true;
-
-cleanup:
-	g_free(str);
-	return result;
-}
-
 struct config_param {
 	const char * const val_name;
 	void * const val;
@@ -1228,80 +1182,6 @@ static void parse_csis(GKeyFile *config)
 					0, UINT8_MAX);
 }
 
-static bool parse_cs_role(GKeyFile *config, const char *group,
-					const char *key, uint8_t *val)
-{
-	GError *err = NULL;
-	char *str = NULL;
-	char *endptr = NULL;
-	int numeric_val;
-
-	/* Try to read as string first */
-	str = g_key_file_get_string(config, group, key, &err);
-	if (err) {
-		if (err->code != G_KEY_FILE_ERROR_KEY_NOT_FOUND)
-			DBG("%s", err->message);
-		g_error_free(err);
-		return false;
-	}
-
-	DBG("%s.%s = %s", group, key, str);
-
-	/* Check if it's a string value */
-	if (!strcmp(str, "Initiator") || !strcmp(str, "initiator")) {
-		if (val)
-			*val = 1;
-		g_free(str);
-		return true;
-	} else if (!strcmp(str, "Reflector") || !strcmp(str, "reflector")) {
-		if (val)
-			*val = 2;
-		g_free(str);
-		return true;
-	} else if (!strcmp(str, "Both") || !strcmp(str, "both")) {
-		if (val)
-			*val = 3;
-		g_free(str);
-		return true;
-	}
-
-	/* Try to parse as numeric value */
-	numeric_val = strtol(str, &endptr, 0);
-
-	if (!endptr || *endptr != '\0') {
-		warn("%s.%s = %s is not a valid value. "
-			"Expected: 1/Initiator, 2/Reflector, or 3/Both",
-			group, key, str);
-		g_free(str);
-		return false;
-	}
-
-	if (numeric_val < 1 || numeric_val > 3) {
-		warn("%s.%s = %d is out of range. "
-			"Valid values: 1 (Initiator), 2 (Reflector), 3 (Both)",
-			group, key, numeric_val);
-		g_free(str);
-		return false;
-	}
-
-	if (val)
-		*val = numeric_val;
-
-	g_free(str);
-	return true;
-}
-
-static void parse_le_cs_config(GKeyFile *config)
-{
-	parse_cs_role(config, "ChannelSounding", "Role",
-			&btd_opts.defaults.bcs.role);
-	parse_config_u8(config, "ChannelSounding", "CsSyncAntennaSel",
-			&btd_opts.defaults.bcs.cs_sync_ant_sel,
-			0x01, 0xFF);
-	parse_config_signed_int(config, "ChannelSounding",
-			"MaxTxPower", &btd_opts.defaults.bcs.max_tx_power,
-			INT8_MIN, INT8_MAX);
-}
 
 static void parse_avdtp_session_mode(GKeyFile *config)
 {
@@ -1381,7 +1261,6 @@ static void parse_config(GKeyFile *config)
 	parse_csis(config);
 	parse_avdtp(config);
 	parse_avrcp(config);
-	parse_le_cs_config(config);
 	parse_advmon(config);
 }
 
@@ -1433,10 +1312,6 @@ static void init_defaults(void)
 
 	btd_opts.advmon.rssi_sampling_period = 0xFF;
 	btd_opts.csis.encrypt = true;
-
-	btd_opts.defaults.bcs.role = 0x03;
-	btd_opts.defaults.bcs.cs_sync_ant_sel = 0xFF;
-	btd_opts.defaults.bcs.max_tx_power = 0x14;
 }
 
 static void log_handler(const gchar *log_domain, GLogLevelFlags log_level,
diff --git a/src/main.conf b/src/main.conf
index 5846ef92d..d31dd1b8f 100644
--- a/src/main.conf
+++ b/src/main.conf
@@ -299,30 +299,6 @@
 # Default = auto
 # Security = auto
 
-[ChannelSounding]
-# Current role of the device
-# Possible values:
-#   1 or "Initiator" - CS Initiator role,
-#   Generally, CS Initiator acts as Client (Gatt role) and Central (Gap role)
-#   2 or "Reflector" - CS Reflector role,
-#   Generally, CS Reflector acts as Server (Gatt role) and Peripheral (Gap role)
-#   3 or "Both"      - Both Initiator and Reflector roles
-# Default: 3 (Both)
-#Role = 3
-
-# Antenna Identifier to be used
-# Possible values:
-# 0x01-0x04 (antenna identifier to be used),
-# 0xFE - Antennas to be used in repetitive order,
-# 0xFF - Host doesn't have recommendation
-# Default: 0xFF (Host doesn't have recommendation)
-#CsSyncAntennaSel = 0xFF
-
-# Maximum Transmit power
-# Possible values: 0x81-0x14 (-127dBm to 20dBm)
-# Default: 0x14 (Max Power possible)
-#MaxTxPower = 0x14
-
 [CSIS]
 # SIRK - Set Identification Resolution Key which is common for all the
 # sets. They SIRK key is used to identify its sets. This can be any
-- 

[PATCH BlueZ v1 4/6] shared: replace the wrapper API

[Naga Bhavani Akella]

Replace API bt_rap_attach_hci_v2 with
bt_rap_attach_hci which has less parameters
to avoid compilation error for individual patches
---
 src/shared/rap.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/src/shared/rap.h b/src/shared/rap.h
index 45be14dde..b6fce8b54 100644
--- a/src/shared/rap.h
+++ b/src/shared/rap.h
@@ -206,11 +206,8 @@ void bt_rap_hci_cs_subevent_result_cont_callback(uint16_t length,
 						  const void *param,
 						  void *user_data);
 
-/* Wrapper to be removed */
-void *bt_rap_attach_hci(struct bt_rap *rap, struct bt_hci *hci,
-			uint8_t role, uint8_t cs_sync_ant_sel,
-			int8_t max_tx_power);
 void *bt_rap_attach_hci_v2(struct bt_rap *rap, struct bt_hci *hci);
+void *bt_rap_attach_hci(struct bt_rap *rap, struct bt_hci *hci);
 void bt_rap_detach_hci(struct bt_rap *rap, void *hci_sm);
 
 /* Connection handle mapping functions */
-- 

[PATCH BlueZ v1 5/6] profiles: replace wrapper API

[Naga Bhavani Akella]

replace bt_rap_attach_hci_v2 with bt_rap_attach_hci API
which has less parameters to avoid compilation error
in individual patchsets
---
 profiles/ranging/rap.c     | 2 +-
 profiles/ranging/rap_hci.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/profiles/ranging/rap.c b/profiles/ranging/rap.c
index da6f1241b..a0bd360cc 100644
--- a/profiles/ranging/rap.c
+++ b/profiles/ranging/rap.c
@@ -383,7 +383,7 @@ static int rap_accept(struct btd_service *service)
 
 	/* per-device HCI state machine */
 	if (!data->hci_sm) {
-		data->hci_sm = bt_rap_attach_hci_v2(data->rap,
+		data->hci_sm = bt_rap_attach_hci(data->rap,
 					data->adapter_data->hci);
 		if (!data->hci_sm) {
 			error("Failed to attach HCI state machine for device");
diff --git a/profiles/ranging/rap_hci.c b/profiles/ranging/rap_hci.c
index 8726ce824..944b7e2a3 100644
--- a/profiles/ranging/rap_hci.c
+++ b/profiles/ranging/rap_hci.c
@@ -1533,7 +1533,7 @@ static void unregister_event_id(void *data, void *user_data)
 	bt_hci_unregister_subevent(hci, PTR_TO_UINT(data));
 }
 
-void *bt_rap_attach_hci_v2(struct bt_rap *rap, struct bt_hci *hci)
+void *bt_rap_attach_hci(struct bt_rap *rap, struct bt_hci *hci)
 {
 	struct cs_state_machine *sm;
 	unsigned int id;
-- 

[PATCH BlueZ v1 6/6] shared: remove wrapper API

[Naga Bhavani Akella]

Remove bt_rap_attach_hci_v2 wrapper API
which was added to prevent compilation error
in individual patchsets
---
 src/shared/rap.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/shared/rap.h b/src/shared/rap.h
index b6fce8b54..09d392e38 100644
--- a/src/shared/rap.h
+++ b/src/shared/rap.h
@@ -206,7 +206,6 @@ void bt_rap_hci_cs_subevent_result_cont_callback(uint16_t length,
 						  const void *param,
 						  void *user_data);
 
-void *bt_rap_attach_hci_v2(struct bt_rap *rap, struct bt_hci *hci);
 void *bt_rap_attach_hci(struct bt_rap *rap, struct bt_hci *hci);
 void bt_rap_detach_hci(struct bt_rap *rap, void *hci_sm);
 
-- 

RE: RAP: Move CS default settings

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1284 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=1118869

---Test result---

Test Summary:
CheckPatch                    PASS      6.55 seconds
GitLint                       FAIL      4.87 seconds
BuildEll                      PASS      20.63 seconds
BluezMake                     PASS      545.94 seconds
CheckSmatch                   WARNING   313.00 seconds
bluezmakeextell               PASS      102.14 seconds
IncrementalBuild              PASS      688.13 seconds
ScanBuild                     PASS      993.49 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,v1,2/6] profiles: update bt_rap_attach_hci call to drop default settings args

1: T1 Title exceeds max length (84>80): "[BlueZ,v1,2/6] profiles: update bt_rap_attach_hci call to drop default settings args"
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/main.c: note: in included file (through src/device.h):


https://github.com/bluez/bluez/pull/2260

---
Regards,
Linux Bluetooth

RE: RAP: Move CS default settings

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 992 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=1119059

---Test result---

Test Summary:
CheckPatch                    PASS      6.67 seconds
GitLint                       PASS      4.73 seconds
BuildEll                      PASS      20.27 seconds
BluezMake                     PASS      494.20 seconds
CheckSmatch                   WARNING   299.18 seconds
bluezmakeextell               PASS      94.71 seconds
IncrementalBuild              PASS      627.32 seconds
ScanBuild                     PASS      888.34 seconds

Details
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
src/main.c: note: in included file (through src/device.h):


https://github.com/bluez/bluez/pull/2261

---
Regards,
Linux Bluetooth