* [PATCH 1/2] gdbus: add method for immediate property update
@ 2015-09-18 4:39 Jakub Pawlowski
2015-09-18 4:39 ` [PATCH 2/2] core/gatt-client: fix losing notifications Jakub Pawlowski
2015-09-18 7:15 ` [PATCH 1/2] gdbus: add method for immediate property update Luiz Augusto von Dentz
0 siblings, 2 replies; 4+ messages in thread
From: Jakub Pawlowski @ 2015-09-18 4:39 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jakub Pawlowski
g_dbus_emit_property_changed doesn't send dbus signal immediately. Instead
it stores changed properties, and schedule signal to be send at
g_iddle_add. Additionally, if this method is called few times for some
property, only last value will be sent in property changed signal.
If remote device sends lots of notifications, they're all scheduled to be
notified using this method. This might result in some notifications being
lost.
This patch adds new method, that immediately sends property changed
signal, instead of sheduling it for nearest iddle moment.
---
gdbus/gdbus.h | 3 +++
gdbus/object.c | 46 +++++++++++++++++++++++++++++++++++-----------
2 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
index d99c254..b31246a 100644
--- a/gdbus/gdbus.h
+++ b/gdbus/gdbus.h
@@ -300,6 +300,9 @@ void g_dbus_pending_property_error(GDBusPendingReply id, const char *name,
void g_dbus_emit_property_changed(DBusConnection *connection,
const char *path, const char *interface,
const char *name);
+void g_dbus_emit_property_changed_immediate(DBusConnection *connection,
+ const char *path, const char *interface,
+ const char *name);
gboolean g_dbus_get_properties(DBusConnection *connection, const char *path,
const char *interface, DBusMessageIter *iter);
diff --git a/gdbus/object.c b/gdbus/object.c
index 96db516..fff9e0d 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -1720,21 +1720,12 @@ static void process_property_changes(struct generic_data *data)
}
}
-void g_dbus_emit_property_changed(DBusConnection *connection,
- const char *path, const char *interface,
- const char *name)
+static void prepare_property_changed(const char *interface, const char *name,
+ struct generic_data *data)
{
const GDBusPropertyTable *property;
- struct generic_data *data;
struct interface_data *iface;
- if (path == NULL)
- return;
-
- if (!dbus_connection_get_object_path_data(connection, path,
- (void **) &data) || data == NULL)
- return;
-
iface = find_interface(data->interfaces, interface);
if (iface == NULL)
return;
@@ -1759,10 +1750,43 @@ void g_dbus_emit_property_changed(DBusConnection *connection,
data->pending_prop = TRUE;
iface->pending_prop = g_slist_prepend(iface->pending_prop,
(void *) property);
+}
+
+void g_dbus_emit_property_changed(DBusConnection *connection, const char *path,
+ const char *interface, const char *name)
+{
+ struct generic_data *data;
+
+ if (path == NULL)
+ return;
+
+ if (!dbus_connection_get_object_path_data(connection, path,
+ (void **) &data) || data == NULL)
+ return;
+
+ prepare_property_changed(interface, name, data);
add_pending(data);
}
+void g_dbus_emit_property_changed_immediate(DBusConnection *connection,
+ const char *path, const char *interface,
+ const char *name)
+{
+ struct generic_data *data;
+
+ if (path == NULL)
+ return;
+
+ if (!dbus_connection_get_object_path_data(connection, path,
+ (void **) &data) || data == NULL)
+ return;
+
+ prepare_property_changed(interface, name, data);
+
+ process_property_changes(data);
+}
+
gboolean g_dbus_get_properties(DBusConnection *connection, const char *path,
const char *interface, DBusMessageIter *iter)
{
--
2.5.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] core/gatt-client: fix losing notifications
2015-09-18 4:39 [PATCH 1/2] gdbus: add method for immediate property update Jakub Pawlowski
@ 2015-09-18 4:39 ` Jakub Pawlowski
2015-09-18 7:15 ` [PATCH 1/2] gdbus: add method for immediate property update Luiz Augusto von Dentz
1 sibling, 0 replies; 4+ messages in thread
From: Jakub Pawlowski @ 2015-09-18 4:39 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jakub Pawlowski
When notifications are quickly send to BlueZ, it tries to signal that
through dbus using g_dbus_emit_property_changed, which would merge all
changes and schedule sending property changed in g_iddle_add.
If the stream of notifications from device is fast enough, there might be
no iddle moment to send properties changed before next notification
arives, which would result in notifications being lost.
This patch fixes that by using method that sends properties changed
immediately, without waiting for iddle moment.
---
src/gatt-client.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/gatt-client.c b/src/gatt-client.c
index 399133a..e94a613 100644
--- a/src/gatt-client.c
+++ b/src/gatt-client.c
@@ -778,8 +778,9 @@ static void write_characteristic_cb(struct gatt_db_attribute *attr, int err,
if (err)
return;
- g_dbus_emit_property_changed(btd_get_dbus_connection(), chrc->path,
- GATT_CHARACTERISTIC_IFACE, "Value");
+ g_dbus_emit_property_changed_immediate(btd_get_dbus_connection(),
+ chrc->path, GATT_CHARACTERISTIC_IFACE, "Value");
+
}
static void chrc_read_cb(bool success, uint8_t att_ecode, const uint8_t *value,
--
2.5.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] gdbus: add method for immediate property update
2015-09-18 4:39 [PATCH 1/2] gdbus: add method for immediate property update Jakub Pawlowski
2015-09-18 4:39 ` [PATCH 2/2] core/gatt-client: fix losing notifications Jakub Pawlowski
@ 2015-09-18 7:15 ` Luiz Augusto von Dentz
2015-09-18 7:52 ` Jakub Pawlowski
1 sibling, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2015-09-18 7:15 UTC (permalink / raw)
To: Jakub Pawlowski; +Cc: linux-bluetooth@vger.kernel.org
Hi Jakub,
On Fri, Sep 18, 2015 at 7:39 AM, Jakub Pawlowski <jpawlowski@google.com> wrote:
> g_dbus_emit_property_changed doesn't send dbus signal immediately. Instead
> it stores changed properties, and schedule signal to be send at
> g_iddle_add. Additionally, if this method is called few times for some
> property, only last value will be sent in property changed signal.
>
> If remote device sends lots of notifications, they're all scheduled to be
> notified using this method. This might result in some notifications being
> lost.
>
> This patch adds new method, that immediately sends property changed
> signal, instead of sheduling it for nearest iddle moment.
> ---
> gdbus/gdbus.h | 3 +++
> gdbus/object.c | 46 +++++++++++++++++++++++++++++++++++-----------
> 2 files changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
> index d99c254..b31246a 100644
> --- a/gdbus/gdbus.h
> +++ b/gdbus/gdbus.h
> @@ -300,6 +300,9 @@ void g_dbus_pending_property_error(GDBusPendingReply id, const char *name,
> void g_dbus_emit_property_changed(DBusConnection *connection,
> const char *path, const char *interface,
> const char *name);
> +void g_dbus_emit_property_changed_immediate(DBusConnection *connection,
> + const char *path, const char *interface,
> + const char *name);
> gboolean g_dbus_get_properties(DBusConnection *connection, const char *path,
> const char *interface, DBusMessageIter *iter);
>
> diff --git a/gdbus/object.c b/gdbus/object.c
> index 96db516..fff9e0d 100644
> --- a/gdbus/object.c
> +++ b/gdbus/object.c
> @@ -1720,21 +1720,12 @@ static void process_property_changes(struct generic_data *data)
> }
> }
>
> -void g_dbus_emit_property_changed(DBusConnection *connection,
> - const char *path, const char *interface,
> - const char *name)
> +static void prepare_property_changed(const char *interface, const char *name,
> + struct generic_data *data)
> {
> const GDBusPropertyTable *property;
> - struct generic_data *data;
> struct interface_data *iface;
>
> - if (path == NULL)
> - return;
> -
> - if (!dbus_connection_get_object_path_data(connection, path,
> - (void **) &data) || data == NULL)
> - return;
> -
> iface = find_interface(data->interfaces, interface);
> if (iface == NULL)
> return;
> @@ -1759,10 +1750,43 @@ void g_dbus_emit_property_changed(DBusConnection *connection,
> data->pending_prop = TRUE;
> iface->pending_prop = g_slist_prepend(iface->pending_prop,
> (void *) property);
> +}
> +
> +void g_dbus_emit_property_changed(DBusConnection *connection, const char *path,
> + const char *interface, const char *name)
> +{
> + struct generic_data *data;
> +
> + if (path == NULL)
> + return;
> +
> + if (!dbus_connection_get_object_path_data(connection, path,
> + (void **) &data) || data == NULL)
> + return;
> +
> + prepare_property_changed(interface, name, data);
>
> add_pending(data);
> }
>
> +void g_dbus_emit_property_changed_immediate(DBusConnection *connection,
> + const char *path, const char *interface,
> + const char *name)
> +{
Perhaps we should call it g_dbus_emit_property_changed_full with flush
flag and make g_dbus_emit_property_changed call it so we share more
code. Too bad we cannot do this transparently by checking if the value
was already schedule to change flushing if that happen, or perhaps we
can do it but it means we need to read the value at the moment
g_dbus_emit_property_changed is called but that would probably need a
temporary DBusMessageIter which complicate things so Im fine in having
this way.
> + struct generic_data *data;
> +
> + if (path == NULL)
> + return;
> +
> + if (!dbus_connection_get_object_path_data(connection, path,
> + (void **) &data) || data == NULL)
> + return;
> +
> + prepare_property_changed(interface, name, data);
> +
> + process_property_changes(data);
> +}
> +
> gboolean g_dbus_get_properties(DBusConnection *connection, const char *path,
> const char *interface, DBusMessageIter *iter)
> {
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 1/2] gdbus: add method for immediate property update
2015-09-18 7:15 ` [PATCH 1/2] gdbus: add method for immediate property update Luiz Augusto von Dentz
@ 2015-09-18 7:52 ` Jakub Pawlowski
0 siblings, 0 replies; 4+ messages in thread
From: Jakub Pawlowski @ 2015-09-18 7:52 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth@vger.kernel.org
Hi Luiz,
On Fri, Sep 18, 2015 at 12:15 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Jakub,
>
> On Fri, Sep 18, 2015 at 7:39 AM, Jakub Pawlowski <jpawlowski@google.com> wrote:
> > g_dbus_emit_property_changed doesn't send dbus signal immediately. Instead
> > it stores changed properties, and schedule signal to be send at
> > g_iddle_add. Additionally, if this method is called few times for some
> > property, only last value will be sent in property changed signal.
> >
> > If remote device sends lots of notifications, they're all scheduled to be
> > notified using this method. This might result in some notifications being
> > lost.
> >
> > This patch adds new method, that immediately sends property changed
> > signal, instead of sheduling it for nearest iddle moment.
> > ---
> > gdbus/gdbus.h | 3 +++
> > gdbus/object.c | 46 +++++++++++++++++++++++++++++++++++-----------
> > 2 files changed, 38 insertions(+), 11 deletions(-)
> >
> > diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
> > index d99c254..b31246a 100644
> > --- a/gdbus/gdbus.h
> > +++ b/gdbus/gdbus.h
> > @@ -300,6 +300,9 @@ void g_dbus_pending_property_error(GDBusPendingReply id, const char *name,
> > void g_dbus_emit_property_changed(DBusConnection *connection,
> > const char *path, const char *interface,
> > const char *name);
> > +void g_dbus_emit_property_changed_immediate(DBusConnection *connection,
> > + const char *path, const char *interface,
> > + const char *name);
> > gboolean g_dbus_get_properties(DBusConnection *connection, const char *path,
> > const char *interface, DBusMessageIter *iter);
> >
> > diff --git a/gdbus/object.c b/gdbus/object.c
> > index 96db516..fff9e0d 100644
> > --- a/gdbus/object.c
> > +++ b/gdbus/object.c
> > @@ -1720,21 +1720,12 @@ static void process_property_changes(struct generic_data *data)
> > }
> > }
> >
> > -void g_dbus_emit_property_changed(DBusConnection *connection,
> > - const char *path, const char *interface,
> > - const char *name)
> > +static void prepare_property_changed(const char *interface, const char *name,
> > + struct generic_data *data)
> > {
> > const GDBusPropertyTable *property;
> > - struct generic_data *data;
> > struct interface_data *iface;
> >
> > - if (path == NULL)
> > - return;
> > -
> > - if (!dbus_connection_get_object_path_data(connection, path,
> > - (void **) &data) || data == NULL)
> > - return;
> > -
> > iface = find_interface(data->interfaces, interface);
> > if (iface == NULL)
> > return;
> > @@ -1759,10 +1750,43 @@ void g_dbus_emit_property_changed(DBusConnection *connection,
> > data->pending_prop = TRUE;
> > iface->pending_prop = g_slist_prepend(iface->pending_prop,
> > (void *) property);
> > +}
> > +
> > +void g_dbus_emit_property_changed(DBusConnection *connection, const char *path,
> > + const char *interface, const char *name)
> > +{
> > + struct generic_data *data;
> > +
> > + if (path == NULL)
> > + return;
> > +
> > + if (!dbus_connection_get_object_path_data(connection, path,
> > + (void **) &data) || data == NULL)
> > + return;
> > +
> > + prepare_property_changed(interface, name, data);
> >
> > add_pending(data);
> > }
> >
> > +void g_dbus_emit_property_changed_immediate(DBusConnection *connection,
> > + const char *path, const char *interface,
> > + const char *name)
> > +{
>
> Perhaps we should call it g_dbus_emit_property_changed_full with flush
> flag and make g_dbus_emit_property_changed call it so we share more
> code. Too bad we cannot do this transparently by checking if the value
> was already schedule to change flushing if that happen, or perhaps we
> can do it but it means we need to read the value at the moment
Name fixed as suggested.
So we can check wether value was scheduled for update:
http://git.kernel.org/cgit/bluetooth/bluez.git/tree/gdbus/object.c?id=5783e002459bfb51488c6180c2e88b77c2ca2cbd#n1756
But now the logic is like this: "If value was scheduled for update,
and I schedule it again, then just send second update" which is not
good for notifications.
> g_dbus_emit_property_changed is called but that would probably need a
> temporary DBusMessageIter which complicate things so Im fine in having
> this way.
>
> > + struct generic_data *data;
> > +
> > + if (path == NULL)
> > + return;
> > +
> > + if (!dbus_connection_get_object_path_data(connection, path,
> > + (void **) &data) || data == NULL)
> > + return;
> > +
> > + prepare_property_changed(interface, name, data);
> > +
> > + process_property_changes(data);
> > +}
> > +
> > gboolean g_dbus_get_properties(DBusConnection *connection, const char *path,
> > const char *interface, DBusMessageIter *iter)
> > {
> > --
> > 2.5.0
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-09-18 7:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-18 4:39 [PATCH 1/2] gdbus: add method for immediate property update Jakub Pawlowski
2015-09-18 4:39 ` [PATCH 2/2] core/gatt-client: fix losing notifications Jakub Pawlowski
2015-09-18 7:15 ` [PATCH 1/2] gdbus: add method for immediate property update Luiz Augusto von Dentz
2015-09-18 7:52 ` Jakub Pawlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox