Hi,
Unfortunately your patch has all its lines ending in ^M and has hard
line wrappings in places. Also a few nitpicks below. Could you simply
send out the patch with 'git send-email ....' ?
On Fri, 2014-10-24 at 11:46 -0500, David Lechner wrote:
> Bluetooth PAN specifies three different roles for different types of
> network connections, PANU, NAP and GN. ConnMan currently supports NAP
> but ignores the other roles. This patch adds support for the GN and
> PANU roles so that we can connect to devices that don't offer NAP.
>
> In the case that a device provides more than one role, we prefer NAP
> over GN and GN over PANU. ConnMan will only use the most preferred role
> and ignore the others so that we don't have duplicate services in
> ConnMan.
> ---
> plugins/bluetooth.c | 50
> +++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/plugins/bluetooth.c b/plugins/bluetooth.c
> index 82217d0..6902dd7 100644
> --- a/plugins/bluetooth.c
> +++ b/plugins/bluetooth.c
> @@ -36,7 +36,9 @@
> #define BLUEZ_SERVICE "org.bluez"
> #define BLUEZ_PATH "/org/bluez"
> +#define BLUETOOTH_PAN_PANU
> "00001115-0000-1000-8000-00805f9b34fb"
> #define BLUETOOTH_PAN_NAP
> "00001116-0000-1000-8000-00805f9b34fb"
> +#define BLUETOOTH_PAN_GN
> "00001117-0000-1000-8000-00805f9b34fb"
> #define BLUETOOTH_ADDR_LEN 6
> @@ -50,6 +52,7 @@ struct bluetooth_pan {
> struct connman_network *network;
> GDBusProxy *btdevice_proxy;
> GDBusProxy *btnetwork_proxy;
> + const char *pan_role;
> };
> static void address2ident(const char *address, char *ident)
> @@ -85,28 +88,36 @@ static bool proxy_get_bool(GDBusProxy *proxy, const
> char *property)
> return value;
> }
> -static bool proxy_get_nap(GDBusProxy *proxy)
> +static const char* proxy_get_role(GDBusProxy *proxy)
> {
> DBusMessageIter iter, value;
> if (!proxy)
> - return false;
> + return NULL;
> if (!g_dbus_proxy_get_property(proxy, "UUIDs", &iter))
> - return false;
> + return NULL;
> dbus_message_iter_recurse(&iter, &value);
> while (dbus_message_iter_get_arg_type(&value) ==
> DBUS_TYPE_STRING) {
> const char *uuid;
> - dbus_message_iter_get_basic(&value, &uuid);
> - if (strcmp(uuid, BLUETOOTH_PAN_NAP) == 0)
> - return true;
> + dbus_message_iter_get_basic(&value, &uuid);
> + /*
> + * Order matters here. If a device offers more than one role,
> + * we prefer NAP, then GN, then PANU.
> + */
> + if (strcmp(uuid, BLUETOOTH_PAN_NAP) == 0)
Here and below we'd prefer to use !strcmp(...
> + return "nap";
> + if (strcmp(uuid, BLUETOOTH_PAN_GN) == 0)
> + return "gn";
> + if (strcmp(uuid, BLUETOOTH_PAN_PANU) == 0)
> + return "panu";
> dbus_message_iter_next(&value);
> }
> - return false;
> + return NULL;
> }
> static int bluetooth_pan_probe(struct connman_network *network)
> @@ -225,9 +236,11 @@ static void pan_connect_cb(DBusMessage *message,
> void *user_data)
> static void pan_connect_append(DBusMessageIter *iter,
> void *user_data)
> {
> - const char *role = BLUETOOTH_PAN_NAP;
> + const char *path = user_data;
> + struct bluetooth_pan *pan;
> - dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &role);
> + pan = g_hash_table_lookup(networks, path);
> + dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &pan->pan_role);
> }
> static int bluetooth_pan_connect(struct connman_network *network)
> @@ -322,8 +335,10 @@ static void btnetwork_property_change(GDBusProxy
> *proxy, const char *name,
> static void pan_create_nap(struct bluetooth_pan *pan)
> {
> struct connman_device *device;
> + const char* role;
> - if (!proxy_get_nap(pan->btdevice_proxy)) {
> + role = proxy_get_role(pan->btdevice_proxy);
> + if (!role) {
> pan_remove_nap(pan);
> return;
> }
> @@ -366,6 +381,7 @@ static void pan_create_nap(struct bluetooth_pan *pan)
> connman_network_set_group(pan->network, ident);
> }
> + pan->pan_role = role;
> connman_device_add_network(device, pan->network);
> if (pan_connect(pan, NULL))
> @@ -376,7 +392,7 @@ static void btdevice_property_change(GDBusProxy
> *proxy, const char *name,
> DBusMessageIter *iter, void *user_data)
> {
> struct bluetooth_pan *pan;
> - bool pan_nap = false;
> + const char* pan_role = NULL;
> if (strcmp(name, "UUIDs") != 0)
same here...
> return;
> @@ -387,12 +403,12 @@ static void btdevice_property_change(GDBusProxy
> *proxy, const char *name,
> if (pan->network &&
> connman_network_get_device(pan->network))
> - pan_nap = true;
> + pan_role = pan->pan_role;
> - DBG("network %p network nap %d proxy nap %d", pan->network, pan_nap,
> - proxy_get_nap(pan->btdevice_proxy));
> + DBG("network %p network role %s proxy role %s", pan->network, pan_role,
> + proxy_get_role(pan->btdevice_proxy));
> - if (proxy_get_nap(pan->btdevice_proxy) == pan_nap)
> + if (strcmp(proxy_get_role(pan->btdevice_proxy), pan_role) == 0)
> return;
> pan_create_nap(pan);
> @@ -447,7 +463,7 @@ static void pan_create(GDBusProxy *network_proxy)
> g_dbus_proxy_set_property_watch(pan->btdevice_proxy,
> btdevice_property_change, NULL);
> - DBG("pan %p %s nap %d", pan, path, proxy_get_nap(pan->btdevice_proxy));
> + DBG("pan %p %s role %s", pan, path,
> proxy_get_role(pan->btdevice_proxy));
> pan_create_nap(pan);
> }
> @@ -756,7 +772,7 @@ static void device_create(GDBusProxy *proxy)
> powered = proxy_get_bool(proxy, "Powered");
> connman_device_set_powered(device, powered);
> - if (proxy_get_nap(proxy) && !bluetooth_tethering)
> + if (proxy_get_role(proxy) != NULL && !bluetooth_tethering)
and here just 'if (proxy_get_role(proxy) && ...'
> tethering_create(path, NULL, NULL, false);
> }
> -- 1.9.1
The last line is also mysteriously off, use git send-email.
Cheers,
Patrik
_______________________________________________
connman mailing list
[email protected]
https://lists.connman.net/mailman/listinfo/connman
