From: Jakub Pawlowski <[email protected]>
This patch adds the opcode and structure for Start Service Discovery
operation.
Signed-off-by: Jakub Pawlowski <[email protected]>
Signed-off-by: Marcel Holtmann <[email protected]>
---
include/net/bluetooth/mgmt.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 9b382ea34fd9..95c34d5180fa 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -498,6 +498,15 @@ struct mgmt_cp_set_public_address {
} __packed;
#define MGMT_SET_PUBLIC_ADDRESS_SIZE 6
+#define MGMT_OP_START_SERVICE_DISCOVERY 0x003A
+struct mgmt_cp_start_service_discovery {
+ __u8 type;
+ __s8 rssi;
+ __le16 uuid_count;
+ __u8 uuids[0][16];
+} __packed;
+#define MGMT_START_SERVICE_DISCOVERY_SIZE 4
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
--
1.9.3
Hi Jakub,
>>>> This patch adds the opcode and structure for Start Service Discovery
>>>> operation.
>>>>
>>>> Signed-off-by: Jakub Pawlowski <[email protected]>
>>>> Signed-off-by: Marcel Holtmann <[email protected]>
>>>> ---
>>>> include/net/bluetooth/mgmt.h | 9 +++++++++
>>>> 1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
>>>> index 9b382ea34fd9..95c34d5180fa 100644
>>>> --- a/include/net/bluetooth/mgmt.h
>>>> +++ b/include/net/bluetooth/mgmt.h
>>>> @@ -498,6 +498,15 @@ struct mgmt_cp_set_public_address {
>>>> } __packed;
>>>> #define MGMT_SET_PUBLIC_ADDRESS_SIZE 6
>>>>
>>>> +#define MGMT_OP_START_SERVICE_DISCOVERY 0x003A
>>>> +struct mgmt_cp_start_service_discovery {
>>>> + __u8 type;
>>>
>>> Maybe we should get rid of type ? service discovery based on
>>> advertisement content makes sense only for LE
>>
>> this works perfectly fine for BR/EDR or BR/EDR + LE discovery. It is up to the call to decide what to do. And we have kept the type in place even for commands that are currently only supported on one transport. It allows us to extend the API without breaking it.
>
> Ok, that sounds reasonable.
>
> I've merged my filtering patch on top of your patches, then run
> everything and did some manual tests - all works great!
> I've just send updated patches.
I did re-work your patches a little bit actually.
The filter should be applied independently on EIR, advertising data and/or scan response data. Running it over the combined data has the danger that if some device are broken it becomes hard to detect wrong scan response data. So I decided to run this manually. And I also decided to skip the scan response data in case we already found a match in the advertising data.
There is also the funny case if either of this data is empty, then we should consider no matching and need to handle that one as well.
You also had some uint8_t in there which I assume you copied from userspace code. We do not use that in kernel code and use u8 instead. So I fixed that as well. If I counted correctly, the the code also had one off-by-one for the EIR field length check. That should be fixed as well now.
The only missing piece is now the LE restart handling. That is the one patch I have actually not yet looked at.
Regards
Marcel
On Thu, Dec 4, 2014 at 11:27 AM, Marcel Holtmann <[email protected]> wrot=
e:
> Hi Jakub,
>
>>> This patch adds the opcode and structure for Start Service Discovery
>>> operation.
>>>
>>> Signed-off-by: Jakub Pawlowski <[email protected]>
>>> Signed-off-by: Marcel Holtmann <[email protected]>
>>> ---
>>> include/net/bluetooth/mgmt.h | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.=
h
>>> index 9b382ea34fd9..95c34d5180fa 100644
>>> --- a/include/net/bluetooth/mgmt.h
>>> +++ b/include/net/bluetooth/mgmt.h
>>> @@ -498,6 +498,15 @@ struct mgmt_cp_set_public_address {
>>> } __packed;
>>> #define MGMT_SET_PUBLIC_ADDRESS_SIZE 6
>>>
>>> +#define MGMT_OP_START_SERVICE_DISCOVERY 0x003A
>>> +struct mgmt_cp_start_service_discovery {
>>> + __u8 type;
>>
>> Maybe we should get rid of type ? service discovery based on
>> advertisement content makes sense only for LE
>
> this works perfectly fine for BR/EDR or BR/EDR + LE discovery. It is up t=
o the call to decide what to do. And we have kept the type in place even fo=
r commands that are currently only supported on one transport. It allows us=
to extend the API without breaking it.
Ok, that sounds reasonable.
I've merged my filtering patch on top of your patches, then run
everything and did some manual tests - all works great!
I've just send updated patches.
>
> Regards
>
> Marcel
>
I
Hi Jakub,
>> This patch adds the opcode and structure for Start Service Discovery
>> operation.
>>
>> Signed-off-by: Jakub Pawlowski <[email protected]>
>> Signed-off-by: Marcel Holtmann <[email protected]>
>> ---
>> include/net/bluetooth/mgmt.h | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
>> index 9b382ea34fd9..95c34d5180fa 100644
>> --- a/include/net/bluetooth/mgmt.h
>> +++ b/include/net/bluetooth/mgmt.h
>> @@ -498,6 +498,15 @@ struct mgmt_cp_set_public_address {
>> } __packed;
>> #define MGMT_SET_PUBLIC_ADDRESS_SIZE 6
>>
>> +#define MGMT_OP_START_SERVICE_DISCOVERY 0x003A
>> +struct mgmt_cp_start_service_discovery {
>> + __u8 type;
>
> Maybe we should get rid of type ? service discovery based on
> advertisement content makes sense only for LE
this works perfectly fine for BR/EDR or BR/EDR + LE discovery. It is up to the call to decide what to do. And we have kept the type in place even for commands that are currently only supported on one transport. It allows us to extend the API without breaking it.
Regards
Marcel
On Thu, Dec 4, 2014 at 2:36 AM, Marcel Holtmann <[email protected]> wrote:
> From: Jakub Pawlowski <[email protected]>
>
> This patch adds the opcode and structure for Start Service Discovery
> operation.
>
> Signed-off-by: Jakub Pawlowski <[email protected]>
> Signed-off-by: Marcel Holtmann <[email protected]>
> ---
> include/net/bluetooth/mgmt.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
> index 9b382ea34fd9..95c34d5180fa 100644
> --- a/include/net/bluetooth/mgmt.h
> +++ b/include/net/bluetooth/mgmt.h
> @@ -498,6 +498,15 @@ struct mgmt_cp_set_public_address {
> } __packed;
> #define MGMT_SET_PUBLIC_ADDRESS_SIZE 6
>
> +#define MGMT_OP_START_SERVICE_DISCOVERY 0x003A
> +struct mgmt_cp_start_service_discovery {
> + __u8 type;
Maybe we should get rid of type ? service discovery based on
advertisement content makes sense only for LE
> + __s8 rssi;
> + __le16 uuid_count;
> + __u8 uuids[0][16];
> +} __packed;
> +#define MGMT_START_SERVICE_DISCOVERY_SIZE 4
> +
> #define MGMT_EV_CMD_COMPLETE 0x0001
> struct mgmt_ev_cmd_complete {
> __le16 opcode;
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html