[Bluez-devel] Device Configuration D-Bus services

[Bluez-devel] Device Configuration D-Bus services

[Claudio Takahasi]

[-- Attachment #1: Type: text/plain, Size: 1999 bytes --]

Hi folks,

I would like to bring up the device configuration services before start
coding, I would like design how these services will work and receive
comments.

The services provided under these paths/interfaces will not be public. Only
an administrator group or a root will be allowed to send messages. We don't
need care about the configuration file rules now, it can be defined later.

In my opinion it's better use the same approach of the Manager. A main path
object "/org/bluez/Manager" (FALLBACK) should catch messages sent to
unregistered
paths and handle services like:
- DeviceList, displays UP/DOWN devices
- UP
- DOWN
- RESET

The device based paths can provide services like:
- SetProperty
- GetProperty
- Commands
- Features

Using SetProperty/GetProperty instead of define one method name for each
property is more clean. However it is important keep in mind that the
documentation of this service must be well-defined, because the method
call/method reply arguments will have different argument number and types.
In the method call, the first argument must be a byte(faster comparison)
indicating the command code instead of use a string to represent the
command.

Suggested paths and Interfaces:
*** Main path
path:
/org/bluez/Devices
interface: org.bluez.Devices

*** Device based
path:
/org/bluez/Devices/hciX
interface: org.bluez.Devices


Another point is the message handler function. Currently, there is only one
function responsible for handle the messages. In my opinion it will be
better
create other DBusObjectPathVTable to address Device messages for make the
logic more clear. For SetProperty/GetProperty will be required extract the
first argument to get the command code in the message handle function.


Is everything ok? Any comments?

Regards,
Claudio.

--
---------------------------------------------------------
Claudio Takahasi
Nokia's Institute of Technology - INdT
claudio.takahasi@indt.org.br

[-- Attachment #2: Type: text/html, Size: 2309 bytes --]

Re: [Bluez-devel] Device Configuration D-Bus services

[Eduardo Rocha]

[-- Attachment #1.1: Type: text/plain, Size: 2944 bytes --]

Hi Claudio and Marcel,

here is the initial patch regarding /org/bluez/Devices. I keep it really
simple so we can improve it in small steps. I've merged the Claudio's code
for register and unregister a new path for a device in two funcions as this
code is used in 3 different parts of the code. For now I've leaved the
syslog call to register and unregister operations so we can see if every
thing is going ok.
My next step is to provide a list of registed devices (not only the devices
that are up) through /org/bluez/Devices path.

One question: the SetProperty and GetProperty will use an interger or a
string constant to determing which operation will be used?

BR,
Eduardo
--
Eduardo Rocha
Instituto Nokia de Tecnologia - INdT

On 10/19/05, Claudio Takahasi <cktakahasi@gmail.com> wrote:
>
> Hi folks,
>
> I would like to bring up the device configuration services before start
> coding, I would like design how these services will work and receive
> comments.
>
> The services provided under these paths/interfaces will not be public.
> Only
> an administrator group or a root will be allowed to send messages. We
> don't
> need care about the configuration file rules now, it can be defined later.
>
> In my opinion it's better use the same approach of the Manager. A main
> path
> object "/org/bluez/Manager" (FALLBACK) should catch messages sent to
> unregistered
> paths and handle services like:
> - DeviceList, displays UP/DOWN devices
> - UP
> - DOWN
> - RESET
>
> The device based paths can provide services like:
> - SetProperty
> - GetProperty
> - Commands
> - Features
>
> Using SetProperty/GetProperty instead of define one method name for each
> property is more clean. However it is important keep in mind that the
> documentation of this service must be well-defined, because the method
> call/method reply arguments will have different argument number and types.
> In the method call, the first argument must be a byte(faster comparison)
> indicating the command code instead of use a string to represent the
> command.
>
> Suggested paths and Interfaces:
> *** Main path
> path:
> /org/bluez/Devices
> interface: org.bluez.Devices
>
> *** Device based
> path:
> /org/bluez/Devices/hciX
> interface: org.bluez.Devices
>
>
> Another point is the message handler function. Currently, there is only
> one
> function responsible for handle the messages. In my opinion it will be
> better
> create other DBusObjectPathVTable to address Device messages for make the
> logic more clear. For SetProperty/GetProperty will be required extract the
> first argument to get the command code in the message handle function.
>
>
> Is everything ok? Any comments?
>
> Regards,
> Claudio.
>
> --
> ---------------------------------------------------------
> Claudio Takahasi
> Nokia's Institute of Technology - INdT
> claudio.takahasi@indt.org.br
>

[-- Attachment #1.2: Type: text/html, Size: 3574 bytes --]

[-- Attachment #2: dbus_devices.patch --]
[-- Type: text/x-patch, Size: 7975 bytes --]

--- bluez-cvs-old/utils/hcid/hcid.h	2005-10-24 15:29:36.204404720 -0300
+++ bluez-cvs-new/utils/hcid/hcid.h	2005-10-24 15:34:58.774366632 -0300
@@ -125,8 +125,10 @@
 #ifdef ENABLE_DBUS
 gboolean hcid_dbus_init(void);
 void hcid_dbus_exit(void);
-gboolean hcid_dbus_register_device(uint16_t id);
-gboolean hcid_dbus_unregister_device(uint16_t id);
+gboolean hcid_dbus_dev_register(uint16_t id);
+gboolean hcid_dbus_mgr_register(uint16_t id);
+gboolean hcid_dbus_dev_unregister(uint16_t id);
+gboolean hcid_dbus_mgr_unregister(uint16_t id);
 void hcid_dbus_request_pin(int dev, struct hci_conn_info *ci);
 void hcid_dbus_inquiry_start(bdaddr_t *local);
 void hcid_dbus_inquiry_complete(bdaddr_t *local);
--- bluez-cvs-old/utils/hcid/main.c	2005-10-24 15:29:36.204404720 -0300
+++ bluez-cvs-new/utils/hcid/main.c	2005-10-24 15:34:58.775366480 -0300
@@ -377,7 +377,9 @@
 			start_security_manager(dr->dev_id);
 
 #ifdef ENABLE_DBUS
-		hcid_dbus_register_device(dr->dev_id);
+		if (hci_test_bit(HCI_UP, &dr->dev_opt ))
+			hcid_dbus_mgr_register(dr->dev_id);
+		hcid_dbus_dev_register(dr->dev_id);
 #endif
 	}
 
@@ -430,10 +432,16 @@
 		syslog(LOG_INFO, "HCI dev %d registered", sd->dev_id);
 		if (hcid.auto_init)
 			init_device(sd->dev_id);
+#ifdef ENABLE_DBUS
+		hcid_dbus_dev_register(sd->dev_id);
+#endif
 		break;
 
 	case HCI_DEV_UNREG:
 		syslog(LOG_INFO, "HCI dev %d unregistered", sd->dev_id);
+#ifdef ENABLE_DBUS
+		hcid_dbus_dev_unregister(sd->dev_id);
+#endif
 		break;
 
 	case HCI_DEV_UP:
@@ -443,7 +451,7 @@
 		if (hcid.security)
 			start_security_manager(sd->dev_id);
 #ifdef ENABLE_DBUS
-		hcid_dbus_register_device(sd->dev_id);
+		hcid_dbus_mgr_register(sd->dev_id);
 #endif
 		break;
 
@@ -452,7 +460,7 @@
 		if (hcid.security)
 			stop_security_manager(sd->dev_id);
 #ifdef ENABLE_DBUS
-		hcid_dbus_unregister_device(sd->dev_id);
+		hcid_dbus_mgr_unregister(sd->dev_id);
 #endif
 		break;
 	}
--- bluez-cvs-old/utils/hcid/dbus.c	2005-10-24 15:29:36.171409736 -0300
+++ bluez-cvs-new/utils/hcid/dbus.c	2005-10-24 17:21:54.264063952 -0300
@@ -52,7 +52,7 @@
 #include "dbus.h"
 
 static DBusConnection *connection;
-static int num_adapters = 0;
+static int up_adapters = 0;
 
 #define TIMEOUT (30 * 1000)		/* 30 seconds */
 #define BLUETOOTH_DEVICE_NAME_LEN    (18)
@@ -359,6 +359,39 @@
 	free(req);
 }
 
+static gboolean register_dbus_path(char *path, uint16_t id)
+{
+	struct hci_dbus_data *data;
+	syslog(LOG_INFO,"Registering DBUS Path: %s", path);
+        data = malloc(sizeof(struct hci_dbus_data));
+        if (data == NULL) {
+        	syslog(LOG_ERR,"Failed to alloc memory to DBUS path register data (%s)", path);
+	    	return FALSE;
+	}
+        data->id = id;
+
+	if (!dbus_connection_register_object_path(connection, path, &obj_vtable, data)) {
+       		syslog(LOG_ERR,"DBUS failed to register %s object", path);
+		free(data);
+		return FALSE;
+       	}
+	return TRUE;
+}
+
+static gboolean unregister_dbus_path(char *path)
+{
+	void *data;
+	syslog(LOG_INFO,"Unregistering DBUS Path: %s", path);
+	if (dbus_connection_get_object_path_data(connection, path, &data) && data) 
+		free(data);
+	
+	if (!dbus_connection_unregister_object_path (connection, path)) {
+		syslog(LOG_ERR,"DBUS failed to unregister %s object", path);
+		return FALSE;
+	} 
+	return TRUE;
+}
+
 void hcid_dbus_request_pin(int dev, struct hci_conn_info *ci)
 {
 	DBusMessage *message;
@@ -739,7 +772,7 @@
 
 	data->id = DEVICE_PATH_ID;
 
-	if (!dbus_connection_register_object_path(connection, DEVICE_PATH,
+	if (!dbus_connection_register_fallback(connection, DEVICE_PATH,
 						&obj_vtable, data)) {
 		syslog(LOG_ERR, "Can't register %s object", DEVICE_PATH);
 		return FALSE;
@@ -855,7 +888,32 @@
 	}
 }
 
-gboolean hcid_dbus_register_device(uint16_t id)
+gboolean hcid_dbus_dev_register(uint16_t id) 
+{
+	char path[MAX_PATH_LENGTH+1];
+	char dev[BLUETOOTH_DEVICE_NAME_LEN+1];
+	const char *pdev = dev;
+
+	snprintf(dev, BLUETOOTH_DEVICE_NAME_LEN, HCI_DEVICE_NAME "%d", id);
+	snprintf(path, MAX_PATH_LENGTH, "%s/%s", DEVICE_PATH, pdev);
+
+	/* register the default path*/
+	return register_dbus_path(path, id);
+}
+
+gboolean hcid_dbus_dev_unregister(uint16_t id)
+{
+	char dev[BLUETOOTH_DEVICE_NAME_LEN+1];
+	char path[MAX_PATH_LENGTH+1];
+	const char *pdev = dev;
+
+	snprintf(dev, BLUETOOTH_DEVICE_NAME_LEN, HCI_DEVICE_NAME "%d", id);
+	snprintf(path, MAX_PATH_LENGTH, "%s/%s", DEVICE_PATH, pdev);
+
+	return unregister_dbus_path(path);
+}
+
+gboolean hcid_dbus_mgr_register(uint16_t id)
 {
 	char dev[BLUETOOTH_DEVICE_NAME_LEN];
 	struct profile_obj_path_data *ptr = obj_path_table;
@@ -873,7 +931,7 @@
 	}
 
 	if (!ret)
-		num_adapters++;
+		up_adapters++;
 
 	message = dbus_message_new_signal(BLUEZ_HCI_PATH,
 			BLUEZ_HCI_INTERFACE, BLUEZ_HCI_DEV_ADDED);
@@ -883,7 +941,7 @@
 		goto failed;
 	}
 
-	sprintf(dev, "hci%d", id);
+	sprintf(dev, HCI_DEVICE_NAME "%d", id);
 
 	dbus_message_iter_init_append(message, &iter);
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING ,&pdev);
@@ -904,7 +962,7 @@
 	return TRUE;
 }
 
-gboolean hcid_dbus_unregister_device(uint16_t id)
+gboolean hcid_dbus_mgr_unregister(uint16_t id)
 {
 	char dev[BLUETOOTH_DEVICE_NAME_LEN];
 	struct profile_obj_path_data *ptr = obj_path_table;
@@ -917,8 +975,8 @@
 		return FALSE;
 
 	for (; ptr->name; ptr++) {
-		dft_unreg = (num_adapters > 1) ? 0 : 1;
-		num_adapters--;
+		dft_unreg = (up_adapters > 1) ? 0 : 1;
+		up_adapters--;
 		ptr->unreg_func(connection, dft_unreg, id);
 
 		if (dft_unreg )
@@ -933,7 +991,7 @@
 		goto failed;
 	}
 
-	sprintf(dev, "hci%d", id);
+	sprintf(dev, HCI_DEVICE_NAME "%d", id);
 
 	dbus_message_iter_init_append(message, &iter);
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING ,&pdev);
@@ -965,39 +1023,17 @@
  */
 static int hci_dbus_reg_obj_path(DBusConnection *conn, int dft_reg, uint16_t id)
 {
-	struct hci_dbus_data *data;
 	char path[MAX_PATH_LENGTH];
 
 	/* register the default path*/
 	if (!dft_reg) {
-
 		sprintf(path, "%s/%s/%s", MANAGER_PATH, HCI_DEFAULT_DEVICE_NAME, BLUEZ_HCI);
-
-		data = malloc(sizeof(struct hci_dbus_data));
-		if (data == NULL)
-			return -1;
-
-		data->id = DEFAULT_DEVICE_PATH_ID;
-
-		if (!dbus_connection_register_object_path(conn, path, &obj_vtable, data)) { 
-			syslog(LOG_ERR,"DBUS failed to register %s object", path);
-			/* ignore, the default path was already registered */
-		}
+		register_dbus_path(path, DEFAULT_DEVICE_PATH_ID);
 	}
 
-	data = malloc(sizeof(struct hci_dbus_data));
-	if (data == NULL)
-		return -1;
-
-	data->id = id;
-
 	/* register the default path*/
 	sprintf(path, "%s/%s%d/%s", MANAGER_PATH, HCI_DEVICE_NAME, id, BLUEZ_HCI);
-
-	if (!dbus_connection_register_object_path(conn, path, &obj_vtable, data)) {
-		syslog(LOG_ERR,"DBUS failed to register %s object", path);
-		/* ignore, the path was already registered */
-	}
+	register_dbus_path(path, id);
 
 	return 0;
 }
@@ -1016,39 +1052,14 @@
 {
 	int ret = 0;
 	char path[MAX_PATH_LENGTH];
-	char dft_path[MAX_PATH_LENGTH];
-	void *data = NULL;
 
 	if (unreg_dft) {
-
-		sprintf(dft_path, "%s/%s/%s", MANAGER_PATH, HCI_DEFAULT_DEVICE_NAME, BLUEZ_HCI);
-
-		if (!dbus_connection_unregister_object_path (connection, dft_path)) {
-			syslog(LOG_ERR,"DBUS failed to unregister %s object", dft_path);
-			ret = -1;
-		} else {
-			if (dbus_connection_get_object_path_data(conn, dft_path, &data)) {
-				if (data) {
-					free(data);
-					data = NULL;
-				}
-			}
-		}
+		sprintf(path, "%s/%s/%s", MANAGER_PATH, HCI_DEFAULT_DEVICE_NAME, BLUEZ_HCI);
+		unregister_dbus_path(path);
 	}
 
 	sprintf(path, "%s/%s%d/%s", MANAGER_PATH, HCI_DEVICE_NAME, id, BLUEZ_HCI);
-
-	if (!dbus_connection_unregister_object_path (connection, path)) {
-		syslog(LOG_ERR,"DBUS failed to unregister %s object", path);
-		ret = -1;
-	} else {
-		if (dbus_connection_get_object_path_data(conn, path, &data)) {
-			if (data) {
-				free(data);
-				data = NULL;
-			}
-		}
-	}
+	unregister_dbus_path(path);
 
 	return ret;
 }

Re: [Bluez-devel] Device Configuration D-Bus services

[Eduardo Rocha]

[-- Attachment #1.1: Type: text/plain, Size: 3370 bytes --]

Sorry about the spaces. Here is the same patch without them ;).

BR,
Eduardo.


On 10/24/05, Eduardo Rocha <folhabranca@gmail.com> wrote:
>
> Hi Claudio and Marcel,
>
> here is the initial patch regarding /org/bluez/Devices. I keep it really
> simple so we can improve it in small steps. I've merged the Claudio's code
> for register and unregister a new path for a device in two funcions as this
> code is used in 3 different parts of the code. For now I've leaved the
> syslog call to register and unregister operations so we can see if every
> thing is going ok.
> My next step is to provide a list of registed devices (not only the
> devices that are up) through /org/bluez/Devices path.
>
> One question: the SetProperty and GetProperty will use an interger or a
> string constant to determing which operation will be used?
>
> BR,
> Eduardo
> --
> Eduardo Rocha
> Instituto Nokia de Tecnologia - INdT
>
> On 10/19/05, Claudio Takahasi <cktakahasi@gmail.com> wrote:
> >
> > Hi folks,
> >
> > I would like to bring up the device configuration services before start
> > coding, I would like design how these services will work and receive
> > comments.
> >
> > The services provided under these paths/interfaces will not be public.
> > Only
> > an administrator group or a root will be allowed to send messages. We
> > don't
> > need care about the configuration file rules now, it can be defined
> > later.
> >
> > In my opinion it's better use the same approach of the Manager. A main
> > path
> > object "/org/bluez/Manager" (FALLBACK) should catch messages sent to
> > unregistered
> > paths and handle services like:
> > - DeviceList, displays UP/DOWN devices
> > - UP
> > - DOWN
> > - RESET
> >
> > The device based paths can provide services like:
> > - SetProperty
> > - GetProperty
> > - Commands
> > - Features
> >
> > Using SetProperty/GetProperty instead of define one method name for each
> > property is more clean. However it is important keep in mind that the
> > documentation of this service must be well-defined, because the method
> > call/method reply arguments will have different argument number and
> > types.
> > In the method call, the first argument must be a byte(faster comparison)
> > indicating the command code instead of use a string to represent the
> > command.
> >
> > Suggested paths and Interfaces:
> > *** Main path
> > path:
> > /org/bluez/Devices
> > interface: org.bluez.Devices
> >
> > *** Device based
> > path:
> > /org/bluez/Devices/hciX
> > interface: org.bluez.Devices
> >
> >
> > Another point is the message handler function. Currently, there is only
> > one
> > function responsible for handle the messages. In my opinion it will be
> > better
> > create other DBusObjectPathVTable to address Device messages for make
> > the
> > logic more clear. For SetProperty/GetProperty will be required extract
> > the
> > first argument to get the command code in the message handle function.
> >
> >
> > Is everything ok? Any comments?
> >
> > Regards,
> > Claudio.
> >
> > --
> > ---------------------------------------------------------
> > Claudio Takahasi
> > Nokia's Institute of Technology - INdT
> > claudio.takahasi@indt.org.br
> >
>
>
>
>
>
>
>


--
Eduardo Rocha
Instituto Nokia de Tecnologia - INdT

[-- Attachment #1.2: Type: text/html, Size: 4266 bytes --]

[-- Attachment #2: dbus_device.patch --]
[-- Type: text/x-patch, Size: 7919 bytes --]

--- bluez-cvs-old/utils/hcid/hcid.h	2005-10-24 15:29:36.204404720 -0300
+++ bluez-cvs-new/utils/hcid/hcid.h	2005-10-24 15:34:58.774366632 -0300
@@ -125,8 +125,10 @@
 #ifdef ENABLE_DBUS
 gboolean hcid_dbus_init(void);
 void hcid_dbus_exit(void);
-gboolean hcid_dbus_register_device(uint16_t id);
-gboolean hcid_dbus_unregister_device(uint16_t id);
+gboolean hcid_dbus_dev_register(uint16_t id);
+gboolean hcid_dbus_mgr_register(uint16_t id);
+gboolean hcid_dbus_dev_unregister(uint16_t id);
+gboolean hcid_dbus_mgr_unregister(uint16_t id);
 void hcid_dbus_request_pin(int dev, struct hci_conn_info *ci);
 void hcid_dbus_inquiry_start(bdaddr_t *local);
 void hcid_dbus_inquiry_complete(bdaddr_t *local);
--- bluez-cvs-old/utils/hcid/main.c	2005-10-24 15:29:36.204404720 -0300
+++ bluez-cvs-new/utils/hcid/main.c	2005-10-24 15:34:58.775366480 -0300
@@ -377,7 +377,9 @@
 			start_security_manager(dr->dev_id);
 
 #ifdef ENABLE_DBUS
-		hcid_dbus_register_device(dr->dev_id);
+		if (hci_test_bit(HCI_UP, &dr->dev_opt ))
+			hcid_dbus_mgr_register(dr->dev_id);
+		hcid_dbus_dev_register(dr->dev_id);
 #endif
 	}
 
@@ -430,10 +432,16 @@
 		syslog(LOG_INFO, "HCI dev %d registered", sd->dev_id);
 		if (hcid.auto_init)
 			init_device(sd->dev_id);
+#ifdef ENABLE_DBUS
+		hcid_dbus_dev_register(sd->dev_id);
+#endif
 		break;
 
 	case HCI_DEV_UNREG:
 		syslog(LOG_INFO, "HCI dev %d unregistered", sd->dev_id);
+#ifdef ENABLE_DBUS
+		hcid_dbus_dev_unregister(sd->dev_id);
+#endif
 		break;
 
 	case HCI_DEV_UP:
@@ -443,7 +451,7 @@
 		if (hcid.security)
 			start_security_manager(sd->dev_id);
 #ifdef ENABLE_DBUS
-		hcid_dbus_register_device(sd->dev_id);
+		hcid_dbus_mgr_register(sd->dev_id);
 #endif
 		break;
 
@@ -452,7 +460,7 @@
 		if (hcid.security)
 			stop_security_manager(sd->dev_id);
 #ifdef ENABLE_DBUS
-		hcid_dbus_unregister_device(sd->dev_id);
+		hcid_dbus_mgr_unregister(sd->dev_id);
 #endif
 		break;
 	}
--- bluez-cvs-old/utils/hcid/dbus.c	2005-10-24 15:29:36.171409736 -0300
+++ bluez-cvs-new/utils/hcid/dbus.c	2005-10-24 18:15:46.837638072 -0300
@@ -52,7 +52,7 @@
 #include "dbus.h"
 
 static DBusConnection *connection;
-static int num_adapters = 0;
+static int up_adapters = 0;
 
 #define TIMEOUT (30 * 1000)		/* 30 seconds */
 #define BLUETOOTH_DEVICE_NAME_LEN    (18)
@@ -359,6 +359,39 @@
 	free(req);
 }
 
