[BlueZ,v2,1/5] shared/vcp: add API to control volume on VCP client

Commit Message

Michal Dzik Jan. 21, 2025, 12:47 p.m. UTC

---
 src/shared/vcp.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++-
 src/shared/vcp.h |  7 ++++
 2 files changed, 89 insertions(+), 1 deletion(-)

Comments

[e-mail] Jan. 21, 2025, 1:41 p.m. UTC | #1

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=927226

---Test result---

Test Summary:
CheckPatch                    PENDING   0.21 seconds
GitLint                       PENDING   0.21 seconds
BuildEll                      PASS      20.30 seconds
BluezMake                     FAIL      17.92 seconds
MakeCheck                     FAIL      35.20 seconds
MakeDistcheck                 PASS      159.38 seconds
CheckValgrind                 FAIL      15.00 seconds
CheckSmatch                   FAIL      20.09 seconds
bluezmakeextell               FAIL      13.34 seconds
IncrementalBuild              PENDING   0.25 seconds
ScanBuild                     PASS      861.73 seconds

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

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

##############################
Test: BluezMake - FAIL
Desc: Build BlueZ
Output:

src/shared/vcp.c:2042:6: error: no previous declaration for ‘bt_vcp_set_volume_client’ [-Werror=missing-declarations]
 2042 | bool bt_vcp_set_volume_client(struct bt_vcp *vcp, uint8_t volume)
      |      ^~~~~~~~~~~~~~~~~~~~~~~~
src/shared/vcp.c:2078:6: error: no previous declaration for ‘bt_vcp_set_volume_server’ [-Werror=missing-declarations]
 2078 | bool bt_vcp_set_volume_server(struct bt_vcp *vcp, uint8_t volume)
      |      ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:8676: src/shared/libshared_mainloop_la-vcp.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4693: all] Error 2
##############################
Test: MakeCheck - FAIL
Desc: Run Bluez Make Check
Output:

src/shared/vcp.c:2042:6: error: no previous declaration for ‘bt_vcp_set_volume_client’ [-Werror=missing-declarations]
 2042 | bool bt_vcp_set_volume_client(struct bt_vcp *vcp, uint8_t volume)
      |      ^~~~~~~~~~~~~~~~~~~~~~~~
src/shared/vcp.c:2078:6: error: no previous declaration for ‘bt_vcp_set_volume_server’ [-Werror=missing-declarations]
 2078 | bool bt_vcp_set_volume_server(struct bt_vcp *vcp, uint8_t volume)
      |      ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:8431: src/shared/libshared_glib_la-vcp.lo] Error 1
make: *** [Makefile:12344: check] Error 2
##############################
Test: CheckValgrind - FAIL
Desc: Run Bluez Make Check with Valgrind
Output:

src/shared/vcp.c:2042:6: error: no previous declaration for ‘bt_vcp_set_volume_client’ [-Werror=missing-declarations]
 2042 | bool bt_vcp_set_volume_client(struct bt_vcp *vcp, uint8_t volume)
      |      ^~~~~~~~~~~~~~~~~~~~~~~~
src/shared/vcp.c:2078:6: error: no previous declaration for ‘bt_vcp_set_volume_server’ [-Werror=missing-declarations]
 2078 | bool bt_vcp_set_volume_server(struct bt_vcp *vcp, uint8_t volume)
      |      ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:8676: src/shared/libshared_mainloop_la-vcp.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:12344: check] Error 2
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:

src/shared/crypto.c:271:21: warning: Variable length array is used.
src/shared/crypto.c:272:23: warning: Variable length array is used.
src/shared/gatt-helpers.c:768:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:830:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:1323:31: warning: Variable length array is used.
src/shared/gatt-helpers.c:1354:23: warning: Variable length array is used.
src/shared/gatt-server.c:278:25: warning: Variable length array is used.
src/shared/gatt-server.c:618:25: warning: Variable length array is used.
src/shared/gatt-server.c:716:25: warning: Variable length array is used.
src/shared/bap.c:296:25: warning: array of flexible structures
src/shared/bap.c: note: in included file:
./src/shared/ascs.h:88:25: warning: array of flexible structures
src/shared/vcp.c:2042:6: error: no previous declaration for ‘bt_vcp_set_volume_client’ [-Werror=missing-declarations]
 2042 | bool bt_vcp_set_volume_client(struct bt_vcp *vcp, uint8_t volume)
      |      ^~~~~~~~~~~~~~~~~~~~~~~~
