[PATCH v2 0/4] android/hf-client: API fixes

[PATCH v2 0/4] android/hf-client: API fixes

[Lukasz Rymanowski]

Couple of fixes to hf-client api.

Those patches previously where a part of bigger set
[PATCH 00/39] android: HF Client initial implementation
but on Luiz request I split it into smaller sets.

Lukasz Rymanowski (4):
  android/hal-ipc-api: Use 2 bytes for location in dial memory
  android/hal-ipc-api: Use Tone instead of Code DTMF
  android/hal-ipc-api: Add missing peer features parameter
  android/hal-ipc-api: Fix define for HF-Client

 android/hal-ipc-api.txt | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

-- 
1.8.4

[PATCH v2 1/4] android/hal-ipc-api: Use 2 bytes for location in dial memory

[Lukasz Rymanowski]

---
 android/hal-ipc-api.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
index ca41cff..26e777e 100644
--- a/android/hal-ipc-api.txt
+++ b/android/hal-ipc-api.txt
@@ -2162,7 +2162,7 @@ Commands and response:
 
 	Opcode 0x09 - Dial Memory command/response
 
-		Command parameters: Location (1 octet)
+		Command parameters: Location (2 octet)
 		Response parameters: <none>
 
 		In case of an error, the error response will be returned.
-- 
1.8.4

Re: [PATCH v2 1/4] android/hal-ipc-api: Use 2 bytes for location in dial memory

[Luiz Augusto von Dentz]

Hi Lukasz,

On Wed, Sep 10, 2014 at 2:11 PM, Lukasz Rymanowski
<lukasz.rymanowski@tieto.com> wrote:
> ---
>  android/hal-ipc-api.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
> index ca41cff..26e777e 100644
> --- a/android/hal-ipc-api.txt
> +++ b/android/hal-ipc-api.txt
> @@ -2162,7 +2162,7 @@ Commands and response:
>
>         Opcode 0x09 - Dial Memory command/response
>
> -               Command parameters: Location (1 octet)
> +               Command parameters: Location (2 octet)
>                 Response parameters: <none>
>
>                 In case of an error, the error response will be returned.
> --
> 1.8.4

I guess we need a little bit more info here, does the spec limit this
to 2 octets because it seems the callback actually takes an int:

bt_status_t (*dial_memory) (int location);

Btw, I guess this is subject to change since it does not take the
address this would limit such callbacks to only one phone, in fact
many callback have this limitation which is kind of weird as both
connect/disconnect and connect_audio/disconnect_audio have the
address.


-- 
Luiz Augusto von Dentz

Re: [PATCH v2 1/4] android/hal-ipc-api: Use 2 bytes for location in dial memory

[Lukasz Rymanowski]

Hi Luiz,

On 11 September 2014 14:08, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Lukasz,
>
> On Wed, Sep 10, 2014 at 2:11 PM, Lukasz Rymanowski
> <lukasz.rymanowski@tieto.com> wrote:
>> ---
>>  android/hal-ipc-api.txt | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
>> index ca41cff..26e777e 100644
>> --- a/android/hal-ipc-api.txt
>> +++ b/android/hal-ipc-api.txt
>> @@ -2162,7 +2162,7 @@ Commands and response:
>>
>>         Opcode 0x09 - Dial Memory command/response
>>
>> -               Command parameters: Location (1 octet)
>> +               Command parameters: Location (2 octet)
>>                 Response parameters: <none>
>>
>>                 In case of an error, the error response will be returned.
>> --
>> 1.8.4
>
> I guess we need a little bit more info here, does the spec limit this
> to 2 octets because it seems the callback actually takes an int:
>
> bt_status_t (*dial_memory) (int location);
>
Yup, but in the end we are doing string out of this number
and this is why I thought that 65535 is enough for number location.

But we can fix it if necessary to be consistent.

\Łukasz

> Btw, I guess this is subject to change since it does not take the
> address this would limit such callbacks to only one phone, in fact
> many callback have this limitation which is kind of weird as both
> connect/disconnect and connect_audio/disconnect_audio have the
> address.
>
>
> --
> Luiz Augusto von Dentz

[PATCH v2 2/4] android/hal-ipc-api: Use Tone instead of Code DTMF

[Lukasz Rymanowski]

To be consistent with existing code for handsfree
---
 android/hal-ipc-api.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
index 26e777e..326fa21 100644
--- a/android/hal-ipc-api.txt
+++ b/android/hal-ipc-api.txt
@@ -2209,9 +2209,9 @@ Commands and response:
 
 		In case of an error, the error response will be returned.
 
-	Opcode 0x14 - Send DTMF Code command/response
+	Opcode 0x14 - Send DTMF Tone command/response
 
-		Command parameters: Code (1 octet)
+		Command parameters: Tone (1 octet)
 		Response parameters: <none>
 
 		In case of an error, the error response will be returned.
