Skip to contentSkip to conversation

hcid patch

Bluez-devel Claudio Takahasi 3 participants 8 messages Sep 30, 2005 – Oct 4, 2005 Bluetooth drivers

lore
Share & tools
lore
#0 lore raw

Hi folks/Marcel,

The patch is attached. The basic structure to start the development of the "sdp", "network", "serial", ... was added.
There are a lot not implemented methods.

Regards,
Claudio.

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

Main Features

1. added multiple bt adapter support
2. added the structures to support profiles
3. Modified paths to "/org/bluez/Devices" and "/org/bluez/Manager". Where "Devices" is going to provide
device configuration services currently provided by hciconfig tool. The "Manager" will handle
services related to connection establishment and setup, including profiles, sdp and device discovery.

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

Next Action

1. Move the functions to to suitable file names( eg: dbus-mgr.c and dbus-dev.c)
2. Define the device based path name.

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

How Test

1. /org/bluez/Devices, device up
$dbus-send --system --dest=org.bluez --type=method_call /org/bluez/Devices org.bluez.Devices.Up

2. /org/bluez/Manager, Device list
$dbus-send --system --dest=org.bluez --type=method_call /org/bluez/Manager org.bluez.Manager.DeviceList

comment: This service returns an array of devices. The reply signature is "a(ss)", where the first string is the device name(eg: hci0, hci1) and the second string is the bluetooth address. You have to develop a python client or a "c" client to see the content.

3. /org/bluez/Manager/default/Controller, Inquiry
$dbus-send --system --dest=org.bluez --type=method_call /org/bluez/Manager/default/Controller org.bluez.Manager.Inquiry byte:4 byte:10 uint16:0
or
$dbus-send --system --dest=org.bluez --type=method_call /org/bluez/Manager/hci0/Controller org.bluez.Manager.Inquiry byte:4 byte:10 uint16:0

comment: This service returns an array of devices. The reply signature is "a(suq)", where the first string is the bt address, the snd is the class and the last is the clock offset. You have to develop a python client or a "c" client to see the content.

4. /org/bluez/Manager/default/Controller, Periodic Inquiry $dbus-send --system --dest=org.bluez --type=method_call /org/bluez/Manager/default/Controller org.bluez.Manager.PeriodicInquirybyte:4 byte:27 byte:49
or
$dbus-send --system --dest=org.bluez --type=method_call /org/bluez/Manager/hci0/Controller org.bluez.Manager.PeriodicInquiry byte:4 byte:27 byte:49

5. /org/bluez/Manager/default/Controller, Cancel Periodic Inquiry dbus-send --system --dest=org.bluez --type=method_call /org/bluez/Manager/default/Controller org.bluez.Manager.CancelPeriodic or
dbus-send --system --dest=org.bluez --type=method_call /org/bluez/Manager/hci0/Controller org.bluez.Manager.CancelPeriodic
6. /org/bluez/Manager/default/Controller, RoleSwitch dbus-send --system --dest=org.bluez --type=method_call /org/bluez/Manager/default/Controller org.bluez.Manager.RoleSwitchstring:AA:AA:AA:AA:AA:AA byte:1

--
---------------------------------------------------------
Claudio Takahasi
Nokia's Institute of Technology - INdT
[email protected]

