Hi Arman,
>> We are working on GATT D-Bus API for Tizen LE GATT Client too. For your proposal, I totally agree your part of proposal such as add Primary property into org.bluez.GattService1, because all the hierarchy services cannot differentiate between primary and included service. And device property is also very useful because It can help user to trace the services belong to which device.
>> However , you add Write/Read/Notify method into GATT-API , I think it is good proposal but not essential. Because it is about implementation method problem, not the GATT DBus API problem.
>
> I have to disagree here. I think making the DBus.Properties interface
> work for this particular case doesn't make the API particularly
> simpler, in fact I think it adds unnecessary complexity and ambiguity
> to the semantics involved. It's not clear, for instance, if Get should
> always return the cached value or if it should issue a Read request to
> the remote end. You suggest making an initial read request and storing
> that value and updating it on notifications. What if the
> characteristic doesn't support notifications but it's value can
> change? What if the client is implementing a profile that warrants a
> read request to the remote end to get the freshest value on demand?
>
> For example, take the "Device Name" characteristic of the Generic
> Access service, which has to support reads, optionally supports
> writes, and doesn't support notifications/indications. The client can
> write its value but won't be notified of the change. Now, the daemon
> could remember that the value was written and update the property
> right upon success, but I think this is unnecessary magic.
>
> Same with control point characteristics, that support writes but no
> notifications or reads. The external application can call "Set" to
> write to the characteristic but does it make sense to send a
> PropertiesChanged signal for a value that we cannot read? Should the
> daemon cache the write value and return that on Get? Or should it
> return an empty value?
>
> The same goes for the PropertyChanged signal, which we will end up
> emitting after a successful read, notifications/indications, and
> portentially writes. I think it's cleaner to have a distinct signal
> that gets only sent on notifications.
>
> We end up with a read-write property that doesn't return a consistent
> value on Get and Set calls. We end up making the property interface
> work for semantics that it wasn't exactly built for.
>
> I want to have an API where the external application has a clear way
> of saying "send a remote read/write request". I'm not opposed to
> having a "Value" property but I'd much rather it represent a read-only
> property that only returns the most recently cached value.
>
>>>> + object Device [read-only, optional]
>>>> +
>>>> + Object path of the Bluetooth device the service
>>>> + belongs to. Only present on services from remote
>>>> + devices.
>>>
>>> I am not sure this is needed. What do you want to use it for?
>
> This is very useful to determine which device a service belongs to
> when an InterfacesAdded signal is sent for a service. We have an
> "Adapter" property for org.bluez.Device1, so why not a "Device"
> property for org.bluez.GattService1?
the back reference should only be added if someone really uses them.
>
>>
>>>> +
>>>> + array{object} Characteristics [read-only]
>>>> +
>>>> + Array of object paths representing the characteristics
>>>> + of this service. This property is set only when the
>>>> + characteristic discovery has been completed, however the
>>>> + characteristic objects will become available via
>>>> + ObjectManager as soon as they get discovered.
>>>> +
>>>
>>> I really wonder if this is a good idea. Why not delay announcing any service or characteristic via D-Bus as long as the service discovery is still running. I think it makes more sense to allow doing it one way and one way only.
>>
>
> I guess I should have said "This property is set only when the
> characteristic discovery has been completed AND an InterfacesAdded
> signal for all characteristics and descriptors have been sent to
> clients". In short, even if we wait until discovery is complete, we
> will create service, characteristic, and descriptor objects in some
> order but I want to have a way for a client to know that all objects
> associated with a service are available via D-Bus. I'm open to
> suggestions :)
So this is problem that InterfacesAdded is only reporting for a single object path. Maybe this will actually work out as you described it.
>
>>>
>>> I have objection to adding these two method. However the error codes should be a bit more descriptive on the level AuthenticationRequired or MissingAuthentication. Then again, I think this detail might be better hidden in the daemon.
>>
>> this should read "no objection". Seems I am swallowing words again ;)
>>
>> For example if you are not encrypted and the characteristic requires an encrypted link, we should upgrade the security level and try again. If no LTK is available, then org.bluez.Error.NoEncryptionKey or similar should be returned. I think it is important that we handle all these details in the daemon to make it transparent for the end user.
>>
>
> I think these should be handled by the daemon too. Then again we
> should report errors to the clients when we can. So yes, I agree.
Okay, then make the error really simple. Maybe even on a high-level like "not paired". Since the only action from the user here is really to start pairing with the device.
>
>
>>> +
>>> + void StartNotify()
>>> +
>>> + Starts a notification session from this characteristic
>>> + if it supports value notifications or indications.
>>> +
>>> + Possible Errors: org.bluez.Error.Failed
>>> + org.bluez.Error.InProgress
>>> + org.bluez.Error.NotSupported
>>> +
>>> + void StopNotify()
>>> +
>>> + This method will cancel any previous StartNotify
>>> + transaction. Note that notifications from a
>>> + characteristic are shared between sessions thus
>>> + calling StopNotify will release a single session.
>>> +
>>> + Possible Errors: org.bluez.Error.Failed
>>
>> Seems sensible as well to me. However I am little bit debating the naming of the method a bit. I would have to read the spec in detail again which is the best name. EnableNotification comes to mind as well.
>>
>
> Yeah, I came up with these names pretty quickly while I was
> time-pressed for the Chrome 37 release. I don't have a strong opinion
> here; maybe EnableNotifications & DisableNotifications? Or maybe
> something that contains the word "session" since these methods behave
> like StartDiscovery and StopDiscovery on org.bluez.Adapter1.
I leave it to others to comment here as well. It is an easy change in the end.
>
>
>>> +
>>> +Signals void ValueUpdated(array{bytes} value)
>>> +
>>> + This signal is launched when a characteristic handle
>>> + value notification or indication is received.
>>
>> Why is just sending a PropertyChanged not good enough here? I would consider the property itself the cached value. Extra signals are costing extra overhead.
>>
>
> I added this signal because I got rid of the property. Even if we keep
> the property, I'm still not convinced that using the PropertyChanged
> signal to mean both "cached value updated after read" and
> "notification/indication received" is the right idea. I sort of prefer
> having a signal that only gets called when there's a notification,
> though if we keep this property then I guess we're going to have to
> work with that.
My thinking is that it really does not matter how the value got updated. Either by a manual read or by a notification or even by a write. The importance is to get the new value notified to all other applications that are not involved. Doing it only one way seems to make sense. And the applications all already have to listen to the PropertiesChanged signal anyway. Less signals to listen on the the better.
>
>
>>> + boolean Notifying [read-only]
>>>
>>> - Value read from the remote Bluetooth device or from
>>> - the external application implementing GATT services.
>>> + True, if notifications or indications on this
>>> + characteristic are currently enabled.
>>
>> As said above, I would leave the Value here as well. Maybe make it optional in case it has not yet been read or does not allow reading at all. It would represent the cached value.
>>
>
> I'm fine with leaving it. I'm ok with changing it to a read-only &
> optional property that represents the cached value.
>
>> I am not fully at ease with the name Notifying yet. Need to think about it a bit or some good convincing.
>>
>
> "NotificationsEnabled" maybe? That would make sense if change the
> methods to EnableNotifications & DisableNotifications. I guess we all
> suck at coming up with names :P
We can keep the current names, but I might object later if I come up with something better.
>
>
>>> + array{object} Descriptors [read-only]
>>> +
>>> + Array of object paths representing the descriptors
>>> + of this service. This property is set only when the
>>> + descriptor discovery has been completed, however the
>>> + descriptor objects will become available via
>>> + ObjectManager as soon as they get discovered.
>>> +
>>
>> Why is this needed?
>>
>
> Same reason as the "Characteristics" property on GattService1 above.
> It's not essential but we should have it for consistency if we keep
> "Characteristics".
Okay. Fair enough.
>
>>> -
>>> - string Permissions [read-only]: To be defined
>>> -
>>> - Defines read/write authentication and authorization
>>> - requirements.
>>
>> Why is this removed now?
>
> To be honest, I don't think this property is that necessary. GATT/ATT
> don't really define a way to obtain these requirements in the
> characteristic declaration and leave them up to the upper layer
> profile and the only way to find out about these requirements is to
> issue a read/write request and see if there's an error. If we properly
> report errors to the client on calls to ReadValue and WriteValue, then
> this property wouldn't really be needed. Let me know if I'm missing
> something though.
We might actually update it from the property does not exist to we have figured this out since someone read the property and thus we are reporting it now. I might be wrong, but I think there was some part in GATT that provided these details.
Regards
Marcel
--
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
