subject:"D21584\: Add LE Advertising and GATT APIs"

D21584: Add LE Advertising and GATT APIs

2019-06-07 Thread David Rosca

drosca added a comment.


  In D21584#475914 , @ngraham wrote:
  
  > What is the net effect of this patch? What does it improve?
  
  
  It adds initial support for Bluetooth Low Energy, it's not used anywhere in 
Plasma.

REPOSITORY
  R269 BluezQt

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

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

D21584: Add LE Advertising and GATT APIs

2019-06-07 Thread Nathaniel Graham

ngraham added a comment.


  What is the net effect of this patch? What does it improve?

REPOSITORY
  R269 BluezQt

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

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

D21584: Add LE Advertising and GATT APIs

2019-06-07 Thread Manuel Weichselbaumer

mweichselbaumer closed this revision.

REPOSITORY
  R269 BluezQt

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

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

D21584: Add LE Advertising and GATT APIs

2019-06-07 Thread Manuel Weichselbaumer

mweichselbaumer updated this revision to Diff 59349.
mweichselbaumer added a comment.


  Fixed according to comments

REPOSITORY
  R269 BluezQt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21584?vs=59346=59349

BRANCH
  ble_gatt

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/fakebluez/CMakeLists.txt
  autotests/fakebluez/adapterinterface.cpp
  autotests/fakebluez/adapterinterface.h
  autotests/fakebluez/devicemanager.cpp
  autotests/fakebluez/devicemanager.h
  autotests/fakebluez/gattmanagerinterface.cpp
  autotests/fakebluez/gattmanagerinterface.h
  autotests/fakebluez/leadvertisingmanagerinterface.cpp
  autotests/fakebluez/leadvertisingmanagerinterface.h
  autotests/gattmanagertest.cpp
  autotests/gattmanagertest.h
  autotests/leadvertisingmanagertest.cpp
  autotests/leadvertisingmanagertest.h
  src/CMakeLists.txt
  src/adapter.cpp
  src/adapter.h
  src/adapter_p.cpp
  src/adapter_p.h
  src/gattapplication.cpp
  src/gattapplication.h
  src/gattapplication_p.cpp
  src/gattapplication_p.h
  src/gattcharacteristic.cpp
  src/gattcharacteristic.h
  src/gattcharacteristic_p.cpp
  src/gattcharacteristic_p.h
  src/gattcharacteristicadaptor.cpp
  src/gattcharacteristicadaptor.h
  src/gattmanager.cpp
  src/gattmanager.h
  src/gattmanager_p.cpp
  src/gattmanager_p.h
  src/gattservice.cpp
  src/gattservice.h
  src/gattservice_p.cpp
  src/gattservice_p.h
  src/gattserviceadaptor.cpp
  src/gattserviceadaptor.h
  src/interfaces/org.bluez.GattCharacteristic1.xml
  src/interfaces/org.bluez.GattManager1.xml
  src/interfaces/org.bluez.LEAdvertisement1.xml
  src/interfaces/org.bluez.LEAdvertisingManager1.xml
  src/leadvertisement.cpp
  src/leadvertisement.h
  src/leadvertisement_p.cpp
  src/leadvertisement_p.h
  src/leadvertisementadaptor.cpp
  src/leadvertisementadaptor.h
  src/leadvertisingmanager.cpp
  src/leadvertisingmanager.h
  src/leadvertisingmanager_p.h
  src/mediaendpoint.cpp
  src/objectmanageradaptor.cpp
  src/objectmanageradaptor.h
  src/pendingcall.cpp
  src/pendingcall.h
  src/types.h
  src/utils.cpp
  src/utils.h
  tests/CMakeLists.txt
  tests/leserver.cpp
  tests/leserver.h

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

D21584: Add LE Advertising and GATT APIs

2019-06-07 Thread David Rosca

drosca accepted this revision.
drosca added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> gattapplication.h:85
> + */
> +virtual QDBusObjectPath objectPath() const;
> +

This should be protected

> gattapplication.h:96
> + */
> +DBusManagerStruct getManagedObjects() const;
> +

This doesn't need doc anymore.

> gattcharacteristicadaptor.cpp:62
> +m_gattCharacteristic->writeValue(value);
> +}
> +void GattCharacteristicAdaptor::StartNotify()