+static gboolean register_dbus_path(char *path, uint16_t id)
+{
+	struct hci_dbus_data *data;
+	syslog(LOG_INFO,"Registering DBUS Path: %s", path);
+	data = malloc(sizeof(struct hci_dbus_data));
+	if (data == NULL) {
+		syslog(LOG_ERR,"Failed to alloc memory to DBUS path register data (%s)", path);
+		return FALSE;
+	}
+	data->id = id;
+
+	if (!dbus_connection_register_object_path(connection, path, &obj_vtable, data)) {
+		syslog(LOG_ERR,"DBUS failed to register %s object", path);
+		free(data);
+		return FALSE;
+	}
+	return TRUE;
+}
+
+static gboolean unregister_dbus_path(char *path)
+{
+	void *data;
+	syslog(LOG_INFO,"Unregistering DBUS Path: %s", path);
+	if (dbus_connection_get_object_path_data(connection, path, &data) && data) 
+		free(data);
+	
+	if (!dbus_connection_unregister_object_path (connection, path)) {
+		syslog(LOG_ERR,"DBUS failed to unregister %s object", path);
+		return FALSE;
+	} 
+	return TRUE;
+}
+
 void hcid_dbus_request_pin(int dev, struct hci_conn_info *ci)
 {
 	DBusMessage *message;
@@ -739,7 +772,7 @@
 
 	data->id = DEVICE_PATH_ID;
 
-	if (!dbus_connection_register_object_path(connection, DEVICE_PATH,
+	if (!dbus_connection_register_fallback(connection, DEVICE_PATH,
 						&obj_vtable, data)) {
 		syslog(LOG_ERR, "Can't register %s object", DEVICE_PATH);
 		return FALSE;
@@ -855,7 +888,32 @@
 	}
 }
 
-gboolean hcid_dbus_register_device(uint16_t id)
+gboolean hcid_dbus_dev_register(uint16_t id) 
+{
+	char path[MAX_PATH_LENGTH+1];
+	char dev[BLUETOOTH_DEVICE_NAME_LEN+1];
+	const char *pdev = dev;
+
+	snprintf(dev, BLUETOOTH_DEVICE_NAME_LEN, HCI_DEVICE_NAME "%d", id);
+	snprintf(path, MAX_PATH_LENGTH, "%s/%s", DEVICE_PATH, pdev);
+
+	/* register the default path*/
+	return register_dbus_path(path, id);
+}
+
+gboolean hcid_dbus_dev_unregister(uint16_t id)
+{
+	char dev[BLUETOOTH_DEVICE_NAME_LEN+1];
+	char path[MAX_PATH_LENGTH+1];
+	const char *pdev = dev;
+
+	snprintf(dev, BLUETOOTH_DEVICE_NAME_LEN, HCI_DEVICE_NAME "%d", id);
+	snprintf(path, MAX_PATH_LENGTH, "%s/%s", DEVICE_PATH, pdev);
+
+	return unregister_dbus_path(path);
+}
+
+gboolean hcid_dbus_mgr_register(uint16_t id)
 {
 	char dev[BLUETOOTH_DEVICE_NAME_LEN];
 	struct profile_obj_path_data *ptr = obj_path_table;
@@ -873,7 +931,7 @@
 	}
 
 	if (!ret)
-		num_adapters++;
+		up_adapters++;
 
 	message = dbus_message_new_signal(BLUEZ_HCI_PATH,
 			BLUEZ_HCI_INTERFACE, BLUEZ_HCI_DEV_ADDED);
@@ -883,7 +941,7 @@
 		goto failed;
 	}
 
-	sprintf(dev, "hci%d", id);
+	sprintf(dev, HCI_DEVICE_NAME "%d", id);
 
 	dbus_message_iter_init_append(message, &iter);
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING ,&pdev);
@@ -904,7 +962,7 @@
 	return TRUE;
 }
 
