Hi Jakub,
On Mon, Dec 8, 2014 at 12:17 PM, Tyszkowski Jakub
<jakub.tyszkowski@xxxxxxxxx> wrote:
> Hi,
>
>
> On 12/08/2014 10:31 AM, Tyszkowski Jakub wrote:
>>
>> Hi Luiz,
>>
>> On 12/08/2014 10:22 AM, Luiz Augusto von Dentz wrote:
>>>
>>> Hi Jakub,
>>>
>>> On Mon, Dec 8, 2014 at 10:55 AM, Luiz Augusto von Dentz
>>> <luiz.dentz@xxxxxxxxx> wrote:
>>>>
>>>> Hi Jakub,
>>>>
>>>> On Mon, Dec 8, 2014 at 10:22 AM, Jakub Tyszkowski
>>>> <jakub.tyszkowski@xxxxxxxxx> wrote:
>>>>>
>>>>> Registering for GATTRIB_ALL_REQS now means only requests and not
>>>>> commands
>>>>> or other type of att operations. Those needs their own handlers to be
>>>>> registered.
>>>>
>>>>
>>>> Is this something we break while with the internal changes to use
>>>> bt_gatt* internally? Maybe we should have it fixed there instead since
>>>> it perhaps breaks the core daemon as well. Going forward this code
>>>> will transition to use bt_gatt* directly.
>>>
>>>
>>> Looks like this code is to blame:
>>>
>>> opcode_match:
>>>
>>> if (opcode == BT_ATT_ALL_REQUESTS &&
>>> get_op_type(test_opcode) == ATT_OP_TYPE_REQ)
>>> return true;
>>>
>>> Id say this turn BT_ATT_ALL_REQUESTS not that convenient for server
>>> since, and looking at gatt-server.c you can clearly see how many
>>> handlers we end up doing, perhaps we could something like this:
>>>
>>> diff --git a/src/shared/att.c b/src/shared/att.c
>>> index bc01827..3b52607 100644
>>> --- a/src/shared/att.c
>>> +++ b/src/shared/att.c
>>> @@ -661,7 +661,7 @@ struct notify_data {
>>> static bool opcode_match(uint8_t opcode, uint8_t test_opcode)
>>> {
>>> if (opcode == BT_ATT_ALL_REQUESTS &&
>>> - get_op_type(test_opcode) ==
>>> ATT_OP_TYPE_REQ)
>>> + get_op_type(test_opcode) !=
>>> ATT_OP_TYPE_RSP)
>>> return true;
>
>
> If we do that, we are still left with other stuff like indications or even
> confirmations which would still be quite quirky for using *ALL_REQESTS in
> name.
Perhaps we check ATT_OP_TYPE_CMD in addition to ATT_OP_TYPE_REQ.
>>>
>>> return opcode == test_opcode;
>>>
>> Yeap, that's the line. I assumed that this change was intended and we
>> should make the daemon compatible with this new semantic.
>> I've looked through the code where ALL_REQUESTS is used and I think
>> there is no other place affected by this.
>>
>> Regards
>
>
> Regards
>
--
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