newline

> gattservice.h:50
> + */
> +GattService(const QString , bool isPrimary, GattApplication 
> *parent);
> +

explicit

REPOSITORY
  R269 BluezQt

BRANCH
  ble_gatt

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

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

D21584: Add LE Advertising and GATT APIs

2019-06-07 Thread Manuel Weichselbaumer

mweichselbaumer updated this revision to Diff 59346.
mweichselbaumer added a comment.


  Fixed according to comments

REPOSITORY
  R269 BluezQt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21584?vs=59277=59346

BRANCH
  ble_gatt

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/fakebluez/CMakeLists.txt
  autotests/fakebluez/adapterinterface.cpp
  autotests/fakebluez/adapterinterface.h
  autotests/fakebluez/devicemanager.cpp
  autotests/fakebluez/devicemanager.h
  autotests/fakebluez/gattmanagerinterface.cpp
  autotests/fakebluez/gattmanagerinterface.h
  autotests/fakebluez/leadvertisingmanagerinterface.cpp
  autotests/fakebluez/leadvertisingmanagerinterface.h
  autotests/gattmanagertest.cpp
  autotests/gattmanagertest.h
  autotests/leadvertisingmanagertest.cpp
  autotests/leadvertisingmanagertest.h
  src/CMakeLists.txt
  src/adapter.cpp
  src/adapter.h
  src/adapter_p.cpp
  src/adapter_p.h
  src/gattapplication.cpp
  src/gattapplication.h
  src/gattapplication_p.cpp
  src/gattapplication_p.h
  src/gattcharacteristic.cpp
  src/gattcharacteristic.h
  src/gattcharacteristic_p.cpp
  src/gattcharacteristic_p.h
  src/gattcharacteristicadaptor.cpp
  src/gattcharacteristicadaptor.h
  src/gattmanager.cpp
  src/gattmanager.h
  src/gattmanager_p.cpp
  src/gattmanager_p.h
  src/gattservice.cpp
  src/gattservice.h
  src/gattservice_p.cpp
  src/gattservice_p.h
  src/gattserviceadaptor.cpp
  src/gattserviceadaptor.h
  src/interfaces/org.bluez.GattCharacteristic1.xml
  src/interfaces/org.bluez.GattManager1.xml
  src/interfaces/org.bluez.LEAdvertisement1.xml
  src/interfaces/org.bluez.LEAdvertisingManager1.xml
  src/leadvertisement.cpp
  src/leadvertisement.h
  src/leadvertisement_p.cpp
  src/leadvertisement_p.h
  src/leadvertisementadaptor.cpp
  src/leadvertisementadaptor.h
  src/leadvertisingmanager.cpp
  src/leadvertisingmanager.h
  src/leadvertisingmanager_p.h
  src/mediaendpoint.cpp
  src/objectmanageradaptor.cpp
  src/objectmanageradaptor.h
  src/pendingcall.cpp
  src/pendingcall.h
  src/types.h
  src/utils.cpp
  src/utils.h
  tests/CMakeLists.txt
  tests/leserver.cpp
  tests/leserver.h

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

D21584: Add LE Advertising and GATT APIs

2019-06-07 Thread Manuel Weichselbaumer

mweichselbaumer marked an inline comment as done.
mweichselbaumer added inline comments.

INLINE COMMENTS

> drosca wrote in objectmanager.h:47
> I don't really like this class at all. This is implementation detail, and is 
> of no use for the users of the library, so it shouldn't be exported. On top 
> of it, there is actually no code, it is just interface class.
> 
> Another issue here, although minor, is that the bluezqt_dbustypes.h is not 
> installed, so you can't use it. Yes, you could install it, but we don't need 
> it.
> 
> As I can see, the only reason for this class is so you can pass it to 
> ObjectManagerAdaptor, but there are other ways to achieve the same thing 
> without adding `ObjectManager` and deriving from it in GattApplication. If we 
> really need it in generic way, we can do it later.
> 
> Something like this would work:
> 
>   ObjectManagerAdaptor(QObject *parent)
>   {
>   GattApplication *app = qobject_cast(parent);
>   }
>   
>   DBusManagerStruct GetManagedObjects()
>   {
>   return app->d->getManagedObjects();
>   }
> 
> Or for now just make it work only with GattApplication, as this is the only 
> one we have now.
> 
> Yes, it will not work in the autotest, since you won't have the hook anymore, 
> but you can just remove that test.

