Message from the future (wrong date) (was: [PATCH 1/3] audio/transport: seprate a2dp and bap transport to update volume)

Message from the future (wrong date) (was: [PATCH 1/3] audio/transport: seprate a2dp and bap transport to update volume)

[Paul Menzel]

Dear Sathish,


Thank you for your patches.


Am 30.09.22 um 20:49 schrieb Sathish Narasimman:

[…]

It looks like the clock on your system is incorrectly configured.

     Received: from intel-latitude-e5450.iind.intel.com ([10.224.186.32])
       by FMSMGA003.fm.intel.com with ESMTP; 30 Sep 2022 02:58:50 -0700
     […]
     Date:   Sat,  1 Oct 2022 00:19:52 +0530

Could you please fix that?

Also there is a typo in the summary/title: seprate → separate


Kind regards,

Paul

Re: Message from the future (wrong date) (was: [PATCH 1/3] audio/transport: seprate a2dp and bap transport to update volume)

[Sathish Narasimman]

 patches

On Fri, Sep 30, 2022 at 3:50 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Sathish,
>
>
> Thank you for your patches.
>
>
> Am 30.09.22 um 20:49 schrieb Sathish Narasimman:
>
> […]
>
> It looks like the clock on your system is incorrectly configured.
>
>      Received: from intel-latitude-e5450.iind.intel.com ([10.224.186.32])
>        by FMSMGA003.fm.intel.com with ESMTP; 30 Sep 2022 02:58:50 -0700
>      […]
>      Date:   Sat,  1 Oct 2022 00:19:52 +0530
>
> Could you please fix that?
>
> Also there is a typo in the summary/title: seprate → separate

Sure will update
>
>
> Kind regards,
>
> Paul

RE: [1/3] audio/transport: seprate a2dp and bap transport to update volume

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      2.50 seconds
GitLint                       PASS      1.47 seconds
Prep - Setup ELL              PASS      32.20 seconds
Build - Prep                  PASS      0.66 seconds
Build - Configure             PASS      10.32 seconds
Build - Make                  PASS      994.72 seconds
Make Check                    PASS      12.20 seconds
Make Check w/Valgrind         PASS      350.89 seconds
Make Distcheck                PASS      294.93 seconds
Build w/ext ELL - Configure   PASS      10.43 seconds
Build w/ext ELL - Make        PASS      102.87 seconds
Incremental Build w/ patches  PASS      364.73 seconds
Scan Build                    PASS      673.99 seconds



---
Regards,
Linux Bluetooth

Re: [PATCH 1/3] audio/transport: seprate a2dp and bap transport to update volume

[Luiz Augusto von Dentz]

Hi Sathish,

