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
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 <[email protected]> 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
Hi Szymon,
On Thu, Sep 11, 2014 at 1:17 PM, Szymon Janc <[email protected]> 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
Hi Luiz,
On 11 September 2014 14:14, Luiz Augusto von Dentz <[email protected]> wrote:
> Hi Lukasz,
>
> On Wed, Sep 10, 2014 at 2:11 PM, Lukasz Rymanowski
> <[email protected]> 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
Hi Luiz,
On 11 September 2014 14:08, Luiz Augusto von Dentz <[email protected]> wrote:
> Hi Lukasz,
>
> On Wed, Sep 10, 2014 at 2:11 PM, Lukasz Rymanowski
> <[email protected]> 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
Hi Lukasz,
On Wed, Sep 10, 2014 at 2:11 PM, Lukasz Rymanowski
<[email protected]> 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
Hi Lukasz,
On Wed, Sep 10, 2014 at 2:11 PM, Lukasz Rymanowski
<[email protected]> 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
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
Hi Luiz,
On 10 September 2014 13:53, Luiz Augusto von Dentz <[email protected]> wrote:
> Hi Lukasz,
>
> On Wed, Sep 10, 2014 at 2:11 PM, Lukasz Rymanowski
> <[email protected]> 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
Hi Lukasz,
On Wed, Sep 10, 2014 at 2:11 PM, Lukasz Rymanowski
<[email protected]> 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
---
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
---
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
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
---
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