hcid/dbus-internal.h +82 -0
@@ -0,0 +1,82 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2000-2001  Qualcomm Incorporated
+ *  Copyright (C) 2002-2003  Maxim Krasnyansky <[email protected]>
+ *  Copyright (C) 2002-2005  Marcel Holtmann <[email protected]>
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation;
+ *
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ *  OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY RIGHTS.
+ *  IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LIABLE FOR ANY
+ *  CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES 
+ *  WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN 
+ *  ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF 
+ *  OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ *  ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS, 
+ *  COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS 
+ *  SOFTWARE IS DISCLAIMED.
+ *
+ *
+ *  $Id: dbus-internal.h,v 0.1 2005/09/16 08:00:00 holtmann Exp $
+ */
+ 
+#ifndef __H_BLUEZ_DBUS_INTERNAL_H__
+#define __H_BLUEZ_DBUS_INTERNAL_H__
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+#include <dbus/dbus.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <string.h>
+
+#define DEVICE_ID_LENGTH   (4)
+
+#define BLUETOOTH_DEVICE_NAME_LEN    (18)
+#define BLUETOOTH_DEVICE_ADDR_LEN    (18)
+
+#define MAX_PATH_LENGTH   (64)
+
+
+
+typedef DBusMessage* (service_handler_func_t)(DBusMessage *, void *);
+typedef int8_t  register_function_t(DBusConnection *conn, uint8_t dft_reg, uint16_t id);
+typedef int8_t  unregister_function_t(DBusConnection *conn, uint8_t unreg_dft, uint16_t id);
+
+typedef struct
+{
+	const char             *name;
+	service_handler_func_t *handler_func;
+	const char             *signature;
+}service_table_t;
+
+typedef const service_table_t* get_svc_table_func_t(void);
+typedef struct {
+	const char		*name;
+	uint8_t			status; /* 1:active  0:disabled */
+	uint8_t			dft_reg; /* dft path registered */
+	register_function_t     *reg_func;
+	unregister_function_t   *unreg_func;
+	get_svc_table_func_t *get_svc_table; /* return the service table */
+}__attribute__((packed))profile_obj_path_data_t;
+
+/* D-Bus Manager functions */
+gboolean mgr_dbus_reg_device_paths(uint16_t id);
+gboolean mgr_dbus_unreg_device_paths(uint16_t id);
+
+/* HCI functions */
+const service_table_t *get_hci_table(void);
+int8_t  hci_dbus_reg_obj_path(DBusConnection *conn, uint8_t dft_reg, uint16_t id);
+int8_t  hci_dbus_unreg_obj_path(DBusConnection *conn, uint8_t unreg_dft, uint16_t id);
+
+
+#endif //__H_BLUEZ_DBUS_INTERNAL_H__
hcid/hcid.h +1 -0
@@ -122,6 +122,7 @@
 
 #ifdef ENABLE_DBUS
 gboolean hcid_dbus_init(void);
+void hcid_dbus_exit(void);
 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);
hcid/dbus.h +200 -0

200 lines not on this page · raw message

hcid/dbus.c +900 -9

974 lines not on this page · raw message

hcid/main.c +16 -1
@@ -57,6 +57,10 @@
 #include "hcid.h"
 #include "lib.h"
 
+#ifdef ENABLE_DBUS
+#include "dbus-internal.h"
+#endif
+
 struct hcid_opts hcid;
 struct device_opts default_device;
 struct device_opts *parser_device;
@@ -375,6 +379,9 @@
 
 		if (hcid.security && hci_test_bit(HCI_UP, &dr->dev_opt))
 			start_security_manager(dr->dev_id);
+#ifdef ENABLE_DBUS
+			mgr_dbus_reg_device_paths(dr->dev_id);
+#endif
 	}
 
 	free(dl);
@@ -438,12 +445,18 @@
 			configure_device(sd->dev_id);
 		if (hcid.security)
 			start_security_manager(sd->dev_id);
+#ifdef ENABLE_DBUS
+		mgr_dbus_reg_device_paths(sd->dev_id);
+#endif
 		break;
 
 	case HCI_DEV_DOWN:
 		syslog(LOG_INFO, "HCI dev %d down", sd->dev_id);
 		if (hcid.security)
 			stop_security_manager(sd->dev_id);
+#ifdef ENABLE_DBUS
+		mgr_dbus_unreg_device_paths(sd->dev_id);
+#endif
 		break;
 	}
 }
@@ -627,7 +640,9 @@
 	g_main_run(event_loop);
 
 	free_device_opts();
-
+#ifdef ENABLE_DBUS	
+	hcid_dbus_exit();
+#endif
 	syslog(LOG_INFO, "Exit.");
 	return 0;
 }
common/util.h +35 -0
@@ -0,0 +1,35 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2005       Claudio Takahasi <[email protected]>
+ *  Copyright (C) 2002-2005  Marcel Holtmann <[email protected]>
+ *
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation;
+ *
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ *  OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY RIGHTS.
+ *  IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LIABLE FOR ANY
+ *  CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES 
+ *  WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN 
+ *  ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF 
+ *  OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ *  ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS, 
+ *  COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS 
+ *  SOFTWARE IS DISCLAIMED.
+ *
+ *
+ *  $Id: util.h,v 0.1 2005/09/23 08:00:00 holtmann Exp $
+ */
+ 
+#ifndef __H_BLUEZ_UTIL_H__
+#define __H_BLUEZ_UTIL_H__
+
+int find_conn(int s, int dev_id, long arg);
+
+#endif //__H_BLUEZ_UTIL_H__
common/Makefile.am +3 -1
@@ -2,10 +2,12 @@
 #  $Id: Makefile.am,v 1.2 2005/08/05 11:07:34 holtmann Exp $
 #
 
