2015-07-19 15:31:07

by Dohyun Pyun

[permalink] [raw]
Subject: [PATCH BLUEZ 0/4] Modify Health profile signals for ObjectManager

From: DoHyun Pyun <[email protected]>

Since BlueZ has ObjectManager interface, ChannelConnected and ChannelDeleted
Signals of HealthDevice interface was not available. ObjectManager reports the
HealthChannel D-Bus interface when g_dbus_emit_signal function is called,
and generates "InterfaceAdded" signal. But ChannelConnected and ChannelDeleted
Siganls was not generated. If health-api document contains these signals,
application or service using Bluez Dbus API will get the unexpected result.

If we just remove g_dbus_emit_signal for ChannelConnected and ChannelDeleted,
InterfaceAdded signal will not sent. Because InterfaceAdded signal is pending
before sending a dbus signal. These patches include new property in HealthChannel
interfacem, and this PropertiesChanged signal can be used to replace the previous
ChannelConnected and ChannelDeleted signals.

Connected signal:

signal sender=:1.140 -> dest=(null destination) serial=280 path=/; interface=org.freedesktop.DBus.ObjectManager; member=InterfacesAdded
object path "/org/bluez/hci0/dev_00_09_1F_80_3B_76/chan65279"
array [
dict entry(
string "org.freedesktop.DBus.Introspectable"
array [
]
)
dict entry(
string "org.bluez.HealthChannel1"
array [
dict entry(
string "Device"
variant object path "/org/bluez/hci0/dev_00_09_1F_80_3B_76"
)
dict entry(
string "Application"
variant object path "/org/bluez/health_app_2"
)
dict entry(
string "Type"
variant string "reliable"
)
dict entry(
string "Connected"
variant boolean false
)
]
)
dict entry(
string "org.freedesktop.DBus.Properties"
array [
]
)
]
signal sender=:1.140 -> dest=(null destination) serial=281 path=/org/bluez/hci0/dev_00_09_1F_80_3B_76/chan65279; interface=org.freedesktop.DBus.Properties; member=PropertiesChanged
string "org.bluez.HealthChannel1"
array [
dict entry(
string "Connected"
variant boolean true
)
]
array [
]
signal sender=:1.140 -> dest=(null destination) serial=282 path=/org/bluez/hci0/dev_00_09_1F_80_3B_76; interface=org.freedesktop.DBus.Properties; member=PropertiesChanged
string "org.bluez.HealthDevice1"
array [
dict entry(
string "MainChannel"
variant object path "/org/bluez/hci0/dev_00_09_1F_80_3B_76/chan65279"
)
]
array [
]


Disconnected signal:

signal sender=:1.50 -> dest=(null destination) serial=284 path=/org/bluez/hci0/dev_00_09_1F_80_3B_76/chan65279; interface=org.freedesktop.DBus.Properties; member=PropertiesChanged
string "org.bluez.HealthChannel1"
array [
dict entry(
string "Connected"
variant boolean false
)
]
array [
]
signal sender=:1.50 -> dest=(null destination) serial=285 path=/; interface=org.freedesktop.DBus.ObjectManager; member=InterfacesRemoved
object path "/org/bluez/hci0/dev_00_09_1F_80_3B_76/chan65279"
array [
string "org.freedesktop.DBus.Properties"
string "org.freedesktop.DBus.Introspectable"
string "org.bluez.HealthChannel1"
]


Dohyun Pyun (4):
doc/health-api: Add Connected property to HealthChannel
profiles/health: Implement connected property in HealthChannel
doc/health-api: Remove ChannelConnected and ChannelDeleted signals
profiles/health: Remove HealthDevice signals

doc/health-api.txt | 19 +++-----
profiles/health/hdp.c | 108 ++++++++++++++++++++++++++++----------------
profiles/health/hdp_types.h | 1 +
3 files changed, 75 insertions(+), 53 deletions(-)

--
1.8.1.2



2015-07-23 13:58:09

by Dohyun Pyun

[permalink] [raw]
Subject: Re: [PATCH BLUEZ 0/4] Modify Health profile signals for ObjectManager

Hi Luiz,

