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
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
