* [PATCH 01/10] hfp_hf_bluez5: Initial GDBusClient for BlueZ
2013-01-22 21:43 [PATCH 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
@ 2013-01-22 21:43 ` Vinicius Costa Gomes
2013-01-23 4:30 ` Denis Kenzior
2013-01-22 21:43 ` [PATCH 02/10] hfp_hf_bluez5: Add GDBusProxy for Bluetooth devices Vinicius Costa Gomes
` (9 subsequent siblings)
10 siblings, 1 reply; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-22 21:43 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 3874 bytes --]
This patch adds the initial callbacks to track when BlueZ connects or
disconnects from the system bus. And tracks the interfaces added or
removed.
---
plugins/bluez5.h | 3 ++-
plugins/hfp_hf_bluez5.c | 72 ++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 65 insertions(+), 10 deletions(-)
diff --git a/plugins/bluez5.h b/plugins/bluez5.h
index 01ecfe8..893072f 100644
--- a/plugins/bluez5.h
+++ b/plugins/bluez5.h
@@ -19,7 +19,8 @@
*
*/
-#define BLUEZ_SERVICE "org.bluez"
+#define BLUEZ_SERVICE "org.bluez"
+#define BLUEZ_MANAGER_PATH "/"
#define BLUEZ_PROFILE_INTERFACE BLUEZ_SERVICE ".Profile1"
#define BLUEZ_ERROR_INTERFACE BLUEZ_SERVICE ".Error"
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index e024838..c5c5cd7 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -42,6 +42,8 @@
#define HFP_EXT_PROFILE_PATH "/bluetooth/profile/hfp_hf"
+static GDBusClient *bluez = NULL;
+
static int hfp_probe(struct ofono_modem *modem)
{
DBG("modem: %p", modem);
@@ -143,6 +145,60 @@ static const GDBusMethodTable profile_methods[] = {
{ }
};
+static void connect_handler(DBusConnection *conn, void *user_data)
+{
+ DBG("Registering External Profile handler ...");
+
+ if (!g_dbus_register_interface(conn, HFP_EXT_PROFILE_PATH,
+ BLUEZ_PROFILE_INTERFACE,
+ profile_methods, NULL,
+ NULL, NULL, NULL)) {
+ ofono_error("Register Profile interface failed: %s",
+ HFP_EXT_PROFILE_PATH);
+ }
+
+ bluetooth_register_profile(conn, HFP_HS_UUID, "hfp_hf",
+ HFP_EXT_PROFILE_PATH);
+}
+
+static void disconnect_handler(DBusConnection *conn, void *user_data)
+{
+
+ DBG("Unregistering External Profile handler ...");
+
+ g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
+ BLUEZ_PROFILE_INTERFACE);
+}
+
+static void proxy_added(GDBusProxy *proxy, void *user_data)
+{
+ const char *path;
+
+ path = g_dbus_proxy_get_path(proxy);
+
+ DBG("Device proxy: %s(%p)", path, proxy);
+}
+
+static void proxy_removed(GDBusProxy *proxy, void *user_data)
+{
+ const char *path;
+
+ path = g_dbus_proxy_get_path(proxy);
+
+ DBG("Device proxy: %s(%p)", path, proxy);
+}
+
+static void property_changed(GDBusProxy *proxy, const char *name,
+ DBusMessageIter *iter, void *user_data)
+{
+ const char *interface, *path;
+
+ interface = g_dbus_proxy_get_interface(proxy);
+ path = g_dbus_proxy_get_path(proxy);
+
+ DBG("path: %s interface: %s", path, interface);
+}
+
static int hfp_init(void)
{
DBusConnection *conn = ofono_dbus_get_connection();
@@ -161,19 +217,16 @@ static int hfp_init(void)
return -EIO;
}
- err = ofono_modem_driver_register(&hfp_driver);
- if (err < 0) {
- g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
- BLUEZ_PROFILE_INTERFACE);
- return err;
- }
+ bluez = g_dbus_client_new(conn, BLUEZ_SERVICE, BLUEZ_MANAGER_PATH);
+ g_dbus_client_set_connect_watch(bluez, connect_handler, NULL);
+ g_dbus_client_set_disconnect_watch(bluez, disconnect_handler, NULL);
+ g_dbus_client_set_proxy_handlers(bluez, proxy_added, proxy_removed,
+ property_changed, NULL);
- err = bluetooth_register_profile(conn, HFP_HS_UUID, "hfp_hf",
- HFP_EXT_PROFILE_PATH);
+ err = ofono_modem_driver_register(&hfp_driver);
if (err < 0) {
g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
BLUEZ_PROFILE_INTERFACE);
- ofono_modem_driver_unregister(&hfp_driver);
return err;
}
@@ -188,6 +241,7 @@ static void hfp_exit(void)
g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
BLUEZ_PROFILE_INTERFACE);
ofono_modem_driver_unregister(&hfp_driver);
+ g_dbus_client_unref(bluez);
}
OFONO_PLUGIN_DEFINE(hfp_bluez5, "External Hands-Free Profile Plugin", VERSION,
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 01/10] hfp_hf_bluez5: Initial GDBusClient for BlueZ
2013-01-22 21:43 ` [PATCH 01/10] hfp_hf_bluez5: Initial GDBusClient for BlueZ Vinicius Costa Gomes
@ 2013-01-23 4:30 ` Denis Kenzior
2013-01-23 14:16 ` Vinicius Costa Gomes
0 siblings, 1 reply; 38+ messages in thread
From: Denis Kenzior @ 2013-01-23 4:30 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 4598 bytes --]
Hi Vinicius,
On 01/22/2013 03:43 PM, Vinicius Costa Gomes wrote:
> This patch adds the initial callbacks to track when BlueZ connects or
> disconnects from the system bus. And tracks the interfaces added or
> removed.
> ---
> plugins/bluez5.h | 3 ++-
> plugins/hfp_hf_bluez5.c | 72 ++++++++++++++++++++++++++++++++++++++++++-------
> 2 files changed, 65 insertions(+), 10 deletions(-)
>
> diff --git a/plugins/bluez5.h b/plugins/bluez5.h
> index 01ecfe8..893072f 100644
> --- a/plugins/bluez5.h
> +++ b/plugins/bluez5.h
> @@ -19,7 +19,8 @@
> *
> */
>
> -#define BLUEZ_SERVICE "org.bluez"
> +#define BLUEZ_SERVICE "org.bluez"
> +#define BLUEZ_MANAGER_PATH "/"
> #define BLUEZ_PROFILE_INTERFACE BLUEZ_SERVICE ".Profile1"
> #define BLUEZ_ERROR_INTERFACE BLUEZ_SERVICE ".Error"
>
> diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
> index e024838..c5c5cd7 100644
> --- a/plugins/hfp_hf_bluez5.c
> +++ b/plugins/hfp_hf_bluez5.c
> @@ -42,6 +42,8 @@
>
> #define HFP_EXT_PROFILE_PATH "/bluetooth/profile/hfp_hf"
>
> +static GDBusClient *bluez = NULL;
> +
> static int hfp_probe(struct ofono_modem *modem)
> {
> DBG("modem: %p", modem);
> @@ -143,6 +145,60 @@ static const GDBusMethodTable profile_methods[] = {
> { }
> };
>
> +static void connect_handler(DBusConnection *conn, void *user_data)
> +{
> + DBG("Registering External Profile handler ...");
> +
> + if (!g_dbus_register_interface(conn, HFP_EXT_PROFILE_PATH,
> + BLUEZ_PROFILE_INTERFACE,
> + profile_methods, NULL,
> + NULL, NULL, NULL)) {
We just performed this operation in hfp_init, why are we doing it again
here?
> + ofono_error("Register Profile interface failed: %s",
> + HFP_EXT_PROFILE_PATH);
> + }
> +
> + bluetooth_register_profile(conn, HFP_HS_UUID, "hfp_hf",
> + HFP_EXT_PROFILE_PATH);
You have a whitespace violation here, mixing tabs & spaces.
> +}
> +
> +static void disconnect_handler(DBusConnection *conn, void *user_data)
> +{
> +
> + DBG("Unregistering External Profile handler ...");
> +
> + g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
> + BLUEZ_PROFILE_INTERFACE);
Why do we bother? Should we not leave this always registered?
> +}
> +
> +static void proxy_added(GDBusProxy *proxy, void *user_data)
> +{
> + const char *path;
> +
> + path = g_dbus_proxy_get_path(proxy);
> +
> + DBG("Device proxy: %s(%p)", path, proxy);
> +}
> +
> +static void proxy_removed(GDBusProxy *proxy, void *user_data)
> +{
> + const char *path;
> +
> + path = g_dbus_proxy_get_path(proxy);
> +
> + DBG("Device proxy: %s(%p)", path, proxy);
> +}
> +
> +static void property_changed(GDBusProxy *proxy, const char *name,
> + DBusMessageIter *iter, void *user_data)
> +{
> + const char *interface, *path;
> +
> + interface = g_dbus_proxy_get_interface(proxy);
> + path = g_dbus_proxy_get_path(proxy);
> +
> + DBG("path: %s interface: %s", path, interface);
> +}
> +
> static int hfp_init(void)
> {
> DBusConnection *conn = ofono_dbus_get_connection();
> @@ -161,19 +217,16 @@ static int hfp_init(void)
> return -EIO;
> }
>
> - err = ofono_modem_driver_register(&hfp_driver);
> - if (err< 0) {
> - g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
> - BLUEZ_PROFILE_INTERFACE);
> - return err;
> - }
> + bluez = g_dbus_client_new(conn, BLUEZ_SERVICE, BLUEZ_MANAGER_PATH);
> + g_dbus_client_set_connect_watch(bluez, connect_handler, NULL);
> + g_dbus_client_set_disconnect_watch(bluez, disconnect_handler, NULL);
> + g_dbus_client_set_proxy_handlers(bluez, proxy_added, proxy_removed,
> + property_changed, NULL);
Error handling here is all messed up. ofono_modem_driver_register
operation might still fail below and you never destroy the 'bluez'
object in that case.
>
> - err = bluetooth_register_profile(conn, HFP_HS_UUID, "hfp_hf",
> - HFP_EXT_PROFILE_PATH);
> + err = ofono_modem_driver_register(&hfp_driver);
> if (err< 0) {
> g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
> BLUEZ_PROFILE_INTERFACE);
> - ofono_modem_driver_unregister(&hfp_driver);
> return err;
> }
>
> @@ -188,6 +241,7 @@ static void hfp_exit(void)
> g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
> BLUEZ_PROFILE_INTERFACE);
> ofono_modem_driver_unregister(&hfp_driver);
> + g_dbus_client_unref(bluez);
> }
>
> OFONO_PLUGIN_DEFINE(hfp_bluez5, "External Hands-Free Profile Plugin", VERSION,
Regards,
-Denis
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 01/10] hfp_hf_bluez5: Initial GDBusClient for BlueZ
2013-01-23 4:30 ` Denis Kenzior
@ 2013-01-23 14:16 ` Vinicius Costa Gomes
0 siblings, 0 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 14:16 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 4248 bytes --]
Hi Denis,
On 22:30 Tue 22 Jan, Denis Kenzior wrote:
> Hi Vinicius,
>
> On 01/22/2013 03:43 PM, Vinicius Costa Gomes wrote:
> >This patch adds the initial callbacks to track when BlueZ connects or
> >disconnects from the system bus. And tracks the interfaces added or
> >removed.
> >---
> > plugins/bluez5.h | 3 ++-
> > plugins/hfp_hf_bluez5.c | 72 ++++++++++++++++++++++++++++++++++++++++++-------
> > 2 files changed, 65 insertions(+), 10 deletions(-)
> >
[snip]
> >@@ -143,6 +145,60 @@ static const GDBusMethodTable profile_methods[] = {
> > { }
> > };
> >
> >+static void connect_handler(DBusConnection *conn, void *user_data)
> >+{
> >+ DBG("Registering External Profile handler ...");
> >+
> >+ if (!g_dbus_register_interface(conn, HFP_EXT_PROFILE_PATH,
> >+ BLUEZ_PROFILE_INTERFACE,
> >+ profile_methods, NULL,
> >+ NULL, NULL, NULL)) {
>
> We just performed this operation in hfp_init, why are we doing it
> again here?
Most probably it was a copy and paste error because our rebase. Fixed now.
>
> >+ ofono_error("Register Profile interface failed: %s",
> >+ HFP_EXT_PROFILE_PATH);
> >+ }
> >+
> >+ bluetooth_register_profile(conn, HFP_HS_UUID, "hfp_hf",
> >+ HFP_EXT_PROFILE_PATH);
>
> You have a whitespace violation here, mixing tabs & spaces.
Don't know what happened here. Fixed.
>
> >+}
> >+
> >+static void disconnect_handler(DBusConnection *conn, void *user_data)
> >+{
> >+
> >+ DBG("Unregistering External Profile handler ...");
> >+
> >+ g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
> >+ BLUEZ_PROFILE_INTERFACE);
>
> Why do we bother? Should we not leave this always registered?
Fixed. This function is unnecessary really. Thanks for pointing this out.
>
> >+}
> >+
> >+static void proxy_added(GDBusProxy *proxy, void *user_data)
> >+{
> >+ const char *path;
> >+
> >+ path = g_dbus_proxy_get_path(proxy);
> >+
> >+ DBG("Device proxy: %s(%p)", path, proxy);
> >+}
> >+
> >+static void proxy_removed(GDBusProxy *proxy, void *user_data)
> >+{
> >+ const char *path;
> >+
> >+ path = g_dbus_proxy_get_path(proxy);
> >+
> >+ DBG("Device proxy: %s(%p)", path, proxy);
> >+}
> >+
> >+static void property_changed(GDBusProxy *proxy, const char *name,
> >+ DBusMessageIter *iter, void *user_data)
> >+{
> >+ const char *interface, *path;
> >+
> >+ interface = g_dbus_proxy_get_interface(proxy);
> >+ path = g_dbus_proxy_get_path(proxy);
> >+
> >+ DBG("path: %s interface: %s", path, interface);
> >+}
> >+
> > static int hfp_init(void)
> > {
> > DBusConnection *conn = ofono_dbus_get_connection();
> >@@ -161,19 +217,16 @@ static int hfp_init(void)
> > return -EIO;
> > }
> >
> >- err = ofono_modem_driver_register(&hfp_driver);
> >- if (err< 0) {
> >- g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
> >- BLUEZ_PROFILE_INTERFACE);
> >- return err;
> >- }
> >+ bluez = g_dbus_client_new(conn, BLUEZ_SERVICE, BLUEZ_MANAGER_PATH);
> >+ g_dbus_client_set_connect_watch(bluez, connect_handler, NULL);
> >+ g_dbus_client_set_disconnect_watch(bluez, disconnect_handler, NULL);
> >+ g_dbus_client_set_proxy_handlers(bluez, proxy_added, proxy_removed,
> >+ property_changed, NULL);
>
> Error handling here is all messed up. ofono_modem_driver_register
> operation might still fail below and you never destroy the 'bluez'
> object in that case.
Fixed.
>
> >
> >- err = bluetooth_register_profile(conn, HFP_HS_UUID, "hfp_hf",
> >- HFP_EXT_PROFILE_PATH);
> >+ err = ofono_modem_driver_register(&hfp_driver);
> > if (err< 0) {
> > g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
> > BLUEZ_PROFILE_INTERFACE);
> >- ofono_modem_driver_unregister(&hfp_driver);
> > return err;
> > }
> >
> >@@ -188,6 +241,7 @@ static void hfp_exit(void)
> > g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
> > BLUEZ_PROFILE_INTERFACE);
> > ofono_modem_driver_unregister(&hfp_driver);
> >+ g_dbus_client_unref(bluez);
> > }
> >
> > OFONO_PLUGIN_DEFINE(hfp_bluez5, "External Hands-Free Profile Plugin", VERSION,
>
> Regards,
> -Denis
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 02/10] hfp_hf_bluez5: Add GDBusProxy for Bluetooth devices
2013-01-22 21:43 [PATCH 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
2013-01-22 21:43 ` [PATCH 01/10] hfp_hf_bluez5: Initial GDBusClient for BlueZ Vinicius Costa Gomes
@ 2013-01-22 21:43 ` Vinicius Costa Gomes
2013-01-22 21:43 ` [PATCH 03/10] hfp_hf_bluez5: Register modem for HFP AG capable devices Vinicius Costa Gomes
` (8 subsequent siblings)
10 siblings, 0 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-22 21:43 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2697 bytes --]
This patch tracks the GDBusProxy for Bluetooth devices in order to be
able to get its properties.
---
plugins/bluez5.h | 2 ++
plugins/hfp_hf_bluez5.c | 23 ++++++++++++++++++++---
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/plugins/bluez5.h b/plugins/bluez5.h
index 893072f..e232fee 100644
--- a/plugins/bluez5.h
+++ b/plugins/bluez5.h
@@ -22,6 +22,8 @@
#define BLUEZ_SERVICE "org.bluez"
#define BLUEZ_MANAGER_PATH "/"
#define BLUEZ_PROFILE_INTERFACE BLUEZ_SERVICE ".Profile1"
+#define BLUEZ_ADAPTER_INTERFACE BLUEZ_SERVICE".Adapter1"
+#define BLUEZ_DEVICE_INTERFACE BLUEZ_SERVICE".Device1"
#define BLUEZ_ERROR_INTERFACE BLUEZ_SERVICE ".Error"
#define HFP_HS_UUID "0000111e-0000-1000-8000-00805f9b34fb"
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index c5c5cd7..2daa4c9 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -42,6 +42,7 @@
#define HFP_EXT_PROFILE_PATH "/bluetooth/profile/hfp_hf"
+static GHashTable *devices_proxies = NULL;
static GDBusClient *bluez = NULL;
static int hfp_probe(struct ofono_modem *modem)
@@ -172,20 +173,31 @@ static void disconnect_handler(DBusConnection *conn, void *user_data)
static void proxy_added(GDBusProxy *proxy, void *user_data)
{
- const char *path;
+ const char *interface, *path;
+ interface = g_dbus_proxy_get_interface(proxy);
path = g_dbus_proxy_get_path(proxy);
+ if (g_str_equal(BLUEZ_DEVICE_INTERFACE, interface) == FALSE)
+ return;
+
+ g_hash_table_insert(devices_proxies, g_strdup(path),
+ g_dbus_proxy_ref(proxy));
DBG("Device proxy: %s(%p)", path, proxy);
}
static void proxy_removed(GDBusProxy *proxy, void *user_data)
{
- const char *path;
+ const char *interface, *path;
+ interface = g_dbus_proxy_get_interface(proxy);
path = g_dbus_proxy_get_path(proxy);
- DBG("Device proxy: %s(%p)", path, proxy);
+ if (g_str_equal(BLUEZ_DEVICE_INTERFACE, interface)) {
+ g_hash_table_remove(devices_proxies, path);
+ DBG("Device proxy: %s(%p)", path, proxy);
+ }
+
}
static void property_changed(GDBusProxy *proxy, const char *name,
@@ -230,6 +242,9 @@ static int hfp_init(void)
return err;
}
+ devices_proxies = g_hash_table_new_full(g_str_hash, g_str_equal,
+ g_free, (GDestroyNotify) g_dbus_proxy_unref);
+
return 0;
}
@@ -242,6 +257,8 @@ static void hfp_exit(void)
BLUEZ_PROFILE_INTERFACE);
ofono_modem_driver_unregister(&hfp_driver);
g_dbus_client_unref(bluez);
+
+ g_hash_table_destroy(devices_proxies);
}
OFONO_PLUGIN_DEFINE(hfp_bluez5, "External Hands-Free Profile Plugin", VERSION,
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 03/10] hfp_hf_bluez5: Register modem for HFP AG capable devices
2013-01-22 21:43 [PATCH 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
2013-01-22 21:43 ` [PATCH 01/10] hfp_hf_bluez5: Initial GDBusClient for BlueZ Vinicius Costa Gomes
2013-01-22 21:43 ` [PATCH 02/10] hfp_hf_bluez5: Add GDBusProxy for Bluetooth devices Vinicius Costa Gomes
@ 2013-01-22 21:43 ` Vinicius Costa Gomes
2013-01-23 4:59 ` Denis Kenzior
2013-01-22 21:43 ` [PATCH 04/10] hfp_hf_bluez5: Keep track of changes of devices Aliases Vinicius Costa Gomes
` (7 subsequent siblings)
10 siblings, 1 reply; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-22 21:43 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 4570 bytes --]
Now that we are able to keep track of devices appearing and
disappearing from BlueZ, we are able to register the modem when a
device that supports the HFP AG UUID appears.
---
plugins/bluez5.h | 1 +
plugins/hfp_hf_bluez5.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 88 insertions(+), 1 deletion(-)
diff --git a/plugins/bluez5.h b/plugins/bluez5.h
index e232fee..dbb1d8d 100644
--- a/plugins/bluez5.h
+++ b/plugins/bluez5.h
@@ -27,6 +27,7 @@
#define BLUEZ_ERROR_INTERFACE BLUEZ_SERVICE ".Error"
#define HFP_HS_UUID "0000111e-0000-1000-8000-00805f9b34fb"
+#define HFP_AG_UUID "0000111f-0000-1000-8000-00805f9b34fb"
int bluetooth_register_profile(DBusConnection *conn, const char *uuid,
const char *name, const char *object);
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 2daa4c9..35b9eb9 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -42,9 +42,37 @@
#define HFP_EXT_PROFILE_PATH "/bluetooth/profile/hfp_hf"
+static GHashTable *modem_hash = NULL;
static GHashTable *devices_proxies = NULL;
static GDBusClient *bluez = NULL;
+static struct ofono_modem *modem_register(const char *device,
+ const char *alias)
+{
+ struct ofono_modem *modem;
+ char *path;
+
+ modem = g_hash_table_lookup(modem_hash, device);
+ if (modem != NULL)
+ return modem;
+
+ path = g_strconcat("hfp", device, NULL);
+
+ modem = ofono_modem_create(path, "hfp");
+
+ g_free(path);
+
+ if (modem == NULL)
+ return NULL;
+
+ ofono_modem_set_name(modem, alias);
+ ofono_modem_register(modem);
+
+ g_hash_table_insert(modem_hash, g_strdup(device), modem);
+
+ return modem;
+}
+
static int hfp_probe(struct ofono_modem *modem)
{
DBG("modem: %p", modem);
@@ -171,9 +199,32 @@ static void disconnect_handler(DBusConnection *conn, void *user_data)
BLUEZ_PROFILE_INTERFACE);
}
+static void parse_uuids(DBusMessageIter *array, gpointer user_data)
+{
+ GSList **uuids = user_data;
+ DBusMessageIter value;
+
+ if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
+ return;
+
+ dbus_message_iter_recurse(array, &value);
+
+ while (dbus_message_iter_get_arg_type(&value) == DBUS_TYPE_STRING) {
+ const char *uuid;
+
+ dbus_message_iter_get_basic(&value, &uuid);
+
+ *uuids = g_slist_prepend(*uuids, g_strdup(uuid));
+
+ dbus_message_iter_next(&value);
+ }
+}
+
static void proxy_added(GDBusProxy *proxy, void *user_data)
{
- const char *interface, *path;
+ const char *interface, *path, *alias;
+ DBusMessageIter iter;
+ GSList *l, *uuids = NULL;
interface = g_dbus_proxy_get_interface(proxy);
path = g_dbus_proxy_get_path(proxy);
@@ -184,11 +235,36 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
g_hash_table_insert(devices_proxies, g_strdup(path),
g_dbus_proxy_ref(proxy));
DBG("Device proxy: %s(%p)", path, proxy);
+
+ if (g_dbus_proxy_get_property(proxy, "UUIDs", &iter) == FALSE)
+ return;
+
+ parse_uuids(&iter, &uuids);
+
+ for (l = uuids; l; l = l->next) {
+ const char *uuid = l->data;
+
+ if (g_str_equal(uuid, HFP_AG_UUID) == TRUE)
+ break;
+ }
+
+ g_slist_free_full(uuids, g_free);
+
+ if (l == NULL)
+ return;
+
+ if (g_dbus_proxy_get_property(proxy, "Alias", &iter) == FALSE)
+ return;
+
+ dbus_message_iter_get_basic(&iter, &alias);
+
+ modem_register(path, alias);
}
static void proxy_removed(GDBusProxy *proxy, void *user_data)
{
const char *interface, *path;
+ struct ofono_modem *modem;
interface = g_dbus_proxy_get_interface(proxy);
path = g_dbus_proxy_get_path(proxy);
@@ -198,6 +274,12 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
DBG("Device proxy: %s(%p)", path, proxy);
}
+ modem = g_hash_table_lookup(modem_hash, path);
+ if (modem == NULL)
+ return;
+
+ g_hash_table_remove(modem_hash, path);
+ ofono_modem_remove(modem);
}
static void property_changed(GDBusProxy *proxy, const char *name,
@@ -242,6 +324,9 @@ static int hfp_init(void)
return err;
}
+ modem_hash = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,
+ NULL);
+
devices_proxies = g_hash_table_new_full(g_str_hash, g_str_equal,
g_free, (GDestroyNotify) g_dbus_proxy_unref);
@@ -258,6 +343,7 @@ static void hfp_exit(void)
ofono_modem_driver_unregister(&hfp_driver);
g_dbus_client_unref(bluez);
+ g_hash_table_destroy(modem_hash);
g_hash_table_destroy(devices_proxies);
}
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 03/10] hfp_hf_bluez5: Register modem for HFP AG capable devices
2013-01-22 21:43 ` [PATCH 03/10] hfp_hf_bluez5: Register modem for HFP AG capable devices Vinicius Costa Gomes
@ 2013-01-23 4:59 ` Denis Kenzior
2013-01-23 14:22 ` Vinicius Costa Gomes
0 siblings, 1 reply; 38+ messages in thread
From: Denis Kenzior @ 2013-01-23 4:59 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 5432 bytes --]
Hi Vinicius,
On 01/22/2013 03:43 PM, Vinicius Costa Gomes wrote:
> Now that we are able to keep track of devices appearing and
> disappearing from BlueZ, we are able to register the modem when a
> device that supports the HFP AG UUID appears.
> ---
> plugins/bluez5.h | 1 +
> plugins/hfp_hf_bluez5.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 88 insertions(+), 1 deletion(-)
>
> diff --git a/plugins/bluez5.h b/plugins/bluez5.h
> index e232fee..dbb1d8d 100644
> --- a/plugins/bluez5.h
> +++ b/plugins/bluez5.h
> @@ -27,6 +27,7 @@
> #define BLUEZ_ERROR_INTERFACE BLUEZ_SERVICE ".Error"
>
> #define HFP_HS_UUID "0000111e-0000-1000-8000-00805f9b34fb"
> +#define HFP_AG_UUID "0000111f-0000-1000-8000-00805f9b34fb"
>
> int bluetooth_register_profile(DBusConnection *conn, const char *uuid,
> const char *name, const char *object);
> diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
> index 2daa4c9..35b9eb9 100644
> --- a/plugins/hfp_hf_bluez5.c
> +++ b/plugins/hfp_hf_bluez5.c
> @@ -42,9 +42,37 @@
>
> #define HFP_EXT_PROFILE_PATH "/bluetooth/profile/hfp_hf"
>
> +static GHashTable *modem_hash = NULL;
> static GHashTable *devices_proxies = NULL;
> static GDBusClient *bluez = NULL;
>
> +static struct ofono_modem *modem_register(const char *device,
> + const char *alias)
> +{
> + struct ofono_modem *modem;
> + char *path;
> +
> + modem = g_hash_table_lookup(modem_hash, device);
> + if (modem != NULL)
> + return modem;
> +
> + path = g_strconcat("hfp", device, NULL);
> +
> + modem = ofono_modem_create(path, "hfp");
> +
> + g_free(path);
> +
> + if (modem == NULL)
> + return NULL;
> +
> + ofono_modem_set_name(modem, alias);
> + ofono_modem_register(modem);
> +
> + g_hash_table_insert(modem_hash, g_strdup(device), modem);
> +
> + return modem;
> +}
> +
> static int hfp_probe(struct ofono_modem *modem)
> {
> DBG("modem: %p", modem);
> @@ -171,9 +199,32 @@ static void disconnect_handler(DBusConnection *conn, void *user_data)
> BLUEZ_PROFILE_INTERFACE);
> }
>
> +static void parse_uuids(DBusMessageIter *array, gpointer user_data)
> +{
> + GSList **uuids = user_data;
> + DBusMessageIter value;
> +
> + if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
> + return;
> +
> + dbus_message_iter_recurse(array,&value);
> +
> + while (dbus_message_iter_get_arg_type(&value) == DBUS_TYPE_STRING) {
> + const char *uuid;
> +
> + dbus_message_iter_get_basic(&value,&uuid);
> +
> + *uuids = g_slist_prepend(*uuids, g_strdup(uuid));
> +
> + dbus_message_iter_next(&value);
Okay, so, we build a list of strings, g_strdup()ing as we go along..
> + }
> +}
> +
> static void proxy_added(GDBusProxy *proxy, void *user_data)
> {
> - const char *interface, *path;
> + const char *interface, *path, *alias;
> + DBusMessageIter iter;
> + GSList *l, *uuids = NULL;
>
> interface = g_dbus_proxy_get_interface(proxy);
> path = g_dbus_proxy_get_path(proxy);
> @@ -184,11 +235,36 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
> g_hash_table_insert(devices_proxies, g_strdup(path),
> g_dbus_proxy_ref(proxy));
> DBG("Device proxy: %s(%p)", path, proxy);
> +
> + if (g_dbus_proxy_get_property(proxy, "UUIDs",&iter) == FALSE)
> + return;
> +
> + parse_uuids(&iter,&uuids);
> +
> + for (l = uuids; l; l = l->next) {
> + const char *uuid = l->data;
> +
Next we traverse the list
> + if (g_str_equal(uuid, HFP_AG_UUID) == TRUE)
> + break;
And if we find the UUID we break
> + }
> +
> + g_slist_free_full(uuids, g_free);
> +
And now we free the all the g_strdup()ed strings. Why? Why do we bother
doing all this work? Just rename parse_uuids into something sensible,
like 'has_hfp_ag_uuid' and make it return a boolean.
> + if (l == NULL)
> + return;
> +
> + if (g_dbus_proxy_get_property(proxy, "Alias",&iter) == FALSE)
> + return;
> +
> + dbus_message_iter_get_basic(&iter,&alias);
> +
> + modem_register(path, alias);
And why do we do all this work without first checking whether the modem
is already in the modem_hash?
> }
>
> static void proxy_removed(GDBusProxy *proxy, void *user_data)
> {
> const char *interface, *path;
> + struct ofono_modem *modem;
>
> interface = g_dbus_proxy_get_interface(proxy);
> path = g_dbus_proxy_get_path(proxy);
> @@ -198,6 +274,12 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
> DBG("Device proxy: %s(%p)", path, proxy);
> }
>
> + modem = g_hash_table_lookup(modem_hash, path);
> + if (modem == NULL)
> + return;
> +
> + g_hash_table_remove(modem_hash, path);
> + ofono_modem_remove(modem);
> }
>
> static void property_changed(GDBusProxy *proxy, const char *name,
> @@ -242,6 +324,9 @@ static int hfp_init(void)
> return err;
> }
>
> + modem_hash = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,
> + NULL);
> +
> devices_proxies = g_hash_table_new_full(g_str_hash, g_str_equal,
> g_free, (GDestroyNotify) g_dbus_proxy_unref);
>
> @@ -258,6 +343,7 @@ static void hfp_exit(void)
> ofono_modem_driver_unregister(&hfp_driver);
> g_dbus_client_unref(bluez);
>
> + g_hash_table_destroy(modem_hash);
> g_hash_table_destroy(devices_proxies);
> }
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: [PATCH 03/10] hfp_hf_bluez5: Register modem for HFP AG capable devices
2013-01-23 4:59 ` Denis Kenzior
@ 2013-01-23 14:22 ` Vinicius Costa Gomes
0 siblings, 0 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 14:22 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 3446 bytes --]
Hi Denis,
On 22:59 Tue 22 Jan, Denis Kenzior wrote:
> Hi Vinicius,
>
> On 01/22/2013 03:43 PM, Vinicius Costa Gomes wrote:
> >Now that we are able to keep track of devices appearing and
> >disappearing from BlueZ, we are able to register the modem when a
> >device that supports the HFP AG UUID appears.
> >---
> > plugins/bluez5.h | 1 +
> > plugins/hfp_hf_bluez5.c | 88 ++++++++++++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 88 insertions(+), 1 deletion(-)
> >
[snip]
> >@@ -184,11 +235,36 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
> > g_hash_table_insert(devices_proxies, g_strdup(path),
> > g_dbus_proxy_ref(proxy));
> > DBG("Device proxy: %s(%p)", path, proxy);
> >+
> >+ if (g_dbus_proxy_get_property(proxy, "UUIDs",&iter) == FALSE)
> >+ return;
> >+
> >+ parse_uuids(&iter,&uuids);
> >+
> >+ for (l = uuids; l; l = l->next) {
> >+ const char *uuid = l->data;
> >+
>
> Next we traverse the list
>
> >+ if (g_str_equal(uuid, HFP_AG_UUID) == TRUE)
> >+ break;
>
> And if we find the UUID we break
>
> >+ }
> >+
> >+ g_slist_free_full(uuids, g_free);
> >+
>
> And now we free the all the g_strdup()ed strings. Why? Why do we
> bother doing all this work? Just rename parse_uuids into something
> sensible, like 'has_hfp_ag_uuid' and make it return a boolean.
Cool. Thanks. I made it return a gboolean, but couldn't find a standard about
to return a gboolean or a ofono_bool_t.
>
> >+ if (l == NULL)
> >+ return;
> >+
> >+ if (g_dbus_proxy_get_property(proxy, "Alias",&iter) == FALSE)
> >+ return;
> >+
> >+ dbus_message_iter_get_basic(&iter,&alias);
> >+
> >+ modem_register(path, alias);
>
> And why do we do all this work without first checking whether the
> modem is already in the modem_hash?
This would only happen on a very uncommon case: we receive from BlueZ a
NewConnection() (we register the modem there) and after that we receive
the InterfacesAdded signal. I don't think the increase in complexity is worth.
And inside the modem_register() we check if the modem is already registered.
>
> > }
> >
> > static void proxy_removed(GDBusProxy *proxy, void *user_data)
> > {
> > const char *interface, *path;
> >+ struct ofono_modem *modem;
> >
> > interface = g_dbus_proxy_get_interface(proxy);
> > path = g_dbus_proxy_get_path(proxy);
> >@@ -198,6 +274,12 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
> > DBG("Device proxy: %s(%p)", path, proxy);
> > }
> >
> >+ modem = g_hash_table_lookup(modem_hash, path);
> >+ if (modem == NULL)
> >+ return;
> >+
> >+ g_hash_table_remove(modem_hash, path);
> >+ ofono_modem_remove(modem);
> > }
> >
> > static void property_changed(GDBusProxy *proxy, const char *name,
> >@@ -242,6 +324,9 @@ static int hfp_init(void)
> > return err;
> > }
> >
> >+ modem_hash = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,
> >+ NULL);
> >+
> > devices_proxies = g_hash_table_new_full(g_str_hash, g_str_equal,
> > g_free, (GDestroyNotify) g_dbus_proxy_unref);
> >
> >@@ -258,6 +343,7 @@ static void hfp_exit(void)
> > ofono_modem_driver_unregister(&hfp_driver);
> > g_dbus_client_unref(bluez);
> >
> >+ g_hash_table_destroy(modem_hash);
> > g_hash_table_destroy(devices_proxies);
> > }
> >
>
> Regards,
> -Denis
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 04/10] hfp_hf_bluez5: Keep track of changes of devices Aliases
2013-01-22 21:43 [PATCH 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (2 preceding siblings ...)
2013-01-22 21:43 ` [PATCH 03/10] hfp_hf_bluez5: Register modem for HFP AG capable devices Vinicius Costa Gomes
@ 2013-01-22 21:43 ` Vinicius Costa Gomes
2013-01-22 21:43 ` [PATCH 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ Vinicius Costa Gomes
` (6 subsequent siblings)
10 siblings, 0 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-22 21:43 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1236 bytes --]
If the device Alias property changes we should also change the name of
the modem.
---
plugins/hfp_hf_bluez5.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 35b9eb9..74ca570 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -285,12 +285,28 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
static void property_changed(GDBusProxy *proxy, const char *name,
DBusMessageIter *iter, void *user_data)
{
- const char *interface, *path;
+ const char *interface, *path, *alias;
+ struct ofono_modem *modem;
+ DBusMessageIter alias_iter;
interface = g_dbus_proxy_get_interface(proxy);
path = g_dbus_proxy_get_path(proxy);
DBG("path: %s interface: %s", path, interface);
+
+ if (g_str_equal(BLUEZ_DEVICE_INTERFACE, interface) == FALSE)
+ return;
+
+ if (g_dbus_proxy_get_property(proxy, "Alias", &alias_iter) == FALSE)
+ return;
+
+ dbus_message_iter_get_basic(&alias_iter, &alias);
+
+ modem = g_hash_table_lookup(modem_hash, path);
+ if (modem == NULL)
+ return;
+
+ ofono_modem_set_name(modem, alias);
}
static int hfp_init(void)
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ
2013-01-22 21:43 [PATCH 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (3 preceding siblings ...)
2013-01-22 21:43 ` [PATCH 04/10] hfp_hf_bluez5: Keep track of changes of devices Aliases Vinicius Costa Gomes
@ 2013-01-22 21:43 ` Vinicius Costa Gomes
2013-01-23 4:54 ` Denis Kenzior
2013-01-23 12:03 ` AW: " Kling, Andreas
2013-01-22 21:43 ` [PATCH 06/10] hfpmodem: Add dynamic hfp_slc_info allocation Vinicius Costa Gomes
` (5 subsequent siblings)
10 siblings, 2 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-22 21:43 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2163 bytes --]
Parse the essential arguments in the message, in this case only the
file descriptor, and register the modem if it is not already
registered. This is necessary because in some cases, we may receive a
NewConnection call, and the SDP process is still taking place.
---
plugins/hfp_hf_bluez5.c | 47 ++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 44 insertions(+), 3 deletions(-)
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 74ca570..39853e3 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -24,6 +24,8 @@
#endif
#include <errno.h>
+#include <unistd.h>
+
#include <glib.h>
#include <gdbus.h>
@@ -124,11 +126,50 @@ static struct ofono_modem_driver hfp_driver = {
static DBusMessage *profile_new_connection(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
+ struct ofono_modem *modem;
+ DBusMessageIter iter;
+ GDBusProxy *proxy;
+ DBusMessageIter entry;
+ const char *device, *alias;
+ int fd;
+
DBG("Profile handler NewConnection");
- return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
- ".NotImplemented",
- "Implementation not provided");
+ if (dbus_message_iter_init(msg, &entry) == FALSE)
+ goto error;
+
+ if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_OBJECT_PATH)
+ goto error;
+
+ dbus_message_iter_get_basic(&entry, &device);
+
+ proxy = g_hash_table_lookup(devices_proxies, device);
+ if (proxy == NULL)
+ goto error;
+
+ if (g_dbus_proxy_get_property(proxy, "Alias", &iter) == FALSE)
+ alias = "unknown";
+ else
+ dbus_message_iter_get_basic(&iter, &alias);
+
+ dbus_message_iter_next(&entry);
+ if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_UNIX_FD)
+ goto error;
+
+ dbus_message_iter_get_basic(&entry, &fd);
+ if (fd < 0)
+ goto error;
+
+ modem = modem_register(device, alias);
+ if (modem == NULL)
+ goto error;
+
+ return NULL;
+
+error:
+ close(fd);
+ return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE ".Rejected",
+ "Invalid arguments in method call");
}
static DBusMessage *profile_release(DBusConnection *conn,
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ
2013-01-22 21:43 ` [PATCH 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ Vinicius Costa Gomes
@ 2013-01-23 4:54 ` Denis Kenzior
2013-01-23 12:03 ` AW: " Kling, Andreas
1 sibling, 0 replies; 38+ messages in thread
From: Denis Kenzior @ 2013-01-23 4:54 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2576 bytes --]
Hi Vinicius,
On 01/22/2013 03:43 PM, Vinicius Costa Gomes wrote:
> Parse the essential arguments in the message, in this case only the
> file descriptor, and register the modem if it is not already
> registered. This is necessary because in some cases, we may receive a
> NewConnection call, and the SDP process is still taking place.
> ---
> plugins/hfp_hf_bluez5.c | 47 ++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 44 insertions(+), 3 deletions(-)
>
> diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
> index 74ca570..39853e3 100644
> --- a/plugins/hfp_hf_bluez5.c
> +++ b/plugins/hfp_hf_bluez5.c
> @@ -24,6 +24,8 @@
> #endif
>
> #include<errno.h>
> +#include<unistd.h>
> +
> #include<glib.h>
>
> #include<gdbus.h>
> @@ -124,11 +126,50 @@ static struct ofono_modem_driver hfp_driver = {
> static DBusMessage *profile_new_connection(DBusConnection *conn,
> DBusMessage *msg, void *user_data)
> {
> + struct ofono_modem *modem;
> + DBusMessageIter iter;
> + GDBusProxy *proxy;
> + DBusMessageIter entry;
> + const char *device, *alias;
> + int fd;
> +
> DBG("Profile handler NewConnection");
>
> - return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
> - ".NotImplemented",
> - "Implementation not provided");
> + if (dbus_message_iter_init(msg,&entry) == FALSE)
> + goto error;
> +
> + if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_OBJECT_PATH)
> + goto error;
> +
> + dbus_message_iter_get_basic(&entry,&device);
> +
> + proxy = g_hash_table_lookup(devices_proxies, device);
> + if (proxy == NULL)
> + goto error;
So we return 'Invalid Arguments' even if we failed to lookup the proxy
in the hash? Lets be a bit nicer here
> +
> + if (g_dbus_proxy_get_property(proxy, "Alias",&iter) == FALSE)
> + alias = "unknown";
> + else
> + dbus_message_iter_get_basic(&iter,&alias);
This looks suspicious, under what circumstances are we missing the alias?
> +
> + dbus_message_iter_next(&entry);
> + if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_UNIX_FD)
> + goto error;
> +
> + dbus_message_iter_get_basic(&entry,&fd);
> + if (fd< 0)
> + goto error;
> +
> + modem = modem_register(device, alias);
> + if (modem == NULL)
> + goto error;
> +
> + return NULL;
> +
> +error:
> + close(fd);
> + return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE ".Rejected",
> + "Invalid arguments in method call");
> }
>
> static DBusMessage *profile_release(DBusConnection *conn,
Regards,
-Denis
^ permalink raw reply [flat|nested] 38+ messages in thread* AW: [PATCH 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ
2013-01-22 21:43 ` [PATCH 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ Vinicius Costa Gomes
2013-01-23 4:54 ` Denis Kenzior
@ 2013-01-23 12:03 ` Kling, Andreas
2013-01-23 14:49 ` Denis Kenzior
1 sibling, 1 reply; 38+ messages in thread
From: Kling, Andreas @ 2013-01-23 12:03 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2880 bytes --]
Hi,
>Parse the essential arguments in the message, in this case only the
>file descriptor, and register the modem if it is not already
>registered. This is necessary because in some cases, we may receive a
>NewConnection call, and the SDP process is still taking place.
>---
> plugins/hfp_hf_bluez5.c | 47 ++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 44 insertions(+), 3 deletions(-)
>
>diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
>index 74ca570..39853e3 100644
>--- a/plugins/hfp_hf_bluez5.c
>+++ b/plugins/hfp_hf_bluez5.c
>@@ -24,6 +24,8 @@
> #endif
>
> #include <errno.h>
>+#include <unistd.h>
>+
> #include <glib.h>
>
> #include <gdbus.h>
>@@ -124,11 +126,50 @@ static struct ofono_modem_driver hfp_driver = {
> static DBusMessage *profile_new_connection(DBusConnection *conn,
> DBusMessage *msg, void *user_data)
> {
>+ struct ofono_modem *modem;
>+ DBusMessageIter iter;
>+ GDBusProxy *proxy;
>+ DBusMessageIter entry;
>+ const char *device, *alias;
>+ int fd;
>+
> DBG("Profile handler NewConnection");
>
>- return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
>- ".NotImplemented",
>- "Implementation not provided");
>+ if (dbus_message_iter_init(msg, &entry) == FALSE)
>+ goto error;
>+
>+ if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_OBJECT_PATH)
>+ goto error;
>+
>+ dbus_message_iter_get_basic(&entry, &device);
>+
>+ proxy = g_hash_table_lookup(devices_proxies, device);
>+ if (proxy == NULL)
>+ goto error;
>+
>+ if (g_dbus_proxy_get_property(proxy, "Alias", &iter) == FALSE)
>+ alias = "unknown";
>+ else
>+ dbus_message_iter_get_basic(&iter, &alias);
>+
>+ dbus_message_iter_next(&entry);
>+ if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_UNIX_FD)
>+ goto error;
>+
>+ dbus_message_iter_get_basic(&entry, &fd);
>+ if (fd < 0)
>+ goto error;
>+
>+ modem = modem_register(device, alias);
>+ if (modem == NULL)
>+ goto error;
>+
>+ return NULL;
>+
>+error:
>+ close(fd);
>+ return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE ".Rejected",
>+ "Invalid arguments in method call");
> }
>
> static DBusMessage *profile_release(DBusConnection *conn,
>--
>1.8.1.1
fd is closed always on error, even if uninitialized
I assume returning NULL is ok here if nothing went wrong?
I'm not familiar with plain dbus API, just using glib gdbus, there I would expect to create an answer to the call.
kind regards
andy
^ permalink raw reply [flat|nested] 38+ messages in thread* Re: AW: [PATCH 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ
2013-01-23 12:03 ` AW: " Kling, Andreas
@ 2013-01-23 14:49 ` Denis Kenzior
2013-01-23 16:39 ` Vinicius Costa Gomes
0 siblings, 1 reply; 38+ messages in thread
From: Denis Kenzior @ 2013-01-23 14:49 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 646 bytes --]
Hi Andreas,
<snip>
> I assume returning NULL is ok here if nothing went wrong?
> I'm not familiar with plain dbus API, just using glib gdbus, there I would expect to create an answer to the call.
The NewConnection method is marked as 'ASYNC' in the method table (see
profile_methods). In our gdbus library, returning NULL here means that
the method return is deferred until a later time. E.g. when we try to
establish a SLC connection and fail / succeed.
In this patch it is left kind of hanging out there and you would need to
peek at patch 10 to really know what happens to it. Confusing, I know ;)
Regards,
-Denis
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: AW: [PATCH 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ
2013-01-23 14:49 ` Denis Kenzior
@ 2013-01-23 16:39 ` Vinicius Costa Gomes
0 siblings, 0 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 16:39 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 979 bytes --]
Hi,
On 08:49 Wed 23 Jan, Denis Kenzior wrote:
> Hi Andreas,
>
> <snip>
>
> >I assume returning NULL is ok here if nothing went wrong?
> >I'm not familiar with plain dbus API, just using glib gdbus, there I would expect to create an answer to the call.
>
> The NewConnection method is marked as 'ASYNC' in the method table
> (see profile_methods). In our gdbus library, returning NULL here
> means that the method return is deferred until a later time. E.g.
> when we try to establish a SLC connection and fail / succeed.
>
> In this patch it is left kind of hanging out there and you would
> need to peek at patch 10 to really know what happens to it.
> Confusing, I know ;)
Do you think it is worth adding a comment in the code, that would be later
removed?
>
> Regards,
> -Denis
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH 06/10] hfpmodem: Add dynamic hfp_slc_info allocation
2013-01-22 21:43 [PATCH 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (4 preceding siblings ...)
2013-01-22 21:43 ` [PATCH 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ Vinicius Costa Gomes
@ 2013-01-22 21:43 ` Vinicius Costa Gomes
2013-01-22 21:43 ` [PATCH 07/10] hfpmodem: Implement hfp_slc_info_free Vinicius Costa Gomes
` (4 subsequent siblings)
10 siblings, 0 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-22 21:43 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 5573 bytes --]
It less error prone to have hfp_slc_info allocated dynamically than
having it allocated statically, as it makes the verification that it
was already de-allocated more direct.
---
drivers/hfpmodem/slc.c | 9 ++++++++-
drivers/hfpmodem/slc.h | 3 ++-
plugins/hfp_hf_bluez4.c | 35 ++++++++++++++++++-----------------
plugins/phonesim.c | 4 ++--
4 files changed, 30 insertions(+), 21 deletions(-)
diff --git a/drivers/hfpmodem/slc.c b/drivers/hfpmodem/slc.c
index 646befa..825b8a9 100644
--- a/drivers/hfpmodem/slc.c
+++ b/drivers/hfpmodem/slc.c
@@ -52,8 +52,13 @@ struct slc_establish_data {
gpointer userdata;
};
-void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version)
+struct hfp_slc_info *hfp_slc_info_init(GAtChat *chat, guint16 version)
{
+ struct hfp_slc_info *info;
+
+ info = g_new0(struct hfp_slc_info, 1);
+ info->chat = g_at_chat_ref(chat);
+
info->ag_features = 0;
info->ag_mpty_features = 0;
@@ -75,6 +80,8 @@ void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version)
done:
memset(info->cind_val, 0, sizeof(info->cind_val));
memset(info->cind_pos, 0, sizeof(info->cind_pos));
+
+ return info;
}
static void slc_establish_data_unref(gpointer userdata)
diff --git a/drivers/hfpmodem/slc.h b/drivers/hfpmodem/slc.h
index b71ffe9..8ae8d2f 100644
--- a/drivers/hfpmodem/slc.h
+++ b/drivers/hfpmodem/slc.h
@@ -55,7 +55,8 @@ struct hfp_slc_info {
unsigned int cind_val[HFP_INDICATOR_LAST];
};
-void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version);
+struct hfp_slc_info *hfp_slc_info_init(GAtChat *chat, guint16 version);
+
void hfp_slc_info_free(struct hfp_slc_info *info);
void hfp_slc_establish(struct hfp_slc_info *info, hfp_slc_cb_t connect_cb,
diff --git a/plugins/hfp_hf_bluez4.c b/plugins/hfp_hf_bluez4.c
index 450c183..b0aa1aa 100644
--- a/plugins/hfp_hf_bluez4.c
+++ b/plugins/hfp_hf_bluez4.c
@@ -62,7 +62,7 @@ static DBusConnection *connection;
static GHashTable *modem_hash = NULL;
struct hfp_data {
- struct hfp_slc_info info;
+ struct hfp_slc_info *info;
char *handsfree_path;
char *handsfree_address;
DBusMessage *slc_msg;
@@ -109,8 +109,8 @@ static void slc_failed(gpointer userdata)
ofono_error("Service level connection failed");
ofono_modem_set_powered(modem, FALSE);
- g_at_chat_unref(data->info.chat);
- data->info.chat = NULL;
+ g_at_chat_unref(data->info->chat);
+ data->info->chat = NULL;
}
static void hfp_disconnected_cb(gpointer user_data)
@@ -120,12 +120,13 @@ static void hfp_disconnected_cb(gpointer user_data)
ofono_modem_set_powered(modem, FALSE);
- g_at_chat_unref(data->info.chat);
- data->info.chat = NULL;
+ g_at_chat_unref(data->info->chat);
+ data->info->chat = NULL;
}
/* either oFono or Phone could request SLC connection */
-static int service_level_connection(struct ofono_modem *modem, int fd)
+static int service_level_connection(struct ofono_modem *modem, int fd,
+ guint16 version)
{
struct hfp_data *data = ofono_modem_get_data(modem);
GIOChannel *io;
@@ -152,8 +153,10 @@ static int service_level_connection(struct ofono_modem *modem, int fd)
if (getenv("OFONO_AT_DEBUG"))
g_at_chat_set_debug(chat, hfp_debug, "");
- data->info.chat = chat;
- hfp_slc_establish(&data->info, slc_established, slc_failed, modem);
+ data->info = hfp_slc_info_init(chat, version);
+ g_at_chat_unref(chat);
+
+ hfp_slc_establish(data->info, slc_established, slc_failed, modem);
return -EINPROGRESS;
}
@@ -170,9 +173,7 @@ static DBusMessage *hfp_agent_new_connection(DBusConnection *conn,
DBUS_TYPE_UINT16, &version, DBUS_TYPE_INVALID))
return __ofono_error_invalid_args(msg);
- hfp_slc_info_init(&hfp_data->info, version);
-
- err = service_level_connection(modem, fd);
+ err = service_level_connection(modem, fd, version);
if (err < 0 && err != -EINPROGRESS)
return __ofono_error_failed(msg);
@@ -462,8 +463,8 @@ static int hfp_disable(struct ofono_modem *modem)
DBG("%p", modem);
- g_at_chat_unref(data->info.chat);
- data->info.chat = NULL;
+ g_at_chat_unref(data->info->chat);
+ data->info->chat = NULL;
if (data->agent_registered) {
status = bluetooth_send_with_reply(data->handsfree_path,
@@ -486,10 +487,10 @@ static void hfp_pre_sim(struct ofono_modem *modem)
DBG("%p", modem);
ofono_devinfo_create(modem, 0, "hfpmodem", data->handsfree_address);
- ofono_voicecall_create(modem, 0, "hfpmodem", &data->info);
- ofono_netreg_create(modem, 0, "hfpmodem", &data->info);
- ofono_call_volume_create(modem, 0, "hfpmodem", &data->info);
- ofono_handsfree_create(modem, 0, "hfpmodem", &data->info);
+ ofono_voicecall_create(modem, 0, "hfpmodem", data->info);
+ ofono_netreg_create(modem, 0, "hfpmodem", data->info);
+ ofono_call_volume_create(modem, 0, "hfpmodem", data->info);
+ ofono_handsfree_create(modem, 0, "hfpmodem", data->info);
}
static void hfp_post_sim(struct ofono_modem *modem)
diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index 26f96d0..9210e02 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -929,8 +929,8 @@ static int localhfp_enable(struct ofono_modem *modem)
g_at_chat_set_disconnect_function(chat, slc_failed, modem);
- hfp_slc_info_init(info, HFP_VERSION_LATEST);
- info->chat = chat;
+ info = hfp_slc_info_init(chat, HFP_VERSION_LATEST);
+ g_at_chat_unref(chat);
hfp_slc_establish(info, slc_established, slc_failed, modem);
return -EINPROGRESS;
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 07/10] hfpmodem: Implement hfp_slc_info_free
2013-01-22 21:43 [PATCH 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (5 preceding siblings ...)
2013-01-22 21:43 ` [PATCH 06/10] hfpmodem: Add dynamic hfp_slc_info allocation Vinicius Costa Gomes
@ 2013-01-22 21:43 ` Vinicius Costa Gomes
2013-01-22 21:43 ` [PATCH 08/10] hfp_hf_bluez4: Use hfp_slc_info_free() Vinicius Costa Gomes
` (3 subsequent siblings)
10 siblings, 0 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-22 21:43 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 572 bytes --]
Add missing hfp_slc_info_free function declaration.
---
drivers/hfpmodem/slc.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/hfpmodem/slc.c b/drivers/hfpmodem/slc.c
index 825b8a9..3ac2b9e 100644
--- a/drivers/hfpmodem/slc.c
+++ b/drivers/hfpmodem/slc.c
@@ -84,6 +84,13 @@ done:
return info;
}
+void hfp_slc_info_free(struct hfp_slc_info *info)
+{
+ g_at_chat_unref(info->chat);
+
+ g_free(info);
+}
+
static void slc_establish_data_unref(gpointer userdata)
{
struct slc_establish_data *sed = userdata;
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 08/10] hfp_hf_bluez4: Use hfp_slc_info_free()
2013-01-22 21:43 [PATCH 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (6 preceding siblings ...)
2013-01-22 21:43 ` [PATCH 07/10] hfpmodem: Implement hfp_slc_info_free Vinicius Costa Gomes
@ 2013-01-22 21:43 ` Vinicius Costa Gomes
2013-01-22 21:43 ` [PATCH 09/10] phonesim: " Vinicius Costa Gomes
` (2 subsequent siblings)
10 siblings, 0 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-22 21:43 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]
Now that we have a function that takes care of free'ing that structure
we should use it.
---
plugins/hfp_hf_bluez4.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/plugins/hfp_hf_bluez4.c b/plugins/hfp_hf_bluez4.c
index b0aa1aa..ca73eaa 100644
--- a/plugins/hfp_hf_bluez4.c
+++ b/plugins/hfp_hf_bluez4.c
@@ -109,8 +109,10 @@ static void slc_failed(gpointer userdata)
ofono_error("Service level connection failed");
ofono_modem_set_powered(modem, FALSE);
- g_at_chat_unref(data->info->chat);
- data->info->chat = NULL;
+ if (data->info) {
+ hfp_slc_info_free(data->info);
+ data->info = NULL;
+ }
}
static void hfp_disconnected_cb(gpointer user_data)
@@ -120,8 +122,10 @@ static void hfp_disconnected_cb(gpointer user_data)
ofono_modem_set_powered(modem, FALSE);
- g_at_chat_unref(data->info->chat);
- data->info->chat = NULL;
+ if (data->info) {
+ hfp_slc_info_free(data->info);
+ data->info = NULL;
+ }
}
/* either oFono or Phone could request SLC connection */
@@ -463,8 +467,10 @@ static int hfp_disable(struct ofono_modem *modem)
DBG("%p", modem);
- g_at_chat_unref(data->info->chat);
- data->info->chat = NULL;
+ if (data->info) {
+ hfp_slc_info_free(data->info);
+ data->info = NULL;
+ }
if (data->agent_registered) {
status = bluetooth_send_with_reply(data->handsfree_path,
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 09/10] phonesim: Use hfp_slc_info_free()
2013-01-22 21:43 [PATCH 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (7 preceding siblings ...)
2013-01-22 21:43 ` [PATCH 08/10] hfp_hf_bluez4: Use hfp_slc_info_free() Vinicius Costa Gomes
@ 2013-01-22 21:43 ` Vinicius Costa Gomes
2013-01-22 21:43 ` [PATCH 10/10] hfp_hf_bluez5: Add SLC establishment procedure Vinicius Costa Gomes
2013-01-23 18:27 ` [PATCH v1 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
10 siblings, 0 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-22 21:43 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 834 bytes --]
Now that we have a function that takes care of free'ing that structure
we should use it.
---
plugins/phonesim.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index 9210e02..a9b77b9 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -885,8 +885,7 @@ static void slc_failed(gpointer userdata)
ofono_modem_set_powered(modem, FALSE);
- g_at_chat_unref(info->chat);
- info->chat = NULL;
+ hfp_slc_info_free(info);
}
static int localhfp_enable(struct ofono_modem *modem)
@@ -940,8 +939,7 @@ static int localhfp_disable(struct ofono_modem *modem)
{
struct hfp_slc_info *info = ofono_modem_get_data(modem);
- g_at_chat_unref(info->chat);
- info->chat = NULL;
+ hfp_slc_info_free(info);
return 0;
}
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH 10/10] hfp_hf_bluez5: Add SLC establishment procedure
2013-01-22 21:43 [PATCH 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (8 preceding siblings ...)
2013-01-22 21:43 ` [PATCH 09/10] phonesim: " Vinicius Costa Gomes
@ 2013-01-22 21:43 ` Vinicius Costa Gomes
2013-01-23 5:26 ` Denis Kenzior
2013-01-23 18:27 ` [PATCH v1 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
10 siblings, 1 reply; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-22 21:43 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 8115 bytes --]
When receiving a NewConnection call from BlueZ, initiates the Service
Level Connection using the received file descriptor. The HFP modem
sub-components (devinfo, voicecall, netreg, handsfree and callvolume)
are created at this point.
---
plugins/hfp_hf_bluez5.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 192 insertions(+), 7 deletions(-)
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 39853e3..79c78e5 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -24,17 +24,28 @@
#endif
#include <errno.h>
+#include <stdint.h>
+#include <stdlib.h>
#include <unistd.h>
+#include <string.h>
#include <glib.h>
#include <gdbus.h>
+#include <gatchat.h>
#define OFONO_API_SUBJECT_TO_CHANGE
#include <ofono/modem.h>
#include <ofono/dbus.h>
#include <ofono/plugin.h>
#include <ofono/log.h>
+#include <ofono/devinfo.h>
+#include <ofono/netreg.h>
+#include <ofono/voicecall.h>
+#include <ofono/call-volume.h>
+#include <ofono/handsfree.h>
+
+#include <drivers/hfpmodem/slc.h>
#include "bluez5.h"
@@ -44,14 +55,153 @@
#define HFP_EXT_PROFILE_PATH "/bluetooth/profile/hfp_hf"
+struct hfp {
+ char *device_address;
+ struct hfp_slc_info *info;
+ DBusMessage *msg;
+ GIOChannel *slcio;
+};
+
static GHashTable *modem_hash = NULL;
static GHashTable *devices_proxies = NULL;
static GDBusClient *bluez = NULL;
+static void hfp_free(gpointer user_data)
+{
+ struct hfp *hfp = user_data;
+
+ if (hfp->msg)
+ dbus_message_unref(hfp->msg);
+
+ if (hfp->slcio)
+ g_io_channel_unref(hfp->slcio);
+
+ if (hfp->info)
+ hfp_slc_info_free(hfp->info);
+
+ g_free(hfp->device_address);
+ g_free(hfp);
+}
+
+static void modem_data_free(gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct hfp *hfp = ofono_modem_get_data(modem);
+
+ hfp_free(hfp);
+}
+
+static void hfp_debug(const char *str, void *user_data)
+{
+ const char *prefix = user_data;
+
+ ofono_info("%s%s", prefix, str);
+}
+
+static void slc_established(gpointer userdata)
+{
+ struct ofono_modem *modem = userdata;
+ struct hfp *hfp = ofono_modem_get_data(modem);
+ DBusMessage *msg;
+
+ ofono_modem_set_powered(modem, TRUE);
+
+ msg = dbus_message_new_method_return(hfp->msg);
+ g_dbus_send_message(ofono_dbus_get_connection(), msg);
+ dbus_message_unref(hfp->msg);
+ hfp->msg = NULL;
+
+ ofono_info("Service level connection established");
+}
+
+static void slc_failed(gpointer userdata)
+{
+ struct ofono_modem *modem = userdata;
+ struct hfp *hfp = ofono_modem_get_data(modem);
+ DBusMessage *msg;
+
+ msg = g_dbus_create_error(hfp->msg, BLUEZ_ERROR_INTERFACE
+ ".Failed",
+ "HFP Handshake failed");
+
+ g_dbus_send_message(ofono_dbus_get_connection(), msg);
+ dbus_message_unref(hfp->msg);
+ hfp->msg = NULL;
+
+ ofono_error("Service level connection failed");
+ ofono_modem_set_powered(modem, FALSE);
+
+ if (hfp->info) {
+ hfp_slc_info_free(hfp->info);
+ hfp->info = NULL;
+ }
+}
+
+static void hfp_disconnected_cb(gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct hfp *hfp = ofono_modem_get_data(modem);
+
+ DBG("HFP disconnected");
+
+ if (hfp->info) {
+ hfp_slc_info_free(hfp->info);
+ hfp->info = NULL;
+ }
+
+ ofono_modem_set_powered(modem, FALSE);
+}
+
+static GIOChannel *service_level_connection(struct ofono_modem *modem,
+ int fd, uint16_t version, int *err)
+{
+ struct hfp *hfp = ofono_modem_get_data(modem);
+ GIOChannel *io;
+ GAtSyntax *syntax;
+ GAtChat *chat;
+
+ io = g_io_channel_unix_new(fd);
+ if (io == NULL) {
+ ofono_error("Service level connection failed: %s (%d)",
+ strerror(errno), errno);
+ *err = -EIO;
+ return NULL;
+ }
+
+ syntax = g_at_syntax_new_gsm_permissive();
+ chat = g_at_chat_new(io, syntax);
+ g_at_syntax_unref(syntax);
+ g_io_channel_set_close_on_unref(io, TRUE);
+
+ if (chat == NULL) {
+ *err = -ENOMEM;
+ goto fail;
+ }
+
+ g_at_chat_set_disconnect_function(chat, hfp_disconnected_cb, modem);
+
+ if (getenv("OFONO_AT_DEBUG"))
+ g_at_chat_set_debug(chat, hfp_debug, "");
+
+ hfp->info = hfp_slc_info_init(chat, version);
+ g_at_chat_unref(chat);
+ hfp_slc_establish(hfp->info, slc_established, slc_failed, modem);
+
+ *err = -EINPROGRESS;
+
+ return io;
+
+fail:
+ g_at_chat_unref(chat);
+ g_io_channel_unref(io);
+ return NULL;
+}
+
static struct ofono_modem *modem_register(const char *device,
- const char *alias)
+ const char *device_address, const char *alias)
{
struct ofono_modem *modem;
+ struct hfp *hfp;
char *path;
modem = g_hash_table_lookup(modem_hash, device);
@@ -70,6 +220,11 @@ static struct ofono_modem *modem_register(const char *device,
ofono_modem_set_name(modem, alias);
ofono_modem_register(modem);
+ hfp = g_new0(struct hfp, 1);
+ hfp->device_address = g_strdup(device_address);
+
+ ofono_modem_set_data(modem, hfp);
+
g_hash_table_insert(modem_hash, g_strdup(device), modem);
return modem;
@@ -104,7 +259,15 @@ static int hfp_disable(struct ofono_modem *modem)
static void hfp_pre_sim(struct ofono_modem *modem)
{
+ struct hfp *hfp = ofono_modem_get_data(modem);
+
DBG("%p", modem);
+
+ ofono_devinfo_create(modem, 0, "hfpmodem", hfp->device_address);
+ ofono_voicecall_create(modem, 0, "hfpmodem", hfp->info);
+ ofono_netreg_create(modem, 0, "hfpmodem", hfp->info);
+ ofono_handsfree_create(modem, 0, "hfpmodem", hfp->info);
+ ofono_call_volume_create(modem, 0, "hfpmodem", hfp->info);
}
static void hfp_post_sim(struct ofono_modem *modem)
@@ -126,12 +289,13 @@ static struct ofono_modem_driver hfp_driver = {
static DBusMessage *profile_new_connection(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
+ struct hfp *hfp;
struct ofono_modem *modem;
DBusMessageIter iter;
GDBusProxy *proxy;
DBusMessageIter entry;
- const char *device, *alias;
- int fd;
+ const char *device, *alias, *address;
+ int fd, err;
DBG("Profile handler NewConnection");
@@ -152,6 +316,11 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
else
dbus_message_iter_get_basic(&iter, &alias);
+ if (g_dbus_proxy_get_property(proxy, "Address", &iter) == FALSE)
+ goto error;
+
+ dbus_message_iter_get_basic(&iter, &address);
+
dbus_message_iter_next(&entry);
if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_UNIX_FD)
goto error;
@@ -160,10 +329,20 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
if (fd < 0)
goto error;
- modem = modem_register(device, alias);
+ modem = modem_register(device, address, alias);
if (modem == NULL)
goto error;
+ hfp = ofono_modem_get_data(modem);
+
+ hfp->msg = dbus_message_ref(msg);
+ hfp->slcio = service_level_connection(modem, fd,
+ HFP_VERSION_LATEST, &err);
+ if (err < 0 && err != -EINPROGRESS) {
+ hfp_free(hfp);
+ goto error;
+ }
+
return NULL;
error:
@@ -263,7 +442,7 @@ static void parse_uuids(DBusMessageIter *array, gpointer user_data)
static void proxy_added(GDBusProxy *proxy, void *user_data)
{
- const char *interface, *path, *alias;
+ const char *interface, *path, *alias, *address;
DBusMessageIter iter;
GSList *l, *uuids = NULL;
@@ -299,7 +478,13 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
dbus_message_iter_get_basic(&iter, &alias);
- modem_register(path, alias);
+
+ if (g_dbus_proxy_get_property(proxy, "Address", &iter) == FALSE)
+ return;
+
+ dbus_message_iter_get_basic(&iter, &address);
+
+ modem_register(path, address, alias);
}
static void proxy_removed(GDBusProxy *proxy, void *user_data)
@@ -382,7 +567,7 @@ static int hfp_init(void)
}
modem_hash = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,
- NULL);
+ modem_data_free);
devices_proxies = g_hash_table_new_full(g_str_hash, g_str_equal,
g_free, (GDestroyNotify) g_dbus_proxy_unref);
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH 10/10] hfp_hf_bluez5: Add SLC establishment procedure
2013-01-22 21:43 ` [PATCH 10/10] hfp_hf_bluez5: Add SLC establishment procedure Vinicius Costa Gomes
@ 2013-01-23 5:26 ` Denis Kenzior
0 siblings, 0 replies; 38+ messages in thread
From: Denis Kenzior @ 2013-01-23 5:26 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 9671 bytes --]
Hi Vinicius,
On 01/22/2013 03:43 PM, Vinicius Costa Gomes wrote:
> When receiving a NewConnection call from BlueZ, initiates the Service
> Level Connection using the received file descriptor. The HFP modem
> sub-components (devinfo, voicecall, netreg, handsfree and callvolume)
> are created at this point.
> ---
> plugins/hfp_hf_bluez5.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 192 insertions(+), 7 deletions(-)
>
> diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
> index 39853e3..79c78e5 100644
> --- a/plugins/hfp_hf_bluez5.c
> +++ b/plugins/hfp_hf_bluez5.c
> @@ -24,17 +24,28 @@
> #endif
>
> #include<errno.h>
> +#include<stdint.h>
> +#include<stdlib.h>
> #include<unistd.h>
> +#include<string.h>
>
> #include<glib.h>
>
> #include<gdbus.h>
> +#include<gatchat.h>
>
> #define OFONO_API_SUBJECT_TO_CHANGE
> #include<ofono/modem.h>
> #include<ofono/dbus.h>
> #include<ofono/plugin.h>
> #include<ofono/log.h>
> +#include<ofono/devinfo.h>
> +#include<ofono/netreg.h>
> +#include<ofono/voicecall.h>
> +#include<ofono/call-volume.h>
> +#include<ofono/handsfree.h>
> +
> +#include<drivers/hfpmodem/slc.h>
>
> #include "bluez5.h"
>
> @@ -44,14 +55,153 @@
>
> #define HFP_EXT_PROFILE_PATH "/bluetooth/profile/hfp_hf"
>
> +struct hfp {
> + char *device_address;
> + struct hfp_slc_info *info;
> + DBusMessage *msg;
> + GIOChannel *slcio;
Why do you want to track the GIOChannel?
> +};
> +
> static GHashTable *modem_hash = NULL;
> static GHashTable *devices_proxies = NULL;
> static GDBusClient *bluez = NULL;
>
> +static void hfp_free(gpointer user_data)
> +{
> + struct hfp *hfp = user_data;
> +
> + if (hfp->msg)
> + dbus_message_unref(hfp->msg);
> +
> + if (hfp->slcio)
> + g_io_channel_unref(hfp->slcio);
> +
> + if (hfp->info)
> + hfp_slc_info_free(hfp->info);
> +
> + g_free(hfp->device_address);
> + g_free(hfp);
What in the world are you doing here? How about using hfp_remove()
function for its intended purpose, and that is to destroy the user data
pertaining to the modem object.
> +}
> +
> +static void modem_data_free(gpointer user_data)
> +{
> + struct ofono_modem *modem = user_data;
> + struct hfp *hfp = ofono_modem_get_data(modem);
> +
> + hfp_free(hfp);
> +}
Yeah... Get rid of this ;)
> +
> +static void hfp_debug(const char *str, void *user_data)
> +{
> + const char *prefix = user_data;
> +
> + ofono_info("%s%s", prefix, str);
> +}
> +
> +static void slc_established(gpointer userdata)
> +{
> + struct ofono_modem *modem = userdata;
> + struct hfp *hfp = ofono_modem_get_data(modem);
> + DBusMessage *msg;
> +
> + ofono_modem_set_powered(modem, TRUE);
> +
> + msg = dbus_message_new_method_return(hfp->msg);
> + g_dbus_send_message(ofono_dbus_get_connection(), msg);
> + dbus_message_unref(hfp->msg);
> + hfp->msg = NULL;
> +
> + ofono_info("Service level connection established");
> +}
> +
> +static void slc_failed(gpointer userdata)
> +{
> + struct ofono_modem *modem = userdata;
> + struct hfp *hfp = ofono_modem_get_data(modem);
> + DBusMessage *msg;
> +
> + msg = g_dbus_create_error(hfp->msg, BLUEZ_ERROR_INTERFACE
> + ".Failed",
> + "HFP Handshake failed");
> +
> + g_dbus_send_message(ofono_dbus_get_connection(), msg);
> + dbus_message_unref(hfp->msg);
> + hfp->msg = NULL;
> +
> + ofono_error("Service level connection failed");
> + ofono_modem_set_powered(modem, FALSE);
> +
> + if (hfp->info) {
> + hfp_slc_info_free(hfp->info);
> + hfp->info = NULL;
> + }
> +}
> +
> +static void hfp_disconnected_cb(gpointer user_data)
> +{
> + struct ofono_modem *modem = user_data;
> + struct hfp *hfp = ofono_modem_get_data(modem);
> +
> + DBG("HFP disconnected");
> +
> + if (hfp->info) {
> + hfp_slc_info_free(hfp->info);
> + hfp->info = NULL;
> + }
> +
> + ofono_modem_set_powered(modem, FALSE);
> +}
> +
> +static GIOChannel *service_level_connection(struct ofono_modem *modem,
> + int fd, uint16_t version, int *err)
> +{
> + struct hfp *hfp = ofono_modem_get_data(modem);
> + GIOChannel *io;
> + GAtSyntax *syntax;
> + GAtChat *chat;
> +
> + io = g_io_channel_unix_new(fd);
> + if (io == NULL) {
> + ofono_error("Service level connection failed: %s (%d)",
> + strerror(errno), errno);
> + *err = -EIO;
> + return NULL;
> + }
> +
> + syntax = g_at_syntax_new_gsm_permissive();
> + chat = g_at_chat_new(io, syntax);
> + g_at_syntax_unref(syntax);
> + g_io_channel_set_close_on_unref(io, TRUE);
> +
In general it is not customary to hold on to the GIOChannel. It is
assumed that the GAtChat object owns it now, so I'm not sure what you're
trying to accomplish here...
> + if (chat == NULL) {
> + *err = -ENOMEM;
> + goto fail;
> + }
> +
> + g_at_chat_set_disconnect_function(chat, hfp_disconnected_cb, modem);
> +
> + if (getenv("OFONO_AT_DEBUG"))
> + g_at_chat_set_debug(chat, hfp_debug, "");
> +
> + hfp->info = hfp_slc_info_init(chat, version);
> + g_at_chat_unref(chat);
> + hfp_slc_establish(hfp->info, slc_established, slc_failed, modem);
> +
> + *err = -EINPROGRESS;
> +
> + return io;
> +
> +fail:
> + g_at_chat_unref(chat);
> + g_io_channel_unref(io);
> + return NULL;
> +}
> +
> static struct ofono_modem *modem_register(const char *device,
> - const char *alias)
> + const char *device_address, const char *alias)
> {
> struct ofono_modem *modem;
> + struct hfp *hfp;
> char *path;
>
> modem = g_hash_table_lookup(modem_hash, device);
> @@ -70,6 +220,11 @@ static struct ofono_modem *modem_register(const char *device,
> ofono_modem_set_name(modem, alias);
> ofono_modem_register(modem);
>
> + hfp = g_new0(struct hfp, 1);
> + hfp->device_address = g_strdup(device_address);
> +
> + ofono_modem_set_data(modem, hfp);
> +
In general this belongs in the probe() function. The data should be
created and ready prior to calling ofono_modem_register(). If the
device_address is to only be used in the pre_sim() function, you can
also use ofono_modem_set_string(modem, "Address", device_address) and
avoid keeping track of this variable here.
> g_hash_table_insert(modem_hash, g_strdup(device), modem);
>
> return modem;
> @@ -104,7 +259,15 @@ static int hfp_disable(struct ofono_modem *modem)
>
> static void hfp_pre_sim(struct ofono_modem *modem)
> {
> + struct hfp *hfp = ofono_modem_get_data(modem);
> +
> DBG("%p", modem);
> +
> + ofono_devinfo_create(modem, 0, "hfpmodem", hfp->device_address);
And then here simply call ofono_modem_get_string(modem, "Address")
> + ofono_voicecall_create(modem, 0, "hfpmodem", hfp->info);
> + ofono_netreg_create(modem, 0, "hfpmodem", hfp->info);
> + ofono_handsfree_create(modem, 0, "hfpmodem", hfp->info);
> + ofono_call_volume_create(modem, 0, "hfpmodem", hfp->info);
> }
>
> static void hfp_post_sim(struct ofono_modem *modem)
> @@ -126,12 +289,13 @@ static struct ofono_modem_driver hfp_driver = {
> static DBusMessage *profile_new_connection(DBusConnection *conn,
> DBusMessage *msg, void *user_data)
> {
> + struct hfp *hfp;
> struct ofono_modem *modem;
> DBusMessageIter iter;
> GDBusProxy *proxy;
> DBusMessageIter entry;
> - const char *device, *alias;
> - int fd;
> + const char *device, *alias, *address;
> + int fd, err;
>
> DBG("Profile handler NewConnection");
>
> @@ -152,6 +316,11 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
> else
> dbus_message_iter_get_basic(&iter,&alias);
>
> + if (g_dbus_proxy_get_property(proxy, "Address",&iter) == FALSE)
> + goto error;
> +
> + dbus_message_iter_get_basic(&iter,&address);
> +
> dbus_message_iter_next(&entry);
> if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_UNIX_FD)
> goto error;
> @@ -160,10 +329,20 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
> if (fd< 0)
> goto error;
>
> - modem = modem_register(device, alias);
> + modem = modem_register(device, address, alias);
> if (modem == NULL)
> goto error;
>
> + hfp = ofono_modem_get_data(modem);
> +
> + hfp->msg = dbus_message_ref(msg);
> + hfp->slcio = service_level_connection(modem, fd,
> + HFP_VERSION_LATEST,&err);
> + if (err< 0&& err != -EINPROGRESS) {
> + hfp_free(hfp);
> + goto error;
> + }
> +
> return NULL;
>
> error:
> @@ -263,7 +442,7 @@ static void parse_uuids(DBusMessageIter *array, gpointer user_data)
>
> static void proxy_added(GDBusProxy *proxy, void *user_data)
> {
> - const char *interface, *path, *alias;
> + const char *interface, *path, *alias, *address;
> DBusMessageIter iter;
> GSList *l, *uuids = NULL;
>
> @@ -299,7 +478,13 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
>
> dbus_message_iter_get_basic(&iter,&alias);
>
> - modem_register(path, alias);
> +
> + if (g_dbus_proxy_get_property(proxy, "Address",&iter) == FALSE)
> + return;
> +
> + dbus_message_iter_get_basic(&iter,&address);
> +
> + modem_register(path, address, alias);
> }
>
> static void proxy_removed(GDBusProxy *proxy, void *user_data)
> @@ -382,7 +567,7 @@ static int hfp_init(void)
> }
>
> modem_hash = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,
> - NULL);
> + modem_data_free);
This chunk should not longer be required.
>
> devices_proxies = g_hash_table_new_full(g_str_hash, g_str_equal,
> g_free, (GDestroyNotify) g_dbus_proxy_unref);
Regards,
-Denis
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v1 00/10] HFP Service Level Connection establishment
2013-01-22 21:43 [PATCH 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (9 preceding siblings ...)
2013-01-22 21:43 ` [PATCH 10/10] hfp_hf_bluez5: Add SLC establishment procedure Vinicius Costa Gomes
@ 2013-01-23 18:27 ` Vinicius Costa Gomes
2013-01-23 18:27 ` [PATCH v1 01/10] hfp_hf_bluez5: Initial GDBusClient for BlueZ Vinicius Costa Gomes
` (9 more replies)
10 siblings, 10 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 18:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]
Hi,
Changes from last version (from review comments):
- Better error handling when using GDBusProxy;
- Fixed some indentation issues;
- Proper use of the ofono_modem functionality to keep associated data;
- Simplifications that these changes allowed;
Cheers,
Vinicius Costa Gomes (10):
hfp_hf_bluez5: Initial GDBusClient for BlueZ
hfp_hf_bluez5: Add GDBusProxy for Bluetooth devices
hfp_hf_bluez5: Register modem for HFP AG capable devices
hfp_hf_bluez5: Keep track of changes of devices Aliases
hfp_hf_bluez5: Handle NewConnection from BlueZ
hfpmodem: Add dynamic hfp_slc_info allocation
hfpmodem: Implement hfp_slc_info_free()
hfp_hf_bluez4: Use hfp_slc_info_free()
phonesim: Use hfp_slc_info_free()
hfp_hf_bluez5: Add SLC establishment procedure
drivers/hfpmodem/slc.c | 19 ++-
drivers/hfpmodem/slc.h | 3 +-
plugins/bluez5.h | 5 +-
plugins/hfp_hf_bluez4.c | 37 ++---
plugins/hfp_hf_bluez5.c | 375 +++++++++++++++++++++++++++++++++++++++++++++++-
plugins/phonesim.c | 10 +-
6 files changed, 416 insertions(+), 33 deletions(-)
--
1.8.1.1
^ permalink raw reply [flat|nested] 38+ messages in thread* [PATCH v1 01/10] hfp_hf_bluez5: Initial GDBusClient for BlueZ
2013-01-23 18:27 ` [PATCH v1 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
@ 2013-01-23 18:27 ` Vinicius Costa Gomes
2013-01-23 20:31 ` Denis Kenzior
2013-01-23 18:27 ` [PATCH v1 02/10] hfp_hf_bluez5: Add GDBusProxy for Bluetooth devices Vinicius Costa Gomes
` (8 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 18:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 3159 bytes --]
This patch adds the initial callbacks to track when BlueZ connects so we can
register our HFP external profile handler. And tracks the interfaces added or
removed.
---
plugins/bluez5.h | 3 ++-
plugins/hfp_hf_bluez5.c | 51 +++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/plugins/bluez5.h b/plugins/bluez5.h
index 01ecfe8..893072f 100644
--- a/plugins/bluez5.h
+++ b/plugins/bluez5.h
@@ -19,7 +19,8 @@
*
*/
-#define BLUEZ_SERVICE "org.bluez"
+#define BLUEZ_SERVICE "org.bluez"
+#define BLUEZ_MANAGER_PATH "/"
#define BLUEZ_PROFILE_INTERFACE BLUEZ_SERVICE ".Profile1"
#define BLUEZ_ERROR_INTERFACE BLUEZ_SERVICE ".Error"
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index e024838..61e6669 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -42,6 +42,8 @@
#define HFP_EXT_PROFILE_PATH "/bluetooth/profile/hfp_hf"
+static GDBusClient *bluez = NULL;
+
static int hfp_probe(struct ofono_modem *modem)
{
DBG("modem: %p", modem);
@@ -143,6 +145,43 @@ static const GDBusMethodTable profile_methods[] = {
{ }
};
+static void connect_handler(DBusConnection *conn, void *user_data)
+{
+ DBG("Registering External Profile handler ...");
+
+ bluetooth_register_profile(conn, HFP_HS_UUID, "hfp_hf",
+ HFP_EXT_PROFILE_PATH);
+}
+
+static void proxy_added(GDBusProxy *proxy, void *user_data)
+{
+ const char *path;
+
+ path = g_dbus_proxy_get_path(proxy);
+
+ DBG("Device proxy: %s(%p)", path, proxy);
+}
+
+static void proxy_removed(GDBusProxy *proxy, void *user_data)
+{
+ const char *path;
+
+ path = g_dbus_proxy_get_path(proxy);
+
+ DBG("Device proxy: %s(%p)", path, proxy);
+}
+
+static void property_changed(GDBusProxy *proxy, const char *name,
+ DBusMessageIter *iter, void *user_data)
+{
+ const char *interface, *path;
+
+ interface = g_dbus_proxy_get_interface(proxy);
+ path = g_dbus_proxy_get_path(proxy);
+
+ DBG("path: %s interface: %s", path, interface);
+}
+
static int hfp_init(void)
{
DBusConnection *conn = ofono_dbus_get_connection();
@@ -168,15 +207,18 @@ static int hfp_init(void)
return err;
}
- err = bluetooth_register_profile(conn, HFP_HS_UUID, "hfp_hf",
- HFP_EXT_PROFILE_PATH);
- if (err < 0) {
+ bluez = g_dbus_client_new(conn, BLUEZ_SERVICE, BLUEZ_MANAGER_PATH);
+ if (bluez == NULL) {
g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
BLUEZ_PROFILE_INTERFACE);
ofono_modem_driver_unregister(&hfp_driver);
- return err;
+ return -ENOMEM;
}
+ g_dbus_client_set_connect_watch(bluez, connect_handler, NULL);
+ g_dbus_client_set_proxy_handlers(bluez, proxy_added, proxy_removed,
+ property_changed, NULL);
+
return 0;
}
@@ -188,6 +230,7 @@ static void hfp_exit(void)
g_dbus_unregister_interface(conn, HFP_EXT_PROFILE_PATH,
BLUEZ_PROFILE_INTERFACE);
ofono_modem_driver_unregister(&hfp_driver);
+ g_dbus_client_unref(bluez);
}
OFONO_PLUGIN_DEFINE(hfp_bluez5, "External Hands-Free Profile Plugin", VERSION,
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v1 02/10] hfp_hf_bluez5: Add GDBusProxy for Bluetooth devices
2013-01-23 18:27 ` [PATCH v1 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
2013-01-23 18:27 ` [PATCH v1 01/10] hfp_hf_bluez5: Initial GDBusClient for BlueZ Vinicius Costa Gomes
@ 2013-01-23 18:27 ` Vinicius Costa Gomes
2013-01-23 20:31 ` Denis Kenzior
2013-01-23 18:27 ` [PATCH v1 03/10] hfp_hf_bluez5: Register modem for HFP AG capable devices Vinicius Costa Gomes
` (7 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 18:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2788 bytes --]
This patch tracks the GDBusProxy for Bluetooth devices in order to be
able to get its properties.
---
plugins/bluez5.h | 1 +
plugins/hfp_hf_bluez5.c | 23 ++++++++++++++++++++---
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/plugins/bluez5.h b/plugins/bluez5.h
index 893072f..65c0b00 100644
--- a/plugins/bluez5.h
+++ b/plugins/bluez5.h
@@ -22,6 +22,7 @@
#define BLUEZ_SERVICE "org.bluez"
#define BLUEZ_MANAGER_PATH "/"
#define BLUEZ_PROFILE_INTERFACE BLUEZ_SERVICE ".Profile1"
+#define BLUEZ_DEVICE_INTERFACE BLUEZ_SERVICE ".Device1"
#define BLUEZ_ERROR_INTERFACE BLUEZ_SERVICE ".Error"
#define HFP_HS_UUID "0000111e-0000-1000-8000-00805f9b34fb"
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 61e6669..e0962a6 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -42,6 +42,7 @@
#define HFP_EXT_PROFILE_PATH "/bluetooth/profile/hfp_hf"
+static GHashTable *devices_proxies = NULL;
static GDBusClient *bluez = NULL;
static int hfp_probe(struct ofono_modem *modem)
@@ -155,20 +156,31 @@ static void connect_handler(DBusConnection *conn, void *user_data)
static void proxy_added(GDBusProxy *proxy, void *user_data)
{
- const char *path;
+ const char *interface, *path;
+ interface = g_dbus_proxy_get_interface(proxy);
path = g_dbus_proxy_get_path(proxy);
+ if (g_str_equal(BLUEZ_DEVICE_INTERFACE, interface) == FALSE)
+ return;
+
+ g_hash_table_insert(devices_proxies, g_strdup(path),
+ g_dbus_proxy_ref(proxy));
DBG("Device proxy: %s(%p)", path, proxy);
}
static void proxy_removed(GDBusProxy *proxy, void *user_data)
{
- const char *path;
+ const char *interface, *path;
+ interface = g_dbus_proxy_get_interface(proxy);
path = g_dbus_proxy_get_path(proxy);
- DBG("Device proxy: %s(%p)", path, proxy);
+ if (g_str_equal(BLUEZ_DEVICE_INTERFACE, interface)) {
+ g_hash_table_remove(devices_proxies, path);
+ DBG("Device proxy: %s(%p)", path, proxy);
+ }
+
}
static void property_changed(GDBusProxy *proxy, const char *name,
@@ -215,6 +227,9 @@ static int hfp_init(void)
return -ENOMEM;
}
+ devices_proxies = g_hash_table_new_full(g_str_hash, g_str_equal,
+ g_free, (GDestroyNotify) g_dbus_proxy_unref);
+
g_dbus_client_set_connect_watch(bluez, connect_handler, NULL);
g_dbus_client_set_proxy_handlers(bluez, proxy_added, proxy_removed,
property_changed, NULL);
@@ -231,6 +246,8 @@ static void hfp_exit(void)
BLUEZ_PROFILE_INTERFACE);
ofono_modem_driver_unregister(&hfp_driver);
g_dbus_client_unref(bluez);
+
+ g_hash_table_destroy(devices_proxies);
}
OFONO_PLUGIN_DEFINE(hfp_bluez5, "External Hands-Free Profile Plugin", VERSION,
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v1 03/10] hfp_hf_bluez5: Register modem for HFP AG capable devices
2013-01-23 18:27 ` [PATCH v1 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
2013-01-23 18:27 ` [PATCH v1 01/10] hfp_hf_bluez5: Initial GDBusClient for BlueZ Vinicius Costa Gomes
2013-01-23 18:27 ` [PATCH v1 02/10] hfp_hf_bluez5: Add GDBusProxy for Bluetooth devices Vinicius Costa Gomes
@ 2013-01-23 18:27 ` Vinicius Costa Gomes
2013-01-23 20:32 ` Denis Kenzior
2013-01-23 18:27 ` [PATCH v1 04/10] hfp_hf_bluez5: Keep track of changes of devices Aliases Vinicius Costa Gomes
` (6 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 18:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 4342 bytes --]
Now that we are able to keep track of devices appearing and
disappearing from BlueZ, we are able to register the modem when a
device that supports the HFP AG UUID appears.
---
plugins/bluez5.h | 1 +
plugins/hfp_hf_bluez5.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 78 insertions(+), 1 deletion(-)
diff --git a/plugins/bluez5.h b/plugins/bluez5.h
index 65c0b00..3aa8ffe 100644
--- a/plugins/bluez5.h
+++ b/plugins/bluez5.h
@@ -26,6 +26,7 @@
#define BLUEZ_ERROR_INTERFACE BLUEZ_SERVICE ".Error"
#define HFP_HS_UUID "0000111e-0000-1000-8000-00805f9b34fb"
+#define HFP_AG_UUID "0000111f-0000-1000-8000-00805f9b34fb"
int bluetooth_register_profile(DBusConnection *conn, const char *uuid,
const char *name, const char *object);
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index e0962a6..fbb1be3 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -42,9 +42,37 @@
#define HFP_EXT_PROFILE_PATH "/bluetooth/profile/hfp_hf"
+static GHashTable *modem_hash = NULL;
static GHashTable *devices_proxies = NULL;
static GDBusClient *bluez = NULL;
+static struct ofono_modem *modem_register(const char *device,
+ const char *alias)
+{
+ struct ofono_modem *modem;
+ char *path;
+
+ modem = g_hash_table_lookup(modem_hash, device);
+ if (modem != NULL)
+ return modem;
+
+ path = g_strconcat("hfp", device, NULL);
+
+ modem = ofono_modem_create(path, "hfp");
+
+ g_free(path);
+
+ if (modem == NULL)
+ return NULL;
+
+ ofono_modem_set_name(modem, alias);
+ ofono_modem_register(modem);
+
+ g_hash_table_insert(modem_hash, g_strdup(device), modem);
+
+ return modem;
+}
+
static int hfp_probe(struct ofono_modem *modem)
{
DBG("modem: %p", modem);
@@ -154,9 +182,33 @@ static void connect_handler(DBusConnection *conn, void *user_data)
HFP_EXT_PROFILE_PATH);
}
+static gboolean has_hfp_ag_uuid(DBusMessageIter *array)
+{
+ DBusMessageIter value;
+
+ if (dbus_message_iter_get_arg_type(array) != DBUS_TYPE_ARRAY)
+ return FALSE;
+
+ dbus_message_iter_recurse(array, &value);
+
+ while (dbus_message_iter_get_arg_type(&value) == DBUS_TYPE_STRING) {
+ const char *uuid;
+
+ dbus_message_iter_get_basic(&value, &uuid);
+
+ if (g_str_equal(uuid, HFP_AG_UUID) == TRUE)
+ return TRUE;
+
+ dbus_message_iter_next(&value);
+ }
+
+ return FALSE;
+}
+
static void proxy_added(GDBusProxy *proxy, void *user_data)
{
- const char *interface, *path;
+ const char *interface, *path, *alias;
+ DBusMessageIter iter;
interface = g_dbus_proxy_get_interface(proxy);
path = g_dbus_proxy_get_path(proxy);
@@ -167,11 +219,25 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
g_hash_table_insert(devices_proxies, g_strdup(path),
g_dbus_proxy_ref(proxy));
DBG("Device proxy: %s(%p)", path, proxy);
+
+ if (g_dbus_proxy_get_property(proxy, "UUIDs", &iter) == FALSE)
+ return;
+
+ if (has_hfp_ag_uuid(&iter) == FALSE)
+ return;
+
+ if (g_dbus_proxy_get_property(proxy, "Alias", &iter) == FALSE)
+ return;
+
+ dbus_message_iter_get_basic(&iter, &alias);
+
+ modem_register(path, alias);
}
static void proxy_removed(GDBusProxy *proxy, void *user_data)
{
const char *interface, *path;
+ struct ofono_modem *modem;
interface = g_dbus_proxy_get_interface(proxy);
path = g_dbus_proxy_get_path(proxy);
@@ -181,6 +247,12 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
DBG("Device proxy: %s(%p)", path, proxy);
}
+ modem = g_hash_table_lookup(modem_hash, path);
+ if (modem == NULL)
+ return;
+
+ g_hash_table_remove(modem_hash, path);
+ ofono_modem_remove(modem);
}
static void property_changed(GDBusProxy *proxy, const char *name,
@@ -227,6 +299,9 @@ static int hfp_init(void)
return -ENOMEM;
}
+ modem_hash = g_hash_table_new_full(g_str_hash, g_str_equal, g_free,
+ NULL);
+
devices_proxies = g_hash_table_new_full(g_str_hash, g_str_equal,
g_free, (GDestroyNotify) g_dbus_proxy_unref);
@@ -247,6 +322,7 @@ static void hfp_exit(void)
ofono_modem_driver_unregister(&hfp_driver);
g_dbus_client_unref(bluez);
+ g_hash_table_destroy(modem_hash);
g_hash_table_destroy(devices_proxies);
}
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v1 04/10] hfp_hf_bluez5: Keep track of changes of devices Aliases
2013-01-23 18:27 ` [PATCH v1 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (2 preceding siblings ...)
2013-01-23 18:27 ` [PATCH v1 03/10] hfp_hf_bluez5: Register modem for HFP AG capable devices Vinicius Costa Gomes
@ 2013-01-23 18:27 ` Vinicius Costa Gomes
2013-01-23 20:32 ` Denis Kenzior
2013-01-23 18:27 ` [PATCH v1 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ Vinicius Costa Gomes
` (5 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 18:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1236 bytes --]
If the device Alias property changes we should also change the name of
the modem.
---
plugins/hfp_hf_bluez5.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index fbb1be3..91bafaf 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -258,12 +258,28 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
static void property_changed(GDBusProxy *proxy, const char *name,
DBusMessageIter *iter, void *user_data)
{
- const char *interface, *path;
+ const char *interface, *path, *alias;
+ struct ofono_modem *modem;
+ DBusMessageIter alias_iter;
interface = g_dbus_proxy_get_interface(proxy);
path = g_dbus_proxy_get_path(proxy);
DBG("path: %s interface: %s", path, interface);
+
+ if (g_str_equal(BLUEZ_DEVICE_INTERFACE, interface) == FALSE)
+ return;
+
+ if (g_dbus_proxy_get_property(proxy, "Alias", &alias_iter) == FALSE)
+ return;
+
+ dbus_message_iter_get_basic(&alias_iter, &alias);
+
+ modem = g_hash_table_lookup(modem_hash, path);
+ if (modem == NULL)
+ return;
+
+ ofono_modem_set_name(modem, alias);
}
static int hfp_init(void)
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v1 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ
2013-01-23 18:27 ` [PATCH v1 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (3 preceding siblings ...)
2013-01-23 18:27 ` [PATCH v1 04/10] hfp_hf_bluez5: Keep track of changes of devices Aliases Vinicius Costa Gomes
@ 2013-01-23 18:27 ` Vinicius Costa Gomes
2013-01-23 20:32 ` Denis Kenzior
2013-01-23 18:27 ` [PATCH v1 06/10] hfpmodem: Add dynamic hfp_slc_info allocation Vinicius Costa Gomes
` (4 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 18:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2341 bytes --]
Parse the essential arguments in the message, in this case only the
file descriptor, and register the modem if it is not already
registered. This is necessary because in some cases, we may receive a
NewConnection call, and the SDP process is still taking place.
---
plugins/hfp_hf_bluez5.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 48 insertions(+), 3 deletions(-)
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index 91bafaf..d4f158e 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -24,6 +24,8 @@
#endif
#include <errno.h>
+#include <unistd.h>
+
#include <glib.h>
#include <gdbus.h>
@@ -124,11 +126,54 @@ static struct ofono_modem_driver hfp_driver = {
static DBusMessage *profile_new_connection(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
+ struct ofono_modem *modem;
+ DBusMessageIter iter;
+ GDBusProxy *proxy;
+ DBusMessageIter entry;
+ const char *device, *alias;
+ int fd;
+
DBG("Profile handler NewConnection");
- return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
- ".NotImplemented",
- "Implementation not provided");
+ if (dbus_message_iter_init(msg, &entry) == FALSE)
+ goto invalid;
+
+ if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_OBJECT_PATH)
+ goto invalid;
+
+ dbus_message_iter_get_basic(&entry, &device);
+
+ proxy = g_hash_table_lookup(devices_proxies, device);
+ if (proxy == NULL)
+ return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
+ ".Rejected",
+ "Unknown Bluetooth device");
+
+ g_dbus_proxy_get_property(proxy, "Alias", &iter);
+
+ dbus_message_iter_get_basic(&iter, &alias);
+
+ dbus_message_iter_next(&entry);
+ if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_UNIX_FD)
+ goto invalid;
+
+ dbus_message_iter_get_basic(&entry, &fd);
+ if (fd < 0)
+ goto invalid;
+
+ modem = modem_register(device, alias);
+ if (modem == NULL) {
+ close(fd);
+ return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
+ ".Rejected",
+ "Could not register HFP modem");
+ }
+
+ return NULL;
+
+invalid:
+ return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE ".Rejected",
+ "Invalid arguments in method call");
}
static DBusMessage *profile_release(DBusConnection *conn,
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v1 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ
2013-01-23 18:27 ` [PATCH v1 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ Vinicius Costa Gomes
@ 2013-01-23 20:32 ` Denis Kenzior
0 siblings, 0 replies; 38+ messages in thread
From: Denis Kenzior @ 2013-01-23 20:32 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 548 bytes --]
Hi Vinicius,
On 01/23/2013 12:27 PM, Vinicius Costa Gomes wrote:
> Parse the essential arguments in the message, in this case only the
> file descriptor, and register the modem if it is not already
> registered. This is necessary because in some cases, we may receive a
> NewConnection call, and the SDP process is still taking place.
> ---
> plugins/hfp_hf_bluez5.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 48 insertions(+), 3 deletions(-)
>
Patch has been applied, thanks.
Regards,
-Denis
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v1 06/10] hfpmodem: Add dynamic hfp_slc_info allocation
2013-01-23 18:27 ` [PATCH v1 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (4 preceding siblings ...)
2013-01-23 18:27 ` [PATCH v1 05/10] hfp_hf_bluez5: Handle NewConnection from BlueZ Vinicius Costa Gomes
@ 2013-01-23 18:27 ` Vinicius Costa Gomes
2013-01-23 20:34 ` Denis Kenzior
2013-01-23 18:27 ` [PATCH v1 07/10] hfpmodem: Implement hfp_slc_info_free() Vinicius Costa Gomes
` (3 subsequent siblings)
9 siblings, 1 reply; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 18:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 5573 bytes --]
It less error prone to have hfp_slc_info allocated dynamically than
having it allocated statically, as it makes the verification that it
was already de-allocated more direct.
---
drivers/hfpmodem/slc.c | 9 ++++++++-
drivers/hfpmodem/slc.h | 3 ++-
plugins/hfp_hf_bluez4.c | 35 ++++++++++++++++++-----------------
plugins/phonesim.c | 4 ++--
4 files changed, 30 insertions(+), 21 deletions(-)
diff --git a/drivers/hfpmodem/slc.c b/drivers/hfpmodem/slc.c
index 646befa..825b8a9 100644
--- a/drivers/hfpmodem/slc.c
+++ b/drivers/hfpmodem/slc.c
@@ -52,8 +52,13 @@ struct slc_establish_data {
gpointer userdata;
};
-void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version)
+struct hfp_slc_info *hfp_slc_info_init(GAtChat *chat, guint16 version)
{
+ struct hfp_slc_info *info;
+
+ info = g_new0(struct hfp_slc_info, 1);
+ info->chat = g_at_chat_ref(chat);
+
info->ag_features = 0;
info->ag_mpty_features = 0;
@@ -75,6 +80,8 @@ void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version)
done:
memset(info->cind_val, 0, sizeof(info->cind_val));
memset(info->cind_pos, 0, sizeof(info->cind_pos));
+
+ return info;
}
static void slc_establish_data_unref(gpointer userdata)
diff --git a/drivers/hfpmodem/slc.h b/drivers/hfpmodem/slc.h
index b71ffe9..8ae8d2f 100644
--- a/drivers/hfpmodem/slc.h
+++ b/drivers/hfpmodem/slc.h
@@ -55,7 +55,8 @@ struct hfp_slc_info {
unsigned int cind_val[HFP_INDICATOR_LAST];
};
-void hfp_slc_info_init(struct hfp_slc_info *info, guint16 version);
+struct hfp_slc_info *hfp_slc_info_init(GAtChat *chat, guint16 version);
+
void hfp_slc_info_free(struct hfp_slc_info *info);
void hfp_slc_establish(struct hfp_slc_info *info, hfp_slc_cb_t connect_cb,
diff --git a/plugins/hfp_hf_bluez4.c b/plugins/hfp_hf_bluez4.c
index 450c183..b0aa1aa 100644
--- a/plugins/hfp_hf_bluez4.c
+++ b/plugins/hfp_hf_bluez4.c
@@ -62,7 +62,7 @@ static DBusConnection *connection;
static GHashTable *modem_hash = NULL;
struct hfp_data {
- struct hfp_slc_info info;
+ struct hfp_slc_info *info;
char *handsfree_path;
char *handsfree_address;
DBusMessage *slc_msg;
@@ -109,8 +109,8 @@ static void slc_failed(gpointer userdata)
ofono_error("Service level connection failed");
ofono_modem_set_powered(modem, FALSE);
- g_at_chat_unref(data->info.chat);
- data->info.chat = NULL;
+ g_at_chat_unref(data->info->chat);
+ data->info->chat = NULL;
}
static void hfp_disconnected_cb(gpointer user_data)
@@ -120,12 +120,13 @@ static void hfp_disconnected_cb(gpointer user_data)
ofono_modem_set_powered(modem, FALSE);
- g_at_chat_unref(data->info.chat);
- data->info.chat = NULL;
+ g_at_chat_unref(data->info->chat);
+ data->info->chat = NULL;
}
/* either oFono or Phone could request SLC connection */
-static int service_level_connection(struct ofono_modem *modem, int fd)
+static int service_level_connection(struct ofono_modem *modem, int fd,
+ guint16 version)
{
struct hfp_data *data = ofono_modem_get_data(modem);
GIOChannel *io;
@@ -152,8 +153,10 @@ static int service_level_connection(struct ofono_modem *modem, int fd)
if (getenv("OFONO_AT_DEBUG"))
g_at_chat_set_debug(chat, hfp_debug, "");
- data->info.chat = chat;
- hfp_slc_establish(&data->info, slc_established, slc_failed, modem);
+ data->info = hfp_slc_info_init(chat, version);
+ g_at_chat_unref(chat);
+
+ hfp_slc_establish(data->info, slc_established, slc_failed, modem);
return -EINPROGRESS;
}
@@ -170,9 +173,7 @@ static DBusMessage *hfp_agent_new_connection(DBusConnection *conn,
DBUS_TYPE_UINT16, &version, DBUS_TYPE_INVALID))
return __ofono_error_invalid_args(msg);
- hfp_slc_info_init(&hfp_data->info, version);
-
- err = service_level_connection(modem, fd);
+ err = service_level_connection(modem, fd, version);
if (err < 0 && err != -EINPROGRESS)
return __ofono_error_failed(msg);
@@ -462,8 +463,8 @@ static int hfp_disable(struct ofono_modem *modem)
DBG("%p", modem);
- g_at_chat_unref(data->info.chat);
- data->info.chat = NULL;
+ g_at_chat_unref(data->info->chat);
+ data->info->chat = NULL;
if (data->agent_registered) {
status = bluetooth_send_with_reply(data->handsfree_path,
@@ -486,10 +487,10 @@ static void hfp_pre_sim(struct ofono_modem *modem)
DBG("%p", modem);
ofono_devinfo_create(modem, 0, "hfpmodem", data->handsfree_address);
- ofono_voicecall_create(modem, 0, "hfpmodem", &data->info);
- ofono_netreg_create(modem, 0, "hfpmodem", &data->info);
- ofono_call_volume_create(modem, 0, "hfpmodem", &data->info);
- ofono_handsfree_create(modem, 0, "hfpmodem", &data->info);
+ ofono_voicecall_create(modem, 0, "hfpmodem", data->info);
+ ofono_netreg_create(modem, 0, "hfpmodem", data->info);
+ ofono_call_volume_create(modem, 0, "hfpmodem", data->info);
+ ofono_handsfree_create(modem, 0, "hfpmodem", data->info);
}
static void hfp_post_sim(struct ofono_modem *modem)
diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index 26f96d0..9210e02 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -929,8 +929,8 @@ static int localhfp_enable(struct ofono_modem *modem)
g_at_chat_set_disconnect_function(chat, slc_failed, modem);
- hfp_slc_info_init(info, HFP_VERSION_LATEST);
- info->chat = chat;
+ info = hfp_slc_info_init(chat, HFP_VERSION_LATEST);
+ g_at_chat_unref(chat);
hfp_slc_establish(info, slc_established, slc_failed, modem);
return -EINPROGRESS;
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* Re: [PATCH v1 06/10] hfpmodem: Add dynamic hfp_slc_info allocation
2013-01-23 18:27 ` [PATCH v1 06/10] hfpmodem: Add dynamic hfp_slc_info allocation Vinicius Costa Gomes
@ 2013-01-23 20:34 ` Denis Kenzior
2013-01-23 23:04 ` Vinicius Costa Gomes
0 siblings, 1 reply; 38+ messages in thread
From: Denis Kenzior @ 2013-01-23 20:34 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 697 bytes --]
Hi Vinicius,
> diff --git a/plugins/phonesim.c b/plugins/phonesim.c
> index 26f96d0..9210e02 100644
> --- a/plugins/phonesim.c
> +++ b/plugins/phonesim.c
> @@ -929,8 +929,8 @@ static int localhfp_enable(struct ofono_modem *modem)
>
> g_at_chat_set_disconnect_function(chat, slc_failed, modem);
>
> - hfp_slc_info_init(info, HFP_VERSION_LATEST);
> - info->chat = chat;
> + info = hfp_slc_info_init(chat, HFP_VERSION_LATEST);
> + g_at_chat_unref(chat);
> hfp_slc_establish(info, slc_established, slc_failed, modem);
>
> return -EINPROGRESS;
This part makes no sense. I refer you to localhfp_probe() function.
Are you testing any of your changes?
Regards,
-Denis
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [PATCH v1 06/10] hfpmodem: Add dynamic hfp_slc_info allocation
2013-01-23 20:34 ` Denis Kenzior
@ 2013-01-23 23:04 ` Vinicius Costa Gomes
0 siblings, 0 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 23:04 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1187 bytes --]
Hi Denis,
On 14:34 Wed 23 Jan, Denis Kenzior wrote:
> Hi Vinicius,
>
> >diff --git a/plugins/phonesim.c b/plugins/phonesim.c
> >index 26f96d0..9210e02 100644
> >--- a/plugins/phonesim.c
> >+++ b/plugins/phonesim.c
> >@@ -929,8 +929,8 @@ static int localhfp_enable(struct ofono_modem *modem)
> >
> > g_at_chat_set_disconnect_function(chat, slc_failed, modem);
> >
> >- hfp_slc_info_init(info, HFP_VERSION_LATEST);
> >- info->chat = chat;
> >+ info = hfp_slc_info_init(chat, HFP_VERSION_LATEST);
> >+ g_at_chat_unref(chat);
> > hfp_slc_establish(info, slc_established, slc_failed, modem);
> >
> > return -EINPROGRESS;
>
> This part makes no sense. I refer you to localhfp_probe() function.
> Are you testing any of your changes?
Ugh. That is what happens when you mess with code that you are not familiar
with, and do not properly test it.
Will send a new version of the remaining of the series, without any changes
related to the allocation of hfp_slc_info. So we delay having to deal with
allocating it until we really need it. And we do not touch code that we
are not sure how it works.
>
> Regards,
> -Denis
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 38+ messages in thread
* [PATCH v1 07/10] hfpmodem: Implement hfp_slc_info_free()
2013-01-23 18:27 ` [PATCH v1 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (5 preceding siblings ...)
2013-01-23 18:27 ` [PATCH v1 06/10] hfpmodem: Add dynamic hfp_slc_info allocation Vinicius Costa Gomes
@ 2013-01-23 18:27 ` Vinicius Costa Gomes
2013-01-23 18:27 ` [PATCH v1 08/10] hfp_hf_bluez4: Use hfp_slc_info_free() Vinicius Costa Gomes
` (2 subsequent siblings)
9 siblings, 0 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 18:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 615 bytes --]
Add missing hfp_slc_info_free() function declaration.
---
drivers/hfpmodem/slc.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/hfpmodem/slc.c b/drivers/hfpmodem/slc.c
index 825b8a9..bf8d02d 100644
--- a/drivers/hfpmodem/slc.c
+++ b/drivers/hfpmodem/slc.c
@@ -84,6 +84,16 @@ done:
return info;
}
+void hfp_slc_info_free(struct hfp_slc_info *info)
+{
+ if (info == NULL)
+ return;
+
+ g_at_chat_unref(info->chat);
+
+ g_free(info);
+}
+
static void slc_establish_data_unref(gpointer userdata)
{
struct slc_establish_data *sed = userdata;
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v1 08/10] hfp_hf_bluez4: Use hfp_slc_info_free()
2013-01-23 18:27 ` [PATCH v1 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (6 preceding siblings ...)
2013-01-23 18:27 ` [PATCH v1 07/10] hfpmodem: Implement hfp_slc_info_free() Vinicius Costa Gomes
@ 2013-01-23 18:27 ` Vinicius Costa Gomes
2013-01-23 18:28 ` [PATCH v1 09/10] phonesim: " Vinicius Costa Gomes
2013-01-23 18:28 ` [PATCH v1 10/10] hfp_hf_bluez5: Add SLC establishment procedure Vinicius Costa Gomes
9 siblings, 0 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 18:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1604 bytes --]
Now that we have a function that takes care of free'ing that structure
we should use it.
---
plugins/hfp_hf_bluez4.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/plugins/hfp_hf_bluez4.c b/plugins/hfp_hf_bluez4.c
index b0aa1aa..d00e019 100644
--- a/plugins/hfp_hf_bluez4.c
+++ b/plugins/hfp_hf_bluez4.c
@@ -109,8 +109,8 @@ static void slc_failed(gpointer userdata)
ofono_error("Service level connection failed");
ofono_modem_set_powered(modem, FALSE);
- g_at_chat_unref(data->info->chat);
- data->info->chat = NULL;
+ hfp_slc_info_free(data->info);
+ data->info = NULL;
}
static void hfp_disconnected_cb(gpointer user_data)
@@ -120,8 +120,8 @@ static void hfp_disconnected_cb(gpointer user_data)
ofono_modem_set_powered(modem, FALSE);
- g_at_chat_unref(data->info->chat);
- data->info->chat = NULL;
+ hfp_slc_info_free(data->info);
+ data->info = NULL;
}
/* either oFono or Phone could request SLC connection */
@@ -369,6 +369,8 @@ static void hfp_remove(struct ofono_modem *modem)
HFP_AGENT_INTERFACE))
hfp_unregister_ofono_handsfree(modem);
+ hfp_slc_info_free(data->info);
+
g_free(data->handsfree_address);
g_free(data->handsfree_path);
g_free(data);
@@ -463,8 +465,8 @@ static int hfp_disable(struct ofono_modem *modem)
DBG("%p", modem);
- g_at_chat_unref(data->info->chat);
- data->info->chat = NULL;
+ hfp_slc_info_free(data->info);
+ data->info = NULL;
if (data->agent_registered) {
status = bluetooth_send_with_reply(data->handsfree_path,
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v1 09/10] phonesim: Use hfp_slc_info_free()
2013-01-23 18:27 ` [PATCH v1 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (7 preceding siblings ...)
2013-01-23 18:27 ` [PATCH v1 08/10] hfp_hf_bluez4: Use hfp_slc_info_free() Vinicius Costa Gomes
@ 2013-01-23 18:28 ` Vinicius Costa Gomes
2013-01-23 18:28 ` [PATCH v1 10/10] hfp_hf_bluez5: Add SLC establishment procedure Vinicius Costa Gomes
9 siblings, 0 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 18:28 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 834 bytes --]
Now that we have a function that takes care of free'ing that structure
we should use it.
---
plugins/phonesim.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index 9210e02..a9b77b9 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -885,8 +885,7 @@ static void slc_failed(gpointer userdata)
ofono_modem_set_powered(modem, FALSE);
- g_at_chat_unref(info->chat);
- info->chat = NULL;
+ hfp_slc_info_free(info);
}
static int localhfp_enable(struct ofono_modem *modem)
@@ -940,8 +939,7 @@ static int localhfp_disable(struct ofono_modem *modem)
{
struct hfp_slc_info *info = ofono_modem_get_data(modem);
- g_at_chat_unref(info->chat);
- info->chat = NULL;
+ hfp_slc_info_free(info);
return 0;
}
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread* [PATCH v1 10/10] hfp_hf_bluez5: Add SLC establishment procedure
2013-01-23 18:27 ` [PATCH v1 00/10] HFP Service Level Connection establishment Vinicius Costa Gomes
` (8 preceding siblings ...)
2013-01-23 18:28 ` [PATCH v1 09/10] phonesim: " Vinicius Costa Gomes
@ 2013-01-23 18:28 ` Vinicius Costa Gomes
9 siblings, 0 replies; 38+ messages in thread
From: Vinicius Costa Gomes @ 2013-01-23 18:28 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 7931 bytes --]
When receiving a NewConnection call from BlueZ, initiates the Service
Level Connection using the received file descriptor. The HFP modem
sub-components (devinfo, voicecall, netreg, handsfree and callvolume)
are created at this point.
---
plugins/hfp_hf_bluez5.c | 176 ++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 170 insertions(+), 6 deletions(-)
diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index d4f158e..cec924c 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -24,17 +24,28 @@
#endif
#include <errno.h>
+#include <stdint.h>
+#include <stdlib.h>
#include <unistd.h>
+#include <string.h>
#include <glib.h>
#include <gdbus.h>
+#include <gatchat.h>
#define OFONO_API_SUBJECT_TO_CHANGE
#include <ofono/modem.h>
#include <ofono/dbus.h>
#include <ofono/plugin.h>
#include <ofono/log.h>
+#include <ofono/devinfo.h>
+#include <ofono/netreg.h>
+#include <ofono/voicecall.h>
+#include <ofono/call-volume.h>
+#include <ofono/handsfree.h>
+
+#include <drivers/hfpmodem/slc.h>
#include "bluez5.h"
@@ -44,12 +55,115 @@
#define HFP_EXT_PROFILE_PATH "/bluetooth/profile/hfp_hf"
+struct hfp {
+ struct hfp_slc_info *info;
+ DBusMessage *msg;
+};
+
static GHashTable *modem_hash = NULL;
static GHashTable *devices_proxies = NULL;
static GDBusClient *bluez = NULL;
+static void hfp_debug(const char *str, void *user_data)
+{
+ const char *prefix = user_data;
+
+ ofono_info("%s%s", prefix, str);
+}
+
+static void slc_established(gpointer userdata)
+{
+ struct ofono_modem *modem = userdata;
+ struct hfp *hfp = ofono_modem_get_data(modem);
+ DBusMessage *msg;
+
+ ofono_modem_set_powered(modem, TRUE);
+
+ msg = dbus_message_new_method_return(hfp->msg);
+ g_dbus_send_message(ofono_dbus_get_connection(), msg);
+ dbus_message_unref(hfp->msg);
+ hfp->msg = NULL;
+
+ ofono_info("Service level connection established");
+}
+
+static void slc_failed(gpointer userdata)
+{
+ struct ofono_modem *modem = userdata;
+ struct hfp *hfp = ofono_modem_get_data(modem);
+ DBusMessage *msg;
+
+ msg = g_dbus_create_error(hfp->msg, BLUEZ_ERROR_INTERFACE
+ ".Failed",
+ "HFP Handshake failed");
+
+ g_dbus_send_message(ofono_dbus_get_connection(), msg);
+ dbus_message_unref(hfp->msg);
+ hfp->msg = NULL;
+
+ ofono_error("Service level connection failed");
+ ofono_modem_set_powered(modem, FALSE);
+
+ if (hfp->info) {
+ hfp_slc_info_free(hfp->info);
+ hfp->info = NULL;
+ }
+}
+
+static void hfp_disconnected_cb(gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct hfp *hfp = ofono_modem_get_data(modem);
+
+ DBG("HFP disconnected");
+
+ if (hfp->info) {
+ hfp_slc_info_free(hfp->info);
+ hfp->info = NULL;
+ }
+
+ ofono_modem_set_powered(modem, FALSE);
+}
+
+static int service_level_connection(struct ofono_modem *modem,
+ int fd, guint16 version)
+{
+ struct hfp *hfp = ofono_modem_get_data(modem);
+ GIOChannel *io;
+ GAtSyntax *syntax;
+ GAtChat *chat;
+
+ io = g_io_channel_unix_new(fd);
+ if (io == NULL) {
+ ofono_error("Service level connection failed: %s (%d)",
+ strerror(errno), errno);
+ return -EIO;
+ }
+
+ syntax = g_at_syntax_new_gsm_permissive();
+ chat = g_at_chat_new(io, syntax);
+ g_at_syntax_unref(syntax);
+
+ g_io_channel_set_close_on_unref(io, TRUE);
+ g_io_channel_unref(io);
+
+ if (chat == NULL)
+ return -ENOMEM;
+
+ g_at_chat_set_disconnect_function(chat, hfp_disconnected_cb, modem);
+
+ if (getenv("OFONO_AT_DEBUG"))
+ g_at_chat_set_debug(chat, hfp_debug, "");
+
+ hfp->info = hfp_slc_info_init(chat, version);
+ g_at_chat_unref(chat);
+ hfp_slc_establish(hfp->info, slc_established, slc_failed, modem);
+
+ return -EINPROGRESS;
+}
+
static struct ofono_modem *modem_register(const char *device,
- const char *alias)
+ const char *device_address, const char *alias)
{
struct ofono_modem *modem;
char *path;
@@ -67,6 +181,8 @@ static struct ofono_modem *modem_register(const char *device,
if (modem == NULL)
return NULL;
+ ofono_modem_set_string(modem, "Address", device_address);
+
ofono_modem_set_name(modem, alias);
ofono_modem_register(modem);
@@ -77,14 +193,32 @@ static struct ofono_modem *modem_register(const char *device,
static int hfp_probe(struct ofono_modem *modem)
{
+ struct hfp *hfp;
+
DBG("modem: %p", modem);
+ hfp = g_new0(struct hfp, 1);
+
+ ofono_modem_set_data(modem, hfp);
+
return 0;
}
static void hfp_remove(struct ofono_modem *modem)
{
+ struct hfp *hfp = ofono_modem_get_data(modem);
+
DBG("modem: %p", modem);
+
+ if (hfp->msg)
+ dbus_message_unref(hfp->msg);
+
+ if (hfp->info)
+ hfp_slc_info_free(hfp->info);
+
+ g_free(hfp);
+
+ ofono_modem_set_data(modem, NULL);
}
/* power up hardware */
@@ -104,7 +238,16 @@ static int hfp_disable(struct ofono_modem *modem)
static void hfp_pre_sim(struct ofono_modem *modem)
{
+ struct hfp *hfp = ofono_modem_get_data(modem);
+ char *address = (char *) ofono_modem_get_string(modem, "Address");
+
DBG("%p", modem);
+
+ ofono_devinfo_create(modem, 0, "hfpmodem", address);
+ ofono_voicecall_create(modem, 0, "hfpmodem", hfp->info);
+ ofono_netreg_create(modem, 0, "hfpmodem", hfp->info);
+ ofono_handsfree_create(modem, 0, "hfpmodem", hfp->info);
+ ofono_call_volume_create(modem, 0, "hfpmodem", hfp->info);
}
static void hfp_post_sim(struct ofono_modem *modem)
@@ -126,12 +269,13 @@ static struct ofono_modem_driver hfp_driver = {
static DBusMessage *profile_new_connection(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
+ struct hfp *hfp;
struct ofono_modem *modem;
DBusMessageIter iter;
GDBusProxy *proxy;
DBusMessageIter entry;
- const char *device, *alias;
- int fd;
+ const char *device, *alias, *address;
+ int fd, err;
DBG("Profile handler NewConnection");
@@ -153,6 +297,11 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
dbus_message_iter_get_basic(&iter, &alias);
+ if (g_dbus_proxy_get_property(proxy, "Address", &iter) == FALSE)
+ goto invalid;
+
+ dbus_message_iter_get_basic(&iter, &address);
+
dbus_message_iter_next(&entry);
if (dbus_message_iter_get_arg_type(&entry) != DBUS_TYPE_UNIX_FD)
goto invalid;
@@ -161,7 +310,7 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
if (fd < 0)
goto invalid;
- modem = modem_register(device, alias);
+ modem = modem_register(device, address, alias);
if (modem == NULL) {
close(fd);
return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
@@ -169,6 +318,15 @@ static DBusMessage *profile_new_connection(DBusConnection *conn,
"Could not register HFP modem");
}
+ err = service_level_connection(modem, fd, HFP_VERSION_LATEST);
+ if (err < 0 && err != -EINPROGRESS)
+ return g_dbus_create_error(msg, BLUEZ_ERROR_INTERFACE
+ ".Rejected",
+ "Not enough resources");
+
+ hfp = ofono_modem_get_data(modem);
+ hfp->msg = dbus_message_ref(msg);
+
return NULL;
invalid:
@@ -252,7 +410,7 @@ static gboolean has_hfp_ag_uuid(DBusMessageIter *array)
static void proxy_added(GDBusProxy *proxy, void *user_data)
{
- const char *interface, *path, *alias;
+ const char *interface, *path, *alias, *address;
DBusMessageIter iter;
interface = g_dbus_proxy_get_interface(proxy);
@@ -276,7 +434,13 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
dbus_message_iter_get_basic(&iter, &alias);
- modem_register(path, alias);
+
+ if (g_dbus_proxy_get_property(proxy, "Address", &iter) == FALSE)
+ return;
+
+ dbus_message_iter_get_basic(&iter, &address);
+
+ modem_register(path, address, alias);
}
static void proxy_removed(GDBusProxy *proxy, void *user_data)
--
1.8.1.1
^ permalink raw reply related [flat|nested] 38+ messages in thread