2015-07-22 21:50 GMT+09:00, Luiz Augusto von Dentz <[email protected]>:
> Hi Dohyun,
>
> On Sun, Jul 19, 2015 at 6:31 PM, Dohyun Pyun <[email protected]> wrote:
>> From: DoHyun Pyun <[email protected]>
>>
>> Since BlueZ has ObjectManager interface, ChannelConnected and
>> ChannelDeleted
>> Signals of HealthDevice interface was not available. ObjectManager reports
>> the
>> HealthChannel D-Bus interface when g_dbus_emit_signal function is called,
>> and generates "InterfaceAdded" signal. But ChannelConnected and
>> ChannelDeleted
>> Siganls was not generated. If health-api document contains these signals,
>> application or service using Bluez Dbus API will get the unexpected
>> result.
>>
>> If we just remove g_dbus_emit_signal for ChannelConnected and
>> ChannelDeleted,
>> InterfaceAdded signal will not sent. Because InterfaceAdded signal is
>> pending
>> before sending a dbus signal. These patches include new property in
>> HealthChannel
>> interfacem, and this PropertiesChanged signal can be used to replace the
>> previous
>> ChannelConnected and ChannelDeleted signals.
>
> Seems like a good idea, but from the logs it looks like the channel
> lifetime is tied with the connection in that case
> InterfacesAdded/InterfacesRemoved can be used directly instead of
> having another property or Im missing something?
>

When I re-test the possible senarios without "PropertiesChanged" Signal
after removing the ChannelConnected and ChannelRemoved signals.
It looks no problem. As you said, we can use InterfacesAdded /
InterfacesRemoved event directly. After verifying the operations, I will
re-make the patchsets - only include remove the previous signals
(ChannelConnected/ ChannelRemoved)


Thanks,
Pyun

2015-07-22 12:50:19

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH BLUEZ 0/4] Modify Health profile signals for ObjectManager

Hi Dohyun,

On Sun, Jul 19, 2015 at 6:31 PM, Dohyun Pyun <[email protected]> wrote:
> From: DoHyun Pyun <[email protected]>
>
> Since BlueZ has ObjectManager interface, ChannelConnected and ChannelDeleted
> Signals of HealthDevice interface was not available. ObjectManager reports the
> HealthChannel D-Bus interface when g_dbus_emit_signal function is called,
> and generates "InterfaceAdded" signal. But ChannelConnected and ChannelDeleted
> Siganls was not generated. If health-api document contains these signals,
> application or service using Bluez Dbus API will get the unexpected result.
>
> If we just remove g_dbus_emit_signal for ChannelConnected and ChannelDeleted,
> InterfaceAdded signal will not sent. Because InterfaceAdded signal is pending
> before sending a dbus signal. These patches include new property in HealthChannel
> interfacem, and this PropertiesChanged signal can be used to replace the previous
> ChannelConnected and ChannelDeleted signals.

Seems like a good idea, but from the logs it looks like the channel
lifetime is tied with the connection in that case
InterfacesAdded/InterfacesRemoved can be used directly instead of
having another property or Im missing something?


--
Luiz Augusto von Dentz

2015-07-19 15:31:11

by Dohyun Pyun

[permalink] [raw]
Subject: [PATCH BLUEZ 4/4] profiles/health: Remove HealthDevice signals

This patch removes ChannelConnected and ChannelDeleted signals in
HealthDevice interface. These signals are not currently generated
and can be replaced InterfaceAdded and PropertiesChanged signals.
---
profiles/health/hdp.c | 44 +-------------------------------------------
1 file changed, 1 insertion(+), 43 deletions(-)

diff --git a/profiles/health/hdp.c b/profiles/health/hdp.c
index 9fd7bf6..b7b1693 100644
--- a/profiles/health/hdp.c
+++ b/profiles/health/hdp.c
@@ -529,11 +529,6 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
&fd, DBUS_TYPE_INVALID);
g_dbus_send_message(conn, reply);

- g_dbus_emit_signal(conn, device_get_path(dc_data->hdp_chann->dev->dev),
- HEALTH_DEVICE, "ChannelConnected",
- DBUS_TYPE_OBJECT_PATH, &dc_data->hdp_chann->path,
- DBUS_TYPE_INVALID);
-
dc_data->hdp_chann->connected = TRUE;