-gboolean hcid_dbus_unregister_device(uint16_t id)
+gboolean hcid_dbus_mgr_unregister(uint16_t id)
 {
 	char dev[BLUETOOTH_DEVICE_NAME_LEN];
 	struct profile_obj_path_data *ptr = obj_path_table;
@@ -917,8 +975,8 @@
 		return FALSE;
 
 	for (; ptr->name; ptr++) {
-		dft_unreg = (num_adapters > 1) ? 0 : 1;
-		num_adapters--;
+		dft_unreg = (up_adapters > 1) ? 0 : 1;
+		up_adapters--;
 		ptr->unreg_func(connection, dft_unreg, id);
 
 		if (dft_unreg )
@@ -933,7 +991,7 @@
 		goto failed;
 	}
 
-	sprintf(dev, "hci%d", id);
+	sprintf(dev, HCI_DEVICE_NAME "%d", id);
 
 	dbus_message_iter_init_append(message, &iter);
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING ,&pdev);
@@ -965,39 +1023,17 @@
  */
 static int hci_dbus_reg_obj_path(DBusConnection *conn, int dft_reg, uint16_t id)
 {
-	struct hci_dbus_data *data;
 	char path[MAX_PATH_LENGTH];
 
 	/* register the default path*/
 	if (!dft_reg) {
-
 		sprintf(path, "%s/%s/%s", MANAGER_PATH, HCI_DEFAULT_DEVICE_NAME, BLUEZ_HCI);
-
-		data = malloc(sizeof(struct hci_dbus_data));
-		if (data == NULL)
-			return -1;
-
-		data->id = DEFAULT_DEVICE_PATH_ID;
-
-		if (!dbus_connection_register_object_path(conn, path, &obj_vtable, data)) { 
-			syslog(LOG_ERR,"DBUS failed to register %s object", path);
-			/* ignore, the default path was already registered */
-		}
+		register_dbus_path(path, DEFAULT_DEVICE_PATH_ID);
 	}
 
-	data = malloc(sizeof(struct hci_dbus_data));
-	if (data == NULL)
-		return -1;
-
-	data->id = id;
-
 	/* register the default path*/
 	sprintf(path, "%s/%s%d/%s", MANAGER_PATH, HCI_DEVICE_NAME, id, BLUEZ_HCI);
-
-	if (!dbus_connection_register_object_path(conn, path, &obj_vtable, data)) {
-		syslog(LOG_ERR,"DBUS failed to register %s object", path);
-		/* ignore, the path was already registered */
-	}
+	register_dbus_path(path, id);
 
 	return 0;
 }