-- 
1.8.4

[PATCH v2 3/4] android/hal-ipc-api: Add missing peer features parameter

[Lukasz Rymanowski]

---
 android/hal-ipc-api.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
index 326fa21..0948af2 100644
--- a/android/hal-ipc-api.txt
+++ b/android/hal-ipc-api.txt
@@ -2228,7 +2228,8 @@ Notifications:
 	Opcode 0x81 - Connection State Changed notification
 
 		Notification parameters: State (1 octet)
-		                         Peer Features (1 octet)
+		                         Peer Features (2 octets)
+		                         CHLD Features (2 octets)
 		                         Address (6 octets)
 
 		Valid State values: 0x00 = Disconnected
-- 
1.8.4

Re: [PATCH v2 3/4] android/hal-ipc-api: Add missing peer features parameter

[Luiz Augusto von Dentz]

Hi Lukasz,

On Wed, Sep 10, 2014 at 2:11 PM, Lukasz Rymanowski
<lukasz.rymanowski@tieto.com> wrote:
> ---
>  android/hal-ipc-api.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
> index 326fa21..0948af2 100644
> --- a/android/hal-ipc-api.txt
> +++ b/android/hal-ipc-api.txt
> @@ -2228,7 +2228,8 @@ Notifications:
>         Opcode 0x81 - Connection State Changed notification
>
>                 Notification parameters: State (1 octet)
> -                                        Peer Features (1 octet)
> +                                        Peer Features (2 octets)
> +                                        CHLD Features (2 octets)
>                                          Address (6 octets)
>
>                 Valid State values: 0x00 = Disconnected
> --
> 1.8.4

You should probably document what are the possible values for CHLD.


-- 
Luiz Augusto von Dentz

Re: [PATCH v2 3/4] android/hal-ipc-api: Add missing peer features parameter

[Lukasz Rymanowski]

Hi Luiz,

On 11 September 2014 14:14, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Lukasz,
>
> On Wed, Sep 10, 2014 at 2:11 PM, Lukasz Rymanowski
> <lukasz.rymanowski@tieto.com> wrote:
>> ---
>>  android/hal-ipc-api.txt | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
>> index 326fa21..0948af2 100644
>> --- a/android/hal-ipc-api.txt
>> +++ b/android/hal-ipc-api.txt
>> @@ -2228,7 +2228,8 @@ Notifications:
>>         Opcode 0x81 - Connection State Changed notification
>>
>>                 Notification parameters: State (1 octet)
>> -                                        Peer Features (1 octet)
>> +                                        Peer Features (2 octets)
>> +                                        CHLD Features (2 octets)
>>                                          Address (6 octets)
>>
>>                 Valid State values: 0x00 = Disconnected
>> --
>> 1.8.4
>
> You should probably document what are the possible values for CHLD.
>
True, I missed that.
\Łukasz
>
> --
> Luiz Augusto von Dentz

[PATCH v2 4/4] android/hal-ipc-api: Fix define for HF-Client

[Lukasz Rymanowski]

---
 android/hal-ipc-api.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
index 0948af2..f6e1680 100644
--- a/android/hal-ipc-api.txt
+++ b/android/hal-ipc-api.txt
@@ -2083,7 +2083,7 @@ Notifications:
 Bluetooth Handsfree Client HAL (ID 10)
 ======================================
 
-Android HAL name: "hf_client" (BT_PROFILE_HF_CLIENT_ID)
+Android HAL name: "hf_client" (BT_PROFILE_HANDSFREE_CLIENT_ID)
 
 Commands and response:
 
-- 
1.8.4

Re: [PATCH v2 0/4] android/hf-client: API fixes

[Luiz Augusto von Dentz]

Hi Lukasz,

On Wed, Sep 10, 2014 at 2:11 PM, Lukasz Rymanowski
<lukasz.rymanowski@tieto.com> wrote:
> Couple of fixes to hf-client api.
>
> Those patches previously where a part of bigger set
> [PATCH 00/39] android: HF Client initial implementation
> but on Luiz request I split it into smaller sets.
>
> Lukasz Rymanowski (4):
>   android/hal-ipc-api: Use 2 bytes for location in dial memory
>   android/hal-ipc-api: Use Tone instead of Code DTMF
>   android/hal-ipc-api: Add missing peer features parameter
>   android/hal-ipc-api: Fix define for HF-Client
>
>  android/hal-ipc-api.txt | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> --
> 1.8.4

First thanks for flooding my inbox, and second congrats for sending
almost 60 emails to the list in one day.


-- 
Luiz Augusto von Dentz

Re: [PATCH v2 0/4] android/hf-client: API fixes

[Lukasz Rymanowski]

Hi Luiz,

On 10 September 2014 13:53, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
> Hi Lukasz,
>
> On Wed, Sep 10, 2014 at 2:11 PM, Lukasz Rymanowski
> <lukasz.rymanowski@tieto.com> wrote:
>> Couple of fixes to hf-client api.
>>
>> Those patches previously where a part of bigger set
>> [PATCH 00/39] android: HF Client initial implementation
>> but on Luiz request I split it into smaller sets.
>>
>> Lukasz Rymanowski (4):
>>   android/hal-ipc-api: Use 2 bytes for location in dial memory
>>   android/hal-ipc-api: Use Tone instead of Code DTMF
>>   android/hal-ipc-api: Add missing peer features parameter
>>   android/hal-ipc-api: Fix define for HF-Client
>>
>>  android/hal-ipc-api.txt | 11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> --
>> 1.8.4
>
> First thanks for flooding my inbox, and second congrats for sending
> almost 60 emails to the list in one day.
>
I should probably not reply on that email as this is yet another email
from me :) - However I will take that risk
First of all, I'm not sending patches to annoy you - lets be clear on
it :) You had comments to my patches so I put you into CC.

On IRC you asked me to send smaller patch sets, so once I fixed your
comment, Szymons ones and couple of mine  I've sent v2 in 2 patch
sets. One set is 4 patch long and second one is 16 patch long. (it is
not 60 :) )

As I wrote on the IRC, this HAL is big and we usually have such big
patch sets when doing skeletons for HALs. Also you were sending 35
patches for AVCTP in one set some time ago. Sometimes it is like that.
But those patches are small so it is easy to review. I don't see
reason to be annoyed.

BR
\Lukasz
>
> --
> Luiz Augusto von Dentz

Re: [PATCH v2 0/4] android/hf-client: API fixes

[Szymon Janc]

Hi Łukasz,

On Wednesday 10 of September 2014 13:11:30 Lukasz Rymanowski wrote:
> Couple of fixes to hf-client api.
> 
> Those patches previously where a part of bigger set
> [PATCH 00/39] android: HF Client initial implementation
> but on Luiz request I split it into smaller sets.
> 
> Lukasz Rymanowski (4):
>   android/hal-ipc-api: Use 2 bytes for location in dial memory
>   android/hal-ipc-api: Use Tone instead of Code DTMF
>   android/hal-ipc-api: Add missing peer features parameter
>   android/hal-ipc-api: Fix define for HF-Client
> 
>  android/hal-ipc-api.txt | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 

Applied. Thanks. 

-- 
Best regards, 
Szymon Janc

Re: [PATCH v2 0/4] android/hf-client: API fixes

[Luiz Augusto von Dentz]

Hi Szymon,

On Thu, Sep 11, 2014 at 1:17 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi Łukasz,
>
> On Wednesday 10 of September 2014 13:11:30 Lukasz Rymanowski wrote:
>> Couple of fixes to hf-client api.
>>
>> Those patches previously where a part of bigger set
>> [PATCH 00/39] android: HF Client initial implementation
>> but on Luiz request I split it into smaller sets.
>>
>> Lukasz Rymanowski (4):
>>   android/hal-ipc-api: Use 2 bytes for location in dial memory
>>   android/hal-ipc-api: Use Tone instead of Code DTMF
>>   android/hal-ipc-api: Add missing peer features parameter
>>   android/hal-ipc-api: Fix define for HF-Client
>>
>>  android/hal-ipc-api.txt | 11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>
> Applied. Thanks.

This one went in without taking my comments into account.

-- 
Luiz Augusto von Dentz

Re: [PATCH v2 0/4] android/hf-client: API fixes

[Szymon Janc]

Hi Luiz,

On Friday 12 of September 2014 14:20:43 Luiz Augusto von Dentz wrote:
> Hi Szymon,
> 
> On Thu, Sep 11, 2014 at 1:17 PM, Szymon Janc <szymon.janc@tieto.com> wrote:
> > Hi Łukasz,
> >
> > On Wednesday 10 of September 2014 13:11:30 Lukasz Rymanowski wrote:
> >> Couple of fixes to hf-client api.
> >>
> >> Those patches previously where a part of bigger set
> >> [PATCH 00/39] android: HF Client initial implementation
> >> but on Luiz request I split it into smaller sets.
> >>
> >> Lukasz Rymanowski (4):
> >>   android/hal-ipc-api: Use 2 bytes for location in dial memory
> >>   android/hal-ipc-api: Use Tone instead of Code DTMF
> >>   android/hal-ipc-api: Add missing peer features parameter
> >>   android/hal-ipc-api: Fix define for HF-Client
> >>
> >>  android/hal-ipc-api.txt | 11 ++++++-----
> >>  1 file changed, 6 insertions(+), 5 deletions(-)
> >>
> >
> > Applied. Thanks.
> 
> This one went in without taking my comments into account.

This was applied before your comments showed up on ML.
I've asked Łukasz to address your comments with follow-up patch.

-- 
Best regards, 
Szymon Janc