g_dbus_emit_property_changed(conn, dc_data->hdp_chann->path,
@@ -726,13 +721,6 @@ static void health_channel_destroy(void *data)

dev->channels = g_slist_remove(dev->channels, hdp_chan);

- if (hdp_chan->mdep != HDP_MDEP_ECHO)
- g_dbus_emit_signal(btd_get_dbus_connection(),
- device_get_path(dev->dev),
- HEALTH_DEVICE, "ChannelDeleted",
- DBUS_TYPE_OBJECT_PATH, &hdp_chan->path,
- DBUS_TYPE_INVALID);
-
if (hdp_chan == dev->fr) {
hdp_channel_unref(dev->fr);
dev->fr = NULL;
@@ -996,11 +984,6 @@ static void hdp_mcap_mdl_connected_cb(struct mcap_mdl *mdl, void *data)
goto end;
}

- g_dbus_emit_signal(btd_get_dbus_connection(), device_get_path(dev->dev),
- HEALTH_DEVICE, "ChannelConnected",
- DBUS_TYPE_OBJECT_PATH, &chan->path,
- DBUS_TYPE_INVALID);
-
chan->connected = TRUE;

g_dbus_emit_property_changed(btd_get_dbus_connection(), chan->path,
@@ -1073,12 +1056,6 @@ static void hdp_mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
hdp_channel_ref(dev->ndc));

if (dev->ndc->mdep != HDP_MDEP_ECHO) {
- g_dbus_emit_signal(btd_get_dbus_connection(),
- device_get_path(dev->dev),
- HEALTH_DEVICE, "ChannelConnected",
- DBUS_TYPE_OBJECT_PATH, &dev->ndc->path,
- DBUS_TYPE_INVALID);
-
dev->ndc->connected = TRUE;

g_dbus_emit_property_changed(btd_get_dbus_connection(),
@@ -1703,12 +1680,6 @@ static void abort_mdl_connection_cb(GError *err, gpointer data)
/* Connection operation has failed but we have to */
/* notify the channel created at MCAP level */
if (hdp_chann->mdep != HDP_MDEP_ECHO) {
- g_dbus_emit_signal(btd_get_dbus_connection(),
- device_get_path(hdp_chann->dev->dev),
- HEALTH_DEVICE, "ChannelConnected",
- DBUS_TYPE_OBJECT_PATH, &hdp_chann->path,
- DBUS_TYPE_INVALID);
-
hdp_chann->connected = TRUE;

g_dbus_emit_property_changed(btd_get_dbus_connection(),
@@ -1750,11 +1721,6 @@ static void hdp_mdl_conn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
DBUS_TYPE_INVALID);
g_dbus_send_message(conn, reply);

- g_dbus_emit_signal(conn, device_get_path(hdp_chann->dev->dev),
- HEALTH_DEVICE, "ChannelConnected",
- DBUS_TYPE_OBJECT_PATH, &hdp_chann->path,
- DBUS_TYPE_INVALID);
-
hdp_chann->connected = TRUE;

g_dbus_emit_property_changed(btd_get_dbus_connection(), hdp_chann->path,
@@ -2189,14 +2155,6 @@ static const GDBusMethodTable health_device_methods[] = {
{ }
};

-static const GDBusSignalTable health_device_signals[] = {
- { GDBUS_SIGNAL("ChannelConnected",
- GDBUS_ARGS({ "channel", "o" })) },
- { GDBUS_SIGNAL("ChannelDeleted",
- GDBUS_ARGS({ "channel", "o" })) },
- { }
-};
-
static const GDBusPropertyTable health_device_properties[] = {
{ "MainChannel", "o", dev_property_get_main_channel, NULL,
dev_property_exists_main_channel },
@@ -2226,7 +2184,7 @@ static struct hdp_device *create_health_device(struct btd_device *device)
if (!g_dbus_register_interface(btd_get_dbus_connection(),
path, HEALTH_DEVICE,
health_device_methods,
- health_device_signals,
+ NULL,
health_device_properties,
dev, health_device_destroy)) {
error("D-Bus failed to register %s interface", HEALTH_DEVICE);
--
1.8.1.2


2015-07-19 15:31:10

by Dohyun Pyun

[permalink] [raw]
Subject: [PATCH BLUEZ 3/4] doc/health-api: Remove ChannelConnected and ChannelDeleted signals

These signals are no longer available since BlueZ uses ObjectManager
interface. Instead of these signals InterfaceAdded and PropertiesChanged
(connected) signals can be used.
---
doc/health-api.txt | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/doc/health-api.txt b/doc/health-api.txt
index 8d4aa72..be82dd0 100644
--- a/doc/health-api.txt
+++ b/doc/health-api.txt
@@ -87,19 +87,6 @@ Methods boolean Echo()
org.bluez.Error.NotFound
org.bluez.Error.NotAllowed

-Signals void ChannelConnected(object channel)
-
- This signal is launched when a new data channel is
- created or when a known data channel is reconnected.
-
- void ChannelDeleted(object channel)
-
- This signal is launched when a data channel is deleted.
-
- After this signal the data channel path will not be
- valid and its path can be reused for future data
- channels.
-
Properties object MainChannel [readonly]

The first reliable channel opened. It is needed by
@@ -155,4 +142,4 @@ Properties string Type [readonly]

Indicates if the HealthChannel is currently connected.
A PropertiesChanged signal indicate changes to this
- status
\ No newline at end of file
+ status
--
1.8.1.2


2015-07-19 15:31:09

by Dohyun Pyun

[permalink] [raw]
Subject: [PATCH BLUEZ 2/4] profiles/health: Implement connected property in HealthChannel

This patch implements the new connected property in HealthChannel,
which gets updated when a health channel is connected and disconnected.
This property will replace ChannelConnected and ChannelDeleted signals
in HealthDevice.

Change-Id: Ie1d2df12bcb9de1a55df3e5f08b664f0a5a61d95
---
profiles/health/hdp.c | 74 +++++++++++++++++++++++++++++++++++++++++++--
profiles/health/hdp_types.h | 1 +
2 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/profiles/health/hdp.c b/profiles/health/hdp.c
index bc3b38a..9fd7bf6 100644
--- a/profiles/health/hdp.c
+++ b/profiles/health/hdp.c
@@ -464,6 +464,18 @@ static gboolean channel_property_get_type(const GDBusPropertyTable *property,
return TRUE;
}

+static gboolean channel_property_get_connected(
+ const GDBusPropertyTable *property,
+ DBusMessageIter *iter, void *data)
+{
+ struct hdp_channel *chan = data;
+
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN,
+ &chan->connected);
+
+ return TRUE;
+}
+
static void hdp_tmp_dc_data_destroy(gpointer data)
{
struct hdp_tmp_dc_data *hdp_conn = data;
@@ -521,6 +533,11 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
HEALTH_DEVICE, "ChannelConnected",
DBUS_TYPE_OBJECT_PATH, &dc_data->hdp_chann->path,
DBUS_TYPE_INVALID);
+
+ dc_data->hdp_chann->connected = TRUE;
+
+ g_dbus_emit_property_changed(conn, dc_data->hdp_chann->path,
+ HEALTH_CHANNEL, "Connected");
}

static void hdp_get_dcpsm_cb(uint16_t dcpsm, gpointer user_data, GError *err)
@@ -737,6 +754,7 @@ static const GDBusPropertyTable health_channels_properties[] = {
{ "Device", "o", channel_property_get_device },
{ "Application", "o", channel_property_get_application },
{ "Type", "s", channel_property_get_type },
+ { "Connected", "b", channel_property_get_connected },
{ }
};

@@ -800,6 +818,12 @@ static void remove_channels(struct hdp_device *dev)

while (dev->channels != NULL) {
chan = dev->channels->data;
+ chan->connected = FALSE;
+
+ if (chan->mdep != HDP_MDEP_ECHO)
+ g_dbus_emit_property_changed(btd_get_dbus_connection(),
+ chan->path,
+ HEALTH_CHANNEL, "Connected");

path = g_strdup(chan->path);
if (!g_dbus_unregister_interface(btd_get_dbus_connection(),
@@ -977,6 +1001,11 @@ static void hdp_mcap_mdl_connected_cb(struct mcap_mdl *mdl, void *data)
DBUS_TYPE_OBJECT_PATH, &chan->path,
DBUS_TYPE_INVALID);

+ chan->connected = TRUE;
+
+ g_dbus_emit_property_changed(btd_get_dbus_connection(), chan->path,
+ HEALTH_CHANNEL, "Connected");
+
if (dev->fr != NULL)
goto end;

@@ -1014,6 +1043,12 @@ static void hdp_mcap_mdl_deleted_cb(struct mcap_mdl *mdl, void *data)
return;

chan = l->data;
+ chan->connected = FALSE;
+
+ if (chan->mdep != HDP_MDEP_ECHO)
+ g_dbus_emit_property_changed(btd_get_dbus_connection(),
+ chan->path,
+ HEALTH_CHANNEL, "Connected");

path = g_strdup(chan->path);
if (!g_dbus_unregister_interface(btd_get_dbus_connection(),
@@ -1037,13 +1072,20 @@ static void hdp_mcap_mdl_aborted_cb(struct mcap_mdl *mdl, void *data)
dev->channels = g_slist_prepend(dev->channels,
hdp_channel_ref(dev->ndc));

- if (dev->ndc->mdep != HDP_MDEP_ECHO)
+ if (dev->ndc->mdep != HDP_MDEP_ECHO) {
g_dbus_emit_signal(btd_get_dbus_connection(),
device_get_path(dev->dev),
HEALTH_DEVICE, "ChannelConnected",
DBUS_TYPE_OBJECT_PATH, &dev->ndc->path,
DBUS_TYPE_INVALID);

+ dev->ndc->connected = TRUE;
+
+ g_dbus_emit_property_changed(btd_get_dbus_connection(),
+ dev->ndc->path,
+ HEALTH_CHANNEL, "Connected");
+ }
+
hdp_channel_unref(dev->ndc);
dev->ndc = NULL;
}
@@ -1266,6 +1308,15 @@ static void mcl_disconnected(struct mcap_mcl *mcl, gpointer data)
hdp_device = l->data;
hdp_device->mcl_conn = FALSE;

+ /* Set channel connected status to false */
+ if (hdp_device->fr) {
+ hdp_device->fr->connected = FALSE;
+
+ g_dbus_emit_property_changed(btd_get_dbus_connection(),
+ hdp_device->fr->path,
+ HEALTH_CHANNEL, "Connected");
+ }
+
DBG("Mcl disconnected %s", device_get_path(hdp_device->dev));
}

@@ -1651,12 +1702,19 @@ static void abort_mdl_connection_cb(GError *err, gpointer data)

/* Connection operation has failed but we have to */
/* notify the channel created at MCAP level */
- if (hdp_chann->mdep != HDP_MDEP_ECHO)
+ if (hdp_chann->mdep != HDP_MDEP_ECHO) {
g_dbus_emit_signal(btd_get_dbus_connection(),
device_get_path(hdp_chann->dev->dev),
HEALTH_DEVICE, "ChannelConnected",
DBUS_TYPE_OBJECT_PATH, &hdp_chann->path,
DBUS_TYPE_INVALID);
+
+ hdp_chann->connected = TRUE;
+
+ g_dbus_emit_property_changed(btd_get_dbus_connection(),
+ hdp_chann->path,
+ HEALTH_CHANNEL, "Connected");
+ }
}

static void hdp_mdl_conn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
@@ -1697,6 +1755,11 @@ static void hdp_mdl_conn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
DBUS_TYPE_OBJECT_PATH, &hdp_chann->path,
DBUS_TYPE_INVALID);

+ hdp_chann->connected = TRUE;
+
+ g_dbus_emit_property_changed(btd_get_dbus_connection(), hdp_chann->path,
+ HEALTH_CHANNEL, "Connected");
+
if (!check_channel_conf(hdp_chann)) {
close_mdl(hdp_chann);
return;
@@ -1984,6 +2047,13 @@ static void hdp_mdl_delete_cb(GError *err, gpointer data)
return;
}

+ del_data->hdp_chann->connected = FALSE;
+
+ if (del_data->hdp_chann->mdep != HDP_MDEP_ECHO)
+ g_dbus_emit_property_changed(btd_get_dbus_connection(),
+ del_data->hdp_chann->path,
+ HEALTH_CHANNEL, "Connected");
+
path = g_strdup(del_data->hdp_chann->path);
g_dbus_unregister_interface(conn, path, HEALTH_CHANNEL);
g_free(path);
diff --git a/profiles/health/hdp_types.h b/profiles/health/hdp_types.h
index b34b5e0..3db38bb 100644
--- a/profiles/health/hdp_types.h
+++ b/profiles/health/hdp_types.h
@@ -115,6 +115,7 @@ struct hdp_channel {
uint16_t omtu; /* Channel outgoing MTU */
struct hdp_echo_data *edata; /* private data used by echo channels */
int ref; /* Reference counter */
+ gboolean connected; /* Connected status */
};

#endif /* __HDP_TYPES_H__ */
--
1.8.1.2


2015-07-19 15:31:08

by Dohyun Pyun

[permalink] [raw]
Subject: [PATCH BLUEZ 1/4] doc/health-api: Add Connected property to HealthChannel

This patch adds Connected property description to HealthChannel.
It will be used to replace ChannelConnected and ChannelDeleted
signals. These Signals was not currently generated.
---
doc/health-api.txt | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/doc/health-api.txt b/doc/health-api.txt
index 2c48ff2..8d4aa72 100644
--- a/doc/health-api.txt
+++ b/doc/health-api.txt
@@ -150,3 +150,9 @@ Properties string Type [readonly]
Identifies the HealthApplication to which this channel
is related to (which indirectly defines its role and
data type).
+
+ boolean Connected [readonly]
+
+ Indicates if the HealthChannel is currently connected.
+ A PropertiesChanged signal indicate changes to this
+ status
\ No newline at end of file
--
1.8.1.2