Hi Jaikumar,
On Wed, Jun 8, 2011 at 11:18 AM, Jaikumar Ganesh <[email protected]> wrote:
> Hi Claudio:
>
> On Tue, Jun 7, 2011 at 11:41 PM, Claudio Takahasi
> <[email protected]> wrote:
>> Hi All,
>>
>> There is a potential race condition in the Generic Attribute API. I'd
>> like to collect opinions to avoid re-work later.
>>
>> How to reproduce:
>> - call CreateDevice() for a device which exports a GATT based service:
>> CreateDevice will discover all primary services
>> and the Generic API will register the object paths for all found services
>> - Call DiscoverCharacteristics() of the primary service followed by
>> Characteristic GetProperties(), expected result: read all
>> characteristics properties outcome: partial characteristics properties
>> may appear.
>>
>> This problem happens because DiscoverCharacteristics() doesn't wait
>> for all characteristic VALUES, it returns after discovering all
>> characteristic DECLARATIONS.
>> Wait for characteristic values is a problem, attributes may require
>> authentication or authorization.
>>
>> Suggestions are:
>> 1. Report discovered characteristic values using PropertyChanged:
>> Returning empty properties values (for GetProperties) if the discovery
>> is in progress
>>
>> 2. Block GetProperties() if there is a discovery pending: returning
>> after discovery finishes. Timeout can still happen!
>>
>> 3. Return Busy for GetProperties() if there is a characteristic
>> discovery in progress
>>
>> Comments?
>
> I don't think we should read characteristic value till the application
> asks for it.
Are you suggesting to trigger the characteristic value and descriptors
read when Characteristic GetProperties() is called and block it?
Remember that GATT timeout is 30seconds and the default D-Bus message
reply timeout is smaller.
> DiscoverCharacteristics() should just return after getting the DECLARATIONS.
This is current behaviour.
> A client might not be interested in all the values and thus we should
> read them only
> when needed by the client. This will save both time and power and
> solve this problem too.
I don't think it is critical, we are not reading them on all
connections/re-connections and the profiles available at the moment
don't expose "complex" characteristics.
BR,
Claudio
>
> Thanks
> Jaikumar
>>
>> BR,
>> Claudio
Hi,
On Thu, Jun 30, 2011 at 2:48 PM, Claudio Takahasi
<[email protected]> wrote:
> On Wed, Jun 29, 2011 at 8:50 PM, ?<[email protected]> wrote:
>> I ran into a similar problem. Cannot we just delay the return of
>> DiscoverCharacteristics method call until after the characteristic
>> values/properties have been acquired? I tried out this solution and it
>> seems to work fine. The trick is to implement internal counter for
>> received responses from the remote device + timeout that is reset on each
>> successful response. Any thoughts on this?
>
> IMO delay the return of DiscoverCharacteristics method is the most
> simple solution.
What about the D-Bus timeout (small) versus GATT timeout (30 seconds)
? We should avoid getting those "org.freedesktop.DBus.Error.NoReply:
Did not receive a reply." errors.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
Hi Inga,
On Wed, Jun 29, 2011 at 8:50 PM, <[email protected]> wrote:
> Hi Claudio,
>
<snip>
>
> Just noticed this thread.
> I ran into a similar problem. Cannot we just delay the return of
> DiscoverCharacteristics method call until after the characteristic
> values/properties have been acquired? I tried out this solution and it
> seems to work fine. The trick is to implement internal counter for
> received responses from the remote device + timeout that is reset on each
> successful response. Any thoughts on this?
IMO delay the return of DiscoverCharacteristics method is the most
simple solution.
I am changing the Generic Attribute to handle on demand connections:
request connection when there is at least one watcher registered or a
write operation pending(SetProperty). After that we will investigate
how to fix the discovery race condition.
>
> Also, "Property Changed" on characteristic obj path/interface signal is
> mentioned in the attribute API doc, but I cannot seem to find
> implementation for the signal in the code. Am I missing something?
>
It is not implemented.
Regards,
Claudio.
<snip>
Hi Claudio,
> Hi Jaikumar
>
> On Wed, Jun 8, 2011 at 11:06 PM, Jaikumar Ganesh <[email protected]>
> wrote:
>> Hey Claudio:
>>
>> On Thu, Jun 9, 2011 at 2:44 AM, Claudio Takahasi
>> <[email protected]> wrote:
>>> Hi Jaikumar,
>>>
>>> On Wed, Jun 8, 2011 at 11:18 AM, Jaikumar Ganesh <[email protected]>
>>> wrote:
>>>> Hi Claudio:
>>>>
>>>> On Tue, Jun 7, 2011 at 11:41 PM, Claudio Takahasi
>>>> <[email protected]> wrote:
>>>>> Hi All,
>>>>>
>>>>> There is a potential race condition in the Generic Attribute API. I'd
>>>>> like to collect opinions to avoid re-work later.
>>>>>
>>>>> How to reproduce:
>>>>> - call CreateDevice() for a device which exports a GATT based
>>>>> service:
>>>>> CreateDevice will discover all primary services
>>>>> and the Generic API will register the object paths for all found
>>>>> services
>>>>> - Call DiscoverCharacteristics() of the primary service followed by
>>>>> Characteristic GetProperties(), expected result: read all
>>>>> characteristics properties outcome: partial characteristics
>>>>> properties
>>>>> may appear.
>>>>>
>>>>> This problem happens because DiscoverCharacteristics() doesn't wait
>>>>> for all characteristic VALUES, it returns after discovering all
>>>>> characteristic DECLARATIONS.
>>>>> Wait for characteristic values is a problem, attributes may require
>>>>> authentication or authorization.
>>>>>
>>>>> Suggestions are:
>>>>> 1. Report discovered characteristic values using PropertyChanged:
>>>>> Returning empty properties values (for GetProperties) if the
>>>>> discovery
>>>>> is in progress
>>>>>
>>>>> 2. Block GetProperties() if there is a discovery pending: returning
>>>>> after discovery finishes. Timeout can still happen!
>>>>>
>>>>> 3. Return Busy for GetProperties() if there is a characteristic
>>>>> discovery in progress
>>>>>
>>>>> Comments?
>>>>
>>>> I don't think we should read characteristic value till the application
>>>> asks for it.
>>> Are you suggesting to trigger the characteristic value and descriptors
>>> read when Characteristic GetProperties() is called and block it?
>>> Remember that GATT timeout is 30seconds and the default D-Bus message
>>> reply timeout is smaller.
>>>
>>>> DiscoverCharacteristics() should just return after getting the
>>>> DECLARATIONS.
>>> This is current behaviour.
>>
>> Sorry misread your email a bit.
>>
>> I assume you are referring to the Characteristic Watcher when you are
>> talking about the PropertyChanged signal,
>> since there is no explicit PropertyChanged signal.
>>
>> I don't think we should block GetProperties. But then if we are
>> returning null and depending on PropertyChanged signal when we have to
>> read the values, clients will need to wait for it after calling
>> GetProperties. We need to guarantee that this signal will be sent
>> (with a timeout, unless we want the clients to have the timeout at
>> their end) even if we are unable to get the value - because of some
>> kind of error.
>
> Signal with a timeout? I don't think that it is necessary.
> We can use persistent storage returning "old" values on GetProperties
> calls and use PropertyChanged to notify "new" values asynchronously.
> This approach should be enough.
Just noticed this thread.
I ran into a similar problem. Cannot we just delay the return of
DiscoverCharacteristics method call until after the characteristic
values/properties have been acquired? I tried out this solution and it
seems to work fine. The trick is to implement internal counter for
received responses from the remote device + timeout that is reset on each
successful response. Any thoughts on this?
Also, "Property Changed" on characteristic obj path/interface signal is
mentioned in the attribute API doc, but I cannot seem to find
implementation for the signal in the code. Am I missing something?
>
>>
>> In all other places we use PropertyChanged signal when the property
>> value actually changes, whereas here we are tying it with the reading
>> of the properties.
>>
>> How about providing separate calls for read and writing values and
>> decoupling it from the Properties ?
>
> I don't think it will help, authorization and GATT timeout is still a
> problem.
> We are trying to cover unusual scenarios, IMO change the API to cover
> these cases doesn't look correct.
>
> BR,
> Claudio.
>
>>
>> Thanks
>>
>>>
>>>> A client might not be interested in all the values and thus we should
>>>> read them only
>>>> when needed by the client. This will save both time and power and
>>>> solve this problem too.
>>>
>>> I don't think it is critical, we are not reading them on all
>>> connections/re-connections and the profiles available at the moment
>>> don't expose "complex" characteristics.
>>
>> I am talking about GATT clients written using the attribute DBUS apis.
>> Accessory vendors
>> want to get started on LE and not wait till all the profiles are
>> approved the SIG and developed
>> and they want to develop things based on the GATT clients APIs.
>>
>>>
>>> BR,
>>> Claudio
>>>
>>>>
>>>> Thanks
>>>> Jaikumar
>>>>>
>>>>> BR,
>>>>> Claudio
>>>
> --
> 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
>
Thanks,
Inga Stotland
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Hi Jaikumar
On Wed, Jun 8, 2011 at 11:06 PM, Jaikumar Ganesh <[email protected]> wrote:
> Hey Claudio:
>
> On Thu, Jun 9, 2011 at 2:44 AM, Claudio Takahasi
> <[email protected]> wrote:
>> Hi Jaikumar,
>>
>> On Wed, Jun 8, 2011 at 11:18 AM, Jaikumar Ganesh <[email protected]> wrote:
>>> Hi Claudio:
>>>
>>> On Tue, Jun 7, 2011 at 11:41 PM, Claudio Takahasi
>>> <[email protected]> wrote:
>>>> Hi All,
>>>>
>>>> There is a potential race condition in the Generic Attribute API. I'd
>>>> like to collect opinions to avoid re-work later.
>>>>
>>>> How to reproduce:
>>>> - call CreateDevice() for a device which exports a GATT based service:
>>>> CreateDevice will discover all primary services
>>>> and the Generic API will register the object paths for all found services
>>>> - Call DiscoverCharacteristics() of the primary service followed by
>>>> Characteristic GetProperties(), expected result: read all
>>>> characteristics properties outcome: partial characteristics properties
>>>> may appear.
>>>>
>>>> This problem happens because DiscoverCharacteristics() doesn't wait
>>>> for all characteristic VALUES, it returns after discovering all
>>>> characteristic DECLARATIONS.
>>>> Wait for characteristic values is a problem, attributes may require
>>>> authentication or authorization.
>>>>
>>>> Suggestions are:
>>>> 1. Report discovered characteristic values using PropertyChanged:
>>>> Returning empty properties values (for GetProperties) if the discovery
>>>> is in progress
>>>>
>>>> 2. Block GetProperties() if there is a discovery pending: returning
>>>> after discovery finishes. Timeout can still happen!
>>>>
>>>> 3. Return Busy for GetProperties() if there is a characteristic
>>>> discovery in progress
>>>>
>>>> Comments?
>>>
>>> I don't think we should read characteristic value till the application
>>> asks for it.
>> Are you suggesting to trigger the characteristic value and descriptors
>> read when Characteristic GetProperties() is called and block it?
>> Remember that GATT timeout is 30seconds and the default D-Bus message
>> reply timeout is smaller.
>>
>>> DiscoverCharacteristics() should just return after getting the DECLARATIONS.
>> This is current behaviour.
>
> Sorry misread your email a bit.
>
> I assume you are referring to the Characteristic Watcher when you are
> talking about the PropertyChanged signal,
> since there is no explicit PropertyChanged signal.
>
> I don't think we should block GetProperties. But then if we are
> returning null and depending on PropertyChanged signal when we have to
> read the values, clients will need to wait for it after calling
> GetProperties. We need to guarantee that this signal will be sent
> (with a timeout, unless we want the clients to have the timeout at
> their end) even if we are unable to get the value - because of some
> kind of error.
Signal with a timeout? I don't think that it is necessary.
We can use persistent storage returning "old" values on GetProperties
calls and use PropertyChanged to notify "new" values asynchronously.
This approach should be enough.
>
> In all other places we use PropertyChanged signal when the property
> value actually changes, whereas here we are tying it with the reading
> of the properties.
>
> How about providing separate calls for read and writing values and
> decoupling it from the Properties ?
I don't think it will help, authorization and GATT timeout is still a problem.
We are trying to cover unusual scenarios, IMO change the API to cover
these cases doesn't look correct.
BR,
Claudio.
>
> Thanks
>
>>
>>> A client might not be interested in all the values and thus we should
>>> read them only
>>> when needed by the client. This will save both time and power and
>>> solve this problem too.
>>
>> I don't think it is critical, we are not reading them on all
>> connections/re-connections and the profiles available at the moment
>> don't expose "complex" characteristics.
>
> I am talking about GATT clients written using the attribute DBUS apis.
> Accessory vendors
> want to get started on LE and not wait till all the profiles are
> approved the SIG and developed
> and they want to develop things based on the GATT clients APIs.
>
>>
>> BR,
>> Claudio
>>
>>>
>>> Thanks
>>> Jaikumar
>>>>
>>>> BR,
>>>> Claudio
>>
Hey Claudio:
On Thu, Jun 9, 2011 at 2:44 AM, Claudio Takahasi
<[email protected]> wrote:
> Hi Jaikumar,
>
> On Wed, Jun 8, 2011 at 11:18 AM, Jaikumar Ganesh <[email protected]> wrote:
>> Hi Claudio:
>>
>> On Tue, Jun 7, 2011 at 11:41 PM, Claudio Takahasi
>> <[email protected]> wrote:
>>> Hi All,
>>>
>>> There is a potential race condition in the Generic Attribute API. I'd
>>> like to collect opinions to avoid re-work later.
>>>
>>> How to reproduce:
>>> - call CreateDevice() for a device which exports a GATT based service:
>>> CreateDevice will discover all primary services
>>> ?and the Generic API will register the object paths for all found services
>>> - Call DiscoverCharacteristics() of the primary service followed by
>>> Characteristic GetProperties(), expected result: read all
>>> characteristics properties outcome: partial characteristics properties
>>> may appear.
>>>
>>> This problem happens because DiscoverCharacteristics() doesn't wait
>>> for all characteristic VALUES, it returns after discovering all
>>> characteristic DECLARATIONS.
>>> Wait for characteristic values is a problem, attributes may require
>>> authentication or authorization.
>>>
>>> Suggestions are:
>>> 1. Report discovered characteristic values using PropertyChanged:
>>> Returning empty properties values (for GetProperties) if the discovery
>>> is in progress
>>>
>>> 2. Block GetProperties() if there is a discovery pending: returning
>>> after discovery finishes. Timeout can still happen!
>>>
>>> 3. Return Busy for GetProperties() if there is a characteristic
>>> discovery in progress
>>>
>>> Comments?
>>
>> I don't think we should read characteristic value till the application
>> asks for it.
> Are you suggesting to trigger the characteristic value and descriptors
> read when Characteristic GetProperties() is called and block it?
> Remember that GATT timeout is 30seconds and the default D-Bus message
> reply timeout is smaller.
>
>> DiscoverCharacteristics() should just return after getting the DECLARATIONS.
> This is current behaviour.
Sorry misread your email a bit.
I assume you are referring to the Characteristic Watcher when you are
talking about the PropertyChanged signal,
since there is no explicit PropertyChanged signal.
I don't think we should block GetProperties. But then if we are
returning null and depending on PropertyChanged signal when we have to
read the values, clients will need to wait for it after calling
GetProperties. We need to guarantee that this signal will be sent
(with a timeout, unless we want the clients to have the timeout at
their end) even if we are unable to get the value - because of some
kind of error.
In all other places we use PropertyChanged signal when the property
value actually changes, whereas here we are tying it with the reading
of the properties.
How about providing separate calls for read and writing values and
decoupling it from the Properties ?
Thanks
>
>> A client might not be interested in all the values and thus we should
>> read them only
>> when needed by the client. This will save both time and power and
>> solve this problem too.
>
> I don't think it is critical, we are not reading them on all
> connections/re-connections and the profiles available at the moment
> don't expose "complex" characteristics.
I am talking about GATT clients written using the attribute DBUS apis.
Accessory vendors
want to get started on LE and not wait till all the profiles are
approved the SIG and developed
and they want to develop things based on the GATT clients APIs.
>
> BR,
> Claudio
>
>>
>> Thanks
>> Jaikumar
>>>
>>> BR,
>>> Claudio
>