I see your point and agree, that ObjectManager is solely used in 
GattApplication.
Just letting you know, that we would need an ObjectManager for the mesh-api as 
well.

Shall we then add constructors for appropriate types to ObjectManagerAdaptor 
(instead of exporting and inheriting from ObjectManager)?

REPOSITORY
  R269 BluezQt

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

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

D21584: Add LE Advertising and GATT APIs

2019-06-07 Thread David Rosca

drosca requested changes to this revision.
drosca added a comment.
This revision now requires changes to proceed.


  After this, it should be good to go.

INLINE COMMENTS

> drosca wrote in gattapplication.h:47
> Maybe it would be best to add constructor that uses default object path. Also 
> please add better documentation as to what object path prefix is.
> 
> `explicit GattApplication(QObject *parent = nullptr);`

Here I meant adding new constructor, because as you did it now, if you want to 
set parent from constructor you would need to copy the default value for the 
objectPathPrefix.

  GattApplication(QObject *parent = nullptr);
  GattApplication(const QString , QObject *parent = nullptr);

> mweichselbaumer wrote in objectmanager.h:47
> 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).

I don't really like this class at all. This is implementation detail, and is of 
no use for the users of the library, so it shouldn't be exported. On top of it, 
there is actually no code, it is just interface class.

Another issue here, although minor, is that the bluezqt_dbustypes.h is not 
installed, so you can't use it. Yes, you could install it, but we don't need it.

As I can see, the only reason for this class is so you can pass it to 
ObjectManagerAdaptor, but there are other ways to achieve the same thing 
without adding `ObjectManager` and deriving from it in GattApplication. If we 
really need it in generic way, we can do it later.

Something like this would work:

  ObjectManagerAdaptor(QObject *parent)
  {
  GattApplication *app = qobject_cast(parent);
  }
  
  DBusManagerStruct GetManagedObjects()
  {
  return app->d->getManagedObjects();
  }

Or for now just make it work only with GattApplication, as this is the only one 
we have now.

Yes, it will not work in the autotest, since you won't have the hook anymore, 
but you can just remove that test.

REPOSITORY
  R269 BluezQt

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

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

D21584: Add LE Advertising and GATT APIs

2019-06-06 Thread Manuel Weichselbaumer

mweichselbaumer added inline comments.

INLINE COMMENTS

> drosca wrote in gattapplication.h:66
> Should this be made private (preferrably in GattApplicationPrivate)?

I override this in test class, so kept as protected.

REPOSITORY
  R269 BluezQt

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

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

D21584: Add LE Advertising and GATT APIs

2019-06-06 Thread Manuel Weichselbaumer

mweichselbaumer updated this revision to Diff 59277.
mweichselbaumer added a comment.


  Fixed according to comments

REPOSITORY
  R269 BluezQt

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21584?vs=59238=59277