On Fri, Sep 30, 2022 at 3:14 AM Sathish Narasimman
<sathish.narasimman@intel.com> wrote:
>
> Identify BAP and a2dp transport to update the transport volume.
> Also add properties volume for bap_transport.
> ---
>  profiles/audio/media.c     |   2 +-
>  profiles/audio/transport.c | 120 ++++++++++++++++++++++++++++++++++---
>  profiles/audio/transport.h |   7 ++-
>  3 files changed, 117 insertions(+), 12 deletions(-)
>
> diff --git a/profiles/audio/media.c b/profiles/audio/media.c
> index c9328ab9bd6e..0e3c7cbbaf2c 100644
> --- a/profiles/audio/media.c
> +++ b/profiles/audio/media.c
> @@ -523,7 +523,7 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
>                 return FALSE;
>
>         init_volume = media_player_get_device_volume(device);
> -       media_transport_update_volume(transport, init_volume);
> +       media_a2dp_transport_update_volume(transport, init_volume);
>
>         msg = dbus_message_new_method_call(endpoint->sender, endpoint->path,
>                                                 MEDIA_ENDPOINT_INTERFACE,
> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> index 41339da51e17..a606d45eb628 100644
> --- a/profiles/audio/transport.c
> +++ b/profiles/audio/transport.c
> @@ -91,6 +91,7 @@ struct bap_transport {
>         uint8_t                 rtn;
>         uint16_t                latency;
>         uint32_t                delay;
> +       int8_t                  volume;
>  };
>
>  struct media_transport {
> @@ -709,7 +710,8 @@ static gboolean get_delay_reporting(const GDBusPropertyTable *property,
>         return TRUE;
>  }
>
> -static gboolean volume_exists(const GDBusPropertyTable *property, void *data)
> +static gboolean volume_a2dp_exists(const GDBusPropertyTable *property,
> +                                       void *data)
>  {
>         struct media_transport *transport = data;
>         struct a2dp_transport *a2dp = transport->data;
> @@ -717,7 +719,8 @@ static gboolean volume_exists(const GDBusPropertyTable *property, void *data)
>         return a2dp->volume >= 0;
>  }
>
> -static gboolean get_volume(const GDBusPropertyTable *property,
> +
> +static gboolean get_a2dp_volume(const GDBusPropertyTable *property,
>                                         DBusMessageIter *iter, void *data)
>  {
>         struct media_transport *transport = data;
> @@ -729,7 +732,7 @@ static gboolean get_volume(const GDBusPropertyTable *property,
>         return TRUE;
>  }
>
> -static void set_volume(const GDBusPropertyTable *property,
> +static void set_a2dp_volume(const GDBusPropertyTable *property,
>                         DBusMessageIter *iter, GDBusPendingPropertySet id,
>                         void *data)
>  {
> @@ -769,6 +772,58 @@ error:
>                                         "Invalid arguments in method call");
>  }
>
> +static gboolean volume_bap_exists(const GDBusPropertyTable *property,
> +                                       void *data)
> +{
> +       struct media_transport *transport = data;
> +       struct bap_transport *bap = transport->data;
> +
> +       return bap->volume >= 0;
> +}
> +
> +static gboolean get_bap_volume(const GDBusPropertyTable *property,
> +                                       DBusMessageIter *iter, void *data)
> +{
> +       struct media_transport *transport = data;
> +       struct bap_transport *bap = transport->data;
> +       uint16_t volume = (uint16_t)bap->volume;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &volume);
> +
> +       return TRUE;
> +}
> +
> +static void set_bap_volume(const GDBusPropertyTable *property,
> +                          DBusMessageIter *iter, GDBusPendingPropertySet id,
> +                          void *data)
> +{
> +       struct media_transport *transport = data;
> +       struct bap_transport *bap = transport->data;
> +       uint16_t arg;
> +       int8_t volume;
> +
> +       if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT16)
> +               goto error;
> +
> +       dbus_message_iter_get_basic(iter, &arg);
> +       if (arg > INT8_MAX)
> +               goto error;
> +
> +       g_dbus_pending_property_success(id);
> +
> +       volume = (int8_t)arg;
> +       if (bap->volume == volume)
> +               return;
> +
> +       /*TODO vcp_send_volume */
> +       return;
> +
> +error:
> +       g_dbus_pending_property_error(id, ERROR_INTERFACE ".InvalidArguments",
> +                                       "Invalid arguments in method call");
> +}
> +
> +
>  static gboolean endpoint_exists(const GDBusPropertyTable *property, void *data)
>  {
>         struct media_transport *transport = data;
> @@ -809,7 +864,8 @@ static const GDBusPropertyTable a2dp_properties[] = {
>         { "Configuration", "ay", get_configuration },
>         { "State", "s", get_state },
>         { "Delay", "q", get_delay_reporting, NULL, delay_reporting_exists },
> -       { "Volume", "q", get_volume, set_volume, volume_exists },
> +       { "Volume", "q", get_a2dp_volume, set_a2dp_volume,
> +                                       volume_a2dp_exists },
>         { "Endpoint", "o", get_endpoint, NULL, endpoint_exists,
>                                 G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
>         { }
> @@ -970,6 +1026,7 @@ static const GDBusPropertyTable bap_properties[] = {
>         { "Retransmissions", "y", get_retransmissions },
>         { "Latency", "q", get_latency },
>         { "Delay", "u", get_delay },
> +       { "Volume", "q", get_bap_volume, set_bap_volume, volume_bap_exists },
>         { "Endpoint", "o", get_endpoint, NULL, endpoint_exists },
>         { "Location", "u", get_location },
>         { "Metadata", "ay", get_metadata },
> @@ -1403,6 +1460,7 @@ static int media_transport_init_bap(struct media_transport *transport,
>         bap->rtn = qos->rtn;
>         bap->latency = qos->latency;
>         bap->delay = qos->delay;
> +       bap->volume = 127;
>         bap->state_id = bt_bap_state_register(bt_bap_stream_get_session(stream),
>                                                 bap_state_changed,
>                                                 bap_connecting,
> @@ -1502,14 +1560,22 @@ struct btd_device *media_transport_get_dev(struct media_transport *transport)
>         return transport->device;
>  }
>
> -int8_t media_transport_get_volume(struct media_transport *transport)
> +int8_t media_a2dp_transport_get_volume(struct media_transport *transport)
>  {
>         struct a2dp_transport *a2dp = transport->data;
> +
>         return a2dp->volume;
>  }
>
> -void media_transport_update_volume(struct media_transport *transport,
> -                                                               int8_t volume)
> +int8_t media_bap_transport_get_volume(struct media_transport *transport)
> +{
> +       struct bap_transport *bap = transport->data;
> +
> +       return bap->volume;
> +}
> +
> +void media_a2dp_transport_update_volume(struct media_transport *transport,
> +                                       int8_t volume)
>  {
>         struct a2dp_transport *a2dp = transport->data;
>
> @@ -1527,6 +1593,25 @@ void media_transport_update_volume(struct media_transport *transport,
>                                         MEDIA_TRANSPORT_INTERFACE, "Volume");
>  }
>
> +void media_bap_transport_update_volume(struct media_transport *transport,
> +                                                               int8_t volume)
> +{
> +       struct bap_transport *bap = transport->data;
> +
> +       if (volume < 0)
> +               return;
> +
> +       /* Check if volume really changed */
> +       if (bap->volume == volume)
> +               return;
> +
> +       bap->volume = volume;
> +
> +       g_dbus_emit_property_changed(btd_get_dbus_connection(),
> +                                       transport->path,
> +                                       MEDIA_TRANSPORT_INTERFACE, "Volume");
> +}
> +
>  int8_t media_transport_get_device_volume(struct btd_device *dev)
>  {
>         GSList *l;
> @@ -1537,12 +1622,21 @@ int8_t media_transport_get_device_volume(struct btd_device *dev)
>         /* Attempt to locate the transport to get its volume */
>         for (l = transports; l; l = l->next) {
>                 struct media_transport *transport = l->data;
> +               const char *uuid = media_endpoint_get_uuid(transport->endpoint);
> +
> +               /* Locate BAP Transport */
> +               if (!strcasecmp(uuid, PAC_SINK_UUID) &&
> +                               !strcasecmp(uuid, PAC_SOURCE_UUID)) {
> +
> +                       return media_bap_transport_get_volume(transport);
> +               }
> +
>                 if (transport->device != dev)
>                         continue;
>
>                 /* Volume is A2DP only */
>                 if (media_endpoint_get_sep(transport->endpoint))
> -                       return media_transport_get_volume(transport);
> +                       return media_a2dp_transport_get_volume(transport);
>         }
>
>         /* If transport volume doesn't exists use device_volume */
> @@ -1560,12 +1654,20 @@ void media_transport_update_device_volume(struct btd_device *dev,
>         /* Attempt to locate the transport to set its volume */
>         for (l = transports; l; l = l->next) {
>                 struct media_transport *transport = l->data;
> +               const char *uuid = media_endpoint_get_uuid(transport->endpoint);
> +
> +               /* Locate BAP Transport */
> +               if (!strcasecmp(uuid, PAC_SINK_UUID)) {
> +                       media_bap_transport_update_volume(transport, volume);
> +                       return;
> +               }
> +
>                 if (transport->device != dev)
>                         continue;
>
>                 /* Volume is A2DP only */
>                 if (media_endpoint_get_sep(transport->endpoint)) {
> -                       media_transport_update_volume(transport, volume);
> +                       media_a2dp_transport_update_volume(transport, volume);
>                         return;
>                 }
>         }
> diff --git a/profiles/audio/transport.h b/profiles/audio/transport.h
> index 102fc3cf1153..3cbefa5d7bee 100644
> --- a/profiles/audio/transport.h
> +++ b/profiles/audio/transport.h
> @@ -20,10 +20,13 @@ struct media_transport *media_transport_create(struct btd_device *device,
>  void media_transport_destroy(struct media_transport *transport);
>  const char *media_transport_get_path(struct media_transport *transport);
>  struct btd_device *media_transport_get_dev(struct media_transport *transport);
> -int8_t media_transport_get_volume(struct media_transport *transport);
> +int8_t media_a2dp_transport_get_volume(struct media_transport *transport);
> +int8_t media_bap_transport_get_volume(struct media_transport *transport);
>  void media_transport_update_delay(struct media_transport *transport,
>                                                         uint16_t delay);
> -void media_transport_update_volume(struct media_transport *transport,
> +void media_a2dp_transport_update_volume(struct media_transport *transport,
> +                                                               int8_t volume);

Not really following what you need to introduce dedicated functions
foreach? The prototype is the same and we could just introduce
callbacks that access the respective data internally, like we do for
suspend/resume.

> +void media_bap_transport_update_volume(struct media_transport *transport,
>                                                                 int8_t volume);
>  void transport_get_properties(struct media_transport *transport,
>                                                         DBusMessageIter *iter);
> --
> 2.25.1
>


-- 
Luiz Augusto von Dentz

[PATCH 1/3] audio/transport: seprate a2dp and bap transport to update volume

[Sathish Narasimman]

Identify BAP and a2dp transport to update the transport volume.
Also add properties volume for bap_transport.
---
 profiles/audio/media.c     |   2 +-
 profiles/audio/transport.c | 120 ++++++++++++++++++++++++++++++++++---
 profiles/audio/transport.h |   7 ++-
 3 files changed, 117 insertions(+), 12 deletions(-)

diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index c9328ab9bd6e..0e3c7cbbaf2c 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -523,7 +523,7 @@ static gboolean set_configuration(struct media_endpoint *endpoint,
 		return FALSE;
 
 	init_volume = media_player_get_device_volume(device);
-	media_transport_update_volume(transport, init_volume);
+	media_a2dp_transport_update_volume(transport, init_volume);
 
 	msg = dbus_message_new_method_call(endpoint->sender, endpoint->path,
 						MEDIA_ENDPOINT_INTERFACE,
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 41339da51e17..a606d45eb628 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -91,6 +91,7 @@ struct bap_transport {
 	uint8_t			rtn;
 	uint16_t		latency;
 	uint32_t		delay;
+	int8_t			volume;
 };
 
 struct media_transport {
@@ -709,7 +710,8 @@ static gboolean get_delay_reporting(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
-static gboolean volume_exists(const GDBusPropertyTable *property, void *data)
+static gboolean volume_a2dp_exists(const GDBusPropertyTable *property,
+					void *data)
 {
 	struct media_transport *transport = data;
 	struct a2dp_transport *a2dp = transport->data;
@@ -717,7 +719,8 @@ static gboolean volume_exists(const GDBusPropertyTable *property, void *data)
 	return a2dp->volume >= 0;
 }
 
-static gboolean get_volume(const GDBusPropertyTable *property,
+
+static gboolean get_a2dp_volume(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
 	struct media_transport *transport = data;
@@ -729,7 +732,7 @@ static gboolean get_volume(const GDBusPropertyTable *property,
 	return TRUE;
 }
 
-static void set_volume(const GDBusPropertyTable *property,
+static void set_a2dp_volume(const GDBusPropertyTable *property,
 			DBusMessageIter *iter, GDBusPendingPropertySet id,
 			void *data)
 {
@@ -769,6 +772,58 @@ error:
 					"Invalid arguments in method call");
 }
 
+static gboolean volume_bap_exists(const GDBusPropertyTable *property,
+					void *data)
+{
+	struct media_transport *transport = data;
+	struct bap_transport *bap = transport->data;
+
+	return bap->volume >= 0;
+}
+
+static gboolean get_bap_volume(const GDBusPropertyTable *property,
+					DBusMessageIter *iter, void *data)
+{
+	struct media_transport *transport = data;
+	struct bap_transport *bap = transport->data;
+	uint16_t volume = (uint16_t)bap->volume;
+
+	dbus_message_iter_append_basic(iter, DBUS_TYPE_UINT16, &volume);
+
+	return TRUE;
+}
+
+static void set_bap_volume(const GDBusPropertyTable *property,
+			   DBusMessageIter *iter, GDBusPendingPropertySet id,
+			   void *data)
+{
+	struct media_transport *transport = data;
+	struct bap_transport *bap = transport->data;
+	uint16_t arg;
+	int8_t volume;
+
+	if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_UINT16)
+		goto error;
+
+	dbus_message_iter_get_basic(iter, &arg);
+	if (arg > INT8_MAX)
+		goto error;
+
+	g_dbus_pending_property_success(id);
+
+	volume = (int8_t)arg;
+	if (bap->volume == volume)
+		return;
+
+	/*TODO vcp_send_volume */
+	return;
+
+error:
+	g_dbus_pending_property_error(id, ERROR_INTERFACE ".InvalidArguments",
+					"Invalid arguments in method call");
+}
+
+
 static gboolean endpoint_exists(const GDBusPropertyTable *property, void *data)
 {
 	struct media_transport *transport = data;
@@ -809,7 +864,8 @@ static const GDBusPropertyTable a2dp_properties[] = {
 	{ "Configuration", "ay", get_configuration },
 	{ "State", "s", get_state },
 	{ "Delay", "q", get_delay_reporting, NULL, delay_reporting_exists },
-	{ "Volume", "q", get_volume, set_volume, volume_exists },
+	{ "Volume", "q", get_a2dp_volume, set_a2dp_volume,
+					volume_a2dp_exists },
 	{ "Endpoint", "o", get_endpoint, NULL, endpoint_exists,
 				G_DBUS_PROPERTY_FLAG_EXPERIMENTAL },
 	{ }
@@ -970,6 +1026,7 @@ static const GDBusPropertyTable bap_properties[] = {
 	{ "Retransmissions", "y", get_retransmissions },
 	{ "Latency", "q", get_latency },
 	{ "Delay", "u", get_delay },
+	{ "Volume", "q", get_bap_volume, set_bap_volume, volume_bap_exists },
 	{ "Endpoint", "o", get_endpoint, NULL, endpoint_exists },
 	{ "Location", "u", get_location },
 	{ "Metadata", "ay", get_metadata },
@@ -1403,6 +1460,7 @@ static int media_transport_init_bap(struct media_transport *transport,
 	bap->rtn = qos->rtn;
 	bap->latency = qos->latency;
 	bap->delay = qos->delay;
+	bap->volume = 127;
 	bap->state_id = bt_bap_state_register(bt_bap_stream_get_session(stream),
 						bap_state_changed,
 						bap_connecting,
@@ -1502,14 +1560,22 @@ struct btd_device *media_transport_get_dev(struct media_transport *transport)
 	return transport->device;
 }
 
-int8_t media_transport_get_volume(struct media_transport *transport)
+int8_t media_a2dp_transport_get_volume(struct media_transport *transport)
 {
 	struct a2dp_transport *a2dp = transport->data;
+
 	return a2dp->volume;
 }
 
-void media_transport_update_volume(struct media_transport *transport,
-								int8_t volume)
+int8_t media_bap_transport_get_volume(struct media_transport *transport)
+{
+	struct bap_transport *bap = transport->data;
+
+	return bap->volume;
+}
+
+void media_a2dp_transport_update_volume(struct media_transport *transport,
+					int8_t volume)
 {
 	struct a2dp_transport *a2dp = transport->data;
 
@@ -1527,6 +1593,25 @@ void media_transport_update_volume(struct media_transport *transport,
 					MEDIA_TRANSPORT_INTERFACE, "Volume");
 }
 
+void media_bap_transport_update_volume(struct media_transport *transport,
+								int8_t volume)
+{
+	struct bap_transport *bap = transport->data;
+
+	if (volume < 0)
+		return;
+
+	/* Check if volume really changed */
+	if (bap->volume == volume)
+		return;
+
+	bap->volume = volume;
+
+	g_dbus_emit_property_changed(btd_get_dbus_connection(),
+					transport->path,
+					MEDIA_TRANSPORT_INTERFACE, "Volume");
+}
+
 int8_t media_transport_get_device_volume(struct btd_device *dev)
 {
 	GSList *l;
@@ -1537,12 +1622,21 @@ int8_t media_transport_get_device_volume(struct btd_device *dev)
 	/* Attempt to locate the transport to get its volume */
 	for (l = transports; l; l = l->next) {
 		struct media_transport *transport = l->data;
+		const char *uuid = media_endpoint_get_uuid(transport->endpoint);
+
+		/* Locate BAP Transport */
+		if (!strcasecmp(uuid, PAC_SINK_UUID) &&
+				!strcasecmp(uuid, PAC_SOURCE_UUID)) {
+
+			return media_bap_transport_get_volume(transport);
+		}
+
 		if (transport->device != dev)
 			continue;
 
 		/* Volume is A2DP only */
 		if (media_endpoint_get_sep(transport->endpoint))
-			return media_transport_get_volume(transport);
+			return media_a2dp_transport_get_volume(transport);
 	}
 
 	/* If transport volume doesn't exists use device_volume */
@@ -1560,12 +1654,20 @@ void media_transport_update_device_volume(struct btd_device *dev,
 	/* Attempt to locate the transport to set its volume */
 	for (l = transports; l; l = l->next) {
 		struct media_transport *transport = l->data;
+		const char *uuid = media_endpoint_get_uuid(transport->endpoint);
+
+		/* Locate BAP Transport */
+		if (!strcasecmp(uuid, PAC_SINK_UUID)) {
+			media_bap_transport_update_volume(transport, volume);
+			return;
+		}
+
 		if (transport->device != dev)
 			continue;
 
 		/* Volume is A2DP only */
 		if (media_endpoint_get_sep(transport->endpoint)) {
-			media_transport_update_volume(transport, volume);
+			media_a2dp_transport_update_volume(transport, volume);
 			return;
 		}
 	}
diff --git a/profiles/audio/transport.h b/profiles/audio/transport.h
index 102fc3cf1153..3cbefa5d7bee 100644
--- a/profiles/audio/transport.h
+++ b/profiles/audio/transport.h
@@ -20,10 +20,13 @@ struct media_transport *media_transport_create(struct btd_device *device,
 void media_transport_destroy(struct media_transport *transport);
 const char *media_transport_get_path(struct media_transport *transport);
 struct btd_device *media_transport_get_dev(struct media_transport *transport);
-int8_t media_transport_get_volume(struct media_transport *transport);
+int8_t media_a2dp_transport_get_volume(struct media_transport *transport);
+int8_t media_bap_transport_get_volume(struct media_transport *transport);
 void media_transport_update_delay(struct media_transport *transport,
 							uint16_t delay);
-void media_transport_update_volume(struct media_transport *transport,
+void media_a2dp_transport_update_volume(struct media_transport *transport,
+								int8_t volume);
+void media_bap_transport_update_volume(struct media_transport *transport,
 								int8_t volume);
 void transport_get_properties(struct media_transport *transport,
 							DBusMessageIter *iter);
-- 
2.25.1

[PATCH 2/3] shared/vcp: Add callback to update media volume

[Sathish Narasimman]

Add support for callback functions to update media transport volume.
Fix to register debug functions. Also invoke vcp_attached function the
right place. Fix check for existing session if available and attach.
---
 src/shared/vcp.c | 139 +++++++++++++++++++++++++++++++++++++++++------
 src/shared/vcp.h |   7 +++
 2 files changed, 130 insertions(+), 16 deletions(-)

diff --git a/src/shared/vcp.c b/src/shared/vcp.c
index 5459cf892a7d..8e1964234338 100644
--- a/src/shared/vcp.c
+++ b/src/shared/vcp.c
@@ -41,6 +41,11 @@ struct bt_vcp_db {
 	struct bt_vcs *vcs;
 };
 
+struct ev_cb {
+	const struct bt_vcp_vr_ops *ops;
+	void *data;
+};
+
 typedef void (*vcp_func_t)(struct bt_vcp *vcp, bool success, uint8_t att_ecode,
 					const uint8_t *value, uint16_t length,
 					void *user_data);
@@ -89,11 +94,16 @@ struct bt_vcp {
 	unsigned int vstate_id;
 	unsigned int vflag_id;
 
+	unsigned int disconn_id;
 	struct queue *notify;
 	struct queue *pending;
 
 	bt_vcp_debug_func_t debug_func;
 	bt_vcp_destroy_func_t debug_destroy;
+
+	struct bt_vcp_vr_ops *ops;
+	struct ev_cb *cb;
+
 	void *debug_data;
 	void *user_data;
 };
@@ -124,6 +134,18 @@ static struct queue *vcp_db;
 static struct queue *vcp_cbs;
 static struct queue *sessions;
 
+static void vcp_debug(struct bt_vcp *vcp, const char *format, ...)
+{
+	va_list ap;
+
+	if (!vcp || !format || !vcp->debug_func)
+		return;
+
+	va_start(ap, format);
+	util_debug_va(vcp->debug_func, vcp->debug_data, format, ap);
+	va_end(ap);
+}
+
 static void *iov_pull_mem(struct iovec *iov, size_t len)
 {
 	void *data = iov->iov_base;
@@ -183,12 +205,17 @@ static void vcp_detached(void *data, void *user_data)
 
 void bt_vcp_detach(struct bt_vcp *vcp)
 {
+	DBG(vcp, "%p", vcp);
+
 	if (!queue_remove(sessions, vcp))
 		return;
 
 	bt_gatt_client_unref(vcp->client);
 	vcp->client = NULL;
 
+	bt_att_unregister_disconnect(vcp->att, vcp->disconn_id);
+	vcp->att = NULL;
+
 	queue_foreach(vcp_cbs, vcp_detached, vcp);
 }
 
@@ -267,24 +294,14 @@ void bt_vcp_unref(struct bt_vcp *vcp)
 	vcp_free(vcp);
 }
 
-static void vcp_debug(struct bt_vcp *vcp, const char *format, ...)
-{
-	va_list ap;
-
-	if (!vcp || !format || !vcp->debug_func)
-		return;
-
-	va_start(ap, format);
-	util_debug_va(vcp->debug_func, vcp->debug_data, format, ap);
-	va_end(ap);
-}
-
 static void vcp_disconnected(int err, void *user_data)
 {
 	struct bt_vcp *vcp = user_data;
 
 	DBG(vcp, "vcp %p disconnected err %d", vcp, err);
 
+	vcp->disconn_id = 0;
+
 	bt_vcp_detach(vcp);
 }
 
@@ -303,12 +320,9 @@ static struct bt_vcp *vcp_get_session(struct bt_att *att, struct gatt_db *db)
 	vcp = bt_vcp_new(db, NULL);
 	vcp->att = att;
 
-	bt_att_register_disconnect(att, vcp_disconnected, vcp, NULL);
-
 	bt_vcp_attach(vcp, NULL);
 
 	return vcp;
-
 }
 
 static uint8_t vcs_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
@@ -317,6 +331,7 @@ static uint8_t vcs_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	struct bt_vcp_db *vdb;
 	struct vol_state *vstate;
 	uint8_t	*change_counter;
+	struct ev_cb *cb =  vcp->cb;
 
 	DBG(vcp, "Volume Down");
 
@@ -344,6 +359,9 @@ static uint8_t vcs_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	vstate->vol_set = MAX((vstate->vol_set - VCP_STEP_SIZE), 0);
 	vstate->counter = -~vstate->counter; /*Increment Change Counter*/
 
+	if (cb && cb->ops && cb->ops->set_volume)
+		cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
 	gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
 				 sizeof(struct vol_state),
 				 bt_vcp_get_att(vcp));
@@ -356,6 +374,7 @@ static uint8_t vcs_rel_vol_up(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	struct bt_vcp_db *vdb;
 	struct vol_state *vstate;
 	uint8_t	*change_counter;
+	struct ev_cb *cb =  vcp->cb;
 
 	DBG(vcp, "Volume Up");
 
@@ -383,6 +402,9 @@ static uint8_t vcs_rel_vol_up(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	vstate->vol_set = MIN((vstate->vol_set + VCP_STEP_SIZE), 255);
 	vstate->counter = -~vstate->counter; /*Increment Change Counter*/
 
+	if (cb && cb->ops && cb->ops->set_volume)
+		cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
 	gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
 				 sizeof(struct vol_state),
 				 bt_vcp_get_att(vcp));
@@ -395,6 +417,7 @@ static uint8_t vcs_unmute_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	struct bt_vcp_db *vdb;
 	struct vol_state *vstate;
 	uint8_t	*change_counter;
+	struct ev_cb *cb =  vcp->cb;
 
 	DBG(vcp, "Un Mute and Volume Down");
 
@@ -423,6 +446,9 @@ static uint8_t vcs_unmute_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	vstate->vol_set = MAX((vstate->vol_set - VCP_STEP_SIZE), 0);
 	vstate->counter = -~vstate->counter; /*Increment Change Counter*/
 
+	if (cb && cb->ops && cb->ops->set_volume)
+		cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
 	gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
 				 sizeof(struct vol_state),
 				 bt_vcp_get_att(vcp));
@@ -435,6 +461,7 @@ static uint8_t vcs_unmute_rel_vol_up(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	struct bt_vcp_db *vdb;
 	struct vol_state *vstate;
 	uint8_t	*change_counter;
+	struct ev_cb *cb =  vcp->cb;
 
 	DBG(vcp, "UN Mute and Volume Up");
 
@@ -463,6 +490,9 @@ static uint8_t vcs_unmute_rel_vol_up(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	vstate->vol_set = MIN((vstate->vol_set + VCP_STEP_SIZE), 255);
 	vstate->counter = -~vstate->counter; /*Increment Change Counter*/
 
+	if (cb && cb->ops && cb->ops->set_volume)
+		cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
 	gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
 				 sizeof(struct vol_state),
 				 bt_vcp_get_att(vcp));
@@ -475,6 +505,7 @@ static uint8_t vcs_set_absolute_vol(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	struct bt_vcp_db *vdb;
 	struct vol_state *vstate;
 	struct bt_vcs_ab_vol *req;
+	struct ev_cb *cb =  vcp->cb;
 
 	DBG(vcp, "Set Absolute Volume");
 
@@ -502,6 +533,9 @@ static uint8_t vcs_set_absolute_vol(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	vstate->vol_set = req->vol_set;
 	vstate->counter = -~vstate->counter; /*Increment Change Counter*/
 
+	if (cb && cb->ops && cb->ops->set_volume)
+		cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
 	gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
 				 sizeof(struct vol_state),
 				 bt_vcp_get_att(vcp));
@@ -514,6 +548,7 @@ static uint8_t vcs_unmute(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	struct bt_vcp_db *vdb;
 	struct vol_state *vstate;
 	uint8_t	*change_counter;
+	struct ev_cb *cb =  vcp->cb;
 
 	DBG(vcp, "Un Mute");
 
@@ -541,6 +576,9 @@ static uint8_t vcs_unmute(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	vstate->mute = 0x00;
 	vstate->counter = -~vstate->counter; /*Increment Change Counter*/
 
+	if (cb && cb->ops && cb->ops->set_volume)
+		cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
 	gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
 				 sizeof(struct vol_state),
 				 bt_vcp_get_att(vcp));
@@ -553,6 +591,7 @@ static uint8_t vcs_mute(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	struct bt_vcp_db *vdb;
 	struct vol_state *vstate;
 	uint8_t	*change_counter;
+	struct ev_cb *cb =  vcp->cb;
 
 	DBG(vcp, "MUTE");
 
@@ -580,6 +619,13 @@ static uint8_t vcs_mute(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	vstate->mute = 0x01;
 	vstate->counter = -~vstate->counter; /*Increment Change Counter*/
 
+	if (cb && cb->ops && cb->ops->set_volume)
+		cb->ops->set_volume(vcp, 0, cb->data);
+
+	gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
+					sizeof(struct vol_state),
+					bt_vcp_get_att(vcp));
+
 	return 0;
 }
 
@@ -689,8 +735,10 @@ static void vcs_state_read(struct gatt_db_attribute *attrib,
 				void *user_data)
 {
 	struct bt_vcs *vcs = user_data;
+	struct bt_vcp *vcp = vcp_get_session(att, vcs->vdb->db);
 	struct iovec iov;
 
+	DBG(vcp, "VCP State Read");
 	iov.iov_base = vcs->vstate;
 	iov.iov_len = sizeof(*vcs->vstate);
 
@@ -704,8 +752,10 @@ static void vcs_flag_read(struct gatt_db_attribute *attrib,
 				void *user_data)
 {
 	struct bt_vcs *vcs = user_data;
+	struct bt_vcp *vcp = vcp_get_session(att, vcs->vdb->db);
 	struct iovec iov;
 
+	DBG(vcp, "VCP Flag Read");
 	iov.iov_base = &vcs->vol_flag;
 	iov.iov_len = sizeof(vcs->vol_flag);
 
@@ -868,6 +918,14 @@ bool bt_vcp_unregister(unsigned int id)
 	return true;
 }
 
+static void vcp_attached(void *data, void *user_data)
+{
+	struct bt_vcp_cb *cb = data;
+	struct bt_vcp *vcp = user_data;
+
+	cb->attached(vcp, cb->user_data);
+}
+
 struct bt_vcp *bt_vcp_new(struct gatt_db *ldb, struct gatt_db *rdb)
 {
 	struct bt_vcp *vcp;
@@ -1068,6 +1126,26 @@ static unsigned int vcp_register_notify(struct bt_vcp *vcp,
 	return notify->id;
 }
 
+bool bt_vcp_vr_set_ops(struct bt_vcp *vcp, struct bt_vcp_vr_ops *ops,
+			void *data)
+{
+	struct ev_cb *cb;
+
+	if (!vcp)
+		return false;
+
+	if (vcp->cb)
+		free(vcp->cb);
+
+	cb = new0(struct ev_cb, 1);
+	cb->ops = ops;
+	cb->data = data;
+
+	vcp->cb = cb;
+
+	return true;
+}
+
 static void foreach_vcs_char(struct gatt_db_attribute *attr, void *user_data)
 {
 	struct bt_vcp *vcp = user_data;
@@ -1141,25 +1219,54 @@ static void foreach_vcs_service(struct gatt_db_attribute *attr,
 	gatt_db_service_foreach_char(attr, foreach_vcs_char, vcp);
 }
 
+static void vcp_attach_att(struct bt_vcp *vcp, struct bt_att *att)
+{
+	if (vcp->disconn_id) {
+		if (att == bt_vcp_get_att(vcp))
+			return;
+		bt_att_unregister_disconnect(vcp->att, vcp->disconn_id);
+	}
+
+	vcp->disconn_id = bt_att_register_disconnect(vcp->att,
+							vcp_disconnected,
+							vcp, NULL);
+}
+
 bool bt_vcp_attach(struct bt_vcp *vcp, struct bt_gatt_client *client)
 {
 	bt_uuid_t uuid;
 
+	if (queue_find(sessions, NULL, vcp)) {
+		/* If instance already been set but there is no client proceed
+		 * to clone it otherwise considered it already attached.
+		 */
+		if (client && !vcp->client)
+			goto clone;
+		return true;
+	}
+
 	if (!sessions)
 		sessions = queue_new();
 
 	queue_push_tail(sessions, vcp);
 
-	if (!client)
+	queue_foreach(vcp_cbs, vcp_attached, vcp);
+
+	if (!client) {
+		vcp_attach_att(vcp, vcp->att);
 		return true;
+	}
 
 	if (vcp->client)
 		return false;
 
+clone:
 	vcp->client = bt_gatt_client_clone(client);
 	if (!vcp->client)
 		return false;
 
+	vcp_attach_att(vcp, bt_gatt_client_get_att(client));
+
 	bt_uuid16_create(&uuid, VCS_UUID);
 	gatt_db_foreach_service(vcp->ldb->db, &uuid, foreach_vcs_service, vcp);
 
diff --git a/src/shared/vcp.h b/src/shared/vcp.h
index 26db5732d19b..7d70aefe0089 100644
--- a/src/shared/vcp.h
+++ b/src/shared/vcp.h
@@ -33,6 +33,13 @@
 
 struct bt_vcp;
 
+struct bt_vcp_vr_ops {
+	void (*set_volume)(struct bt_vcp *vcp, int8_t volume, void *data);
+};
+
+bool bt_vcp_vr_set_ops(struct bt_vcp *vcp, struct bt_vcp_vr_ops *ops,
+			void *data);
+
 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);
-- 
2.25.1

[PATCH 3/3] profiles: Register callback function to update volume

[Sathish Narasimman]

Callback function has to be registered to call
media_transport_update_device_volume to change transport volume.
---
 profiles/audio/vcp.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/profiles/audio/vcp.c b/profiles/audio/vcp.c
index b42b0a4f79dd..4b790b03c032 100644
--- a/profiles/audio/vcp.c
+++ b/profiles/audio/vcp.c
@@ -50,6 +50,7 @@
 #include "src/service.h"
 #include "src/log.h"
 #include "src/error.h"
+#include "transport.h"
 
 #define VCS_UUID_STR "00001844-0000-1000-8000-00805f9b34fb"
 #define MEDIA_ENDPOINT_INTERFACE "org.bluez.MediaEndpoint1"
@@ -83,6 +84,20 @@ static struct vcp_data *vcp_data_new(struct btd_device *device)
 	return data;
 }
 
+static void vr_set_volume(struct bt_vcp *vcp, int8_t volume, void *data)
+{
+	struct vcp_data *user_data = data;
+	struct btd_device *device = user_data->device;
+
+	DBG("set volume");
+
+	media_transport_update_device_volume(device, volume);
+}
+
+static struct bt_vcp_vr_ops vcp_vr_ops = {
+	.set_volume	= vr_set_volume,
+};
+
 static void vcp_data_add(struct vcp_data *data)
 {
 	DBG("data %p", data);
@@ -94,6 +109,7 @@ static void vcp_data_add(struct vcp_data *data)
 
 	bt_vcp_set_debug(data->vcp, vcp_debug, NULL, NULL);
 
+	bt_vcp_vr_set_ops(data->vcp, &vcp_vr_ops, data);
 	if (!sessions)
 		sessions = queue_new();
 
@@ -178,6 +194,7 @@ static void vcp_attached(struct bt_vcp *vcp, void *user_data)
 	data->vcp = vcp;
 
 	vcp_data_add(data);
+
 }
 
 static int vcp_probe(struct btd_service *service)
-- 
2.25.1

[PATCH 2/3] shared/vcp: Add callback to update media volume

[Sathish Narasimman]

Add support for callback functions to update media transport volume.
Fix to register debug functions. Also invoke vcp_attached function the
right place. Fix check for existing session if available and attach.
---
 src/shared/vcp.c | 139 +++++++++++++++++++++++++++++++++++++++++------
 src/shared/vcp.h |   7 +++
 2 files changed, 130 insertions(+), 16 deletions(-)

diff --git a/src/shared/vcp.c b/src/shared/vcp.c
index 5459cf892a7d..8e1964234338 100644
--- a/src/shared/vcp.c
+++ b/src/shared/vcp.c
@@ -41,6 +41,11 @@ struct bt_vcp_db {
 	struct bt_vcs *vcs;
 };
 
+struct ev_cb {
+	const struct bt_vcp_vr_ops *ops;
+	void *data;
+};
+
 typedef void (*vcp_func_t)(struct bt_vcp *vcp, bool success, uint8_t att_ecode,
 					const uint8_t *value, uint16_t length,
 					void *user_data);
@@ -89,11 +94,16 @@ struct bt_vcp {
 	unsigned int vstate_id;
 	unsigned int vflag_id;
 
+	unsigned int disconn_id;
 	struct queue *notify;
 	struct queue *pending;
 
 	bt_vcp_debug_func_t debug_func;
 	bt_vcp_destroy_func_t debug_destroy;
+
+	struct bt_vcp_vr_ops *ops;
+	struct ev_cb *cb;
+
 	void *debug_data;
 	void *user_data;
 };
@@ -124,6 +134,18 @@ static struct queue *vcp_db;
 static struct queue *vcp_cbs;
 static struct queue *sessions;
 
+static void vcp_debug(struct bt_vcp *vcp, const char *format, ...)
+{
+	va_list ap;
+
+	if (!vcp || !format || !vcp->debug_func)
+		return;
+
+	va_start(ap, format);
+	util_debug_va(vcp->debug_func, vcp->debug_data, format, ap);
+	va_end(ap);
+}
+
 static void *iov_pull_mem(struct iovec *iov, size_t len)
 {
 	void *data = iov->iov_base;
@@ -183,12 +205,17 @@ static void vcp_detached(void *data, void *user_data)
 
 void bt_vcp_detach(struct bt_vcp *vcp)
 {
+	DBG(vcp, "%p", vcp);
+
 	if (!queue_remove(sessions, vcp))
 		return;
 
 	bt_gatt_client_unref(vcp->client);
 	vcp->client = NULL;
 
+	bt_att_unregister_disconnect(vcp->att, vcp->disconn_id);
+	vcp->att = NULL;
+
 	queue_foreach(vcp_cbs, vcp_detached, vcp);
 }
 
@@ -267,24 +294,14 @@ void bt_vcp_unref(struct bt_vcp *vcp)
 	vcp_free(vcp);
 }
 
-static void vcp_debug(struct bt_vcp *vcp, const char *format, ...)
-{
-	va_list ap;
-
-	if (!vcp || !format || !vcp->debug_func)
-		return;
-
-	va_start(ap, format);
-	util_debug_va(vcp->debug_func, vcp->debug_data, format, ap);
-	va_end(ap);
-}
-
 static void vcp_disconnected(int err, void *user_data)
 {
 	struct bt_vcp *vcp = user_data;
 
 	DBG(vcp, "vcp %p disconnected err %d", vcp, err);
 
+	vcp->disconn_id = 0;
+
 	bt_vcp_detach(vcp);
 }
 
@@ -303,12 +320,9 @@ static struct bt_vcp *vcp_get_session(struct bt_att *att, struct gatt_db *db)
 	vcp = bt_vcp_new(db, NULL);
 	vcp->att = att;
 
-	bt_att_register_disconnect(att, vcp_disconnected, vcp, NULL);
-
 	bt_vcp_attach(vcp, NULL);
 
 	return vcp;
-
 }
 
 static uint8_t vcs_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
@@ -317,6 +331,7 @@ static uint8_t vcs_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	struct bt_vcp_db *vdb;
 	struct vol_state *vstate;
 	uint8_t	*change_counter;
+	struct ev_cb *cb =  vcp->cb;
 
 	DBG(vcp, "Volume Down");
 
@@ -344,6 +359,9 @@ static uint8_t vcs_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	vstate->vol_set = MAX((vstate->vol_set - VCP_STEP_SIZE), 0);
 	vstate->counter = -~vstate->counter; /*Increment Change Counter*/
 
+	if (cb && cb->ops && cb->ops->set_volume)
+		cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
 	gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
 				 sizeof(struct vol_state),
 				 bt_vcp_get_att(vcp));
@@ -356,6 +374,7 @@ static uint8_t vcs_rel_vol_up(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	struct bt_vcp_db *vdb;
 	struct vol_state *vstate;
 	uint8_t	*change_counter;
+	struct ev_cb *cb =  vcp->cb;
 
 	DBG(vcp, "Volume Up");
 
@@ -383,6 +402,9 @@ static uint8_t vcs_rel_vol_up(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	vstate->vol_set = MIN((vstate->vol_set + VCP_STEP_SIZE), 255);
 	vstate->counter = -~vstate->counter; /*Increment Change Counter*/
 
+	if (cb && cb->ops && cb->ops->set_volume)
+		cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
 	gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
 				 sizeof(struct vol_state),
 				 bt_vcp_get_att(vcp));
@@ -395,6 +417,7 @@ static uint8_t vcs_unmute_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	struct bt_vcp_db *vdb;
 	struct vol_state *vstate;
 	uint8_t	*change_counter;
+	struct ev_cb *cb =  vcp->cb;
 
 	DBG(vcp, "Un Mute and Volume Down");
 
@@ -423,6 +446,9 @@ static uint8_t vcs_unmute_rel_vol_down(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	vstate->vol_set = MAX((vstate->vol_set - VCP_STEP_SIZE), 0);
 	vstate->counter = -~vstate->counter; /*Increment Change Counter*/
 
+	if (cb && cb->ops && cb->ops->set_volume)
+		cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
 	gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
 				 sizeof(struct vol_state),
 				 bt_vcp_get_att(vcp));
@@ -435,6 +461,7 @@ static uint8_t vcs_unmute_rel_vol_up(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	struct bt_vcp_db *vdb;
 	struct vol_state *vstate;
 	uint8_t	*change_counter;
+	struct ev_cb *cb =  vcp->cb;
 
 	DBG(vcp, "UN Mute and Volume Up");
 
@@ -463,6 +490,9 @@ static uint8_t vcs_unmute_rel_vol_up(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	vstate->vol_set = MIN((vstate->vol_set + VCP_STEP_SIZE), 255);
 	vstate->counter = -~vstate->counter; /*Increment Change Counter*/
 
+	if (cb && cb->ops && cb->ops->set_volume)
+		cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
 	gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
 				 sizeof(struct vol_state),
 				 bt_vcp_get_att(vcp));
@@ -475,6 +505,7 @@ static uint8_t vcs_set_absolute_vol(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	struct bt_vcp_db *vdb;
 	struct vol_state *vstate;
 	struct bt_vcs_ab_vol *req;
+	struct ev_cb *cb =  vcp->cb;
 
 	DBG(vcp, "Set Absolute Volume");
 
@@ -502,6 +533,9 @@ static uint8_t vcs_set_absolute_vol(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	vstate->vol_set = req->vol_set;
 	vstate->counter = -~vstate->counter; /*Increment Change Counter*/
 
+	if (cb && cb->ops && cb->ops->set_volume)
+		cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
 	gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
 				 sizeof(struct vol_state),
 				 bt_vcp_get_att(vcp));
@@ -514,6 +548,7 @@ static uint8_t vcs_unmute(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	struct bt_vcp_db *vdb;
 	struct vol_state *vstate;
 	uint8_t	*change_counter;
+	struct ev_cb *cb =  vcp->cb;
 
 	DBG(vcp, "Un Mute");
 
@@ -541,6 +576,9 @@ static uint8_t vcs_unmute(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	vstate->mute = 0x00;
 	vstate->counter = -~vstate->counter; /*Increment Change Counter*/
 
+	if (cb && cb->ops && cb->ops->set_volume)
+		cb->ops->set_volume(vcp, vstate->vol_set, cb->data);
+
 	gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
 				 sizeof(struct vol_state),
 				 bt_vcp_get_att(vcp));
@@ -553,6 +591,7 @@ static uint8_t vcs_mute(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	struct bt_vcp_db *vdb;
 	struct vol_state *vstate;
 	uint8_t	*change_counter;
+	struct ev_cb *cb =  vcp->cb;
 
 	DBG(vcp, "MUTE");
 
@@ -580,6 +619,13 @@ static uint8_t vcs_mute(struct bt_vcs *vcs, struct bt_vcp *vcp,
 	vstate->mute = 0x01;
 	vstate->counter = -~vstate->counter; /*Increment Change Counter*/
 
+	if (cb && cb->ops && cb->ops->set_volume)
+		cb->ops->set_volume(vcp, 0, cb->data);
+
+	gatt_db_attribute_notify(vdb->vcs->vs, (void *)vstate,
+					sizeof(struct vol_state),
+					bt_vcp_get_att(vcp));
+
 	return 0;
 }
 
@@ -689,8 +735,10 @@ static void vcs_state_read(struct gatt_db_attribute *attrib,
 				void *user_data)
 {
 	struct bt_vcs *vcs = user_data;
+	struct bt_vcp *vcp = vcp_get_session(att, vcs->vdb->db);
 	struct iovec iov;
 
+	DBG(vcp, "VCP State Read");
 	iov.iov_base = vcs->vstate;
 	iov.iov_len = sizeof(*vcs->vstate);
 
@@ -704,8 +752,10 @@ static void vcs_flag_read(struct gatt_db_attribute *attrib,
 				void *user_data)
 {
 	struct bt_vcs *vcs = user_data;
+	struct bt_vcp *vcp = vcp_get_session(att, vcs->vdb->db);
 	struct iovec iov;
 
+	DBG(vcp, "VCP Flag Read");
 	iov.iov_base = &vcs->vol_flag;
 	iov.iov_len = sizeof(vcs->vol_flag);
 
@@ -868,6 +918,14 @@ bool bt_vcp_unregister(unsigned int id)
 	return true;
 }
 
+static void vcp_attached(void *data, void *user_data)
+{
+	struct bt_vcp_cb *cb = data;
+	struct bt_vcp *vcp = user_data;
+
+	cb->attached(vcp, cb->user_data);
+}
+
 struct bt_vcp *bt_vcp_new(struct gatt_db *ldb, struct gatt_db *rdb)
 {
 	struct bt_vcp *vcp;
@@ -1068,6 +1126,26 @@ static unsigned int vcp_register_notify(struct bt_vcp *vcp,
 	return notify->id;
 }
 
+bool bt_vcp_vr_set_ops(struct bt_vcp *vcp, struct bt_vcp_vr_ops *ops,
+			void *data)
+{
+	struct ev_cb *cb;
+
+	if (!vcp)
+		return false;
+
+	if (vcp->cb)
+		free(vcp->cb);
+
+	cb = new0(struct ev_cb, 1);
+	cb->ops = ops;
+	cb->data = data;
+
+	vcp->cb = cb;
+
+	return true;
+}
+
 static void foreach_vcs_char(struct gatt_db_attribute *attr, void *user_data)
 {
 	struct bt_vcp *vcp = user_data;
@@ -1141,25 +1219,54 @@ static void foreach_vcs_service(struct gatt_db_attribute *attr,
 	gatt_db_service_foreach_char(attr, foreach_vcs_char, vcp);
 }
 
+static void vcp_attach_att(struct bt_vcp *vcp, struct bt_att *att)
+{
+	if (vcp->disconn_id) {
+		if (att == bt_vcp_get_att(vcp))
+			return;
+		bt_att_unregister_disconnect(vcp->att, vcp->disconn_id);
+	}
+
+	vcp->disconn_id = bt_att_register_disconnect(vcp->att,
+							vcp_disconnected,
+							vcp, NULL);
+}
+
 bool bt_vcp_attach(struct bt_vcp *vcp, struct bt_gatt_client *client)
 {
 	bt_uuid_t uuid;
 
+	if (queue_find(sessions, NULL, vcp)) {
+		/* If instance already been set but there is no client proceed
+		 * to clone it otherwise considered it already attached.
+		 */
+		if (client && !vcp->client)
+			goto clone;
+		return true;
+	}
+
 	if (!sessions)
 		sessions = queue_new();
 
 	queue_push_tail(sessions, vcp);
 
-	if (!client)
+	queue_foreach(vcp_cbs, vcp_attached, vcp);
+
+	if (!client) {
+		vcp_attach_att(vcp, vcp->att);
 		return true;
+	}
 
 	if (vcp->client)
 		return false;
 
+clone:
 	vcp->client = bt_gatt_client_clone(client);
 	if (!vcp->client)
 		return false;
 
+	vcp_attach_att(vcp, bt_gatt_client_get_att(client));
+
 	bt_uuid16_create(&uuid, VCS_UUID);
 	gatt_db_foreach_service(vcp->ldb->db, &uuid, foreach_vcs_service, vcp);
 
diff --git a/src/shared/vcp.h b/src/shared/vcp.h
index 26db5732d19b..7d70aefe0089 100644
--- a/src/shared/vcp.h
+++ b/src/shared/vcp.h
@@ -33,6 +33,13 @@
 
 struct bt_vcp;
 
+struct bt_vcp_vr_ops {
+	void (*set_volume)(struct bt_vcp *vcp, int8_t volume, void *data);
+};
+
+bool bt_vcp_vr_set_ops(struct bt_vcp *vcp, struct bt_vcp_vr_ops *ops,
+			void *data);
+
 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);
-- 
2.25.1