[PATCH BlueZ 1/3] btdev: pass sent SCO data to bthost

[PATCH BlueZ 1/3] btdev: pass sent SCO data to bthost

[Pauli Virtanen]

Actually send SCO data to the linked connection, if any.
---
 emulator/btdev.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/emulator/btdev.c b/emulator/btdev.c
index ec52c5242..c44b52c49 100644
--- a/emulator/btdev.c
+++ b/emulator/btdev.c
@@ -2807,6 +2807,10 @@ static int cmd_enhanced_setup_sync_conn_complete(struct btdev *dev,
 done:
 	send_event(dev, BT_HCI_EVT_SYNC_CONN_COMPLETE, &cc, sizeof(cc));
 
+	if (conn)
+		send_event(conn->link->dev, BT_HCI_EVT_SYNC_CONN_COMPLETE,
+							&cc, sizeof(cc));
+
 	return 0;
 }
 
@@ -2853,6 +2857,10 @@ static int cmd_setup_sync_conn_complete(struct btdev *dev, const void *data,
 done:
 	send_event(dev, BT_HCI_EVT_SYNC_CONN_COMPLETE, &cc, sizeof(cc));
 
+	if (conn)
+		send_event(conn->link->dev, BT_HCI_EVT_SYNC_CONN_COMPLETE,
+							&cc, sizeof(cc));
+
 	return 0;
 }
 
@@ -7655,6 +7663,33 @@ static void send_acl(struct btdev *dev, const void *data, uint16_t len)
 	send_packet(conn->link->dev, iov, 3);
 }
 