BRANCH
  ble_gatt

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/fakebluez/CMakeLists.txt
  autotests/fakebluez/adapterinterface.cpp
  autotests/fakebluez/adapterinterface.h
  autotests/fakebluez/devicemanager.cpp
  autotests/fakebluez/devicemanager.h
  autotests/fakebluez/gattmanagerinterface.cpp
  autotests/fakebluez/gattmanagerinterface.h
  autotests/fakebluez/leadvertisingmanagerinterface.cpp
  autotests/fakebluez/leadvertisingmanagerinterface.h
  autotests/gattmanagertest.cpp
  autotests/gattmanagertest.h
  autotests/leadvertisingmanagertest.cpp
  autotests/leadvertisingmanagertest.h
  src/CMakeLists.txt
  src/adapter.cpp
  src/adapter.h
  src/adapter_p.cpp
  src/adapter_p.h
  src/gattapplication.cpp
  src/gattapplication.h
  src/gattapplication_p.cpp
  src/gattapplication_p.h
  src/gattcharacteristic.cpp
  src/gattcharacteristic.h
  src/gattcharacteristic_p.cpp
  src/gattcharacteristic_p.h
  src/gattcharacteristicadaptor.cpp
  src/gattcharacteristicadaptor.h
  src/gattmanager.cpp
  src/gattmanager.h
  src/gattmanager_p.cpp
  src/gattmanager_p.h
  src/gattservice.cpp
  src/gattservice.h
  src/gattservice_p.cpp
  src/gattservice_p.h
  src/gattserviceadaptor.cpp
  src/gattserviceadaptor.h
  src/interfaces/org.bluez.GattCharacteristic1.xml
  src/interfaces/org.bluez.GattManager1.xml
  src/interfaces/org.bluez.LEAdvertisement1.xml
  src/interfaces/org.bluez.LEAdvertisingManager1.xml
  src/leadvertisement.cpp
  src/leadvertisement.h
  src/leadvertisement_p.cpp
  src/leadvertisement_p.h
  src/leadvertisementadaptor.cpp
  src/leadvertisementadaptor.h
  src/leadvertisingmanager.cpp
  src/leadvertisingmanager.h
  src/leadvertisingmanager_p.h
  src/mediaendpoint.cpp
  src/objectmanager.cpp
  src/objectmanager.h
  src/objectmanageradaptor.cpp
  src/objectmanageradaptor.h
  src/pendingcall.cpp
  src/pendingcall.h
  src/types.h
  src/utils.cpp
  src/utils.h
  tests/CMakeLists.txt
  tests/leserver.cpp
  tests/leserver.h

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

D21584: Add LE Advertising and GATT APIs

2019-06-06 Thread David Rosca

drosca requested changes to this revision.
drosca added a comment.
This revision now requires changes to proceed.


  It seems you uploaded only diff with latest changes, you need to upload the 
entire diff against master.

INLINE COMMENTS

> gattapplication.h:47
>   */
> -explicit GattApplication(QObject *parent = nullptr);
> +explicit GattApplication(const QString , QObject 
> *parent = nullptr);
>  

Maybe it would be best to add constructor that uses default object path. Also 
please add better documentation as to what object path prefix is.

`explicit GattApplication(QObject *parent = nullptr);`

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

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

D21584: Add LE Advertising and GATT APIs

2019-06-05 Thread Manuel Weichselbaumer

mweichselbaumer updated this revision to Diff 59238.
mweichselbaumer added a comment.


  Fixed according to comments

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21584?vs=59237=59238

BRANCH
  ble_gatt

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

AFFECTED FILES
  autotests/fakebluez/gattmanagerinterface.cpp
  autotests/fakebluez/gattmanagerinterface.h
  autotests/gattmanagertest.cpp
  src/CMakeLists.txt
  src/gattapplication.cpp
  src/gattapplication.h
  src/gattapplication_p.cpp
  src/gattapplication_p.h
  src/gattcharacteristic.cpp
  src/gattcharacteristic.h
  src/gattcharacteristic_p.cpp
  src/gattcharacteristic_p.h
  src/gattcharacteristicadaptor.cpp
  src/gattcharacteristicadaptor.h
  src/gattmanager.cpp
  src/gattmanager.h
  src/gattmanager_p.cpp
  src/gattmanager_p.h
  src/gattservice.cpp
  src/gattservice.h
  src/gattservice_p.cpp
  src/gattservice_p.h
  src/gattserviceadaptor.cpp
  src/gattserviceadaptor.h
  src/objectmanageradaptor.cpp
  src/objectmanageradaptor.h
  tests/leserver.cpp

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

D21584: Add LE Advertising and GATT APIs

2019-06-05 Thread Manuel Weichselbaumer

mweichselbaumer updated this revision to Diff 59237.
mweichselbaumer added a comment.


  Fixed according to comments

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21584?vs=59236=59237

BRANCH
  ble_gatt

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

