* [PATCH v2 1/7] android: Add flags parameter to register service command
@ 2014-03-01 21:28 Szymon Janc
2014-03-01 21:45 ` Marcel Holtmann
0 siblings, 1 reply; 5+ messages in thread
From: Szymon Janc @ 2014-03-01 21:28 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
This will allow to configure daemon services.
---
android/hal-a2dp.c | 1 +
android/hal-avrcp.c | 1 +
android/hal-bluetooth.c | 1 +
android/hal-handsfree.c | 1 +
android/hal-hidhost.c | 1 +
android/hal-ipc-api.txt | 7 +++++++
android/hal-msg.h | 1 +
android/hal-pan.c | 1 +
8 files changed, 14 insertions(+)
diff --git a/android/hal-a2dp.c b/android/hal-a2dp.c
index c898995..87d89a2 100644
--- a/android/hal-a2dp.c
+++ b/android/hal-a2dp.c
@@ -109,6 +109,7 @@ static bt_status_t init(btav_callbacks_t *callbacks)
sizeof(ev_handlers)/sizeof(ev_handlers[0]));
cmd.service_id = HAL_SERVICE_ID_A2DP;
+ cmd.flags = 0;
ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
sizeof(cmd), &cmd, 0, NULL, NULL);
diff --git a/android/hal-avrcp.c b/android/hal-avrcp.c
index 46e25a0..4907f4a 100644
--- a/android/hal-avrcp.c
+++ b/android/hal-avrcp.c
@@ -220,6 +220,7 @@ static bt_status_t init(btrc_callbacks_t *callbacks)
sizeof(ev_handlers) / sizeof(ev_handlers[0]));
cmd.service_id = HAL_SERVICE_ID_AVRCP;
+ cmd.flags = 0;
ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
sizeof(cmd), &cmd, 0, NULL, NULL);
diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
index 6871f5d..4b31ff1 100644
--- a/android/hal-bluetooth.c
+++ b/android/hal-bluetooth.c
@@ -442,6 +442,7 @@ static int init(bt_callbacks_t *callbacks)
}
cmd.service_id = HAL_SERVICE_ID_SOCKET;
+ cmd.flags = 0;
status = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
sizeof(cmd), &cmd, NULL, NULL, NULL);
diff --git a/android/hal-handsfree.c b/android/hal-handsfree.c
index 1b150c3..422f52c 100644
--- a/android/hal-handsfree.c
+++ b/android/hal-handsfree.c
@@ -212,6 +212,7 @@ static bt_status_t init(bthf_callbacks_t *callbacks)
sizeof(ev_handlers)/sizeof(ev_handlers[0]));
cmd.service_id = HAL_SERVICE_ID_HANDSFREE;
+ cmd.flags = 0;
ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
sizeof(cmd), &cmd, 0, NULL, NULL);
diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
index c758d2a..fe1b4df 100644
--- a/android/hal-hidhost.c
+++ b/android/hal-hidhost.c
@@ -354,6 +354,7 @@ static bt_status_t init(bthh_callbacks_t *callbacks)
sizeof(ev_handlers)/sizeof(ev_handlers[0]));
cmd.service_id = HAL_SERVICE_ID_HIDHOST;
+ cmd.flags = 0;
ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
sizeof(cmd), &cmd, 0, NULL, NULL);
diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
index 1a19c80..f68324d 100644
--- a/android/hal-ipc-api.txt
+++ b/android/hal-ipc-api.txt
@@ -124,12 +124,16 @@ Core Service (ID 0)
Opcode 0x01 - Register module command/response
Command parameters: Service id (1 octet)
+ Flags (1 octet)
Response parameters: <none>
In case a command is sent for an undeclared service ID, it will
be rejected. Also there will be no notifications for undeclared
service ID.
+ Flags parameter values should be defined as needed by
+ respective services.
+
In case of an error, the error response will be returned.
Opcode 0x02 - Unregister module command/response
@@ -749,6 +753,9 @@ Bluetooth Handsfree HAL (ID 5)
Android HAL name: "handsfree" (BT_PROFILE_HANDSFREE_ID)
+ Service flags: 0x01 = Disable HSP AG
+ 0x02 = Disable HFP AG
+
Opcode 0x00 - Error response
Response parameters: Status (1 octet)
diff --git a/android/hal-msg.h b/android/hal-msg.h
index 1e12868..e66043e 100644
--- a/android/hal-msg.h
+++ b/android/hal-msg.h
@@ -57,6 +57,7 @@ static const char BLUEZ_HAL_SK_PATH[] = "\0bluez_hal_socket";
#define HAL_OP_REGISTER_MODULE 0x01
struct hal_cmd_register_module {
uint8_t service_id;
+ uint8_t flags;
} __attribute__((packed));
#define HAL_OP_UNREGISTER_MODULE 0x02
diff --git a/android/hal-pan.c b/android/hal-pan.c
index 5ee49ef..e6a351d 100644
--- a/android/hal-pan.c
+++ b/android/hal-pan.c
@@ -157,6 +157,7 @@ static bt_status_t pan_init(const btpan_callbacks_t *callbacks)
sizeof(ev_handlers)/sizeof(ev_handlers[0]));
cmd.service_id = HAL_SERVICE_ID_PAN;
+ cmd.flags = 0;
ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
sizeof(cmd), &cmd, 0, NULL, NULL);
--
1.8.5.3
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2 1/7] android: Add flags parameter to register service command
2014-03-01 21:28 [PATCH v2 1/7] android: Add flags parameter to register service command Szymon Janc
@ 2014-03-01 21:45 ` Marcel Holtmann
2014-03-01 22:25 ` Szymon Janc
0 siblings, 1 reply; 5+ messages in thread
From: Marcel Holtmann @ 2014-03-01 21:45 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
Hi Szymon,
> This will allow to configure daemon services.
> ---
> android/hal-a2dp.c | 1 +
> android/hal-avrcp.c | 1 +
> android/hal-bluetooth.c | 1 +
> android/hal-handsfree.c | 1 +
> android/hal-hidhost.c | 1 +
> android/hal-ipc-api.txt | 7 +++++++
please do not mix code with API docs updates.
> android/hal-msg.h | 1 +
> android/hal-pan.c | 1 +
> 8 files changed, 14 insertions(+)
>
> diff --git a/android/hal-a2dp.c b/android/hal-a2dp.c
> index c898995..87d89a2 100644
> --- a/android/hal-a2dp.c
> +++ b/android/hal-a2dp.c
> @@ -109,6 +109,7 @@ static bt_status_t init(btav_callbacks_t *callbacks)
> sizeof(ev_handlers)/sizeof(ev_handlers[0]));
>
> cmd.service_id = HAL_SERVICE_ID_A2DP;
> + cmd.flags = 0;
>
> ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> sizeof(cmd), &cmd, 0, NULL, NULL);
> diff --git a/android/hal-avrcp.c b/android/hal-avrcp.c
> index 46e25a0..4907f4a 100644
> --- a/android/hal-avrcp.c
> +++ b/android/hal-avrcp.c
> @@ -220,6 +220,7 @@ static bt_status_t init(btrc_callbacks_t *callbacks)
> sizeof(ev_handlers) / sizeof(ev_handlers[0]));
>
> cmd.service_id = HAL_SERVICE_ID_AVRCP;
> + cmd.flags = 0;
>
> ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> sizeof(cmd), &cmd, 0, NULL, NULL);
> diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> index 6871f5d..4b31ff1 100644
> --- a/android/hal-bluetooth.c
> +++ b/android/hal-bluetooth.c
> @@ -442,6 +442,7 @@ static int init(bt_callbacks_t *callbacks)
> }
>
> cmd.service_id = HAL_SERVICE_ID_SOCKET;
> + cmd.flags = 0;
>
> status = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> sizeof(cmd), &cmd, NULL, NULL, NULL);
> diff --git a/android/hal-handsfree.c b/android/hal-handsfree.c
> index 1b150c3..422f52c 100644
> --- a/android/hal-handsfree.c
> +++ b/android/hal-handsfree.c
> @@ -212,6 +212,7 @@ static bt_status_t init(bthf_callbacks_t *callbacks)
> sizeof(ev_handlers)/sizeof(ev_handlers[0]));
>
> cmd.service_id = HAL_SERVICE_ID_HANDSFREE;
> + cmd.flags = 0;
>
> ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> sizeof(cmd), &cmd, 0, NULL, NULL);
> diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
> index c758d2a..fe1b4df 100644
> --- a/android/hal-hidhost.c
> +++ b/android/hal-hidhost.c
> @@ -354,6 +354,7 @@ static bt_status_t init(bthh_callbacks_t *callbacks)
> sizeof(ev_handlers)/sizeof(ev_handlers[0]));
>
> cmd.service_id = HAL_SERVICE_ID_HIDHOST;
> + cmd.flags = 0;
>
> ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> sizeof(cmd), &cmd, 0, NULL, NULL);
> diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
> index 1a19c80..f68324d 100644
> --- a/android/hal-ipc-api.txt
> +++ b/android/hal-ipc-api.txt
> @@ -124,12 +124,16 @@ Core Service (ID 0)
> Opcode 0x01 - Register module command/response
>
> Command parameters: Service id (1 octet)
> + Flags (1 octet)
We might better go with a Flags (4 octets) here. Otherwise you only have 8 options which might be enough or not. Your choice.
Or instead of a bit mask, we do a Configuration (1 octet) were we have the default configuration with 0x00 and then alternatives with all other values. That might be actually a bit better.
> Response parameters: <none>
>
> In case a command is sent for an undeclared service ID, it will
> be rejected. Also there will be no notifications for undeclared
> service ID.
>
> + Flags parameter values should be defined as needed by
> + respective services.
> +
> In case of an error, the error response will be returned.
>
> Opcode 0x02 - Unregister module command/response
> @@ -749,6 +753,9 @@ Bluetooth Handsfree HAL (ID 5)
>
> Android HAL name: "handsfree" (BT_PROFILE_HANDSFREE_ID)
>
> + Service flags: 0x01 = Disable HSP AG
> + 0x02 = Disable HFP AG
> +
If you do not want HSP and HFP, then there is no point in registering handsfree ID at all. I am tending currently towards the configuration option.
0x00 = Provide Handsfree profile
0x01 = Provide Headset profile
There is really no point in providing both? Or is there a case where you would provide both?
Regards
Marcel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/7] android: Add flags parameter to register service command
2014-03-01 21:45 ` Marcel Holtmann
@ 2014-03-01 22:25 ` Szymon Janc
2014-03-02 17:56 ` Marcel Holtmann
0 siblings, 1 reply; 5+ messages in thread
From: Szymon Janc @ 2014-03-01 22:25 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hi Marcel,
On Saturday 01 of March 2014 13:45:15 Marcel Holtmann wrote:
> Hi Szymon,
>
> > This will allow to configure daemon services.
> > ---
> > android/hal-a2dp.c | 1 +
> > android/hal-avrcp.c | 1 +
> > android/hal-bluetooth.c | 1 +
> > android/hal-handsfree.c | 1 +
> > android/hal-hidhost.c | 1 +
> > android/hal-ipc-api.txt | 7 +++++++
>
> please do not mix code with API docs updates.
OK.
>
> > android/hal-msg.h | 1 +
> > android/hal-pan.c | 1 +
> > 8 files changed, 14 insertions(+)
> >
> > diff --git a/android/hal-a2dp.c b/android/hal-a2dp.c
> > index c898995..87d89a2 100644
> > --- a/android/hal-a2dp.c
> > +++ b/android/hal-a2dp.c
> > @@ -109,6 +109,7 @@ static bt_status_t init(btav_callbacks_t *callbacks)
> >
> > sizeof(ev_handlers)/sizeof(ev_handlers[0]));
> >
> > cmd.service_id = HAL_SERVICE_ID_A2DP;
> >
> > + cmd.flags = 0;
> >
> > ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> >
> > sizeof(cmd), &cmd, 0, NULL, NULL);
> >
> > diff --git a/android/hal-avrcp.c b/android/hal-avrcp.c
> > index 46e25a0..4907f4a 100644
> > --- a/android/hal-avrcp.c
> > +++ b/android/hal-avrcp.c
> > @@ -220,6 +220,7 @@ static bt_status_t init(btrc_callbacks_t *callbacks)
> >
> > sizeof(ev_handlers) / sizeof(ev_handlers[0]));
> >
> > cmd.service_id = HAL_SERVICE_ID_AVRCP;
> >
> > + cmd.flags = 0;
> >
> > ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> >
> > sizeof(cmd), &cmd, 0, NULL, NULL);
> >
> > diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> > index 6871f5d..4b31ff1 100644
> > --- a/android/hal-bluetooth.c
> > +++ b/android/hal-bluetooth.c
> > @@ -442,6 +442,7 @@ static int init(bt_callbacks_t *callbacks)
> >
> > }
> >
> > cmd.service_id = HAL_SERVICE_ID_SOCKET;
> >
> > + cmd.flags = 0;
> >
> > status = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> >
> > sizeof(cmd), &cmd, NULL, NULL, NULL);
> >
> > diff --git a/android/hal-handsfree.c b/android/hal-handsfree.c
> > index 1b150c3..422f52c 100644
> > --- a/android/hal-handsfree.c
> > +++ b/android/hal-handsfree.c
> > @@ -212,6 +212,7 @@ static bt_status_t init(bthf_callbacks_t *callbacks)
> >
> > sizeof(ev_handlers)/sizeof(ev_handlers[0]));
> >
> > cmd.service_id = HAL_SERVICE_ID_HANDSFREE;
> >
> > + cmd.flags = 0;
> >
> > ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> >
> > sizeof(cmd), &cmd, 0, NULL, NULL);
> >
> > diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
> > index c758d2a..fe1b4df 100644
> > --- a/android/hal-hidhost.c
> > +++ b/android/hal-hidhost.c
> > @@ -354,6 +354,7 @@ static bt_status_t init(bthh_callbacks_t *callbacks)
> >
> > sizeof(ev_handlers)/sizeof(ev_handlers[0]));
> >
> > cmd.service_id = HAL_SERVICE_ID_HIDHOST;
> >
> > + cmd.flags = 0;
> >
> > ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> >
> > sizeof(cmd), &cmd, 0, NULL, NULL);
> >
> > diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
> > index 1a19c80..f68324d 100644
> > --- a/android/hal-ipc-api.txt
> > +++ b/android/hal-ipc-api.txt
> > @@ -124,12 +124,16 @@ Core Service (ID 0)
> >
> > Opcode 0x01 - Register module command/response
> >
> > Command parameters: Service id (1 octet)
> >
> > + Flags (1 octet)
>
> We might better go with a Flags (4 octets) here. Otherwise you only have 8
> options which might be enough or not. Your choice.
>
> Or instead of a bit mask, we do a Configuration (1 octet) were we have the
> default configuration with 0x00 and then alternatives with all other
> values. That might be actually a bit better.
I'll see how it goes with configuration options, but flags could be used also
to enable eg. debug options which might be clumsy without flags.
> > Response parameters: <none>
> >
> > In case a command is sent for an undeclared service ID, it will
> > be rejected. Also there will be no notifications for undeclared
> > service ID.
> >
> > + Flags parameter values should be defined as needed by
> > + respective services.
> > +
> >
> > In case of an error, the error response will be returned.
> >
> > Opcode 0x02 - Unregister module command/response
> >
> > @@ -749,6 +753,9 @@ Bluetooth Handsfree HAL (ID 5)
> >
> > Android HAL name: "handsfree" (BT_PROFILE_HANDSFREE_ID)
> >
> > + Service flags: 0x01 = Disable HSP AG
> > + 0x02 = Disable HFP AG
> > +
>
> If you do not want HSP and HFP, then there is no point in registering
> handsfree ID at all. I am tending currently towards the configuration
> option.
> 0x00 = Provide Handsfree profile
> 0x01 = Provide Headset profile
>
> There is really no point in providing both? Or is there a case where you
> would provide both?
Android on phones provides both. On tablets HSP only. So we could go with
0x00 = Provide HFP and HSP
0x01 = Provide HSP only
or just with single flag to disable HFP.
--
BR
Szymon Janc
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/7] android: Add flags parameter to register service command
2014-03-01 22:25 ` Szymon Janc
@ 2014-03-02 17:56 ` Marcel Holtmann
2014-03-02 20:53 ` Szymon Janc
0 siblings, 1 reply; 5+ messages in thread
From: Marcel Holtmann @ 2014-03-02 17:56 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
Hi Szymon,
>>> This will allow to configure daemon services.
>>> ---
>>> android/hal-a2dp.c | 1 +
>>> android/hal-avrcp.c | 1 +
>>> android/hal-bluetooth.c | 1 +
>>> android/hal-handsfree.c | 1 +
>>> android/hal-hidhost.c | 1 +
>>> android/hal-ipc-api.txt | 7 +++++++
>>
>> please do not mix code with API docs updates.
>
> OK.
>
>>
>>> android/hal-msg.h | 1 +
>>> android/hal-pan.c | 1 +
>>> 8 files changed, 14 insertions(+)
>>>
>>> diff --git a/android/hal-a2dp.c b/android/hal-a2dp.c
>>> index c898995..87d89a2 100644
>>> --- a/android/hal-a2dp.c
>>> +++ b/android/hal-a2dp.c
>>> @@ -109,6 +109,7 @@ static bt_status_t init(btav_callbacks_t *callbacks)
>>>
>>> sizeof(ev_handlers)/sizeof(ev_handlers[0]));
>>>
>>> cmd.service_id = HAL_SERVICE_ID_A2DP;
>>>
>>> + cmd.flags = 0;
>>>
>>> ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
>>>
>>> sizeof(cmd), &cmd, 0, NULL, NULL);
>>>
>>> diff --git a/android/hal-avrcp.c b/android/hal-avrcp.c
>>> index 46e25a0..4907f4a 100644
>>> --- a/android/hal-avrcp.c
>>> +++ b/android/hal-avrcp.c
>>> @@ -220,6 +220,7 @@ static bt_status_t init(btrc_callbacks_t *callbacks)
>>>
>>> sizeof(ev_handlers) / sizeof(ev_handlers[0]));
>>>
>>> cmd.service_id = HAL_SERVICE_ID_AVRCP;
>>>
>>> + cmd.flags = 0;
>>>
>>> ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
>>>
>>> sizeof(cmd), &cmd, 0, NULL, NULL);
>>>
>>> diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
>>> index 6871f5d..4b31ff1 100644
>>> --- a/android/hal-bluetooth.c
>>> +++ b/android/hal-bluetooth.c
>>> @@ -442,6 +442,7 @@ static int init(bt_callbacks_t *callbacks)
>>>
>>> }
>>>
>>> cmd.service_id = HAL_SERVICE_ID_SOCKET;
>>>
>>> + cmd.flags = 0;
>>>
>>> status = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
>>>
>>> sizeof(cmd), &cmd, NULL, NULL, NULL);
>>>
>>> diff --git a/android/hal-handsfree.c b/android/hal-handsfree.c
>>> index 1b150c3..422f52c 100644
>>> --- a/android/hal-handsfree.c
>>> +++ b/android/hal-handsfree.c
>>> @@ -212,6 +212,7 @@ static bt_status_t init(bthf_callbacks_t *callbacks)
>>>
>>> sizeof(ev_handlers)/sizeof(ev_handlers[0]));
>>>
>>> cmd.service_id = HAL_SERVICE_ID_HANDSFREE;
>>>
>>> + cmd.flags = 0;
>>>
>>> ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
>>>
>>> sizeof(cmd), &cmd, 0, NULL, NULL);
>>>
>>> diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
>>> index c758d2a..fe1b4df 100644
>>> --- a/android/hal-hidhost.c
>>> +++ b/android/hal-hidhost.c
>>> @@ -354,6 +354,7 @@ static bt_status_t init(bthh_callbacks_t *callbacks)
>>>
>>> sizeof(ev_handlers)/sizeof(ev_handlers[0]));
>>>
>>> cmd.service_id = HAL_SERVICE_ID_HIDHOST;
>>>
>>> + cmd.flags = 0;
>>>
>>> ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
>>>
>>> sizeof(cmd), &cmd, 0, NULL, NULL);
>>>
>>> diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
>>> index 1a19c80..f68324d 100644
>>> --- a/android/hal-ipc-api.txt
>>> +++ b/android/hal-ipc-api.txt
>>> @@ -124,12 +124,16 @@ Core Service (ID 0)
>>>
>>> Opcode 0x01 - Register module command/response
>>>
>>> Command parameters: Service id (1 octet)
>>>
>>> + Flags (1 octet)
>>
>> We might better go with a Flags (4 octets) here. Otherwise you only have 8
>> options which might be enough or not. Your choice.
>>
>> Or instead of a bit mask, we do a Configuration (1 octet) were we have the
>> default configuration with 0x00 and then alternatives with all other
>> values. That might be actually a bit better.
>
> I'll see how it goes with configuration options, but flags could be used also
> to enable eg. debug options which might be clumsy without flags.
what debug options are you thinking about?
I also think we should just call it Mode (1 octet). Keep it simple.
>>> Response parameters: <none>
>>>
>>> In case a command is sent for an undeclared service ID, it will
>>> be rejected. Also there will be no notifications for undeclared
>>> service ID.
>>>
>>> + Flags parameter values should be defined as needed by
>>> + respective services.
>>> +
>>>
>>> In case of an error, the error response will be returned.
>>>
>>> Opcode 0x02 - Unregister module command/response
>>>
>>> @@ -749,6 +753,9 @@ Bluetooth Handsfree HAL (ID 5)
>>>
>>> Android HAL name: "handsfree" (BT_PROFILE_HANDSFREE_ID)
>>>
>>> + Service flags: 0x01 = Disable HSP AG
>>> + 0x02 = Disable HFP AG
>>> +
>>
>> If you do not want HSP and HFP, then there is no point in registering
>> handsfree ID at all. I am tending currently towards the configuration
>> option.
>
>> 0x00 = Provide Handsfree profile
>> 0x01 = Provide Headset profile
>>
>> There is really no point in providing both? Or is there a case where you
>> would provide both?
>
> Android on phones provides both. On tablets HSP only. So we could go with
> 0x00 = Provide HFP and HSP
> 0x01 = Provide HSP only
>
> or just with single flag to disable HFP.
Do we really want to expose HSP gateway when HFP is available. I am not even sure this makes sense. With Bluedroid, are they using the same RFCOMM channel or how does that actually work. You can not connect them at the same time anyway.
I would really go for the simple case of either Handsfree or Headset profile, but not both.
Regards
Marcel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/7] android: Add flags parameter to register service command
2014-03-02 17:56 ` Marcel Holtmann
@ 2014-03-02 20:53 ` Szymon Janc
0 siblings, 0 replies; 5+ messages in thread
From: Szymon Janc @ 2014-03-02 20:53 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hi Marcel,
On Sunday 02 of March 2014 09:56:15 Marcel Holtmann wrote:
> Hi Szymon,
>
> >>> This will allow to configure daemon services.
> >>> ---
> >>> android/hal-a2dp.c | 1 +
> >>> android/hal-avrcp.c | 1 +
> >>> android/hal-bluetooth.c | 1 +
> >>> android/hal-handsfree.c | 1 +
> >>> android/hal-hidhost.c | 1 +
> >>> android/hal-ipc-api.txt | 7 +++++++
> >>
> >> please do not mix code with API docs updates.
> >
> > OK.
> >
> >>> android/hal-msg.h | 1 +
> >>> android/hal-pan.c | 1 +
> >>> 8 files changed, 14 insertions(+)
> >>>
> >>> diff --git a/android/hal-a2dp.c b/android/hal-a2dp.c
> >>> index c898995..87d89a2 100644
> >>> --- a/android/hal-a2dp.c
> >>> +++ b/android/hal-a2dp.c
> >>> @@ -109,6 +109,7 @@ static bt_status_t init(btav_callbacks_t *callbacks)
> >>>
> >>> sizeof(ev_handlers)/sizeof(ev_handlers[0]));
> >>>
> >>> cmd.service_id = HAL_SERVICE_ID_A2DP;
> >>>
> >>> + cmd.flags = 0;
> >>>
> >>> ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> >>>
> >>> sizeof(cmd), &cmd, 0, NULL, NULL);
> >>>
> >>> diff --git a/android/hal-avrcp.c b/android/hal-avrcp.c
> >>> index 46e25a0..4907f4a 100644
> >>> --- a/android/hal-avrcp.c
> >>> +++ b/android/hal-avrcp.c
> >>> @@ -220,6 +220,7 @@ static bt_status_t init(btrc_callbacks_t *callbacks)
> >>>
> >>> sizeof(ev_handlers) / sizeof(ev_handlers[0]));
> >>>
> >>> cmd.service_id = HAL_SERVICE_ID_AVRCP;
> >>>
> >>> + cmd.flags = 0;
> >>>
> >>> ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> >>>
> >>> sizeof(cmd), &cmd, 0, NULL, NULL);
> >>>
> >>> diff --git a/android/hal-bluetooth.c b/android/hal-bluetooth.c
> >>> index 6871f5d..4b31ff1 100644
> >>> --- a/android/hal-bluetooth.c
> >>> +++ b/android/hal-bluetooth.c
> >>> @@ -442,6 +442,7 @@ static int init(bt_callbacks_t *callbacks)
> >>>
> >>> }
> >>>
> >>> cmd.service_id = HAL_SERVICE_ID_SOCKET;
> >>>
> >>> + cmd.flags = 0;
> >>>
> >>> status = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> >>>
> >>> sizeof(cmd), &cmd, NULL, NULL, NULL);
> >>>
> >>> diff --git a/android/hal-handsfree.c b/android/hal-handsfree.c
> >>> index 1b150c3..422f52c 100644
> >>> --- a/android/hal-handsfree.c
> >>> +++ b/android/hal-handsfree.c
> >>> @@ -212,6 +212,7 @@ static bt_status_t init(bthf_callbacks_t *callbacks)
> >>>
> >>> sizeof(ev_handlers)/sizeof(ev_handlers[0]));
> >>>
> >>> cmd.service_id = HAL_SERVICE_ID_HANDSFREE;
> >>>
> >>> + cmd.flags = 0;
> >>>
> >>> ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> >>>
> >>> sizeof(cmd), &cmd, 0, NULL, NULL);
> >>>
> >>> diff --git a/android/hal-hidhost.c b/android/hal-hidhost.c
> >>> index c758d2a..fe1b4df 100644
> >>> --- a/android/hal-hidhost.c
> >>> +++ b/android/hal-hidhost.c
> >>> @@ -354,6 +354,7 @@ static bt_status_t init(bthh_callbacks_t *callbacks)
> >>>
> >>> sizeof(ev_handlers)/sizeof(ev_handlers[0]));
> >>>
> >>> cmd.service_id = HAL_SERVICE_ID_HIDHOST;
> >>>
> >>> + cmd.flags = 0;
> >>>
> >>> ret = hal_ipc_cmd(HAL_SERVICE_ID_CORE, HAL_OP_REGISTER_MODULE,
> >>>
> >>> sizeof(cmd), &cmd, 0, NULL, NULL);
> >>>
> >>> diff --git a/android/hal-ipc-api.txt b/android/hal-ipc-api.txt
> >>> index 1a19c80..f68324d 100644
> >>> --- a/android/hal-ipc-api.txt
> >>> +++ b/android/hal-ipc-api.txt
> >>> @@ -124,12 +124,16 @@ Core Service (ID 0)
> >>>
> >>> Opcode 0x01 - Register module command/response
> >>>
> >>> Command parameters: Service id (1 octet)
> >>>
> >>> + Flags (1 octet)
> >>
> >> We might better go with a Flags (4 octets) here. Otherwise you only have
> >> 8
> >> options which might be enough or not. Your choice.
> >>
> >> Or instead of a bit mask, we do a Configuration (1 octet) were we have
> >> the
> >> default configuration with 0x00 and then alternatives with all other
> >> values. That might be actually a bit better.
> >
> > I'll see how it goes with configuration options, but flags could be used
> > also to enable eg. debug options which might be clumsy without flags.
>
> what debug options are you thinking about?
>
> I also think we should just call it Mode (1 octet). Keep it simple.
Like enabling debug linkkeys or AT dumping in handsfree. But I guess we can
work this out later, so I'll go with 'Mode' as you suggested.
Will send V3 shortly.
> >>> Response parameters: <none>
> >>>
> >>> In case a command is sent for an undeclared service ID, it will
> >>> be rejected. Also there will be no notifications for undeclared
> >>> service ID.
> >>>
> >>> + Flags parameter values should be defined as needed by
> >>> + respective services.
> >>> +
> >>>
> >>> In case of an error, the error response will be returned.
> >>>
> >>> Opcode 0x02 - Unregister module command/response
> >>>
> >>> @@ -749,6 +753,9 @@ Bluetooth Handsfree HAL (ID 5)
> >>>
> >>> Android HAL name: "handsfree" (BT_PROFILE_HANDSFREE_ID)
> >>>
> >>> + Service flags: 0x01 = Disable HSP AG
> >>> + 0x02 = Disable HFP AG
> >>> +
> >>
> >> If you do not want HSP and HFP, then there is no point in registering
> >> handsfree ID at all. I am tending currently towards the configuration
> >> option.
> >>
> >> 0x00 = Provide Handsfree profile
> >> 0x01 = Provide Headset profile
> >>
> >> There is really no point in providing both? Or is there a case where you
> >> would provide both?
> >
> > Android on phones provides both. On tablets HSP only. So we could go with
> > 0x00 = Provide HFP and HSP
> > 0x01 = Provide HSP only
> >
> > or just with single flag to disable HFP.
>
> Do we really want to expose HSP gateway when HFP is available. I am not even
> sure this makes sense. With Bluedroid, are they using the same RFCOMM
> channel or how does that actually work. You can not connect them at the
> same time anyway.
>
> I would really go for the simple case of either Handsfree or Headset
> profile, but not both.
Those are running on separate channels and with this serie we have the same.
If either is connected, another connection will be refused.
I'm not sure why having both HSP and HFP would be of any problem..
--
BR
Szymon Janc
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-03-02 20:53 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-01 21:28 [PATCH v2 1/7] android: Add flags parameter to register service command Szymon Janc
2014-03-01 21:45 ` Marcel Holtmann
2014-03-01 22:25 ` Szymon Janc
2014-03-02 17:56 ` Marcel Holtmann
2014-03-02 20:53 ` Szymon Janc
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).