mbox series

[0/3] bluez: Export SDP "Remote audio volume control" item for HSP profile

Message ID 20200413162513.2221-1-pali@kernel.org (mailing list archive)
Headers show
Series bluez: Export SDP "Remote audio volume control" item for HSP profile | expand

Message

Pali Rohár April 13, 2020, 4:25 p.m. UTC
This patch series fixes handling of zero value in feature list and
provides Remote audio volume control support for HSP profile in both HS
and AG roles.

Luiz, you wrote that you do not have time to work on this, so I
implemented it myself in this patch series. Could you please find time
at least for reviewing and merging these patches? Thanks.

Pali Rohár (3):
  src/profile: Distinguish between zero-set HFP AG features and unset
    HFP AG features
  src/profile: Export Remote Audio Volume Control SDP value for HSP HS
    role via first bit in features value
  src/profile: Add default SDP record for Headset role of HSP 1.2
    profile with Erratum 3507

 src/profile.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 104 insertions(+), 8 deletions(-)

Comments

Luiz Augusto von Dentz April 14, 2020, 12:09 a.m. UTC | #1
Hi Pali,

On Mon, Apr 13, 2020 at 9:25 AM Pali Rohár <pali@kernel.org> wrote:
>
> This patch series fixes handling of zero value in feature list and
> provides Remote audio volume control support for HSP profile in both HS
> and AG roles.
>
> Luiz, you wrote that you do not have time to work on this, so I
> implemented it myself in this patch series. Could you please find time
> at least for reviewing and merging these patches? Thanks.
>
> Pali Rohár (3):
>   src/profile: Distinguish between zero-set HFP AG features and unset
>     HFP AG features
>   src/profile: Export Remote Audio Volume Control SDP value for HSP HS
>     role via first bit in features value
>   src/profile: Add default SDP record for Headset role of HSP 1.2
>     profile with Erratum 3507
>
>  src/profile.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 104 insertions(+), 8 deletions(-)
>
> --
> 2.20.1

Ive make some changes to remove the need to a flag to detect when the
features should be added or not, it is now applied, thanks.
Pali Rohár April 14, 2020, 8:09 a.m. UTC | #2
On Monday 13 April 2020 17:09:42 Luiz Augusto von Dentz wrote:
> Hi Pali,
> 
> On Mon, Apr 13, 2020 at 9:25 AM Pali Rohár <pali@kernel.org> wrote:
> >
> > This patch series fixes handling of zero value in feature list and
> > provides Remote audio volume control support for HSP profile in both HS
> > and AG roles.
> >
> > Luiz, you wrote that you do not have time to work on this, so I
> > implemented it myself in this patch series. Could you please find time
> > at least for reviewing and merging these patches? Thanks.
> >
> > Pali Rohár (3):
> >   src/profile: Distinguish between zero-set HFP AG features and unset
> >     HFP AG features
> >   src/profile: Export Remote Audio Volume Control SDP value for HSP HS
> >     role via first bit in features value
> >   src/profile: Add default SDP record for Headset role of HSP 1.2
> >     profile with Erratum 3507
> >
> >  src/profile.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 104 insertions(+), 8 deletions(-)
> >
> > --
> > 2.20.1
> 
> Ive make some changes to remove the need to a flag to detect when the
> features should be added or not, it is now applied, thanks.

Ok, thank you!

In commit you did one mistake:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/commit/?id=040bd56a948f4d1ecd6987cdf0ba51779dc0c02a

+		/* HSP AG role does not provide any features */
+		return 0;

It should return -ENODATA as AG role does not provide any features, so
has_features needs to be set to false.
Pali Rohár April 14, 2020, 7:53 p.m. UTC | #3
Hello Luiz!

I have there another simple change for updating documentation:

diff --git a/doc/profile-api.txt b/doc/profile-api.txt
index ec18034a6..183c6c11a 100644
--- a/doc/profile-api.txt
+++ b/doc/profile-api.txt
@@ -16,10 +16,33 @@ Object path	/org/bluez
 			If an application disconnects from the bus all
 			its registered profiles will be removed.
 
+			Some predefined services:
+
+			HFP AG UUID: 0000111f-0000-1000-8000-00805f9b34fb
+
+				Default profile Version is 1.7, profile Features
+				is 0b001001 and RFCOMM channel is 13.
+				Authentication is required.
+
 			HFP HS UUID: 0000111e-0000-1000-8000-00805f9b34fb
 
-				Default RFCOMM channel is 6. And this requires
-				authentication.
+				Default profile Version is 1.7, profile Features
+				is 0b000000 and RFCOMM channel is 7.
+				Authentication is required.
+
+			HSP AG UUID: 00001112-0000-1000-8000-00805f9b34fb
+
+				Default profile Version is 1.2, RFCOMM channel
+				is 12 and Authentication is required. Does not
+				support any Features, option is ignored.
+
+			HSP HS UUID: 00001108-0000-1000-8000-00805f9b34fb
+
+				Default profile Version is 1.2, profile Features
+				is 0b0 and RFCOMM channel is 6. Authentication
+				is required. Features is one bit value, specify
+				capability of Remote Audio Volume Control
+				(by default turned off).
 
 			Available options:
 


There is a mistake in documentation that default rfcomm channel for HFP
HS profile is 6. But in reality default rfcomm channel for HFP is 7.
Channel 6 is default for HSP HS profile as can be seen in src/profile.c.
Luiz Augusto von Dentz April 14, 2020, 8:28 p.m. UTC | #4
Hi Pali,

Can you send a proper patch adding these, have you changed the code
already to use channel 7?

On Tue, Apr 14, 2020 at 12:53 PM Pali Rohár <pali@kernel.org> wrote:
>
> Hello Luiz!
>
> I have there another simple change for updating documentation:
>
> diff --git a/doc/profile-api.txt b/doc/profile-api.txt
> index ec18034a6..183c6c11a 100644
> --- a/doc/profile-api.txt
> +++ b/doc/profile-api.txt
> @@ -16,10 +16,33 @@ Object path /org/bluez
>                         If an application disconnects from the bus all
>                         its registered profiles will be removed.
>
> +                       Some predefined services:
> +
> +                       HFP AG UUID: 0000111f-0000-1000-8000-00805f9b34fb
> +
> +                               Default profile Version is 1.7, profile Features
> +                               is 0b001001 and RFCOMM channel is 13.
> +                               Authentication is required.
> +
>                         HFP HS UUID: 0000111e-0000-1000-8000-00805f9b34fb
>
> -                               Default RFCOMM channel is 6. And this requires
> -                               authentication.
> +                               Default profile Version is 1.7, profile Features
> +                               is 0b000000 and RFCOMM channel is 7.
> +                               Authentication is required.
> +
> +                       HSP AG UUID: 00001112-0000-1000-8000-00805f9b34fb
> +
> +                               Default profile Version is 1.2, RFCOMM channel
> +                               is 12 and Authentication is required. Does not
> +                               support any Features, option is ignored.
> +
> +                       HSP HS UUID: 00001108-0000-1000-8000-00805f9b34fb
> +
> +                               Default profile Version is 1.2, profile Features
> +                               is 0b0 and RFCOMM channel is 6. Authentication
> +                               is required. Features is one bit value, specify
> +                               capability of Remote Audio Volume Control
> +                               (by default turned off).
>
>                         Available options:
>
>
>
> There is a mistake in documentation that default rfcomm channel for HFP
> HS profile is 6. But in reality default rfcomm channel for HFP is 7.
> Channel 6 is default for HSP HS profile as can be seen in src/profile.c.