src/shared/vcp.c:2078:6: error: no previous declaration for ‘bt_vcp_set_volume_server’ [-Werror=missing-declarations]
 2078 | bool bt_vcp_set_volume_server(struct bt_vcp *vcp, uint8_t volume)
      |      ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:8676: src/shared/libshared_mainloop_la-vcp.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4693: all] Error 2
##############################
Test: bluezmakeextell - FAIL
Desc: Build Bluez with External ELL
Output:

src/shared/vcp.c:2042:6: error: no previous declaration for ‘bt_vcp_set_volume_client’ [-Werror=missing-declarations]
 2042 | bool bt_vcp_set_volume_client(struct bt_vcp *vcp, uint8_t volume)
      |      ^~~~~~~~~~~~~~~~~~~~~~~~
src/shared/vcp.c:2078:6: error: no previous declaration for ‘bt_vcp_set_volume_server’ [-Werror=missing-declarations]
 2078 | bool bt_vcp_set_volume_server(struct bt_vcp *vcp, uint8_t volume)
      |      ^~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:8676: src/shared/libshared_mainloop_la-vcp.lo] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:4693: all] Error 2
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Patch

diff --git a/src/shared/vcp.c b/src/shared/vcp.c
index cfc426624..abc5a35a3 100644
--- a/src/shared/vcp.c
+++ b/src/shared/vcp.c
@@ -147,6 +147,12 @@  struct bt_vcs_ab_vol {
 	uint8_t	vol_set;
 } __packed;
 
+struct bt_vcs_client_ab_vol {
+	uint8_t	op;
+	uint8_t	change_counter;
+	uint8_t	vol_set;
+} __packed;
+
 struct bt_vocs_set_vol_off {
 	uint8_t	change_counter;
 	int16_t set_vol_offset;
@@ -192,6 +198,11 @@  struct bt_vcp {
 
 	bt_vcp_debug_func_t debug_func;
 	bt_vcp_destroy_func_t debug_destroy;
+	bt_vcp_volume_func_t volume_changed;
+
+	uint8_t volume;
+	uint8_t volume_counter;
+
 	void *debug_data;
 	void *user_data;
 };
@@ -1874,6 +1885,15 @@  bool bt_vcp_set_debug(struct bt_vcp *vcp, bt_vcp_debug_func_t func,
 	return true;
 }
 
+bool bt_vcp_set_volume_callback(struct bt_vcp *vcp,
+				bt_vcp_volume_func_t volume_changed)
+{
+	if (!vcp)
+		return false;
+
+	vcp->volume_changed = volume_changed;
+	return true;
+}
 unsigned int bt_vcp_register(bt_vcp_func_t attached, bt_vcp_func_t detached,
 							void *user_data)
 {
@@ -1959,6 +1979,65 @@  static void vcp_vstate_notify(struct bt_vcp *vcp, uint16_t value_handle,
 	DBG(vcp, "Vol Settings 0x%x", vstate.vol_set);
 	DBG(vcp, "Mute Status 0x%x", vstate.mute);
 	DBG(vcp, "Vol Counter 0x%x", vstate.counter);
+
+	vcp->volume = vstate.vol_set;
+	vcp->volume_counter = vstate.counter;
+
+	if (vcp->volume_changed)
+		vcp->volume_changed(vcp, vcp->volume);
+}
+
+static void vcp_volume_cp_sent(bool success, uint8_t err, void *user_data)
+{
+	struct bt_vcp *vcp = user_data;
+
+	if (!success) {
+		if (err == BT_ATT_ERROR_INVALID_CHANGE_COUNTER)
+			DBG(vcp, "setting volume failed: invalid counter");
+		else
+			DBG(vcp, "setting volume failed: error 0x%x", err);
+	}
+}
+
+uint8_t bt_vcp_get_volume(struct bt_vcp *vcp)
+{
+	return vcp->volume;
+}
+
+bool bt_vcp_set_volume(struct bt_vcp *vcp, uint8_t volume)
+{
+	struct bt_vcs_client_ab_vol req;
+	uint16_t value_handle;
+	struct bt_vcs *vcs = vcp_get_vcs(vcp);
+
+	if (!vcs) {
+		DBG(vcp, "error: vcs not available");
+		return false;
+	}
+
+	if (!vcs->vol_cp) {
+		DBG(vcp, "error: vol_cp characteristics not available");
+		return false;
+	}
+
+	if (!gatt_db_attribute_get_char_data(vcs->vol_cp, NULL, &value_handle,
+							NULL, NULL, NULL)) {
+		DBG(vcp, "error: vol_cp characteristics not available");
+		return false;
+	}
+
+	vcp->volume = volume;
+	req.op = BT_VCS_SET_ABSOLUTE_VOL;
+	req.vol_set = vcp->volume;
+	req.change_counter = vcp->volume_counter;
+
+	if (!bt_gatt_client_write_value(vcp->client, value_handle, (void *) &req,
+		sizeof(struct bt_vcs_client_ab_vol), vcp_volume_cp_sent, vcp,
+									NULL)) {
+		DBG(vcp, "error writing volume");
+		return false;
+	}
+	return true;
 }
 
 static void vcp_voffset_state_notify(struct bt_vcp *vcp, uint16_t value_handle,
@@ -2061,6 +2140,9 @@  static void read_vol_state(struct bt_vcp *vcp, bool success, uint8_t att_ecode,
 	DBG(vcp, "Vol Set:%x", vs->vol_set);
 	DBG(vcp, "Vol Mute:%x", vs->mute);
 	DBG(vcp, "Vol Counter:%x", vs->counter);
+
+	vcp->volume = vs->vol_set;
+	vcp->volume_counter = vs->counter;
 }
 
 static void read_vol_offset_state(struct bt_vcp *vcp, bool success,
@@ -2757,4 +2839,3 @@  bool bt_vcp_attach(struct bt_vcp *vcp, struct bt_gatt_client *client)
 
 	return true;
 }
-
diff --git a/src/shared/vcp.h b/src/shared/vcp.h
index 26db5732d..705b6f301 100644
--- a/src/shared/vcp.h
+++ b/src/shared/vcp.h
@@ -36,6 +36,7 @@  struct bt_vcp;
 typedef void (*bt_vcp_destroy_func_t)(void *user_data);
 typedef void (*bt_vcp_debug_func_t)(const char *str, void *user_data);
 typedef void (*bt_vcp_func_t)(struct bt_vcp *vcp, void *user_data);
+typedef void (*bt_vcp_volume_func_t)(struct bt_vcp *vcp, uint8_t volume);
 
 struct bt_vcp *bt_vcp_ref(struct bt_vcp *vcp);
 void bt_vcp_unref(struct bt_vcp *vcp);
@@ -45,9 +46,15 @@  void bt_vcp_add_db(struct gatt_db *db);
 bool bt_vcp_attach(struct bt_vcp *vcp, struct bt_gatt_client *client);
 void bt_vcp_detach(struct bt_vcp *vcp);
 
+uint8_t bt_vcp_get_volume(struct bt_vcp *vcp);
+bool bt_vcp_set_volume(struct bt_vcp *vcp, uint8_t volume);
+
 bool bt_vcp_set_debug(struct bt_vcp *vcp, bt_vcp_debug_func_t cb,
 			void *user_data, bt_vcp_destroy_func_t destroy);
 
+bool bt_vcp_set_volume_callback(struct bt_vcp *vcp,
+				bt_vcp_volume_func_t volume_changed);
+
 struct bt_att *bt_vcp_get_att(struct bt_vcp *vcp);
 
 bool bt_vcp_set_user_data(struct bt_vcp *vcp, void *user_data);