@@ -1016,39 +1052,14 @@
 {
 	int ret = 0;
 	char path[MAX_PATH_LENGTH];
-	char dft_path[MAX_PATH_LENGTH];
-	void *data = NULL;
 
 	if (unreg_dft) {
-
-		sprintf(dft_path, "%s/%s/%s", MANAGER_PATH, HCI_DEFAULT_DEVICE_NAME, BLUEZ_HCI);
-
-		if (!dbus_connection_unregister_object_path (connection, dft_path)) {
-			syslog(LOG_ERR,"DBUS failed to unregister %s object", dft_path);
-			ret = -1;
-		} else {
-			if (dbus_connection_get_object_path_data(conn, dft_path, &data)) {
-				if (data) {
-					free(data);
-					data = NULL;
-				}
-			}
-		}
+		sprintf(path, "%s/%s/%s", MANAGER_PATH, HCI_DEFAULT_DEVICE_NAME, BLUEZ_HCI);
+		unregister_dbus_path(path);
 	}
 
 	sprintf(path, "%s/%s%d/%s", MANAGER_PATH, HCI_DEVICE_NAME, id, BLUEZ_HCI);
-
-	if (!dbus_connection_unregister_object_path (connection, path)) {
-		syslog(LOG_ERR,"DBUS failed to unregister %s object", path);
-		ret = -1;
-	} else {
-		if (dbus_connection_get_object_path_data(conn, path, &data)) {
-			if (data) {
-				free(data);
-				data = NULL;
-			}
-		}
-	}
+	unregister_dbus_path(path);
 
 	return ret;
 }

Re: [Bluez-devel] Device Configuration D-Bus services

[Marcel Holtmann]

Hi Eduardo,

> here is the initial patch regarding /org/bluez/Devices. I keep it
> really simple so we can improve it in small steps. I've merged the
> Claudio's code for register and unregister a new path for a device in
> two funcions as this code is used in 3 different parts of the code.

call it *register_device() and *register_manager().

> For now I've leaved the syslog call to register and unregister
> operations so we can see if every thing is going ok. 

I think it is a good idea to introduce some DBG() defines.

> My next step is to provide a list of registed devices (not only the
> devices that are up) through /org/bluez/Devices path.

Feel free to send another patch on top of this one.

> One question: the SetProperty and GetProperty will use an interger or
> a string constant to determing which operation will be used?

You need to explain to me what you are talking about.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc.
Get Certified Today * Register for a JBoss Training Course
Free Certification Exam for All Training Attendees Through End of 2005
Visit http://www.jboss.com/services/certification for more information
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] Device Configuration D-Bus services

[Eduardo Rocha]

[-- Attachment #1.1: Type: text/plain, Size: 2557 bytes --]

Hi Marcel,

I've changed the functions names. Here is the patch.

About the SetProperty:

Claudio said: " The device based paths can provide services like:
- SetProperty
- GetProperty
- Commands
- Features

Using SetProperty/GetProperty instead of define one method name for each
property is more clean. However it is important keep in mind that the
documentation of this service must be well-defined, because the method
call/method reply arguments will have different argument number and types.
In the method call, the first argument must be a byte(faster comparison)
indicating the command code instead of use a string to represent the
command"

The point is, as we have merged the main operations in two functions (set
and get), so we have to identify the operation by an integer or by a string
constant. It seems that a string is better for python binding (can anyone
confirm that?).

BR,
Eduardo.



On 10/24/05, Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Eduardo,
>
> > here is the initial patch regarding /org/bluez/Devices. I keep it
> > really simple so we can improve it in small steps. I've merged the
> > Claudio's code for register and unregister a new path for a device in
> > two funcions as this code is used in 3 different parts of the code.
>
> call it *register_device() and *register_manager().
>
> > For now I've leaved the syslog call to register and unregister
> > operations so we can see if every thing is going ok.
>
> I think it is a good idea to introduce some DBG() defines.
>
> > My next step is to provide a list of registed devices (not only the
> > devices that are up) through /org/bluez/Devices path.
>
> Feel free to send another patch on top of this one.
>
> > One question: the SetProperty and GetProperty will use an interger or
> > a string constant to determing which operation will be used?
>
> You need to explain to me what you are talking about.
>
> Regards
>
> Marcel
>
>
>
>
> -------------------------------------------------------
> This SF.Net email is sponsored by the JBoss Inc.
> Get Certified Today * Register for a JBoss Training Course
> Free Certification Exam for All Training Attendees Through End of 2005
> Visit http://www.jboss.com/services/certification for more information
> _______________________________________________
> Bluez-devel mailing list
> Bluez-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/bluez-devel
>



--
Eduardo Rocha
Instituto Nokia de Tecnologia - INdT

[-- Attachment #1.2: Type: text/html, Size: 3236 bytes --]

[-- Attachment #2: dbus_device.patch --]
[-- Type: text/x-patch, Size: 7806 bytes --]

--- bluez-cvs-old/utils/hcid/hcid.h	2005-10-24 15:29:36.204404720 -0300
+++ bluez-cvs-new/utils/hcid/hcid.h	2005-10-25 08:23:37.995046640 -0300
@@ -126,7 +126,9 @@
 gboolean hcid_dbus_init(void);
 void hcid_dbus_exit(void);
 gboolean hcid_dbus_register_device(uint16_t id);
+gboolean hcid_dbus_register_manager(uint16_t id);
 gboolean hcid_dbus_unregister_device(uint16_t id);
+gboolean hcid_dbus_unregister_manager(uint16_t id);
 void hcid_dbus_request_pin(int dev, struct hci_conn_info *ci);
 void hcid_dbus_inquiry_start(bdaddr_t *local);
 void hcid_dbus_inquiry_complete(bdaddr_t *local);
--- bluez-cvs-old/utils/hcid/main.c	2005-10-24 15:29:36.204404720 -0300
+++ bluez-cvs-new/utils/hcid/main.c	2005-10-25 08:25:34.803289088 -0300
@@ -377,6 +377,8 @@
 			start_security_manager(dr->dev_id);
 
 #ifdef ENABLE_DBUS
+		if (hci_test_bit(HCI_UP, &dr->dev_opt ))
+			hcid_dbus_register_manager(dr->dev_id);
 		hcid_dbus_register_device(dr->dev_id);
 #endif
 	}
@@ -430,10 +432,16 @@
 		syslog(LOG_INFO, "HCI dev %d registered", sd->dev_id);
 		if (hcid.auto_init)
 			init_device(sd->dev_id);
+#ifdef ENABLE_DBUS
+		hcid_dbus_register_device(sd->dev_id);
+#endif
 		break;
 
 	case HCI_DEV_UNREG:
 		syslog(LOG_INFO, "HCI dev %d unregistered", sd->dev_id);
+#ifdef ENABLE_DBUS
+		hcid_dbus_unregister_device(sd->dev_id);
+#endif
 		break;
 
 	case HCI_DEV_UP:
@@ -443,7 +451,7 @@
 		if (hcid.security)
 			start_security_manager(sd->dev_id);
 #ifdef ENABLE_DBUS
-		hcid_dbus_register_device(sd->dev_id);
+		hcid_dbus_register_manager(sd->dev_id);
 #endif
 		break;
 
@@ -452,7 +460,7 @@
 		if (hcid.security)
 			stop_security_manager(sd->dev_id);
 #ifdef ENABLE_DBUS
-		hcid_dbus_unregister_device(sd->dev_id);
+		hcid_dbus_unregister_manager(sd->dev_id);
 #endif
 		break;
 	}
--- bluez-cvs-old/utils/hcid/dbus.c	2005-10-24 15:29:36.171409736 -0300
+++ bluez-cvs-new/utils/hcid/dbus.c	2005-10-25 08:23:03.191337608 -0300
@@ -52,7 +52,7 @@
 #include "dbus.h"
 
 static DBusConnection *connection;
-static int num_adapters = 0;
+static int up_adapters = 0;
 
 #define TIMEOUT (30 * 1000)		/* 30 seconds */
 #define BLUETOOTH_DEVICE_NAME_LEN    (18)
@@ -359,6 +359,39 @@
 	free(req);
 }
 
