Relaxes the requirement that object provide ObjectManager AND
GattService interfaces.
Maintains check that GattService and GattCharacteristics are
children of ObjectManager, but loses the check that
GattCharacteristics are children of GattService. Therefore,
meant as a temporary workaround.
---
src/gatt-database.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/src/gatt-database.c b/src/gatt-database.c
index 69a814d..e82165c 100644
--- a/src/gatt-database.c
+++ b/src/gatt-database.c
@@ -85,6 +85,7 @@ struct external_service {
bool failed;
char *owner;
char *path; /* Path to GattService1 */
+ char *manager_path; /* Path to ObjectManager */
DBusMessage *reg;
GDBusClient *client;
GDBusProxy *proxy;
@@ -372,6 +373,7 @@ static void service_free(void *data)
g_free(service->owner);
g_free(service->path);
+ g_free(service->manager_path);
free(service);
}
@@ -1042,7 +1044,7 @@ static bool match_service(const void *a, const void *b)
const struct external_service *service = a;
const struct svc_match_data *data = b;
- return g_strcmp0(service->path, data->path) == 0 &&
+ return g_strcmp0(service->manager_path, data->path) == 0 &&
g_strcmp0(service->owner, data->sender) == 0;
}
@@ -1321,7 +1323,7 @@ static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
iface = g_dbus_proxy_get_interface(proxy);
path = g_dbus_proxy_get_path(proxy);
- if (!g_str_has_prefix(path, service->path))
+ if (!g_str_has_prefix(path, service->manager_path))
return;
if (g_strcmp0(iface, GATT_SERVICE_IFACE) == 0) {
@@ -1332,12 +1334,20 @@ static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
* TODO: We may want to support adding included services in a
* single hierarchy.
*/
- if (g_strcmp0(path, service->path) != 0) {
+ if (service->path != NULL) {
error("Multiple services added within hierarchy");
service->failed = true;
return;
}
+ // Set service path now that we've found a service
+ service->path = g_strdup(path);
+ if (!service->path) {
+ error("Failed to allocate service path");
+ service->failed = true;
+ return;
+ }
+
/* Add 1 for the service declaration */
if (!incr_attr_count(service, 1)) {
error("Failed to increment attribute count");
@@ -2171,8 +2181,8 @@ static struct external_service *service_create(DBusConnection *conn,
if (!service->owner)
goto fail;
- service->path = g_strdup(path);
- if (!service->path)
+ service->manager_path = g_strdup(path);
+ if (!service->manager_path)
goto fail;
service->chrcs = queue_new();
--
2.4.6
Statement of Confidentiality
The contents of this e-mail message and any attachments are confidential and are intended solely for the addressee. The information may also be legally privileged. This transmission is sent in trust, and the sole purpose of delivery to the intended recipient. If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please immediately notify the sender by reply e-mail or at 508.683.2500 and delete this message and its attachments, if any.
On 8/2/15, 2:04 AM, "Luiz Augusto von Dentz" <[email protected]> wrote:
>Hi Adam,
>
>On Fri, Jul 31, 2015 at 8:42 PM, Adam Moore
><[email protected]> wrote:
>> Relaxes the requirement that object provide ObjectManager AND
>> GattService interfaces.
>>
>> Maintains check that GattService and GattCharacteristics are
>> children of ObjectManager, but loses the check that
>> GattCharacteristics are children of GattService. Therefore,
>> meant as a temporary workaround.
>> ---
>> src/gatt-database.c | 20 +++++++++++++++-----
>> 1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/gatt-database.c b/src/gatt-database.c
>> index 69a814d..e82165c 100644
>> --- a/src/gatt-database.c
>> +++ b/src/gatt-database.c
>> @@ -85,6 +85,7 @@ struct external_service {
>> bool failed;
>> char *owner;
>> char *path; /* Path to GattService1 */
>> + char *manager_path; /* Path to ObjectManager */
>> DBusMessage *reg;
>> GDBusClient *client;
>> GDBusProxy *proxy;
>> @@ -372,6 +373,7 @@ static void service_free(void *data)
>>
>> g_free(service->owner);
>> g_free(service->path);
>> + g_free(service->manager_path);
>>
>> free(service);
>> }
>> @@ -1042,7 +1044,7 @@ static bool match_service(const void *a, const
>>void *b)
>> const struct external_service *service = a;
>> const struct svc_match_data *data = b;
>>
>> - return g_strcmp0(service->path, data->path) == 0 &&
>> + return g_strcmp0(service->manager_path, data->path) == 0 &&
>> g_strcmp0(service->owner, data->sender)
>>== 0;
>> }
>>
>> @@ -1321,7 +1323,7 @@ static void proxy_added_cb(GDBusProxy *proxy,
>>void *user_data)
>> iface = g_dbus_proxy_get_interface(proxy);
>> path = g_dbus_proxy_get_path(proxy);
>>
>> - if (!g_str_has_prefix(path, service->path))
>> + if (!g_str_has_prefix(path, service->manager_path))
>> return;
>>
>> if (g_strcmp0(iface, GATT_SERVICE_IFACE) == 0) {
>> @@ -1332,12 +1334,20 @@ static void proxy_added_cb(GDBusProxy *proxy,
>>void *user_data)
>> * TODO: We may want to support adding included
>>services in a
>> * single hierarchy.
>> */
>> - if (g_strcmp0(path, service->path) != 0) {
>> + if (service->path != NULL) {
>> error("Multiple services added within
>>hierarchy");
>> service->failed = true;
>> return;
>> }
>>
>> + // Set service path now that we've found a service
>> + service->path = g_strdup(path);
>> + if (!service->path) {
>> + error("Failed to allocate service path");
>> + service->failed = true;
>> + return;
>> + }
>> +
>> /* Add 1 for the service declaration */
>> if (!incr_attr_count(service, 1)) {
>> error("Failed to increment attribute count");
>> @@ -2171,8 +2181,8 @@ static struct external_service
>>*service_create(DBusConnection *conn,
>> if (!service->owner)
>> goto fail;
>>
>> - service->path = g_strdup(path);
>> - if (!service->path)
>> + service->manager_path = g_strdup(path);
>> + if (!service->manager_path)
>> goto fail;
>>
>> service->chrcs = queue_new();
>> --
>> 2.4.6
>
>Well if we are doing to do that then perhaps we should list all the
>services under the path ObjectManager path not only one.
Hi Luiz,
You are right - though I still like the initial design of having one
service per ObjectManager, as it makes it easier to cleanup or restart (or
D-Bus export/unexport) an individual service at the ObjectManager API
level.
I only added the pointer to manager_path because it was better to test
that for that prefix than it was to not do any checking. I couldn?t do
the stronger test for the GattService path because there is no guaranteed
order to the proxy callbacks.
How strict do you think we need to be here in validating the parent/child
relationships in the object tree?
-Adam
>
>
>--
>Luiz Augusto von Dentz
Statement of Confidentiality
The contents of this e-mail message and any attachments are confidential and are intended solely for the addressee. The information may also be legally privileged. This transmission is sent in trust, and the sole purpose of delivery to the intended recipient. If you have received this transmission in error, any use, reproduction or dissemination of this transmission is strictly prohibited. If you are not the intended recipient, please immediately notify the sender by reply e-mail or at 508.683.2500 and delete this message and its attachments, if any.
Hi Adam,
On Fri, Jul 31, 2015 at 8:42 PM, Adam Moore
<[email protected]> wrote:
> Relaxes the requirement that object provide ObjectManager AND
> GattService interfaces.
>
> Maintains check that GattService and GattCharacteristics are
> children of ObjectManager, but loses the check that
> GattCharacteristics are children of GattService. Therefore,
> meant as a temporary workaround.
> ---
> src/gatt-database.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> index 69a814d..e82165c 100644
> --- a/src/gatt-database.c
> +++ b/src/gatt-database.c
> @@ -85,6 +85,7 @@ struct external_service {
> bool failed;
> char *owner;
> char *path; /* Path to GattService1 */
> + char *manager_path; /* Path to ObjectManager */
> DBusMessage *reg;
> GDBusClient *client;
> GDBusProxy *proxy;
> @@ -372,6 +373,7 @@ static void service_free(void *data)
>
> g_free(service->owner);
> g_free(service->path);
> + g_free(service->manager_path);
>
> free(service);
> }
> @@ -1042,7 +1044,7 @@ static bool match_service(const void *a, const void *b)
> const struct external_service *service = a;
> const struct svc_match_data *data = b;
>
> - return g_strcmp0(service->path, data->path) == 0 &&
> + return g_strcmp0(service->manager_path, data->path) == 0 &&
> g_strcmp0(service->owner, data->sender) == 0;
> }
>
> @@ -1321,7 +1323,7 @@ static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
> iface = g_dbus_proxy_get_interface(proxy);
> path = g_dbus_proxy_get_path(proxy);
>
> - if (!g_str_has_prefix(path, service->path))
> + if (!g_str_has_prefix(path, service->manager_path))
> return;
>
> if (g_strcmp0(iface, GATT_SERVICE_IFACE) == 0) {
> @@ -1332,12 +1334,20 @@ static void proxy_added_cb(GDBusProxy *proxy, void *user_data)
> * TODO: We may want to support adding included services in a
> * single hierarchy.
> */
> - if (g_strcmp0(path, service->path) != 0) {
> + if (service->path != NULL) {
> error("Multiple services added within hierarchy");
> service->failed = true;
> return;
> }
>
> + // Set service path now that we've found a service
> + service->path = g_strdup(path);
> + if (!service->path) {
> + error("Failed to allocate service path");
> + service->failed = true;
> + return;
> + }
> +
> /* Add 1 for the service declaration */
> if (!incr_attr_count(service, 1)) {
> error("Failed to increment attribute count");
> @@ -2171,8 +2181,8 @@ static struct external_service *service_create(DBusConnection *conn,
> if (!service->owner)
> goto fail;
>
> - service->path = g_strdup(path);
> - if (!service->path)
> + service->manager_path = g_strdup(path);
> + if (!service->manager_path)
> goto fail;
>
> service->chrcs = queue_new();
> --
> 2.4.6
Well if we are doing to do that then perhaps we should list all the
services under the path ObjectManager path not only one.
--
Luiz Augusto von Dentz
DQoNCk9uIDEvMTAvMTYsIDI6MzcgUE0sICJMdWl6IEF1Z3VzdG8gdm9uIERlbnR6IiA8bHVpei5k
ZW50ekBnbWFpbC5jb20+IHdyb3RlOg0KDQo+SGkgQWRhbSwNCj4NCj4+IEhpIEx1aXosDQo+Pg0K
Pj4gWW91IGFyZSByaWdodCAtIHRob3VnaCBJIHN0aWxsIGxpa2UgdGhlIGluaXRpYWwgZGVzaWdu
IG9mIGhhdmluZyBvbmUNCj4+IHNlcnZpY2UgcGVyIE9iamVjdE1hbmFnZXIsIGFzIGl0IG1ha2Vz
IGl0IGVhc2llciB0byBjbGVhbnVwIG9yIHJlc3RhcnQNCj4+KG9yDQo+PiBELUJ1cyBleHBvcnQv
dW5leHBvcnQpIGFuIGluZGl2aWR1YWwgc2VydmljZSBhdCB0aGUgT2JqZWN0TWFuYWdlciBBUEkN
Cj4+IGxldmVsLg0KPj4NCj4+IEkgb25seSBhZGRlZCB0aGUgcG9pbnRlciB0byBtYW5hZ2VyX3Bh
dGggYmVjYXVzZSBpdCB3YXMgYmV0dGVyIHRvIHRlc3QNCj4+IHRoYXQgZm9yIHRoYXQgcHJlZml4
IHRoYW4gaXQgd2FzIHRvIG5vdCBkbyBhbnkgY2hlY2tpbmcuICBJIGNvdWxkbqn2dCBkbw0KPj4g
dGhlIHN0cm9uZ2VyIHRlc3QgZm9yIHRoZSBHYXR0U2VydmljZSBwYXRoIGJlY2F1c2UgdGhlcmUg
aXMgbm8NCj4+Z3VhcmFudGVlZA0KPj4gb3JkZXIgdG8gdGhlIHByb3h5IGNhbGxiYWNrcy4NCj4N
Cj5QbGVhc2UgY2hlY2sgdGhlIHBhdGNoLXNldCBJdmUgc2VudCBsYXN0IHdlZWssIHRoZSBpZGVh
IGlzIHRoYXQgeW91DQo+d291bGQgcmVnaXN0ZXIgdGhlIGFwcGxpY2F0aW9uIG9ubHkgb25jZSwg
bGF0ZXIgb24gd2UgbWlnaHQgaW1wbGVtZW50DQo+c3VwcG9ydCBmb3IgSW50ZXJmYWNlc0FkZGVk
L0ludGVyZmFjZXNSZW1vdmVkIGFsbG93aW5nIHNlcnZpY2VzIHRvIGJlDQo+YWRkZWQvcmVtb3Zl
ZCB3aXRob3V0IGhhdmluZyB0byB1bnJlZ2lzdGVyaW5nIGFuZCByZWdpc3RlcmluZyBhZ2Fpbi4N
Cg0KVGhhbmtzIHZlcnkgbXVjaCwgTHVpeiEgIExvb2tzIGdvb2QgLSBJIHRyaWVkIGFsaWduaW5n
IG15IGFwcGxpY2F0aW9uIHRvDQp0aGUgbmV3IEFQSSBhbmQgaXQgd29ya3MgZmluZSBhZnRlciBy
ZW1vdmluZyBteSB3b3JrYXJvdW5kIHBhdGNoLg0KDQoNCj4NCj4+IEhvdyBzdHJpY3QgZG8geW91
IHRoaW5rIHdlIG5lZWQgdG8gYmUgaGVyZSBpbiB2YWxpZGF0aW5nIHRoZQ0KPj5wYXJlbnQvY2hp
bGQNCj4+IHJlbGF0aW9uc2hpcHMgaW4gdGhlIG9iamVjdCB0cmVlPw0KPg0KPlJpZ2h0IG5vdyBp
dCBleHBlY3RzIHRoZSBvYmplY3RzIHRvIGFwcGVhciBpbiBvcmRlciwgc2VydmljZSAtPg0KPmNo
YXJhY3RlcmlzdGljIC0+IGRlc2NyaXB0b3IsIHNvIHRoYXQgd2UgY2FuIGVzdGFibGlzaGVkIHBy
b3Blcg0KPnJlbGF0aW9uc2hpcCBiZXR3ZWVuIHRoZSBvYmplY3RzLiBJIGRpZCBub3QgYWRkIGFu
eSBjaGVjayBmb3IgbXVsdGlwbGUNCj5hcHBsaWNhdGlvbnMgd2l0aGluIHRoZSBzYW1lIGNvbm5l
Y3Rpb24gaWQgZXhjZXB0IGlmIGl0IGlzIHRoZSB2ZXJ5DQo+c2FtZSBvYmplY3QgcGF0aCBnaXZl
biBmb3IgdGhlIG9iamVjdCBtYW5hZ2VyLg0KDQoNClN0YXRlbWVudCBvZiBDb25maWRlbnRpYWxp
dHkNCg0KVGhlIGNvbnRlbnRzIG9mIHRoaXMgZS1tYWlsIG1lc3NhZ2UgYW5kIGFueSBhdHRhY2ht
ZW50cyBhcmUgY29uZmlkZW50aWFsIGFuZCBhcmUgaW50ZW5kZWQgc29sZWx5IGZvciB0aGUgYWRk
cmVzc2VlLiBUaGUgaW5mb3JtYXRpb24gbWF5IGFsc28gYmUgbGVnYWxseSBwcml2aWxlZ2VkLiBU
aGlzIHRyYW5zbWlzc2lvbiBpcyBzZW50IGluIHRydXN0LCBhbmQgdGhlIHNvbGUgcHVycG9zZSBv
ZiBkZWxpdmVyeSB0byB0aGUgaW50ZW5kZWQgcmVjaXBpZW50LiBJZiB5b3UgaGF2ZSByZWNlaXZl
ZCB0aGlzIHRyYW5zbWlzc2lvbiBpbiBlcnJvciwgYW55IHVzZSwgcmVwcm9kdWN0aW9uIG9yIGRp
c3NlbWluYXRpb24gb2YgdGhpcyB0cmFuc21pc3Npb24gaXMgc3RyaWN0bHkgcHJvaGliaXRlZC4g
SWYgeW91IGFyZSBub3QgdGhlIGludGVuZGVkIHJlY2lwaWVudCwgcGxlYXNlIGltbWVkaWF0ZWx5
IG5vdGlmeSB0aGUgc2VuZGVyIGJ5IHJlcGx5IGUtbWFpbCBvciBhdCA1MDguNjgzLjI1MDAgYW5k
IGRlbGV0ZSB0aGlzIG1lc3NhZ2UgYW5kIGl0cyBhdHRhY2htZW50cywgaWYgYW55Lg0KDQo=
Hi Adam,
> Hi Luiz,
>
> You are right - though I still like the initial design of having one
> service per ObjectManager, as it makes it easier to cleanup or restart (or
> D-Bus export/unexport) an individual service at the ObjectManager API
> level.
>
> I only added the pointer to manager_path because it was better to test
> that for that prefix than it was to not do any checking. I couldn¹t do
> the stronger test for the GattService path because there is no guaranteed
> order to the proxy callbacks.
Please check the patch-set Ive sent last week, the idea is that you
would register the application only once, later on we might implement
support for InterfacesAdded/InterfacesRemoved allowing services to be
added/removed without having to unregistering and registering again.
> How strict do you think we need to be here in validating the parent/child
> relationships in the object tree?
Right now it expects the objects to appear in order, service ->
characteristic -> descriptor, so that we can established proper
relationship between the objects. I did not add any check for multiple
applications within the same connection id except if it is the very
same object path given for the object manager.