-noinst_LIBRARIES = libtextfile.a
+noinst_LIBRARIES = libtextfile.a libutil.a
 
 libtextfile_a_SOURCES = textfile.h textfile.c
 
+libutil_a_SOURCES = util.h util.c
+
 noinst_PROGRAMS = test_textfile
 
 test_textfile_LDADD = libtextfile.a
common/util.c +80 -0

80 lines not on this page · raw message

tools/Makefile.am +1 -1
@@ -44,7 +44,7 @@
 hciconfig_LDADD = @BLUEZ_LIBS@ $(top_builddir)/common/libtextfile.a
 
 hcitool_SOURCES = hcitool.c oui.h oui.c
-hcitool_LDADD = @BLUEZ_LIBS@ $(top_builddir)/common/libtextfile.a
+hcitool_LDADD = @BLUEZ_LIBS@ $(top_builddir)/common/libtextfile.a $(top_builddir)/common/libutil.a
 
 l2ping_LDADD = @BLUEZ_LIBS@
 
tools/hcitool.c +1 -26
@@ -49,6 +49,7 @@
 
 #include "textfile.h"
 #include "oui.h"
+#include "util.h"
 
 #define for_each_opt(opt, long, short) while ((opt=getopt_long(argc, argv, short ? short:"+", long, NULL)) != -1)
 
@@ -103,32 +104,6 @@
 	return 0;
 }
 
-static int find_conn(int s, int dev_id, long arg)
-{
-	struct hci_conn_list_req *cl;
-	struct hci_conn_info *ci;
-	int i;
-
-	if (!(cl = malloc(10 * sizeof(*ci) + sizeof(*cl)))) {
-		perror("Can't allocate memory");
-		exit(1);
-	}
-	cl->dev_id = dev_id;
-	cl->conn_num = 10;
-	ci = cl->conn_info;
-
-	if (ioctl(s, HCIGETCONNLIST, (void *) cl)) {
-		perror("Can't get connection list");
-		exit(1);
-	}
-
-	for (i = 0; i < cl->conn_num; i++, ci++)
-		if (!bacmp((bdaddr_t *) arg, &ci->bdaddr))
-			return 1;
-
-	return 0;
-}
-
 static void hex_dump(char *pref, int width, unsigned char *buf, int len)
 {
 	register int i,n;
maintainer
#1 lore raw

Hi Claudio,

Quoted 3 lines

The patch is attached. The basic structure to start the development of the "sdp", "network", "serial", ... was added.
There are a lot not implemented methods.

this patch looks like this:

 common/Makefile.am   |    4
 common/util.c        |   80 ++++
 common/util.h        |   35 +
 hcid/dbus-internal.h |   82 ++++
 hcid/dbus.c          |  909 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 hcid/dbus.h          |  200 +++++++++++
 hcid/hcid.h          |    1
 hcid/main.c          |   17
 tools/Makefile.am    |    2
 tools/hcitool.c      |   27 -
 10 files changed, 1319 insertions(+), 38 deletions(-)

It is changing everything and looks like a previous patch that I reviewed.

I don't wanna see any changes outside hcid/dbus.[ch] at the moment and the error definitions we agreed on are not included. It seems you just modified one of your older patches with the new objects and paths. I want to include something before the next release, but I am going the full patch at the moment.

Regards

Marcel

-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions, and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

#2 lore raw

Hi Marcel,

I kept some log information, if you want remove it I can send another patch. The error code definition was included. I am using "hciX" in the path name until we receive a answer from dbus list. Services under the path /org/bluez/Devices weren't implemented yet.

Is there other changes in this patch?

Regards,
Claudio

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

Main Features

1. added multiple bt adapter support
2. added the structures to support profiles
3. Modified paths to "/org/bluez/Devices" and "/org/bluez/Manager". Where "Devices" is going to provide device configuration services currently provided by hciconfig tool. The "Manager" will handle services related to connection establishment and setup, including profiles, sdp and device discovery.
4. Added error new code definition

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

Next Action

1. Move the functions to to suitable file names( eg: dbus-mgr.c and dbus-dev.c)
2. Define the device based path name. (waiting D-Bus mailing list answers)
3. Investigate fallback behaviour(how handle properly unregistered paths)

Quoted 37 lines · Marcel Holtmann

Hi Claudio,

Quoted 3 lines

The patch is attached. The basic structure to start the development of the "sdp", "network", "serial", ... was added.
There are a lot not implemented methods.

this patch looks like this:

common/Makefile.am | 4
common/util.c | 80 ++++
common/util.h | 35 +
hcid/dbus-internal.h | 82 ++++

hcid/dbus.c | 909 ++++++++++++++++++++++++++++++++++++++++++++++++++-
hcid/dbus.h | 200 +++++++++++
hcid/hcid.h | 1
hcid/main.c | 17
tools/Makefile.am | 2
tools/hcitool.c | 27 -
10 files changed, 1319 insertions(+), 38 deletions(-)

It is changing everything and looks like a previous patch that I reviewed.

I don't wanna see any changes outside hcid/dbus.[ch] at the moment and the error definitions we agreed on are not included. It seems you just modified one of your older patches with the new objects and paths. I want to include something before the next release, but I am going the full patch at the moment.

Regards

Marcel

Quoted 9 lines

-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions, and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

--
---------------------------------------------------------
Claudio Takahasi
Nokia's Institute of Technology - INdT
[email protected]

hcid/hcid.h +3 -0
@@ -122,6 +122,9 @@
 
 #ifdef ENABLE_DBUS
 gboolean hcid_dbus_init(void);
+void hcid_dbus_exit(void);
+gboolean mgr_dbus_reg_device_paths(uint16_t id);
+gboolean mgr_dbus_unreg_device_paths(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);
hcid/dbus.h +209 -0

209 lines not on this page · raw message

hcid/dbus.c +993 -9

1,070 lines not on this page · raw message

hcid/main.c +12 -1
@@ -375,6 +375,9 @@
 
 		if (hcid.security && hci_test_bit(HCI_UP, &dr->dev_opt))
 			start_security_manager(dr->dev_id);
+#ifdef ENABLE_DBUS
+			mgr_dbus_reg_device_paths(dr->dev_id);
+#endif
 	}
 
 	free(dl);