AFFECTED FILES
  autotests/fakebluez/gattmanagerinterface.cpp
  autotests/fakebluez/gattmanagerinterface.h
  autotests/gattmanagertest.cpp
  src/CMakeLists.txt
  src/gattapplication.cpp
  src/gattapplication.h
  src/gattapplication_p.cpp
  src/gattapplication_p.h
  src/gattcharacteristic.cpp
  src/gattcharacteristic.h
  src/gattcharacteristic_p.cpp
  src/gattcharacteristic_p.h
  src/gattcharacteristicadaptor.cpp
  src/gattcharacteristicadaptor.h
  src/gattmanager.cpp
  src/gattmanager.h
  src/gattmanager_p.cpp
  src/gattmanager_p.h
  src/gattservice.cpp
  src/gattservice.h
  src/gattservice_p.cpp
  src/gattservice_p.h
  src/gattserviceadaptor.cpp
  src/gattserviceadaptor.h
  src/objectmanageradaptor.cpp
  src/objectmanageradaptor.h
  tests/leserver.cpp

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

D21584: Add LE Advertising and GATT APIs

2019-06-05 Thread Manuel Weichselbaumer

mweichselbaumer updated this revision to Diff 59236.
mweichselbaumer marked an inline comment as done.
mweichselbaumer added a comment.


  Fixed according to comments

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21584?vs=59151=59236

BRANCH
  ble_gatt

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

AFFECTED FILES
  CMakeLists.txt
  autotests/fakebluez/gattmanagerinterface.cpp
  autotests/fakebluez/gattmanagerinterface.h
  autotests/gattmanagertest.cpp
  src/CMakeLists.txt
  src/gattapplication.cpp
  src/gattapplication.h
  src/gattapplication_p.cpp
  src/gattapplication_p.h
  src/gattcharacteristic.cpp
  src/gattcharacteristic.h
  src/gattcharacteristic_p.cpp
  src/gattcharacteristic_p.h
  src/gattcharacteristicadaptor.cpp
  src/gattcharacteristicadaptor.h
  src/gattmanager.cpp
  src/gattmanager.h
  src/gattmanager_p.cpp
  src/gattmanager_p.h
  src/gattservice.cpp
  src/gattservice.h
  src/gattservice_p.cpp
  src/gattservice_p.h
  src/gattserviceadaptor.cpp
  src/gattserviceadaptor.h
  src/objectmanageradaptor.cpp
  src/objectmanageradaptor.h
  tests/leserver.cpp

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

D21584: Add LE Advertising and GATT APIs

2019-06-05 Thread Manuel Weichselbaumer

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

D21584: Add LE Advertising and GATT APIs

2019-06-05 Thread David Rosca

drosca requested changes to this revision.
drosca added a comment.
This revision now requires changes to proceed.


  Looks really good!

INLINE COMMENTS

> gattapplication.cpp:38
> +
> +GattApplication::GattApplication(QObject *parent) :
> +ObjectManager(parent),

Coding style:

  GattApplication::GattApplication(QObject *parent)
  : ObjectManager(parent)
  , d(new GattApplicationPrivate())
  {
  }

> gattapplication.cpp:53
> +
> +auto serviceAdaptors = findChildren();
> +auto charcAdaptors = findChildren();

const

