Subject: [BUG] LEAdvertisingManager suffers from ObjectManager hierarchy

Hi,

I managed to get up my GATT Server using my patched BlueZ (I'm working on getting all clearances allowing me to contribute the patch to BlueZ, but this may take some weeks).
Meanwhile I added the advertisement part to my code and found the LEAdvertisementManager1 to suffer from a similar problem as the GattManager:
The DBUS ObjectManager is required to be in the same path as the managed objects. Luiz fixed a similar issue for the GattManager with his Patch from January, 5th by introducing the Application API.
I'll again try to identify a solution for patching my working copy of BlueZ, but it may take a while until I am able to share it.

Re contributing:
Are the guidelines within the repository up to date: "HACKING" "doc/coding-style.txt" "doc/maintainer-guidelines"?
I'm a bit confused about Marcels recent remark re signed-off-by.

As I do not have any experiences in contributing to open source projects:
Other hints/specialties I should be aware of to prepare a clean patch are welcome.
Especially, the "how/where to commit/send?" is not really clear to me.

Cheers,
Markus


Subject: AW: [BUG] LEAdvertisingManager suffers from ObjectManager hierarchy

SGkgTHVpeiwNCg0KPiBWb246IEx1aXogQXVndXN0byB2b24gRGVudHogW21haWx0bzpsdWl6LmRl
bnR6QGdtYWlsLmNvbV0NCj4gDQo+IEkgd2FzIGFib3V0IHRvIGNvbW1pdCB0aGlzIGJ1dCB0aGVu
IEkgcmVhbGl6ZSB0aGUgY29kZSB1bmRlciB0ZXN0L2V4YW1wbGUtDQo+IGFkdmVydGlzaW5nIGRv
ZXMgbm90IGFjdHVhbGx5IHVzZXMgT2JqZWN0TWFuYWdlciBhbmQgc3RpbGwgd29ya3MsIHNvIGV2
ZW4NCj4gdGhvdWdoIEdldE1hbmFnZWRPYmplY3RzIGRvZXMgcmV0dXJuIGFuIGVycm9yIHRoZSBj
b2RlIHNlZW1zIHRvIGNvcGUNCj4gd2l0aCBpdCBqdXN0IGZpbmUgc28gSW0gbm90IHN1cmUgd2hh
dCBleGFjdGx5IGlzIHlvdXIgYnVnPw0KDQpJIHJldmVydGVkIGFsbCBjaGFuZ2VzIHRvIGFkdmVy
dGlzaW5nIGNvZGUgdG8gdmVyaWZ5IGFuZCBhZ3JlZTogYXMgeW91IGNsYWltZWQgLS0+IGl0IHdv
cmtzLiANCkknbSByZWFsbHkgbm90IGFibGUgdG8gcmVwcm9kdWNlIHdoYXQgd2VudCB3cm9uZyBh
bnltb3JlLiBBZnRlciBwbGF5aW5nIGEgYml0IHdpdGggdGhlIGNoYW5nZXMgSSByZWNlbnRseSBt
YWRlLCBteSBndWVzcyBpcyB0aGUgZXJyb3Igd2FzIGR1ZSB0byBzb21lIG90aGVyIGJ1ZyBpbiBt
eSBjb2RlLg0KV2hlbiByZXBvcnRpbmcgYXMgYSBidWcgSSBvYnNlcnZlZCAiZmFpbGVkIHRvIHBh
cnNlIGFkdmVydGlzaW5nIiBlcnJvcnMgb24gdGhlIGRidXMgYW5kIHBhcnNpbmcgcHJvYmxlbXMg
Zm9yICJUeXBlIiB3aXRoaW4gYmx1ZXRvb3RoZCdzIGRlYnVnIG91dHB1dCB3aGVuIHJlZ2lzdGVy
aW5nIGFkdmVydGlzaW5nIHVzaW5nIHRoZSBvYmplY3RtYW5hZ2VyJ3MgcGF0aC4NCk9uIHRoZSBv
dGhlciBoYW5kIHdoZW4gYWR2ZXJ0aXNpbmcgdXNpbmcgdGhlIExFQWR2ZXJ0aXNlbWVudDEgcGF0
aCBJIGdvdCBhbm90aGVyIGJ1bmNoIG9mIGVycm9ycy4gVGhvc2UgYm9pbGVkIGRvd24gdG8gc29t
ZXRoaW5nICJubyBjb25uZWN0aW9uIi1pc2ggDQpmcm9tIG15IGRpZmZlcmVudCBkYnVzLXByb3hp
ZXMgKGluY2x1ZGluZyB0aGUgd29ya2luZyBHQVRUTWFuYWdlcjEpIHRoYXQgb2NjdXJyZWQgb25s
eSB3aGVuIEkgbWFkZSB0aGUgY2FsbCB0byBSZWdpc3RlckFkdmVydGlzZW1lbnQuIA0KDQpNeSBn
dWVzcyBpcyBzb21lIGZsYXdlZCBkYXRhL2NvZGUgd2l0aGluIG15IHRlc3QtYWR2ZXJ0aXNlbWVu
dCBhbmQgSSBtb3N0IGxpa2VseSBjbGVhbmVkIGl0IHNvbWV3aGVyZSBpbiB0aGUgcHJvY2VzcyBv
ZiBwYXRjaGluZyB0aGUgImV2aWwiIHBhcnQgb2YgdGhlIGFkdmVydGlzaW5nLmMgDQpSZXNwb25z
aWJsZSBmb3IgdGhlIG9ic2VydmVkIGRidXMgZXJyb3JzLiBBcyBteSBwcm9ibGVtcyB3ZXJlIGdv
bmUgYWZ0ZXIgcGF0Y2hpbmcgdGhlIGJsdWVaIHBhcnQsIEkgb2J2aW91c2x5IGdvdCBvbiB0aGUg
d3JvbmcgdHJhY2suDQpJJ20gc29ycnkgZm9yIHRoZSBjb25mdXNpb24uIA0KDQpJIHRoaW5rIHlv
dSBzaG91bGQgY29tbWl0IHRoZSBwYXRjaCBhbnl3YXksIGV2ZW4gaWYgaXQncyBub3cgImp1c3Qi
IGEgbWlub3IgaW1wcm92ZW1lbnQgdG93YXJkcyBjbGVhbmVyIGNvZGUgaW5zdGVhZCBvZiBhIGJ1
Z2ZpeC4gDQoNClRoYW5rcywNCk1hcmt1cw0KDQo=

2016-01-25 11:54:12

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [BUG] LEAdvertisingManager suffers from ObjectManager hierarchy

Hi Markus,

On Fri, Jan 22, 2016 at 5:43 PM, Kasper Markus (ETAS-PSC/ECE1)
<[email protected]> wrote:
> Hi Luiz,
>
>
>> -----Ursprüngliche Nachricht-----
>> Von: Luiz Augusto von Dentz [mailto:[email protected]]
>> Hi Markus,
>>
>> On Thu, Jan 21, 2016 at 5:53 PM, Kasper Markus (ETAS-PSC/ECE1)
>> <[email protected]> wrote:
>> Actually LEAdvertisementManager1.RegisterAdvertisement does no actually
>> require ObjectManager, so the following shall probably work:
>>
>> iff --git a/src/advertising.c b/src/advertising.c index 59c8c3d..4b87d42 100644
>> --- a/src/advertising.c
>> +++ b/src/advertising.c
>> @@ -591,7 +591,7 @@ static struct advertisement
>> *advertisement_create(DBusConnection *conn,
>> return NULL;
>>
>> ad = new0(struct advertisement, 1);
>> - ad->client = g_dbus_client_new_full(conn, sender, path, path);
>> + ad->client = g_dbus_client_new_full(conn, sender, path, NULL);
>> if (!ad->client)
>> goto fail;
> I tried this solution --> works like a charm. Previously I fixed it by adapting the api to take path and rootpath (= obj. Manager path) and then added rootpath as the fourth parameter to the call above.
> But your fix is obviously much simpler. Will you commit it? You'd most likely be much faster than I am.

I was about to commit this but then I realize the code under
test/example-advertising does not actually uses ObjectManager and
still works, so even though GetManagedObjects does return an error the
code seems to cope with it just fine so Im not sure what exactly is
your bug?

>
>> > Re contributing:
> Thanks for clearing this up. The part in "HACKING" didn't mention both (user & kernel) and I missed the cleaner statement within the maintainers guidelines.
>> > Especially, the "how/where to commit/send?" is not really clear to me.
> What I read is: send patches formatted as text to this list (i.e. output of diff -Nurp).

The easiest method is to use git format-patch followed by git send-email.

--
Luiz Augusto von Dentz

Subject: AW: [BUG] LEAdvertisingManager suffers from ObjectManager hierarchy

SGkgTHVpeiwNCg0KDQo+IC0tLS0tVXJzcHLDvG5nbGljaGUgTmFjaHJpY2h0LS0tLS0NCj4gVm9u
OiBMdWl6IEF1Z3VzdG8gdm9uIERlbnR6IFttYWlsdG86bHVpei5kZW50ekBnbWFpbC5jb21dDQo+
IEhpIE1hcmt1cywNCj4gDQo+IE9uIFRodSwgSmFuIDIxLCAyMDE2IGF0IDU6NTMgUE0sIEthc3Bl
ciBNYXJrdXMgKEVUQVMtUFNDL0VDRTEpDQo+IDxNYXJrdXMuS2FzcGVyQGVzY3J5cHQuY29tPiB3
cm90ZToNCj4gQWN0dWFsbHkgTEVBZHZlcnRpc2VtZW50TWFuYWdlcjEuUmVnaXN0ZXJBZHZlcnRp
c2VtZW50IGRvZXMgbm8gYWN0dWFsbHkNCj4gcmVxdWlyZSBPYmplY3RNYW5hZ2VyLCBzbyB0aGUg
Zm9sbG93aW5nIHNoYWxsIHByb2JhYmx5IHdvcms6DQo+IA0KPiBpZmYgLS1naXQgYS9zcmMvYWR2
ZXJ0aXNpbmcuYyBiL3NyYy9hZHZlcnRpc2luZy5jIGluZGV4IDU5YzhjM2QuLjRiODdkNDIgMTAw
NjQ0DQo+IC0tLSBhL3NyYy9hZHZlcnRpc2luZy5jDQo+ICsrKyBiL3NyYy9hZHZlcnRpc2luZy5j
DQo+IEBAIC01OTEsNyArNTkxLDcgQEAgc3RhdGljIHN0cnVjdCBhZHZlcnRpc2VtZW50DQo+ICph
ZHZlcnRpc2VtZW50X2NyZWF0ZShEQnVzQ29ubmVjdGlvbiAqY29ubiwNCj4gICAgICAgICAgICAg
ICAgIHJldHVybiBOVUxMOw0KPiANCj4gICAgICAgICBhZCA9IG5ldzAoc3RydWN0IGFkdmVydGlz
ZW1lbnQsIDEpOw0KPiAtICAgICAgIGFkLT5jbGllbnQgPSBnX2RidXNfY2xpZW50X25ld19mdWxs
KGNvbm4sIHNlbmRlciwgcGF0aCwgcGF0aCk7DQo+ICsgICAgICAgYWQtPmNsaWVudCA9IGdfZGJ1
c19jbGllbnRfbmV3X2Z1bGwoY29ubiwgc2VuZGVyLCBwYXRoLCBOVUxMKTsNCj4gICAgICAgICBp
ZiAoIWFkLT5jbGllbnQpDQo+ICAgICAgICAgICAgICAgICBnb3RvIGZhaWw7DQpJIHRyaWVkIHRo
aXMgc29sdXRpb24gLS0+IHdvcmtzIGxpa2UgYSBjaGFybS4gUHJldmlvdXNseSBJIGZpeGVkIGl0
IGJ5IGFkYXB0aW5nIHRoZSBhcGkgdG8gdGFrZSBwYXRoIGFuZCByb290cGF0aCAoPSBvYmouIE1h
bmFnZXIgcGF0aCkgYW5kIHRoZW4gYWRkZWQgcm9vdHBhdGggYXMgdGhlIGZvdXJ0aCBwYXJhbWV0
ZXIgdG8gdGhlIGNhbGwgYWJvdmUuDQpCdXQgeW91ciBmaXggaXMgb2J2aW91c2x5IG11Y2ggc2lt
cGxlci4gV2lsbCB5b3UgY29tbWl0IGl0PyBZb3UnZCBtb3N0IGxpa2VseSBiZSBtdWNoIGZhc3Rl
ciB0aGFuIEkgYW0uIA0KDQo+ID4gUmUgY29udHJpYnV0aW5nOg0KVGhhbmtzIGZvciBjbGVhcmlu
ZyB0aGlzIHVwLiBUaGUgcGFydCBpbiAiSEFDS0lORyIgZGlkbid0IG1lbnRpb24gYm90aCAodXNl
ciAmIGtlcm5lbCkgYW5kIEkgbWlzc2VkIHRoZSBjbGVhbmVyIHN0YXRlbWVudCB3aXRoaW4gdGhl
IG1haW50YWluZXJzIGd1aWRlbGluZXMuDQo+ID4gRXNwZWNpYWxseSwgdGhlICJob3cvd2hlcmUg
dG8gY29tbWl0L3NlbmQ/IiBpcyBub3QgcmVhbGx5IGNsZWFyIHRvIG1lLg0KV2hhdCBJIHJlYWQg
aXM6IHNlbmQgcGF0Y2hlcyBmb3JtYXR0ZWQgYXMgdGV4dCB0byB0aGlzIGxpc3QgKGkuZS4gb3V0
cHV0IG9mIGRpZmYgLU51cnApLg0KDQpUaGFua3MgYWdhaW4sDQpNYXJrdXMNCg0KDQo=

2016-01-22 13:33:34

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [BUG] LEAdvertisingManager suffers from ObjectManager hierarchy

Hi Markus,

On Thu, Jan 21, 2016 at 5:53 PM, Kasper Markus (ETAS-PSC/ECE1)
<[email protected]> wrote:
> Hi,
>
> I managed to get up my GATT Server using my patched BlueZ (I'm working on getting all clearances allowing me to contribute the patch to BlueZ, but this may take some weeks).
> Meanwhile I added the advertisement part to my code and found the LEAdvertisementManager1 to suffer from a similar problem as the GattManager:
> The DBUS ObjectManager is required to be in the same path as the managed objects. Luiz fixed a similar issue for the GattManager with his Patch from January, 5th by introducing the Application API.
> I'll again try to identify a solution for patching my working copy of BlueZ, but it may take a while until I am able to share it.

Actually LEAdvertisementManager1.RegisterAdvertisement does no
actually require ObjectManager, so the following shall probably work:

iff --git a/src/advertising.c b/src/advertising.c
index 59c8c3d..4b87d42 100644
--- a/src/advertising.c
+++ b/src/advertising.c
@@ -591,7 +591,7 @@ static struct advertisement
*advertisement_create(DBusConnection *conn,
return NULL;

ad = new0(struct advertisement, 1);
- ad->client = g_dbus_client_new_full(conn, sender, path, path);
+ ad->client = g_dbus_client_new_full(conn, sender, path, NULL);
if (!ad->client)
goto fail;

> Re contributing:
> Are the guidelines within the repository up to date: "HACKING" "doc/coding-style.txt" "doc/maintainer-guidelines"?
> I'm a bit confused about Marcels recent remark re signed-off-by.

This refer to userspace:

'Commit messages for bluez.git shall not contain Signed-off-by
signatures. They are not used in userspace and with that it is the
maintainers job to ensure they do not get committed to the repository.'

And this to the kernel only:

'For bluetooth.git and bluetooth-next.git The Signed-off-by process is
used and the signatures are required.'

Perhaps it is a good idea to remove any reference to the kernel since
that has it already documented in the kernel itself, this should
probably remove any confusion.

> As I do not have any experiences in contributing to open source projects:
> Other hints/specialties I should be aware of to prepare a clean patch are welcome.
> Especially, the "how/where to commit/send?" is not really clear to me.

It is mostly documented, and don't take any comments from the review
process as a personal offense, it is all for making the code as clean
possible.

--
Luiz Augusto von Dentz