* Re: [Bluez-devel] [PATCH] DBus GetRemoteName problem
2006-06-06 16:52 ` Claudio Takahasi
@ 2006-06-07 12:45 ` Claudio Takahasi
0 siblings, 0 replies; 3+ messages in thread
From: Claudio Takahasi @ 2006-06-07 12:45 UTC (permalink / raw)
To: BlueZ development
[-- Attachment #1: Type: text/plain, Size: 2607 bytes --]
Hi Marcel, Fred,
Here is the patch to fix the extra DiscoveryCompleted signal sent. I
added an extra field in the discovered_dev_info list to handle the
scenario when GetRemoteName request is received while there is a
DiscoverDevicesWithoutNameResolving running.
I also included some code cleanup.
Do you have comments or another suggestion to fix this problem?
Claudio.
On 6/6/06, Claudio Takahasi <cktakahasi@gmail.com> wrote:
> Hi Frederic,
>
> It's necessary consider some extra conditions:
>
> 1. DiscoverDevices running
> 2. DiscoverDevicesWithoutNameResolving running
> 3. non D-Bus inquiry request(eg: hcitool, kbluetoothd, ..)
> 4. After receive a connection complete event, a HCI remote name
> request is sent to update the remote name
>
> The GetRemoteName request can arrive under these conditions.
>
> I already fixed this bug, but it needs more tests.
>
> Claudio.
>
> --
> ---------------------------------------------------------
> Claudio Takahasi
> Instituto Nokia de Tecnologia - INdT
>
> On 6/6/06, Frederic Danis <frederic.danis@palmsource.com> wrote:
> > Hello,
> >
> > When I use GetRemoteName of dbus-test program I receive a
> > DiscoveryCompleted signal after the RemoteNameUpdated (or
> > RemoteNameFailed) signal.
> > You can find attached a patch to fix this.
> >
> > Hope this helps.
> >
> > Regards
> >
> > Fred
> >
> > --
> > -----------------------------------------------
> > It is not by improving the oil lamp that one invents the electric bulb!
> > -----------------------------------------------
> > Danis Frederic PalmSource Europe
> > Software engineer
> > Mail : mailto:frederic.danis@palmsource.com
> > -----------------------------------------------
> >
> >
> >
> > --- dbus-adapter.c.orig 2006-06-06 11:35:32.000000000 +0200
> > +++ dbus-adapter.c 2006-06-06 14:51:57.000000000 +0200
> > @@ -1255,8 +1255,7 @@
> > * Otherwise, send the HCI cmd to get the remote name
> > */
> > if (dbus_data->discover_state == STATE_IDLE) {
> > - if (!disc_device_req_name(dbus_data))
> > - dbus_data->discover_state = STATE_RESOLVING_NAMES;
> > + disc_device_req_name(dbus_data);
> > }
> >
> > return error_request_deferred(conn, msg);
> >
> >
> >
> >
> > _______________________________________________
> > Bluez-devel mailing list
> > Bluez-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/bluez-devel
> >
> >
> >
>
--
---------------------------------------------------------
Claudio Takahasi
Instituto Nokia de Tecnologia - INdT
[-- Attachment #2: discover02.patch --]
[-- Type: text/x-patch, Size: 11682 bytes --]
Index: dbus-adapter.c
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/dbus-adapter.c,v
retrieving revision 1.55
diff -u -r1.55 dbus-adapter.c
--- dbus-adapter.c 6 Jun 2006 18:02:21 -0000 1.55
+++ dbus-adapter.c 7 Jun 2006 12:30:46 -0000
@@ -1248,16 +1248,14 @@
/* put the request name in the queue to resolve name */
str2ba(peer_addr, &peer_bdaddr);
- disc_device_append(&dbus_data->disc_devices, &peer_bdaddr, NAME_PENDING);
+ disc_device_append(&dbus_data->disc_devices, &peer_bdaddr, NAME_PENDING, RESOLVE_NAME);
/*
* if there is a discover process running, just queue the request.
* Otherwise, send the HCI cmd to get the remote name
*/
- if (dbus_data->discover_state == STATE_IDLE) {
- if (!disc_device_req_name(dbus_data))
- dbus_data->discover_state = STATE_RESOLVING_NAMES;
- }
+ if (dbus_data->discover_state == STATE_IDLE)
+ disc_device_req_name(dbus_data);
return error_request_deferred(conn, msg);
}
@@ -2070,7 +2068,7 @@
if (strcmp("DiscoverDevicesWithoutNameResolving", method) == 0)
dbus_data->discover_type = WITHOUT_NAME_RESOLVING;
else
- dbus_data->discover_type = RESOLVE_NAMES;
+ dbus_data->discover_type = RESOLVE_NAME;
dbus_data->requestor_name = strdup(dbus_message_get_sender(msg));
Index: dbus.c
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/dbus.c,v
retrieving revision 1.126
diff -u -r1.126 dbus.c
--- dbus.c 6 Jun 2006 13:28:21 -0000 1.126
+++ dbus.c 7 Jun 2006 12:30:47 -0000
@@ -91,47 +91,58 @@
free(dev);
}
-static int disc_device_find_by_bdaddr(const void *data, const void *user_data)
+static int disc_device_find(const struct discovered_dev_info *d1, const struct discovered_dev_info *d2)
{
- const struct discovered_dev_info *dev = data;
- const bdaddr_t *bdaddr = user_data;
+ int ret;
- return bacmp(&dev->bdaddr, bdaddr);
-}
+ if (bacmp(&d2->bdaddr, BDADDR_ANY)) {
+ ret = bacmp(&d1->bdaddr, &d2->bdaddr);
+ if (ret)
+ return ret;
+ }
-static int disc_device_find_by_name_status(const void *data, const void *user_data)
-{
- const struct discovered_dev_info *dev = data;
- const name_status_t *name_status = user_data;
+ /* if not any */
+ if (d2->name_status) {
+ ret = (d1->name_status - d2->name_status);
+ if (ret)
+ return ret;
+ }
- if (dev->name_status == *name_status)
- return 0;
+ if (d2->discover_type)
+ return (d1->discover_type - d2->discover_type);
- return -1;
+ return 0;
}
-int disc_device_append(struct slist **list, bdaddr_t *bdaddr, name_status_t name_status)
+int disc_device_append(struct slist **list, bdaddr_t *bdaddr, name_status_t name_status, int discover_type)
{
- struct discovered_dev_info *dev = NULL;
+ struct discovered_dev_info *dev = NULL, match;
struct slist *l;
- /* ignore repeated entries */
- l = slist_find(*list, bdaddr, disc_device_find_by_bdaddr);
+ memset(&match, 0, sizeof(struct discovered_dev_info));
+ bacpy(&match.bdaddr, bdaddr);
+ match.name_status = NAME_ANY;
+ /* ignore repeated entries */
+ l = slist_find(*list, &match, (cmp_func_t)disc_device_find);
if (l) {
/* device found, update the attributes */
dev = l->data;
dev->name_status = name_status;
- return -1;
+ /* get remote name received while discovering */
+ if (dev->discover_type != RESOLVE_NAME)
+ dev->discover_type = discover_type;
+ return -EALREADY;
}
dev = malloc(sizeof(*dev));
if (!dev)
- return -1;
+ return -ENOMEM;
memset(dev, 0, sizeof(*dev));
bacpy(&dev->bdaddr, bdaddr);
dev->name_status = name_status;
+ dev->discover_type = discover_type;
*list = slist_append(*list, dev);
@@ -140,11 +151,14 @@
static int disc_device_remove(struct slist **list, bdaddr_t *bdaddr)
{
- struct discovered_dev_info *dev;
+ struct discovered_dev_info *dev, match;
struct slist *l;
int ret_val = -1;
- l = slist_find(*list, bdaddr, disc_device_find_by_bdaddr);
+ memset(&match, 0, sizeof(struct discovered_dev_info));
+ bacpy(&match.bdaddr, bdaddr);
+
+ l = slist_find(*list, &match, (cmp_func_t)disc_device_find);
if (l) {
dev = l->data;
@@ -341,6 +355,7 @@
data->dev_id = dev_id;
data->mode = SCAN_DISABLED;
data->discoverable_timeout = get_discoverable_timeout(dev_id);
+ data->discover_type = WITHOUT_NAME_RESOLVING; /* default discover type */
if (fallback) {
if (!dbus_connection_register_fallback(connection, path, pvtable, data)) {
@@ -751,29 +766,32 @@
evt_cmd_status rp;
remote_name_req_cp cp;
bdaddr_t tmp;
- struct discovered_dev_info *dev;
+ struct discovered_dev_info *dev, match;
DBusMessage *message = NULL;
struct slist *l = NULL;
char *peer_addr = NULL;
- int dd, req_sent, ret_val = 0;
- name_status_t name_status = NAME_PENDING;
+ int dd, req_sent, ret_val = -ENODATA;
- /*get the next remote address */
+ /* get the next remote address */
if (!dbus_data->disc_devices)
- return -1;
+ return ret_val;
- l = slist_find(dbus_data->disc_devices, &name_status, disc_device_find_by_name_status);
+ memset(&match, 0, sizeof(struct discovered_dev_info));
+ bacpy(&match.bdaddr, BDADDR_ANY);
+ match.name_status = NAME_PENDING;
+ match.discover_type = RESOLVE_NAME;
+ l = slist_find(dbus_data->disc_devices, &match, (cmp_func_t)disc_device_find);
if (!l)
- return -1;
+ return ret_val;
dev = l->data;
if (!dev)
- return -1;
+ return ret_val;
dd = hci_open_dev(dbus_data->dev_id);
if (dd < 0)
- return -1;
+ return -errno;
memset(&rq, 0, sizeof(rq));
rq.ogf = OGF_LINK_CTL;
@@ -822,18 +840,18 @@
free(dev);
/* get the next element */
- l = slist_find(dbus_data->disc_devices, &name_status, disc_device_find_by_name_status);
+ l = slist_find(dbus_data->disc_devices, &match, (cmp_func_t)disc_device_find);
- if (!l) {
- /* no more devices: exit */
- ret_val = -1;
+ /* no more devices: exit */
+ if (!l)
goto failed;
- }
dev = l->data;
}
} while (!req_sent);
+ ret_val = 0;
+
failed:
hci_close_dev(dd);
@@ -859,44 +877,37 @@
snprintf(path, sizeof(path), "%s/hci%d", BASE_PATH, id);
- if (dbus_connection_get_object_path_data(connection, path, (void *) &pdata)) {
-
- if (pdata->discover_type == RESOLVE_NAMES) {
- if (!disc_device_req_name(pdata)) {
- pdata->discover_state = STATE_RESOLVING_NAMES;
- goto failed; /* skip - there is name to resolve */
- }
- }
- pdata->discover_state = STATE_IDLE;
+ if (!dbus_connection_get_object_path_data(connection, path, (void *) &pdata)) {
+ error("Getting %s path data failed!", path);
+ goto failed;
+ }
- /* free discovered devices list */
- slist_foreach(pdata->disc_devices, disc_device_info_free, NULL);
- slist_free(pdata->disc_devices);
- pdata->disc_devices = NULL;
+ /* reset the discover type to be able to handle D-Bus and non D-Bus requests */
+ pdata->discover_type = WITHOUT_NAME_RESOLVING;
- if (pdata->requestor_name) {
- free(pdata->requestor_name);
- pdata->requestor_name = NULL;
- }
+ if (!disc_device_req_name(pdata)) {
+ pdata->discover_state = STATE_RESOLVING_NAMES;
+ goto failed; /* skip - there is name to resolve */
}
- message = dbus_message_new_signal(path, ADAPTER_INTERFACE,
- "DiscoveryCompleted");
- if (message == NULL) {
- error("Can't allocate D-Bus message");
- goto failed;
- }
+ pdata->discover_state = STATE_IDLE;
- if (dbus_connection_send(connection, message, NULL) == FALSE) {
- error("Can't send D-Bus inquiry complete message");
- goto failed;
+ /* free discovered devices list */
+ slist_foreach(pdata->disc_devices, disc_device_info_free, NULL);
+ slist_free(pdata->disc_devices);
+ pdata->disc_devices = NULL;
+
+ if (pdata->requestor_name) {
+ free(pdata->requestor_name);
+ pdata->requestor_name = NULL;
}
- dbus_connection_flush(connection);
+ /* Send discovery completed signal if there isn't name to resolve */
+ message = dbus_message_new_signal(path, ADAPTER_INTERFACE,
+ "DiscoveryCompleted");
+ send_reply_and_unref(connection, message);
failed:
- if (message)
- dbus_message_unref(message);
bt_free(local_addr);
}
@@ -915,7 +926,7 @@
char path[MAX_PATH_LENGTH];
struct hci_dbus_data *pdata = NULL;
struct slist *l = NULL;
- struct discovered_dev_info *dev;
+ struct discovered_dev_info match;
char *local_addr, *peer_addr, *name = NULL;
const char *major_ptr, *minor_ptr;
struct slist *service_classes;
@@ -971,23 +982,18 @@
slist_free(service_classes);
- if (dbus_connection_send(connection, signal_device, NULL) == FALSE) {
- error("Can't send D-Bus remote device found signal");
- goto failed;
- }
+ send_reply_and_unref(connection, signal_device);
- /* send the remote name signal */
- l = slist_find(pdata->disc_devices, peer, disc_device_find_by_bdaddr);
-
- if (l) {
- dev = l->data;
- if (dev->name_status == NAME_SENT)
- goto failed; /* don't sent the name again */
- }
+ memset(&match, 0, sizeof(struct discovered_dev_info));
+ bacpy(&match.bdaddr, peer);
+ match.name_status = NAME_SENT;
+ /* if found: don't sent the name again */
+ l = slist_find(pdata->disc_devices, &match, (cmp_func_t)disc_device_find);
+ if (l)
+ goto failed;
snprintf(filename, PATH_MAX, "%s/%s/names", STORAGEDIR, local_addr);
name = textfile_get(filename, peer_addr);
-
if (name) {
signal_name = dev_signal_factory(pdata->dev_id, "RemoteNameUpdated",
DBUS_TYPE_STRING, &peer_addr,
@@ -998,17 +1004,11 @@
free(name);
name_status = NAME_SENT;
}
- /* queue only results triggered by D-Bus clients */
- if (pdata->requestor_name)
- disc_device_append(&pdata->disc_devices, peer, name_status);
-
- disc_device_append(&pdata->disc_devices, peer, name_status);
+ /* add in the list to track name sent/pending */
+ disc_device_append(&pdata->disc_devices, peer, name_status, pdata->discover_type);
failed:
- if (signal_device)
- dbus_message_unref(signal_device);
-
bt_free(local_addr);
bt_free(peer_addr);
}
@@ -1059,7 +1059,11 @@
slist_free(pdata->disc_devices);
pdata->disc_devices = NULL;
- if (pdata->discover_state == STATE_RESOLVING_NAMES ) {
+ /*
+ * The discovery completed signal must be sent only for discover
+ * devices request WITH name resolving
+ */
+ if (pdata->discover_state == STATE_RESOLVING_NAMES) {
message = dbus_message_new_signal(path, ADAPTER_INTERFACE,
"DiscoveryCompleted");
Index: dbus.h
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/dbus.h,v
retrieving revision 1.88
diff -u -r1.88 dbus.h
--- dbus.h 6 Jun 2006 18:02:21 -0000 1.88
+++ dbus.h 7 Jun 2006 12:30:47 -0000
@@ -63,13 +63,14 @@
STATE_IDLE,
STATE_DISCOVER,
STATE_RESOLVING_NAMES
-}discover_state_t;
+} discover_state_t;
/* discover type */
-#define WITHOUT_NAME_RESOLVING 0
-#define RESOLVE_NAMES 1
+#define WITHOUT_NAME_RESOLVING 1 /* D-Bus and non D-Bus request */
+#define RESOLVE_NAME 2
typedef enum {
+ NAME_ANY,
NAME_PENDING,
NAME_SENT
} name_status_t;
@@ -77,6 +78,7 @@
struct discovered_dev_info {
bdaddr_t bdaddr;
name_status_t name_status;
+ int discover_type;
};
struct bonding_request_info {
@@ -190,7 +192,7 @@
int active_conn_find_by_bdaddr(const void *data, const void *user_data);
void bonding_request_free(struct bonding_request_info *dev);
void disc_device_info_free(void *data, void *user_data);
-int disc_device_append(struct slist **list, bdaddr_t *bdaddr, name_status_t name_status);
+int disc_device_append(struct slist **list, bdaddr_t *bdaddr, name_status_t name_status, int discover_type);
int disc_device_req_name(struct hci_dbus_data *dbus_data);
int discoverable_timeout_handler(void *data);
[-- Attachment #3: Type: text/plain, Size: 0 bytes --]
[-- Attachment #4: Type: text/plain, Size: 164 bytes --]
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel
^ permalink raw reply [flat|nested] 3+ messages in thread