@@ -438,12 +441,18 @@
 			configure_device(sd->dev_id);
 		if (hcid.security)
 			start_security_manager(sd->dev_id);
+#ifdef ENABLE_DBUS
+		mgr_dbus_reg_device_paths(sd->dev_id);
+#endif
 		break;
 
 	case HCI_DEV_DOWN:
 		syslog(LOG_INFO, "HCI dev %d down", sd->dev_id);
 		if (hcid.security)
 			stop_security_manager(sd->dev_id);
+#ifdef ENABLE_DBUS
+		mgr_dbus_unreg_device_paths(sd->dev_id);
+#endif
 		break;
 	}
 }
@@ -627,7 +636,9 @@
 	g_main_run(event_loop);
 
 	free_device_opts();
-
+#ifdef ENABLE_DBUS	
+	hcid_dbus_exit();
+#endif
 	syslog(LOG_INFO, "Exit.");
 	return 0;
 }
maintainer
#3 lore raw

Hi Claudio,

Quoted 6 lines

I kept some log information, if you want remove it I can send another patch. The error code definition was included. I am using "hciX" in the path name until we receive a answer from dbus list. Services under the path /org/bluez/Devices weren't implemented yet.

Is there other changes in this patch?

this thing looks quite good, but now we need to clean up some other parts of your constants and function naming. And please try to keep the BlueZ/kernel coding style.

The overall BLUEZ_DBUS prefix looks like a good idea at the beginning, but I think we don't need it at all. So this should be enough:

#define DEVICE_PATH "/org/bluez/Device"
#define DEVICE_INTERFACE "org.bluez.Device"

#define MANAGER_PATH "/org/bluez/Manager"
#define MANAGER_INTERFACE "org.bluez.Manager"

Please also stop to shortcut some names. For example DFT for DEFAULT. Simple write the full name, because then it is easy to read this code.

The other thing is that I don't like the difference between *_REQ and *_SIG in the names/constants. Please don't use them.

Also please don't include any RFCOMM, PAN, HID etc. related things, because they are not important right now and they make my life of merging the initial step into better D-Bus a lot harder.

For the error part we will use org.bluez.Error and not EFailed or anything like this. Since all the error stuff is currently not used, I don't think it is a good idea to add it to the initial patch. Looking at HAL the use of org.bluez.NoSuchDevice, etc. seems to make sense, too.

