mweichselbaumer marked 11 inline comments as done.
mweichselbaumer added inline comments.

INLINE COMMENTS

> drosca wrote in gattapplication_p.cpp:31
> Shouldn't the caller be made responsible for choosing object path?
> If we force a path then we should use "our" namespace - `/org/kde/bluez-qt/`.

Ok, will add possibility for custom prefix.

> drosca wrote in gattcharacteristic_p.cpp:33
> Any reason to use uint8?

I tried to figure out the maximum number of charcs (per service). I do not know 
exact numbers, but uint8 would intrinsically add limits.

> drosca wrote in gattservice.h:50
> Probably would be better to add setters for uuid/primary (and note that it 
> can only be set during creation), as if we need in future more properties we 
> will need to add new constructors.
> Or make it virtual?

GATT is well defined and i do not expect any changes (regarding additional 
properties).
This also follows the RAII idiom and is less error prone.
Could be made virtual though, however i believe adding further constructors 
won't harm.
Or if further properties are coming, we add setters for them, since they are 
then expected not to be mandatory.

> drosca wrote in leadvertisement.h:52
> Same as above, add setters?

see above

> drosca wrote in objectmanager.h:47
> Will this be used for more classes? Right now only GattApplication inherits 
> it.

QDbusAdaptors can only realize one single DBUS interface.
This is the base class the org.freedesktop.DBus.ObjectManager adaptor is 
handling.
Sure, this could (and should) be reused if other DBUS/BlueZ objects shall 
realize this interface.
GattApplication is meant to realize the org.freedesktop.DBus.ObjectManager 
interface (see gatt-api.txt).

REPOSITORY
  R269 BluezQt

REVISION DETAIL
  https://phabricator.kde.org/D21584

To: mweichselbaumer, drosca
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

Reply via email to