+static gboolean register_dbus_path(char *path, uint16_t id)
+{
+	struct hci_dbus_data *data;
+	syslog(LOG_INFO,"Registering DBUS Path: %s", path);
+	data = malloc(sizeof(struct hci_dbus_data));
+	if (data == NULL) {
+		syslog(LOG_ERR,"Failed to alloc memory to DBUS path register data (%s)", path);
+		return FALSE;
+	}
+	data->id = id;
+
+	if (!dbus_connection_register_object_path(connection, path, &obj_vtable, data)) {
+		syslog(LOG_ERR,"DBUS failed to register %s object", path);
+		free(data);
+		return FALSE;
+	}
+	return TRUE;
+}
+
+static gboolean unregister_dbus_path(char *path)
+{
+	void *data;
+	syslog(LOG_INFO,"Unregistering DBUS Path: %s", path);
+	if (dbus_connection_get_object_path_data(connection, path, &data) && data) 
+		free(data);
+	
+	if (!dbus_connection_unregister_object_path (connection, path)) {
+		syslog(LOG_ERR,"DBUS failed to unregister %s object", path);
+		return FALSE;
+	} 
+	return TRUE;
+}
+
 void hcid_dbus_request_pin(int dev, struct hci_conn_info *ci)
 {
 	DBusMessage *message;
@@ -739,7 +772,7 @@
 
 	data->id = DEVICE_PATH_ID;
 
-	if (!dbus_connection_register_object_path(connection, DEVICE_PATH,
+	if (!dbus_connection_register_fallback(connection, DEVICE_PATH,
 						&obj_vtable, data)) {
 		syslog(LOG_ERR, "Can't register %s object", DEVICE_PATH);
 		return FALSE;
@@ -855,7 +888,32 @@
 	}
 }
 
-gboolean hcid_dbus_register_device(uint16_t id)
+gboolean hcid_dbus_register_device(uint16_t id) 
+{
+	char path[MAX_PATH_LENGTH+1];
+	char dev[BLUETOOTH_DEVICE_NAME_LEN+1];
+	const char *pdev = dev;
+
+	snprintf(dev, BLUETOOTH_DEVICE_NAME_LEN, HCI_DEVICE_NAME "%d", id);
+	snprintf(path, MAX_PATH_LENGTH, "%s/%s", DEVICE_PATH, pdev);
+
+	/* register the default path*/
+	return register_dbus_path(path, id);
+}
+
+gboolean hcid_dbus_unregister_device(uint16_t id)
+{
+	char dev[BLUETOOTH_DEVICE_NAME_LEN+1];
+	char path[MAX_PATH_LENGTH+1];
+	const char *pdev = dev;
+
+	snprintf(dev, BLUETOOTH_DEVICE_NAME_LEN, HCI_DEVICE_NAME "%d", id);
+	snprintf(path, MAX_PATH_LENGTH, "%s/%s", DEVICE_PATH, pdev);
+
+	return unregister_dbus_path(path);
+}
+
+gboolean hcid_dbus_register_manager(uint16_t id)
 {
 	char dev[BLUETOOTH_DEVICE_NAME_LEN];
 	struct profile_obj_path_data *ptr = obj_path_table;
@@ -873,7 +931,7 @@
 	}
 
 	if (!ret)
-		num_adapters++;
+		up_adapters++;
 
 	message = dbus_message_new_signal(BLUEZ_HCI_PATH,
 			BLUEZ_HCI_INTERFACE, BLUEZ_HCI_DEV_ADDED);
@@ -883,7 +941,7 @@
 		goto failed;
 	}
 
-	sprintf(dev, "hci%d", id);
+	sprintf(dev, HCI_DEVICE_NAME "%d", id);
 
 	dbus_message_iter_init_append(message, &iter);
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING ,&pdev);
@@ -904,7 +962,7 @@
 	return TRUE;
 }
 
-gboolean hcid_dbus_unregister_device(uint16_t id)
+gboolean hcid_dbus_unregister_manager(uint16_t id)
 {
 	char dev[BLUETOOTH_DEVICE_NAME_LEN];
 	struct profile_obj_path_data *ptr = obj_path_table;
@@ -917,8 +975,8 @@
 		return FALSE;
 
 	for (; ptr->name; ptr++) {
-		dft_unreg = (num_adapters > 1) ? 0 : 1;
-		num_adapters--;
+		dft_unreg = (up_adapters > 1) ? 0 : 1;
+		up_adapters--;
 		ptr->unreg_func(connection, dft_unreg, id);
 
 		if (dft_unreg )
@@ -933,7 +991,7 @@
 		goto failed;
 	}
 
-	sprintf(dev, "hci%d", id);
+	sprintf(dev, HCI_DEVICE_NAME "%d", id);
 
 	dbus_message_iter_init_append(message, &iter);
 	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING ,&pdev);
@@ -965,39 +1023,17 @@
  */
 static int hci_dbus_reg_obj_path(DBusConnection *conn, int dft_reg, uint16_t id)
 {
-	struct hci_dbus_data *data;
 	char path[MAX_PATH_LENGTH];
 
 	/* register the default path*/
 	if (!dft_reg) {
-
 		sprintf(path, "%s/%s/%s", MANAGER_PATH, HCI_DEFAULT_DEVICE_NAME, BLUEZ_HCI);
-
-		data = malloc(sizeof(struct hci_dbus_data));
-		if (data == NULL)
-			return -1;
-
-		data->id = DEFAULT_DEVICE_PATH_ID;
-
-		if (!dbus_connection_register_object_path(conn, path, &obj_vtable, data)) { 
-			syslog(LOG_ERR,"DBUS failed to register %s object", path);
-			/* ignore, the default path was already registered */
-		}
+		register_dbus_path(path, DEFAULT_DEVICE_PATH_ID);
 	}
 
-	data = malloc(sizeof(struct hci_dbus_data));
-	if (data == NULL)
-		return -1;
-
-	data->id = id;
-
 	/* register the default path*/
 	sprintf(path, "%s/%s%d/%s", MANAGER_PATH, HCI_DEVICE_NAME, id, BLUEZ_HCI);
-
-	if (!dbus_connection_register_object_path(conn, path, &obj_vtable, data)) {
-		syslog(LOG_ERR,"DBUS failed to register %s object", path);
-		/* ignore, the path was already registered */
-	}
+	register_dbus_path(path, id);
 
 	return 0;
 }
@@ -1016,39 +1052,14 @@
 {
 	int ret = 0;
 	char path[MAX_PATH_LENGTH];
-	char dft_path[MAX_PATH_LENGTH];
-	void *data = NULL;
 
 	if (unreg_dft) {
-
-		sprintf(dft_path, "%s/%s/%s", MANAGER_PATH, HCI_DEFAULT_DEVICE_NAME, BLUEZ_HCI);
-
-		if (!dbus_connection_unregister_object_path (connection, dft_path)) {
-			syslog(LOG_ERR,"DBUS failed to unregister %s object", dft_path);
-			ret = -1;
-		} else {
-			if (dbus_connection_get_object_path_data(conn, dft_path, &data)) {
-				if (data) {
-					free(data);
-					data = NULL;
-				}
-			}
-		}
+		sprintf(path, "%s/%s/%s", MANAGER_PATH, HCI_DEFAULT_DEVICE_NAME, BLUEZ_HCI);
+		unregister_dbus_path(path);
 	}
 
 	sprintf(path, "%s/%s%d/%s", MANAGER_PATH, HCI_DEVICE_NAME, id, BLUEZ_HCI);
-
-	if (!dbus_connection_unregister_object_path (connection, path)) {
-		syslog(LOG_ERR,"DBUS failed to unregister %s object", path);
-		ret = -1;
-	} else {
-		if (dbus_connection_get_object_path_data(conn, path, &data)) {
-			if (data) {
-				free(data);
-				data = NULL;
-			}
-		}
-	}
+	unregister_dbus_path(path);
 
 	return ret;
 }

