[PATCH BlueZ v2] shared/vcp: have only one volume change in flight at a time

[Pauli Virtanen <pav@xxxxxx>]

If bt_vcp_set_volume() is called again before the previous operation has
completed, the requests get the same change counter, and all except the
first one fail.

Fix by sending another volume set request only after the previous has
either failed or generated a volume notification.

Send only volume requests that change the value to a different one than
last notification we have seen: in this case the request either fails,
or succeeds and generates a new notification.  In all these cases, we
exit the wait state correctly.
---

Notes:
    v2: reset pending_ops when attaching, needs to be cleared on reconnect

 src/shared/vcp.c | 39 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/shared/vcp.c b/src/shared/vcp.c
index 6b0f2f9db..2b5d186b8 100644
--- a/src/shared/vcp.c
+++ b/src/shared/vcp.c
@@ -203,6 +203,9 @@ struct bt_vcp {
 	uint8_t volume;
 	uint8_t volume_counter;
 
+	uint8_t pending_volume;
+	unsigned int pending_ops;
+
 	void *debug_data;
 	void *user_data;
 };
@@ -2003,6 +2006,19 @@ done:
 	return vcp;
 }
 
+static void vcp_set_pending_volume(struct bt_vcp *vcp)
+{
+	/* Send pending request if any */
+	if (vcp->pending_ops <= 1) {
+		vcp->pending_ops = 0;
+		return;
+	}
+	vcp->pending_ops = 0;
+
+	DBG(vcp, "set pending volume 0x%x", vcp->pending_volume);
+	bt_vcp_set_volume(vcp, vcp->pending_volume);
+}
+
 static void vcp_vstate_notify(struct bt_vcp *vcp, uint16_t value_handle,
 				const uint8_t *value, uint16_t length,
 				void *user_data)
@@ -2020,6 +2036,8 @@ static void vcp_vstate_notify(struct bt_vcp *vcp, uint16_t value_handle,
 
 	if (vcp->volume_changed)
 		vcp->volume_changed(vcp, vcp->volume);
+
+	vcp_set_pending_volume(vcp);
 }
 
 static void vcp_volume_cp_sent(bool success, uint8_t err, void *user_data)
@@ -2031,6 +2049,8 @@ static void vcp_volume_cp_sent(bool success, uint8_t err, void *user_data)
 			DBG(vcp, "setting volume failed: invalid counter");
 		else
 			DBG(vcp, "setting volume failed: error 0x%x", err);
+
+		vcp_set_pending_volume(vcp);
 	}
 }
 
@@ -2061,9 +2081,20 @@ static bool vcp_set_volume_client(struct bt_vcp *vcp, uint8_t volume)
 		return false;
 	}
 
-	vcp->volume = volume;
+	vcp->pending_volume = volume;
+	if (vcp->pending_ops) {
+		/* Wait for current operation to complete */
+		vcp->pending_ops++;
+		return true;
+	} else if (vcp->volume == vcp->pending_volume) {
+		/* Do not set to current value, as that doesn't generate
+		 * a notification
+		 */
+		return true;
+	}
+
 	req.op = BT_VCS_SET_ABSOLUTE_VOL;
-	req.vol_set = vcp->volume;
+	req.vol_set = vcp->pending_volume;
 	req.change_counter = vcp->volume_counter;
 
 	if (!bt_gatt_client_write_value(vcp->client, value_handle, (void *)&req,
@@ -2072,6 +2103,8 @@ static bool vcp_set_volume_client(struct bt_vcp *vcp, uint8_t volume)
 		DBG(vcp, "error writing volume");
 		return false;
 	}
+
+	vcp->pending_ops++;
 	return true;
 }
 
@@ -2896,6 +2929,8 @@ bool bt_vcp_attach(struct bt_vcp *vcp, struct bt_gatt_client *client)
 	if (!vcp->client)
 		return false;
 
+	vcp->pending_ops = 0;
+
 	bt_uuid16_create(&uuid, VCS_UUID);
 	gatt_db_foreach_service(vcp->rdb->db, &uuid, foreach_vcs_service, vcp);
 
-- 
2.48.1