Do we really need that service_table_t thingy now? I think we should start without it.

Regards

Marcel

-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions, and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

#4 lore raw

Hi Marcel,

Your suggestions were done!
Regarding service_table_t, if we remove it, the logic will be based on "if"and "else", see the HAL message filter function. In my opinion service_table_t is more clear.

If you think that is really necessary remove the error handling and the service_table_t I can do it in the next patch.

Regards,
Claudio.

Quoted 48 lines · Marcel Holtmann

Hi Claudio,

Quoted 6 lines

I kept some log information, if you want remove it I can send another patch. The error code definition was included. I am using "hciX" in the path name until we receive a answer from dbus list. Services under the path /org/bluez/Devices weren't implemented yet.

Is there other changes in this patch?

this thing looks quite good, but now we need to clean up some other parts of your constants and function naming. And please try to keep the BlueZ/kernel coding style.

The overall BLUEZ_DBUS prefix looks like a good idea at the beginning, but I think we don't need it at all. So this should be enough:

#define DEVICE_PATH "/org/bluez/Device"
#define DEVICE_INTERFACE "org.bluez.Device"

#define MANAGER_PATH "/org/bluez/Manager"
#define MANAGER_INTERFACE "org.bluez.Manager"

Please also stop to shortcut some names. For example DFT for DEFAULT. Simple write the full name, because then it is easy to read this code.

The other thing is that I don't like the difference between *_REQ and *_SIG in the names/constants. Please don't use them.

Also please don't include any RFCOMM, PAN, HID etc. related things, because they are not important right now and they make my life of merging the initial step into better D-Bus a lot harder.

For the error part we will use org.bluez.Error and not EFailed or anything like this. Since all the error stuff is currently not used, I don't think it is a good idea to add it to the initial patch. Looking at HAL the use of org.bluez.NoSuchDevice, etc. seems to make sense, too.

Do we really need that service_table_t thingy now? I think we should start without it.

Regards

Marcel

Quoted 9 lines

-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions, and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

--
---------------------------------------------------------
Claudio Takahasi
Nokia's Institute of Technology - INdT
[email protected]

#5 lore raw

Hi folks,

Sorry, I forgot attach the patch.

Quoted 82 lines · Claudio Takahasi

Hi Marcel,

Your suggestions were done!
Regarding service_table_t, if we remove it, the logic will be based on "if"and "else", see the HAL message filter function. In my opinion service_table_t is more clear.

If you think that is really necessary remove the error handling and the service_table_t I can do it in the next patch.

Regards,
Claudio.

Quoted 49 lines · Marcel Holtmann

Hi Claudio,

Quoted 7 lines

I kept some log information, if you want remove it I can send another patch. The error code definition was included. I am using "hciX" in the path name until we receive a answer from dbus list. Services under

the path /org/bluez/Devices weren't implemented yet.

Is there other changes in this patch?

this thing looks quite good, but now we need to clean up some other parts of your constants and function naming. And please try to keep the BlueZ/kernel coding style.

The overall BLUEZ_DBUS prefix looks like a good idea at the beginning, but I think we don't need it at all. So this should be enough:

#define DEVICE_PATH "/org/bluez/Device"
#define DEVICE_INTERFACE "org.bluez.Device"

#define MANAGER_PATH "/org/bluez/Manager"
#define MANAGER_INTERFACE "org.bluez.Manager"

Please also stop to shortcut some names. For example DFT for DEFAULT. Simple write the full name, because then it is easy to read this code.

The other thing is that I don't like the difference between *_REQ and *_SIG in the names/constants. Please don't use them.

Also please don't include any RFCOMM, PAN, HID etc. related things, because they are not important right now and they make my life of merging the initial step into better D-Bus a lot harder.

For the error part we will use org.bluez.Error and not EFailed or anything like this. Since all the error stuff is currently not used, I don't think it is a good idea to add it to the initial patch. Looking at HAL the use of org.bluez.NoSuchDevice, etc. seems to make sense, too.

Do we really need that service_table_t thingy now? I think we should start without it.

Regards

Marcel

Quoted 10 lines

-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Quoted 7 lines

--
---------------------------------------------------------
Claudio Takahasi
Nokia's Institute of Technology - INdT
[email protected]

--
---------------------------------------------------------
Claudio Takahasi
Nokia's Institute of Technology - INdT
[email protected]

hcid/hcid.h +3 -0
@@ -122,6 +122,9 @@
 
 #ifdef ENABLE_DBUS
 gboolean hcid_dbus_init(void);
+void hcid_dbus_exit(void);
+gboolean mgr_dbus_reg_device_paths(uint16_t id);
+gboolean mgr_dbus_unreg_device_paths(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);
hcid/dbus.h +169 -0

169 lines not on this page · raw message

hcid/dbus.c +998 -17

1,088 lines not on this page · raw message

hcid/main.c +12 -1
@@ -375,6 +375,9 @@
 
 		if (hcid.security && hci_test_bit(HCI_UP, &dr->dev_opt))
 			start_security_manager(dr->dev_id);
+#ifdef ENABLE_DBUS
+			mgr_dbus_reg_device_paths(dr->dev_id);
+#endif
 	}
 
 	free(dl);
@@ -438,12 +441,18 @@
 			configure_device(sd->dev_id);
 		if (hcid.security)
 			start_security_manager(sd->dev_id);
+#ifdef ENABLE_DBUS
+		mgr_dbus_reg_device_paths(sd->dev_id);
+#endif
 		break;
 
 	case HCI_DEV_DOWN:
 		syslog(LOG_INFO, "HCI dev %d down", sd->dev_id);
 		if (hcid.security)
 			stop_security_manager(sd->dev_id);
+#ifdef ENABLE_DBUS
+		mgr_dbus_unreg_device_paths(sd->dev_id);
+#endif
 		break;
 	}
 }
@@ -627,7 +636,9 @@
 	g_main_run(event_loop);
 
 	free_device_opts();
-
+#ifdef ENABLE_DBUS	
+	hcid_dbus_exit();
+#endif
 	syslog(LOG_INFO, "Exit.");
 	return 0;
 }
#6 lore raw

Hi Claudio,

I see that you have removed "Req" from the end of method call names. However, you still have "Sig" appended to the signal names. Is that intentional or have you just forgotten to remove that?

Johan

-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions, and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

#7 lore raw

Hi folks/Johan,

Sorry, I forgot remove it.
I fixed it in this new patch.

Regards,
Claudio

Quoted 10 lines · Johan Hedberg

Hi Claudio,

I see that you have removed "Req" from the end of method call names. However, you still have "Sig" appended to the signal names. Is that intentional or have you just forgotten to remove that?

Johan

Quoted 9 lines

-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions, and more. http://solutions.newsforge.com/ibmarch.tmpl
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

--
---------------------------------------------------------
Claudio Takahasi
Nokia's Institute of Technology - INdT
[email protected]

hcid/hcid.h +3 -0
@@ -122,6 +122,9 @@
 
 #ifdef ENABLE_DBUS
 gboolean hcid_dbus_init(void);
+void hcid_dbus_exit(void);
+gboolean mgr_dbus_reg_device_paths(uint16_t id);
+gboolean mgr_dbus_unreg_device_paths(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);
hcid/dbus.h +169 -0

169 lines not on this page · raw message

hcid/dbus.c +998 -17

1,088 lines not on this page · raw message

hcid/main.c +12 -1
@@ -375,6 +375,9 @@
 
 		if (hcid.security && hci_test_bit(HCI_UP, &dr->dev_opt))
 			start_security_manager(dr->dev_id);
+#ifdef ENABLE_DBUS
+			mgr_dbus_reg_device_paths(dr->dev_id);
+#endif
 	}
 
 	free(dl);
@@ -438,12 +441,18 @@
 			configure_device(sd->dev_id);
 		if (hcid.security)
 			start_security_manager(sd->dev_id);
+#ifdef ENABLE_DBUS
+		mgr_dbus_reg_device_paths(sd->dev_id);
+#endif
 		break;
 
 	case HCI_DEV_DOWN:
 		syslog(LOG_INFO, "HCI dev %d down", sd->dev_id);
 		if (hcid.security)
 			stop_security_manager(sd->dev_id);
+#ifdef ENABLE_DBUS
+		mgr_dbus_unreg_device_paths(sd->dev_id);
+#endif
 		break;
 	}
 }
@@ -627,7 +636,9 @@
 	g_main_run(event_loop);
 
 	free_device_opts();
-
+#ifdef ENABLE_DBUS	
+	hcid_dbus_exit();
+#endif
 	syslog(LOG_INFO, "Exit.");
 	return 0;
 }