> gattapplication.cpp:64
> +
> +GattService* service = 
> qobject_cast(serviceAdaptor->parent());
> +if (service) {

`GattService *service`

> gattapplication.h:66
> +
> +DBusManagerStruct getManagedObjects() const override;
> +

Should this be made private (preferrably in GattApplicationPrivate)?

> gattapplication_p.cpp:31
> +static uint8_t appNumber = 0;
> +m_objectPath.setPath(QStringLiteral("/org/bluez/app") + 
> QString::number(appNumber++));
> +}

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/`.

> gattcharacteristic.cpp:50
> +
> +void GattCharacteristic::writeValue(const QByteArray& value)
> +{

`const QByteArray )`

> gattcharacteristic_p.cpp:33
> +{
> +static uint8_t charcNumber = 0;
> +m_objectPath.setPath(m_service->objectPath().path() + 
> QStringLiteral("/char") + QString::number(charcNumber++));

Any reason to use uint8?

> gattcharacteristic_p.h:39
> +QDBusObjectPath m_objectPath;
> +QByteArray  m_value;
> +GattCharacteristic::ReadCallback m_readCallback = nullptr;

No additional spaces please.

> gattcharacteristicadaptor.cpp:57
> +return m_gattCharacteristic->readValue();
> +}
> +void GattCharacteristicAdaptor::WriteValue(const QByteArray , const 
> QVariantMap &/*options*/)

newline

> gattcharacteristicadaptor.h:58
> +private:
> +GattCharacteristic* m_gattCharacteristic;
> +};

`GattCharacteristic *m_gattCharacteristic`

> gattmanager.cpp:59
> +
> +for (auto child : application->children()) {
> +GattService* service = qobject_cast(child);

const auto children =  application->children();
  for (const auto  : children()

Also you can use `findChildren` as above.

> gattmanager.h:103
> +private:
> +explicit GattManager(const QString , QObject *parent = nullptr);
> +

This should have d-ptr as it's exported class.

> gattservice.h:50
> + */
> +GattService(const QString , bool isPrimary, GattApplication 
> *parent);
> +

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?

> leadvertisement.h:52
> + */
> +explicit LEAdvertisement(const QStringList , QObject 
> *parent = nullptr);
> +

Same as above, add setters?

> objectmanager.h:47
> + */
> +class BLUEZQT_EXPORT ObjectManager : public QObject
> +{

Will this be used for more classes? Right now only GattApplication inherits it.

REPOSITORY
  R269 BluezQt

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

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

D21584: Add LE Advertising and GATT APIs

2019-06-04 Thread Manuel Weichselbaumer

mweichselbaumer added a reviewer: drosca.

REPOSITORY
  R269 BluezQt

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

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

D21584: Add LE Advertising and GATT APIs

2019-06-04 Thread Manuel Weichselbaumer

mweichselbaumer created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
mweichselbaumer requested review of this revision.

REPOSITORY
  R269 BluezQt

BRANCH
  ble_gatt

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

AFFECTED FILES
  autotests/CMakeLists.txt
  autotests/fakebluez/CMakeLists.txt
  autotests/fakebluez/adapterinterface.cpp
  autotests/fakebluez/adapterinterface.h
  autotests/fakebluez/devicemanager.cpp
  autotests/fakebluez/devicemanager.h
  autotests/fakebluez/gattmanagerinterface.cpp
  autotests/fakebluez/gattmanagerinterface.h
  autotests/fakebluez/leadvertisingmanagerinterface.cpp
  autotests/fakebluez/leadvertisingmanagerinterface.h
  autotests/gattmanagertest.cpp
  autotests/gattmanagertest.h
  autotests/leadvertisingmanagertest.cpp
  autotests/leadvertisingmanagertest.h
  src/CMakeLists.txt
  src/adapter.cpp
  src/adapter.h
  src/adapter_p.cpp
  src/adapter_p.h
  src/gattapplication.cpp
  src/gattapplication.h
  src/gattapplication_p.cpp
  src/gattapplication_p.h
  src/gattcharacteristic.cpp
  src/gattcharacteristic.h
  src/gattcharacteristic_p.cpp
  src/gattcharacteristic_p.h
  src/gattcharacteristicadaptor.cpp
  src/gattcharacteristicadaptor.h
  src/gattmanager.cpp
  src/gattmanager.h
  src/gattservice.cpp
  src/gattservice.h
  src/gattservice_p.cpp
  src/gattservice_p.h
  src/gattserviceadaptor.cpp
  src/gattserviceadaptor.h
  src/interfaces/org.bluez.GattCharacteristic1.xml
  src/interfaces/org.bluez.GattManager1.xml
  src/interfaces/org.bluez.LEAdvertisement1.xml
  src/interfaces/org.bluez.LEAdvertisingManager1.xml
  src/leadvertisement.cpp
  src/leadvertisement.h
  src/leadvertisement_p.cpp
  src/leadvertisement_p.h
  src/leadvertisementadaptor.cpp
  src/leadvertisementadaptor.h
  src/leadvertisingmanager.cpp
  src/leadvertisingmanager.h
  src/leadvertisingmanager_p.h
  src/mediaendpoint.cpp
  src/objectmanager.cpp
  src/objectmanager.h
  src/objectmanageradaptor.cpp
  src/objectmanageradaptor.h
  src/pendingcall.cpp
  src/pendingcall.h
  src/types.h
  src/utils.cpp
  src/utils.h
  tests/CMakeLists.txt
  tests/leserver.cpp
  tests/leserver.h

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

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

D21584: Add LE Advertising and GATT APIs

18 matches

Site Navigation

Mail list logo

Footer information