Re: [Bluez-devel] Device Configuration D-Bus services

[Claudio Takahasi]

[-- Attachment #1: Type: text/plain, Size: 2983 bytes --]

Hi,

Basically the services under the path "/org/bluez/Devices" will
provide a way to userspace applications setup the devices, a similar
functionality provided by hciconfig. The security issues will be handled by
the D-Bus configuration file.

Regarding the SetProperty/GetProperty...
There are a lot of commands that we intend provide: up, down, reset, rstat,
auth, noauth, encrypt, ....
My initial suggestion was divide the services into TWO groups:
* Device independant, path: /org/bluez/Devices, interface: path:
org.bluez.Devices
Services:
- Up
- Down
- Reset

* Device Based services, path: /org/bluez/Devices/hciX, interface: path:
org.bluez.Devices
Services:
- auth
- noauth
- encrypt
- noencrypt
- secmgr
- nosecmgr
- piscan
- noscan
- iscan
- pscan
...

In order to make the API more clean, the device based services will use the
method
name(SerProperty/GetProperty), where the first message argument is the
command
identification (auth, noauth, encrypt,...)

The question is: if we use an String representation of the command the
client coding
will be more "legible" by other hand the hcid message argument
extracting/comparison
will be more expensive than extract a byte or a integer. Which approach do
you prefer?

Regards,
Claudio



On 10/24/05, Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Eduardo,
>
> > here is the initial patch regarding /org/bluez/Devices. I keep it
> > really simple so we can improve it in small steps. I've merged the
> > Claudio's code for register and unregister a new path for a device in
> > two funcions as this code is used in 3 different parts of the code.
>
> call it *register_device() and *register_manager().
>
> > For now I've leaved the syslog call to register and unregister
> > operations so we can see if every thing is going ok.
>
> I think it is a good idea to introduce some DBG() defines.
>
> > My next step is to provide a list of registed devices (not only the
> > devices that are up) through /org/bluez/Devices path.
>
> Feel free to send another patch on top of this one.
>
> > One question: the SetProperty and GetProperty will use an interger or
> > a string constant to determing which operation will be used?
>
> You need to explain to me what you are talking about.


Regards
>
> Marcel
>
>
>
>
> -------------------------------------------------------
> This SF.Net email is sponsored by the JBoss Inc.
> Get Certified Today * Register for a JBoss Training Course
> Free Certification Exam for All Training Attendees Through End of 2005
> Visit http://www.jboss.com/services/certification for more information
> _______________________________________________
> 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: Type: text/html, Size: 4381 bytes --]

Re: [Bluez-devel] Device Configuration D-Bus services

[Johan Hedberg]

On Tue, Oct 25, 2005, Eduardo Rocha wrote:
> The point is, as we have merged the main operations in two functions (set
> and get), so we have to identify the operation by an integer or by a string
> constant. It seems that a string is better for python binding (can anyone
> confirm that?).

At least by using strings it would be much easier to understand what the
client code is doing, e.g. if you compare
SetProperty("page-timeout", 10)
vs. 
SetProperty(3, 10)

So, I vote for using strings instead of integers.

Johan


-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc.
Get Certified Today * Register for a JBoss Training Course
Free Certification Exam for All Training Attendees Through End of 2005
Visit http://www.jboss.com/services/certification for more information
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] Device Configuration D-Bus services

[Eduardo Rocha]

[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]

Hi Marcel,

here is some code cleanup. This patch should be applied after the
dbus_device.patch.

BR,
Eduardo.

--
Eduardo Rocha
Instituto Nokia de Tecnologia - INdT

On 10/25/05, Johan Hedberg <johan.hedberg@nokia.com> wrote:
> On Tue, Oct 25, 2005, Eduardo Rocha wrote:
> > The point is, as we have merged the main operations in two functions (set
> > and get), so we have to identify the operation by an integer or by a string
> > constant. It seems that a string is better for python binding (can anyone
> > confirm that?).
>
> At least by using strings it would be much easier to understand what the
> client code is doing, e.g. if you compare
> SetProperty("page-timeout", 10)
> vs.
> SetProperty(3, 10)
>
> So, I vote for using strings instead of integers.
>
> Johan
>
>
> -------------------------------------------------------
> This SF.Net email is sponsored by the JBoss Inc.
> Get Certified Today * Register for a JBoss Training Course
> Free Certification Exam for All Training Attendees Through End of 2005
> Visit http://www.jboss.com/services/certification for more information
> _______________________________________________
> Bluez-devel mailing list
> Bluez-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/bluez-devel
>