+static void send_sco(struct btdev *dev, const void *data, uint16_t len)
+{
+	struct bt_hci_sco_hdr hdr;
+	struct iovec iov[2];
+	struct btdev_conn *conn;
+	uint8_t pkt_type = BT_H4_SCO_PKT;
+
+	/* Packet type */
+	iov[0].iov_base = &pkt_type;
+	iov[0].iov_len = sizeof(pkt_type);
+
+	iov[1].iov_base = (void *) (data);
+	iov[1].iov_len = len;
+
+	memcpy(&hdr, data, sizeof(hdr));
+
+	conn = queue_find(dev->conns, match_handle,
+					UINT_TO_PTR(acl_handle(hdr.handle)));
+	if (!conn)
+		return;
+
+	/* TODO: flow control */
+
+	if (conn->link)
+		send_packet(conn->link->dev, iov, 2);
+}
+
 static void send_iso(struct btdev *dev, const void *data, uint16_t len)
 {
 	struct bt_hci_acl_hdr *hdr;
@@ -7702,6 +7737,9 @@ void btdev_receive_h4(struct btdev *btdev, const void *data, uint16_t len)
 	case BT_H4_ACL_PKT:
 		send_acl(btdev, data + 1, len - 1);
 		break;
+	case BT_H4_SCO_PKT:
+		send_sco(btdev, data + 1, len - 1);
+		break;
 	case BT_H4_ISO_PKT:
 		send_iso(btdev, data + 1, len - 1);
 		break;
-- 
2.48.1

[PATCH BlueZ 2/3] bthost: add hooks receiving SCO connections and data

[Pauli Virtanen]

Add hooks for new SCO connections and receiving SCO data. SCO
connection accept/reject flow remains unimplemented.
---
 emulator/bthost.c | 122 +++++++++++++++++++++++++++++++++++++++++++++-
 emulator/bthost.h |  10 ++++
 monitor/bt.h      |   1 +
 3 files changed, 132 insertions(+), 1 deletion(-)

diff --git a/emulator/bthost.c b/emulator/bthost.c
index a76b02ecc..9fb68a12a 100644
--- a/emulator/bthost.c
+++ b/emulator/bthost.c
@@ -39,6 +39,8 @@
 #define acl_handle(h)		(h & 0x0fff)
 #define acl_flags(h)		(h >> 12)
 
+#define sco_flags_status(f)	(f & 0x03)
+
 #define iso_flags_pb(f)		(f & 0x0003)
 #define iso_flags_ts(f)		((f >> 2) & 0x0001)
 #define iso_flags_pack(pb, ts)	(((pb) & 0x03) | (((ts) & 0x01) << 2))
@@ -138,8 +140,14 @@ struct rfcomm_chan_hook {
 	struct rfcomm_chan_hook *next;
 };
 
+struct sco_hook {
+	bthost_sco_hook_func_t func;
+	void *user_data;
+	bthost_destroy_func_t destroy;
+};
+
 struct iso_hook {
-	bthost_cid_hook_func_t func;
+	bthost_iso_hook_func_t func;
 	void *user_data;
 	bthost_destroy_func_t destroy;
 };
@@ -155,6 +163,7 @@ struct btconn {
 	struct rcconn *rcconns;
 	struct cid_hook *cid_hooks;
 	struct rfcomm_chan_hook *rfcomm_chan_hooks;
+	struct sco_hook *sco_hook;
 	struct iso_hook *iso_hook;
 	struct btconn *next;
 	void *smp_data;
@@ -241,6 +250,8 @@ struct bthost {
 	void *cmd_complete_data;
 	bthost_new_conn_cb new_conn_cb;
 	void *new_conn_data;
+	bthost_new_conn_cb new_sco_cb;
+	void *new_sco_data;
 	bthost_accept_conn_cb accept_iso_cb;
 	bthost_new_conn_cb new_iso_cb;
 	void *new_iso_data;
@@ -326,9 +337,13 @@ static void btconn_free(struct btconn *conn)
 		free(hook);
 	}
 
+	if (conn->sco_hook && conn->sco_hook->destroy)
+		conn->sco_hook->destroy(conn->sco_hook->user_data);
+
 	if (conn->iso_hook && conn->iso_hook->destroy)
 		conn->iso_hook->destroy(conn->iso_hook->user_data);
 
+	free(conn->sco_hook);
 	free(conn->iso_hook);
 	free(conn->recv_data);
 	free(conn);
@@ -722,6 +737,30 @@ void bthost_add_cid_hook(struct bthost *bthost, uint16_t handle, uint16_t cid,
 	conn->cid_hooks = hook;
 }
 
+void bthost_add_sco_hook(struct bthost *bthost, uint16_t handle,
+				bthost_sco_hook_func_t func, void *user_data,
+				bthost_destroy_func_t destroy)
+{
+	struct sco_hook *hook;
+	struct btconn *conn;
+
+	conn = bthost_find_conn(bthost, handle);
+	if (!conn || conn->sco_hook)
+		return;
+
+	hook = malloc(sizeof(*hook));
+	if (!hook)
+		return;
+
+	memset(hook, 0, sizeof(*hook));
+
+	hook->func = func;
+	hook->user_data = user_data;
+	hook->destroy = destroy;
+
+	conn->sco_hook = hook;
+}
+
 void bthost_add_iso_hook(struct bthost *bthost, uint16_t handle,
 				bthost_iso_hook_func_t func, void *user_data,
 				bthost_destroy_func_t destroy)
@@ -1397,6 +1436,43 @@ static void evt_simple_pairing_complete(struct bthost *bthost, const void *data,
 		return;
 }
 
+static void init_sco(struct bthost *bthost, uint16_t handle,
+				const uint8_t *bdaddr, uint8_t addr_type)
+{
+	struct btconn *conn;
+
+	bthost_debug(bthost, "SCO handle 0x%4.4x", handle);
+
+	conn = malloc(sizeof(*conn));
+	if (!conn)
+		return;
+
+	memset(conn, 0, sizeof(*conn));
+	conn->handle = handle;
+	memcpy(conn->bdaddr, bdaddr, 6);
+	conn->addr_type = addr_type;
+
+	conn->next = bthost->conns;
+	bthost->conns = conn;
+
+	if (bthost->new_sco_cb)
+		bthost->new_sco_cb(handle, bthost->new_sco_data);
+}
+
+static void evt_sync_conn_complete(struct bthost *bthost, const void *data,
+								uint8_t len)
+{
+	const struct bt_hci_evt_sync_conn_complete *ev = data;
+
+	if (len < sizeof(*ev))
+		return;
+
+	if (ev->status)
+		return;
+
+	init_sco(bthost, le16_to_cpu(ev->handle), ev->bdaddr, BDADDR_BREDR);
+}
+
 static void evt_le_conn_complete(struct bthost *bthost, const void *data,
 								uint8_t len)
 {
@@ -1705,6 +1781,10 @@ static void process_evt(struct bthost *bthost, const void *data, uint16_t len)
 		evt_disconn_complete(bthost, param, hdr->plen);
 		break;
 
+	case BT_HCI_EVT_SYNC_CONN_COMPLETE:
+		evt_sync_conn_complete(bthost, param, hdr->plen);
+		break;
+
 	case BT_HCI_EVT_NUM_COMPLETED_PACKETS:
 		evt_num_completed_packets(bthost, param, hdr->plen);
 		break;
@@ -2947,6 +3027,36 @@ static void process_acl(struct bthost *bthost, const void *data, uint16_t len)
 	}
 }
 
+static void process_sco(struct bthost *bthost, const void *data, uint16_t len)
+{
+	const struct bt_hci_sco_hdr *sco_hdr = data;
+	uint16_t handle, sco_len;
+	uint8_t status;
+	struct btconn *conn;
+	struct sco_hook *hook;
+
+	sco_len = le16_to_cpu(sco_hdr->dlen);
+	if (len != sizeof(*sco_hdr) + sco_len)
+		return;
+
+	handle = acl_handle(sco_hdr->handle);
+	status = sco_flags_status(acl_flags(sco_hdr->handle));
+
+	conn = bthost_find_conn(bthost, handle);
+	if (!conn) {
+		bthost_debug(bthost, "Unknown handle: 0x%4.4x", handle);
+		return;
+	}
+
+	bthost_debug(bthost, "SCO data: %u bytes", sco_len);
+
+	hook = conn->sco_hook;
+	if (!hook)
+		return;
+
+	hook->func(sco_hdr->data, sco_len, status, hook->user_data);
+}
+
 static void process_iso_data(struct bthost *bthost, struct btconn *conn,
 					const void *data, uint16_t len)
 {
@@ -3062,6 +3172,9 @@ void bthost_receive_h4(struct bthost *bthost, const void *data, uint16_t len)
 	case BT_H4_ACL_PKT:
 		process_acl(bthost, data + 1, len - 1);
 		break;
+	case BT_H4_SCO_PKT:
+		process_sco(bthost, data + 1, len - 1);
+		break;
 	case BT_H4_ISO_PKT:
 		process_iso(bthost, data + 1, len - 1);
 		break;
@@ -3085,6 +3198,13 @@ void bthost_set_connect_cb(struct bthost *bthost, bthost_new_conn_cb cb,
 	bthost->new_conn_data = user_data;
 }
 
+void bthost_set_sco_cb(struct bthost *bthost, bthost_new_conn_cb cb,
+							void *user_data)
+{
+	bthost->new_sco_cb = cb;
+	bthost->new_sco_data = user_data;
+}
+
 void bthost_set_iso_cb(struct bthost *bthost, bthost_accept_conn_cb accept,
 				bthost_new_conn_cb cb, void *user_data)
 {
diff --git a/emulator/bthost.h b/emulator/bthost.h
index 2c5b0d516..405d66bf0 100644
--- a/emulator/bthost.h
+++ b/emulator/bthost.h
@@ -51,6 +51,9 @@ typedef void (*bthost_new_conn_cb) (uint16_t handle, void *user_data);
 void bthost_set_connect_cb(struct bthost *bthost, bthost_new_conn_cb cb,
 							void *user_data);
 
+void bthost_set_sco_cb(struct bthost *bthost, bthost_new_conn_cb cb,
+							void *user_data);
+
 void bthost_set_iso_cb(struct bthost *bthost, bthost_accept_conn_cb accept,
 				bthost_new_conn_cb cb, void *user_data);
 
@@ -69,6 +72,13 @@ typedef void (*bthost_cid_hook_func_t)(const void *data, uint16_t len,
 void bthost_add_cid_hook(struct bthost *bthost, uint16_t handle, uint16_t cid,
 				bthost_cid_hook_func_t func, void *user_data);
 
+typedef void (*bthost_sco_hook_func_t)(const void *data, uint16_t len,
+					uint8_t status, void *user_data);
+
+void bthost_add_sco_hook(struct bthost *bthost, uint16_t handle,
+				bthost_sco_hook_func_t func, void *user_data,
+				bthost_destroy_func_t destroy);
+
 typedef void (*bthost_iso_hook_func_t)(const void *data, uint16_t len,
 							void *user_data);
 
diff --git a/monitor/bt.h b/monitor/bt.h
index 6fb81abfe..e708e580f 100644
--- a/monitor/bt.h
+++ b/monitor/bt.h
@@ -523,6 +523,7 @@ struct bt_hci_acl_hdr {
 struct bt_hci_sco_hdr {
 	uint16_t handle;
 	uint8_t  dlen;
+	uint8_t  data[];
 } __attribute__ ((packed));
 
 struct bt_hci_iso_hdr {
-- 
2.48.1

[PATCH BlueZ 3/3] sco-tester: check sent SCO data is received at bthost

[Pauli Virtanen]

When sending data, also check that the data is received by bthost.
---
 tools/sco-tester.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/tools/sco-tester.c b/tools/sco-tester.c
index 7f37ca5cf..9886481ff 100644
--- a/tools/sco-tester.c
+++ b/tools/sco-tester.c
@@ -318,10 +318,51 @@ static void client_connectable_complete(uint16_t opcode, uint8_t status,
 		tester_setup_complete();
 }
 
+static void bthost_recv_data(const void *buf, uint16_t len, uint8_t status,
+								void *user_data)
+{
+	struct test_data *data = user_data;
+	const struct sco_client_data *scodata = data->test_data;
+
+	--data->step;
+
+	tester_print("Client received %u bytes of data", len);
+
+	if (scodata->send_data && (scodata->data_len != len ||
+			memcmp(scodata->send_data, buf, len)))
+		tester_test_failed();
+	else if (!data->step)
+		tester_test_passed();
+}
+
+static void bthost_sco_disconnected(void *user_data)
+{
+	struct test_data *data = user_data;
+
+	tester_print("SCO handle 0x%04x disconnected", data->handle);
+
+	data->handle = 0x0000;
+}
+
+static void sco_new_conn(uint16_t handle, void *user_data)
+{
+	struct test_data *data = user_data;
+	struct bthost *host;
+
+	tester_print("New client connection with handle 0x%04x", handle);
+
+	data->handle = handle;
+
+	host = hciemu_client_get_host(data->hciemu);
+	bthost_add_sco_hook(host, data->handle, bthost_recv_data, data,
+				bthost_sco_disconnected);
+}
+
 static void setup_powered_callback(uint8_t status, uint16_t length,
 					const void *param, void *user_data)
 {
 	struct test_data *data = tester_get_data();
+	const struct sco_client_data *scodata = data->test_data;
 	struct bthost *bthost;
 
 	if (status != MGMT_STATUS_SUCCESS) {
@@ -334,6 +375,9 @@ static void setup_powered_callback(uint8_t status, uint16_t length,
 	bthost = hciemu_client_get_host(data->hciemu);
 	bthost_set_cmd_complete_cb(bthost, client_connectable_complete, data);
 	bthost_write_scan_enable(bthost, 0x03);
+
+	if (scodata && scodata->send_data)
+		bthost_set_sco_cb(bthost, sco_new_conn, data);
 }
 
 static void setup_powered(const void *test_data)
@@ -740,8 +784,6 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
 		ssize_t ret = 0;
 		unsigned int count;
 
-		data->step = 0;
-
 		sco_tx_timestamping(data, io);
 
 		tester_print("Writing %u*%u bytes of data",
@@ -751,6 +793,7 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
 			ret = write(sk, scodata->send_data, scodata->data_len);
 			if (scodata->data_len != ret)
 				break;
+			data->step++;
 		}
 		if (scodata->data_len != ret) {
 			tester_warn("Failed to write %u bytes: %zu %s (%d)",
-- 
2.48.1

RE: [BlueZ,1/3] btdev: pass sent SCO data to bthost

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PENDING   0.28 seconds
GitLint                       PENDING   0.22 seconds
BuildEll                      PASS      20.47 seconds
BluezMake                     PASS      1583.23 seconds
MakeCheck                     PASS      14.16 seconds
MakeDistcheck                 PASS      162.66 seconds
CheckValgrind                 PASS      219.55 seconds
CheckSmatch                   WARNING   287.84 seconds
bluezmakeextell               PASS      99.66 seconds
IncrementalBuild              PENDING   0.29 seconds
ScanBuild                     PASS      875.85 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
emulator/btdev.c:450:29: warning: Variable length array is used.emulator/bthost.c:628:28: warning: Variable length array is used.emulator/bthost.c:826:28: warning: Variable length array is used.tools/sco-tester.c: note: in included file:./lib/bluetooth.h:232:15: warning: array of flexible structures./lib/bluetooth.h:237:31: warning: array of flexible structures
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ 1/3] btdev: pass sent SCO data to bthost

[Luiz Augusto von Dentz]

Hi Pauli,

On Wed, Mar 5, 2025 at 10:58 AM Pauli Virtanen <pav@iki.fi> wrote:
>
> Actually send SCO data to the linked connection, if any.
> ---
>  emulator/btdev.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/emulator/btdev.c b/emulator/btdev.c
> index ec52c5242..c44b52c49 100644
> --- a/emulator/btdev.c
> +++ b/emulator/btdev.c
> @@ -2807,6 +2807,10 @@ static int cmd_enhanced_setup_sync_conn_complete(struct btdev *dev,
>  done:
>         send_event(dev, BT_HCI_EVT_SYNC_CONN_COMPLETE, &cc, sizeof(cc));
>
> +       if (conn)
> +               send_event(conn->link->dev, BT_HCI_EVT_SYNC_CONN_COMPLETE,
> +                                                       &cc, sizeof(cc));
> +
>         return 0;
>  }
>
> @@ -2853,6 +2857,10 @@ static int cmd_setup_sync_conn_complete(struct btdev *dev, const void *data,
>  done:
>         send_event(dev, BT_HCI_EVT_SYNC_CONN_COMPLETE, &cc, sizeof(cc));
>
> +       if (conn)
> +               send_event(conn->link->dev, BT_HCI_EVT_SYNC_CONN_COMPLETE,
> +                                                       &cc, sizeof(cc));

Great work, this might be handy for adding tests for SCO data.

>         return 0;
>  }
>
> @@ -7655,6 +7663,33 @@ static void send_acl(struct btdev *dev, const void *data, uint16_t len)
>         send_packet(conn->link->dev, iov, 3);
>  }
>
> +static void send_sco(struct btdev *dev, const void *data, uint16_t len)
> +{
> +       struct bt_hci_sco_hdr hdr;
> +       struct iovec iov[2];
> +       struct btdev_conn *conn;
> +       uint8_t pkt_type = BT_H4_SCO_PKT;
> +
> +       /* Packet type */
> +       iov[0].iov_base = &pkt_type;
> +       iov[0].iov_len = sizeof(pkt_type);
> +
> +       iov[1].iov_base = (void *) (data);
> +       iov[1].iov_len = len;
> +
> +       memcpy(&hdr, data, sizeof(hdr));
> +
> +       conn = queue_find(dev->conns, match_handle,
> +                                       UINT_TO_PTR(acl_handle(hdr.handle)));
> +       if (!conn)
> +               return;
> +
> +       /* TODO: flow control */
> +
> +       if (conn->link)
> +               send_packet(conn->link->dev, iov, 2);
> +}
> +
>  static void send_iso(struct btdev *dev, const void *data, uint16_t len)
>  {
>         struct bt_hci_acl_hdr *hdr;
> @@ -7702,6 +7737,9 @@ void btdev_receive_h4(struct btdev *btdev, const void *data, uint16_t len)
>         case BT_H4_ACL_PKT:
>                 send_acl(btdev, data + 1, len - 1);
>                 break;
> +       case BT_H4_SCO_PKT:
> +               send_sco(btdev, data + 1, len - 1);
> +               break;

Ive done something very similar but adding support for Sync Flow
Control, might be a good idea to rebase once we are done with enabling
that in the kernel so we can start adding more tests to sco-tester.

>         case BT_H4_ISO_PKT:
>                 send_iso(btdev, data + 1, len - 1);
>                 break;
> --
> 2.48.1
>
>


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 3/3] sco-tester: check sent SCO data is received at bthost

[Luiz Augusto von Dentz]

Hi Pauli,

On Wed, Mar 5, 2025 at 10:58 AM Pauli Virtanen <pav@iki.fi> wrote:
>
> When sending data, also check that the data is received by bthost.
> ---
>  tools/sco-tester.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/tools/sco-tester.c b/tools/sco-tester.c
> index 7f37ca5cf..9886481ff 100644
> --- a/tools/sco-tester.c
> +++ b/tools/sco-tester.c
> @@ -318,10 +318,51 @@ static void client_connectable_complete(uint16_t opcode, uint8_t status,
>                 tester_setup_complete();
>  }
>
> +static void bthost_recv_data(const void *buf, uint16_t len, uint8_t status,
> +                                                               void *user_data)
> +{
> +       struct test_data *data = user_data;
> +       const struct sco_client_data *scodata = data->test_data;

I had to add the following change in order to pass with these changes:

+       /* Ignore empty packet as that is used to confirm NOCP is being
+        * generated.
+        */
+       if (!len)
+               return;
+

Now I wonder if this may be a problem since it does show up as a data
packet even though it is empty, I guess for the purpose of HFP
streaming it shouldn't cause problems but if someone attempts to do
packet based checks like this it may stop working.

> +       --data->step;
> +
> +       tester_print("Client received %u bytes of data", len);
> +
> +       if (scodata->send_data && (scodata->data_len != len ||
> +                       memcmp(scodata->send_data, buf, len)))
> +               tester_test_failed();
> +       else if (!data->step)
> +               tester_test_passed();
> +}
> +
> +static void bthost_sco_disconnected(void *user_data)
> +{
> +       struct test_data *data = user_data;
> +
> +       tester_print("SCO handle 0x%04x disconnected", data->handle);
> +
> +       data->handle = 0x0000;
> +}
> +
> +static void sco_new_conn(uint16_t handle, void *user_data)
> +{
> +       struct test_data *data = user_data;
> +       struct bthost *host;
> +
> +       tester_print("New client connection with handle 0x%04x", handle);
> +
> +       data->handle = handle;
> +
> +       host = hciemu_client_get_host(data->hciemu);
> +       bthost_add_sco_hook(host, data->handle, bthost_recv_data, data,
> +                               bthost_sco_disconnected);
> +}
> +
>  static void setup_powered_callback(uint8_t status, uint16_t length,
>                                         const void *param, void *user_data)
>  {
>         struct test_data *data = tester_get_data();
> +       const struct sco_client_data *scodata = data->test_data;
>         struct bthost *bthost;
>
>         if (status != MGMT_STATUS_SUCCESS) {
> @@ -334,6 +375,9 @@ static void setup_powered_callback(uint8_t status, uint16_t length,
>         bthost = hciemu_client_get_host(data->hciemu);
>         bthost_set_cmd_complete_cb(bthost, client_connectable_complete, data);
>         bthost_write_scan_enable(bthost, 0x03);
> +
> +       if (scodata && scodata->send_data)
> +               bthost_set_sco_cb(bthost, sco_new_conn, data);
>  }
>
>  static void setup_powered(const void *test_data)
> @@ -740,8 +784,6 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
>                 ssize_t ret = 0;
>                 unsigned int count;
>
> -               data->step = 0;
> -
>                 sco_tx_timestamping(data, io);
>
>                 tester_print("Writing %u*%u bytes of data",
> @@ -751,6 +793,7 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
>                         ret = write(sk, scodata->send_data, scodata->data_len);
>                         if (scodata->data_len != ret)
>                                 break;
> +                       data->step++;
>                 }
>                 if (scodata->data_len != ret) {
>                         tester_warn("Failed to write %u bytes: %zu %s (%d)",
> --
> 2.48.1
>
>


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 3/3] sco-tester: check sent SCO data is received at bthost

[Pauli Virtanen]

Hi,

ke, 2025-03-05 kello 13:58 -0500, Luiz Augusto von Dentz kirjoitti:
> On Wed, Mar 5, 2025 at 10:58 AM Pauli Virtanen <pav@iki.fi> wrote:
> > 
> > When sending data, also check that the data is received by bthost.
> > ---
> >  tools/sco-tester.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 45 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/sco-tester.c b/tools/sco-tester.c
> > index 7f37ca5cf..9886481ff 100644
> > --- a/tools/sco-tester.c
> > +++ b/tools/sco-tester.c
> > @@ -318,10 +318,51 @@ static void client_connectable_complete(uint16_t opcode, uint8_t status,
> >                 tester_setup_complete();
> >  }
> > 
> > +static void bthost_recv_data(const void *buf, uint16_t len, uint8_t status,
> > +                                                               void *user_data)
> > +{
> > +       struct test_data *data = user_data;
> > +       const struct sco_client_data *scodata = data->test_data;
> 
> I had to add the following change in order to pass with these changes:
> 
> +       /* Ignore empty packet as that is used to confirm NOCP is being
> +        * generated.
> +        */
> +       if (!len)
> +               return;
> +
> 
> Now I wonder if this may be a problem since it does show up as a data
> packet even though it is empty, I guess for the purpose of HFP
> streaming it shouldn't cause problems but if someone attempts to do
> packet based checks like this it may stop working.

It's probably a change in behavior if the controller interprets the
zero-length packet as an instruction to send zero-filled SCO packet
(with valid CRC etc) over the air. I'm not sure what the controllers do
here.

The timer causes a small hiccup at startup, but that is only once per
controller index added. Since then the controller is not spec-
compliant, maybe it is OK and there could be a quirk...

> 
> > +       --data->step;
> > +
> > +       tester_print("Client received %u bytes of data", len);
> > +
> > +       if (scodata->send_data && (scodata->data_len != len ||
> > +                       memcmp(scodata->send_data, buf, len)))
> > +               tester_test_failed();
> > +       else if (!data->step)
> > +               tester_test_passed();
> > +}
> > +
> > +static void bthost_sco_disconnected(void *user_data)
> > +{
> > +       struct test_data *data = user_data;
> > +
> > +       tester_print("SCO handle 0x%04x disconnected", data->handle);
> > +
> > +       data->handle = 0x0000;
> > +}
> > +
> > +static void sco_new_conn(uint16_t handle, void *user_data)
> > +{
> > +       struct test_data *data = user_data;
> > +       struct bthost *host;
> > +
> > +       tester_print("New client connection with handle 0x%04x", handle);
> > +
> > +       data->handle = handle;
> > +
> > +       host = hciemu_client_get_host(data->hciemu);
> > +       bthost_add_sco_hook(host, data->handle, bthost_recv_data, data,
> > +                               bthost_sco_disconnected);
> > +}
> > +
> >  static void setup_powered_callback(uint8_t status, uint16_t length,
> >                                         const void *param, void *user_data)
> >  {
> >         struct test_data *data = tester_get_data();
> > +       const struct sco_client_data *scodata = data->test_data;
> >         struct bthost *bthost;
> > 
> >         if (status != MGMT_STATUS_SUCCESS) {
> > @@ -334,6 +375,9 @@ static void setup_powered_callback(uint8_t status, uint16_t length,
> >         bthost = hciemu_client_get_host(data->hciemu);
> >         bthost_set_cmd_complete_cb(bthost, client_connectable_complete, data);
> >         bthost_write_scan_enable(bthost, 0x03);
> > +
> > +       if (scodata && scodata->send_data)
> > +               bthost_set_sco_cb(bthost, sco_new_conn, data);
> >  }
> > 
> >  static void setup_powered(const void *test_data)
> > @@ -740,8 +784,6 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
> >                 ssize_t ret = 0;
> >                 unsigned int count;
> > 
> > -               data->step = 0;
> > -
> >                 sco_tx_timestamping(data, io);
> > 
> >                 tester_print("Writing %u*%u bytes of data",
> > @@ -751,6 +793,7 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
> >                         ret = write(sk, scodata->send_data, scodata->data_len);
> >                         if (scodata->data_len != ret)
> >                                 break;
> > +                       data->step++;
> >                 }
> >                 if (scodata->data_len != ret) {
> >                         tester_warn("Failed to write %u bytes: %zu %s (%d)",
> > --
> > 2.48.1
> > 
> > 
> 
> 

Re: [PATCH BlueZ 3/3] sco-tester: check sent SCO data is received at bthost

[Luiz Augusto von Dentz]

Hi Pauli,

On Thu, Mar 6, 2025 at 2:37 PM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi,
>
> ke, 2025-03-05 kello 13:58 -0500, Luiz Augusto von Dentz kirjoitti:
> > On Wed, Mar 5, 2025 at 10:58 AM Pauli Virtanen <pav@iki.fi> wrote:
> > >
> > > When sending data, also check that the data is received by bthost.
> > > ---
> > >  tools/sco-tester.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--
> > >  1 file changed, 45 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tools/sco-tester.c b/tools/sco-tester.c
> > > index 7f37ca5cf..9886481ff 100644
> > > --- a/tools/sco-tester.c
> > > +++ b/tools/sco-tester.c
> > > @@ -318,10 +318,51 @@ static void client_connectable_complete(uint16_t opcode, uint8_t status,
> > >                 tester_setup_complete();
> > >  }
> > >
> > > +static void bthost_recv_data(const void *buf, uint16_t len, uint8_t status,
> > > +                                                               void *user_data)
> > > +{
> > > +       struct test_data *data = user_data;
> > > +       const struct sco_client_data *scodata = data->test_data;
> >
> > I had to add the following change in order to pass with these changes:
> >
> > +       /* Ignore empty packet as that is used to confirm NOCP is being
> > +        * generated.
> > +        */
> > +       if (!len)
> > +               return;
> > +
> >
> > Now I wonder if this may be a problem since it does show up as a data
> > packet even though it is empty, I guess for the purpose of HFP
> > streaming it shouldn't cause problems but if someone attempts to do
> > packet based checks like this it may stop working.
>
> It's probably a change in behavior if the controller interprets the
> zero-length packet as an instruction to send zero-filled SCO packet
> (with valid CRC etc) over the air. I'm not sure what the controllers do
> here.
>
> The timer causes a small hiccup at startup, but that is only once per
> controller index added. Since then the controller is not spec-
> compliant, maybe it is OK and there could be a quirk...

Yeah, that said we could perhaps add a quirk to mark Sync Flow Control
as supported, rather than adding one for saying it is broken, that way
we can enable it only in the controllers that are known to work
otherwise there is always a chance that controllers don't behave as
expected.

> >
> > > +       --data->step;
> > > +
> > > +       tester_print("Client received %u bytes of data", len);
> > > +
> > > +       if (scodata->send_data && (scodata->data_len != len ||
> > > +                       memcmp(scodata->send_data, buf, len)))
> > > +               tester_test_failed();
> > > +       else if (!data->step)
> > > +               tester_test_passed();
> > > +}
> > > +
> > > +static void bthost_sco_disconnected(void *user_data)
> > > +{
> > > +       struct test_data *data = user_data;
> > > +
> > > +       tester_print("SCO handle 0x%04x disconnected", data->handle);
> > > +
> > > +       data->handle = 0x0000;
> > > +}
> > > +
> > > +static void sco_new_conn(uint16_t handle, void *user_data)
> > > +{
> > > +       struct test_data *data = user_data;
> > > +       struct bthost *host;
> > > +
> > > +       tester_print("New client connection with handle 0x%04x", handle);
> > > +
> > > +       data->handle = handle;
> > > +
> > > +       host = hciemu_client_get_host(data->hciemu);
> > > +       bthost_add_sco_hook(host, data->handle, bthost_recv_data, data,
> > > +                               bthost_sco_disconnected);
> > > +}
> > > +
> > >  static void setup_powered_callback(uint8_t status, uint16_t length,
> > >                                         const void *param, void *user_data)
> > >  {
> > >         struct test_data *data = tester_get_data();
> > > +       const struct sco_client_data *scodata = data->test_data;
> > >         struct bthost *bthost;
> > >
> > >         if (status != MGMT_STATUS_SUCCESS) {
> > > @@ -334,6 +375,9 @@ static void setup_powered_callback(uint8_t status, uint16_t length,
> > >         bthost = hciemu_client_get_host(data->hciemu);
> > >         bthost_set_cmd_complete_cb(bthost, client_connectable_complete, data);
> > >         bthost_write_scan_enable(bthost, 0x03);
> > > +
> > > +       if (scodata && scodata->send_data)
> > > +               bthost_set_sco_cb(bthost, sco_new_conn, data);
> > >  }
> > >
> > >  static void setup_powered(const void *test_data)
> > > @@ -740,8 +784,6 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
> > >                 ssize_t ret = 0;
> > >                 unsigned int count;
> > >
> > > -               data->step = 0;
> > > -
> > >                 sco_tx_timestamping(data, io);
> > >
> > >                 tester_print("Writing %u*%u bytes of data",
> > > @@ -751,6 +793,7 @@ static gboolean sco_connect_cb(GIOChannel *io, GIOCondition cond,
> > >                         ret = write(sk, scodata->send_data, scodata->data_len);
> > >                         if (scodata->data_len != ret)
> > >                                 break;
> > > +                       data->step++;
> > >                 }
> > >                 if (scodata->data_len != ret) {
> > >                         tester_warn("Failed to write %u bytes: %zu %s (%d)",
> > > --
> > > 2.48.1
> > >
> > >
> >
> >
>


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 1/3] btdev: pass sent SCO data to bthost

[patchwork-bot+bluetooth]

Hello:

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

On Wed,  5 Mar 2025 17:58:24 +0200 you wrote:
> Actually send SCO data to the linked connection, if any.
> ---
>  emulator/btdev.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)

Here is the summary with links:
  - [BlueZ,1/3] btdev: pass sent SCO data to bthost
    (no matching commit)
  - [BlueZ,2/3] bthost: add hooks receiving SCO connections and data
    (no matching commit)
  - [BlueZ,3/3] sco-tester: check sent SCO data is received at bthost
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=87d4f95dc312

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