[-- Attachment #2: dbus_cleanup.patch --]
[-- Type: text/x-patch, Size: 6811 bytes --]

--- bluez-cvs-25102005/utils/hcid/dbus.c	2005-10-25 08:23:03.191337608 -0300
+++ bluez-cvs-new/utils/hcid/dbus.c	2005-10-25 15:19:53.656169024 -0300
@@ -1215,11 +1215,12 @@
 	uint8_t length;
 	uint8_t max_period;
 	uint8_t min_period;
-	int sock = -1;
+	int dd = -1;
 	int dev_id = -1;
 
 	if (dbus_data->id == DEFAULT_DEVICE_PATH_ID) {
-		if ((dev_id = hci_get_route(NULL)) < 0) {
+		dev_id = hci_get_route(NULL);
+		if (dev_id < 0) {
 			syslog(LOG_ERR, "Bluetooth device is not available");
 			reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_ENODEV);
 			goto failed;
@@ -1228,8 +1229,8 @@
 	} else
 		dev_id =  dbus_data->id;
 
-	sock = hci_open_dev(dev_id);
-	if (sock < 0) {
+	dd = hci_open_dev(dev_id);
+	if (dd < 0) {
 		syslog(LOG_ERR, "HCI device open failed");
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_ENODEV);
 		goto failed;
@@ -1260,14 +1261,14 @@
 
 	inq_mode.mode = 1; //INQUIRY_WITH_RSSI;
 
-	if (hci_send_cmd(sock, OGF_HOST_CTL, OCF_WRITE_INQUIRY_MODE,
+	if (hci_send_cmd(dd, OGF_HOST_CTL, OCF_WRITE_INQUIRY_MODE,
 				WRITE_INQUIRY_MODE_CP_SIZE, &inq_mode) < 0) {
 		syslog(LOG_ERR, "Can't set inquiry mode:%s.", strerror(errno));
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + errno);
 		goto failed;
 	}
 
-	if (hci_send_cmd(sock, OGF_LINK_CTL, OCF_PERIODIC_INQUIRY,
+	if (hci_send_cmd(dd, OGF_LINK_CTL, OCF_PERIODIC_INQUIRY,
 				PERIODIC_INQUIRY_CP_SIZE, &inq_param) < 0) {
 		syslog(LOG_ERR, "Can't send HCI commands:%s.", strerror(errno));
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + errno);
@@ -1281,8 +1282,8 @@
 	}
 
 failed:
-	if (sock > 0)
-		close(sock);
+	if (dd >= 0)
+		close(dd);
 
 	return reply;
 }
@@ -1292,11 +1293,12 @@
 	DBusMessageIter iter;
 	DBusMessage *reply = NULL;
 	struct hci_dbus_data *dbus_data = data;
-	int sock = -1;
+	int dd = -1;
 	int dev_id = -1;
 
 	if (dbus_data->id == DEFAULT_DEVICE_PATH_ID) {
-		if ((dev_id = hci_get_route(NULL)) < 0) {
+		dev_id = hci_get_route(NULL);
+		if (dev_id < 0) {
 			syslog(LOG_ERR, "Bluetooth device is not available");
 			reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_ENODEV);
 			goto failed;
@@ -1304,13 +1306,14 @@
 	} else
 		dev_id = dbus_data->id;
 
-	if ((sock = hci_open_dev(dev_id)) < 0) {
+	dd = hci_open_dev(dev_id);
+	if (dd < 0) {
 		syslog(LOG_ERR, "HCI device open failed");
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_ENODEV);
 		goto failed;
 	}
 
-	if (hci_send_cmd(sock, OGF_LINK_CTL, OCF_EXIT_PERIODIC_INQUIRY, 0 , NULL) < 0) {
+	if (hci_send_cmd(dd, OGF_LINK_CTL, OCF_EXIT_PERIODIC_INQUIRY, 0 , NULL) < 0) {
 		syslog(LOG_ERR, "Send hci command failed.");
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + errno);
 	} else {
@@ -1322,8 +1325,8 @@
 	}
 
 failed:
-	if (sock >= 0)
-		close(sock);
+	if (dd >= 0)
+		close(dd);
 
 	return reply;
 }
@@ -1447,7 +1450,7 @@
 	bdaddr_t bdaddr;
 	uint8_t role;
 	int dev_id = -1;
-	int sock = -1;
+	int dd = -1;
 
 	dbus_message_iter_init(msg, &iter);
 	dbus_message_iter_get_basic(&iter, &str_bdaddr);
@@ -1470,14 +1473,14 @@
 		goto failed;
 	}
 
-	sock = hci_open_dev(dev_id);
-	if (sock < 0) {
+	dd = hci_open_dev(dev_id);
+	if (dd < 0) {
 		syslog(LOG_ERR, "HCI device open failed\n");
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_ENODEV);
 		goto failed;
 	}
 
-	if (hci_switch_role(sock, &bdaddr, role, 10000) < 0) {
+	if (hci_switch_role(dd, &bdaddr, role, 10000) < 0) {
 		syslog(LOG_ERR, "Switch role request failed\n");
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + errno);
 	} else {
@@ -1511,7 +1514,8 @@
 	str2ba(str_bdaddr, &bdaddr);
 
 	if (dbus_data->id == DEFAULT_DEVICE_PATH_ID) {
-		if ((dev_id = hci_get_route(&bdaddr)) < 0) {
+		dev_id = hci_get_route(&bdaddr);
+		if (dev_id  < 0) {
 			syslog(LOG_ERR, "Bluetooth device is not available");
 			reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_ENODEV);
 			goto failed;
@@ -1571,7 +1575,8 @@
 	int sk = -1;
 
 	if (dbus_data->id == DEFAULT_DEVICE_PATH_ID) {
-		if ((dev_id = hci_get_route(NULL)) < 0) {
+		dev_id = hci_get_route(NULL);
+		if (dev_id < 0) {
 			syslog(LOG_ERR, "Bluetooth device is not available");
 			reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_ENODEV);
 			goto failed;
@@ -1621,7 +1626,7 @@
 	dbus_message_iter_close_container(&iter, &array_iter);
 
 failed:
-	if (sk >= 0)
+	if (sk > 0)
 		close(sk);
 
 	if (cl)
@@ -1642,7 +1647,7 @@
 	struct hci_conn_info_req *cr = NULL;
 	bdaddr_t bdaddr;
 	int dev_id = -1;
-	int sock = -1;
+	int dd = -1;
 
 	dbus_message_iter_init(msg, &iter);
 	dbus_message_iter_get_basic(&iter, &str_bdaddr);
@@ -1660,8 +1665,8 @@
 		goto failed;
 	}
 
-	sock = hci_open_dev(dev_id);
-	if (sock < 0) {
+	dd = hci_open_dev(dev_id);
+	if (dd < 0) {
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_ENODEV);
 		goto failed;
 	}
@@ -1675,7 +1680,7 @@
 	bacpy(&cr->bdaddr, &bdaddr);
 	cr->type = ACL_LINK;
 
-	if (ioctl(sock, HCIGETCONNINFO, (unsigned long) cr) < 0) {
+	if (ioctl(dd, HCIGETCONNINFO, (unsigned long) cr) < 0) {
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + errno);
 		goto failed;
 	}
@@ -1692,15 +1697,15 @@
 	rq.rlen = EVT_CMD_STATUS_SIZE;
 	rq.event = EVT_CMD_STATUS;
 
-	if (hci_send_req(sock, &rq, 25000) < 0) {
+	if (hci_send_req(dd, &rq, 25000) < 0) {
 		syslog(LOG_ERR, "Unable to send authentication request: %s", strerror(errno));
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + errno);
 		goto failed;
 	}
 
 failed:
-	if (sock >= 0)
-		close(sock);
+	if (dd >= 0)
+		close(dd);
 
 	if (cr)
 		free(cr);
@@ -1725,7 +1730,7 @@
 	struct hci_dev_req *dr      = NULL;
 	struct hci_dev_info di;
 	int i;
-	int sock = -1;
+	int sk = -1;
 
 	char aname[BLUETOOTH_DEVICE_NAME_LEN];
 	char aaddr[BLUETOOTH_DEVICE_ADDR_LEN];
@@ -1734,8 +1739,8 @@
 	const char array_sig[] = HCI_DEVICE_STRUCT_SIGNATURE;
 
 	/* Create and bind HCI socket */
-	sock = socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI);
-	if (sock < 0) {
+	sk = socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI);
+	if (sk < 0) {
 		syslog(LOG_ERR, "Can't open HCI socket: %s (%d)", strerror(errno), errno);
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + errno);
 		goto failed;
@@ -1751,7 +1756,7 @@
 	dl->dev_num = HCI_MAX_DEV;
 	dr = dl->dev_req;
 
-	if (ioctl(sock, HCIGETDEVLIST, dl) < 0) {
+	if (ioctl(sk, HCIGETDEVLIST, dl) < 0) {
 		reply = bluez_new_failure_msg(msg, BLUEZ_ESYSTEM_OFFSET + errno);
 		goto failed;
 	}
@@ -1769,7 +1774,7 @@
 		memset(&di, 0 , sizeof(struct hci_dev_info));
 		di.dev_id = dr->dev_id;
 
-		if (ioctl(sock, HCIGETDEVINFO, &di) < 0)
+		if (ioctl(sk, HCIGETDEVINFO, &di) < 0)
 			continue;
 
 		strcpy(aname, di.name);
@@ -1787,8 +1792,8 @@
 	dbus_message_iter_close_container(&iter, &array_iter);
 
 failed:
-	if (sock >= 0)
-		close(sock);
+	if (sk > 0)
+		close(sk);
 
 	if (dl)
 		free(dl);

Re: [Bluez-devel] Device Configuration D-Bus services

[Marcel Holtmann]

Hi Eduardo,

> I've changed the functions names.  Here is the  patch. 

the patch is now in the CVS. Thanks.

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc.
Get Certified Today * Register for a JBoss Training Course
Free Certification Exam for All Training Attendees Through End of 2005
Visit http://www.jboss.com/services/certification for more information
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] Device Configuration D-Bus services

[Marcel Holtmann]

Hi Eduardo,

> here is some code cleanup. This patch should be applied after the
> dbus_device.patch.

the patch is now in the CVS, but I had to fix "if (sk > 0)" ;)

Regards

Marcel




-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc.
Get Certified Today * Register for a JBoss Training Course
Free Certification Exam for All Training Attendees Through End of 2005
Visit http://www.jboss.com/services/certification for more information
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel