[Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Johan Hedberg]

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

Hi,

These patches have the following changes:

1. Return the right error if a matching handler is not found in the
device message handler function. Eduardo said he would fix it in his
next patch, but I think it's good to get the fix in as soon as possible.

2. Change a couple of places which use DBusMessageIter to use
dbus_message_get_args instead (just one function call and removes the
need to have the DBusMessageIter variable).

3. Use uint16 instead of uint8 for periodic inquiry period length
parameters (since they are two octets each in the HCI spec).

4. Fix checking for the range of inquiry parameters (HCI spec says
length 0x01-0x30 and num_resp 0x00-0xFF).

The range checking for periodic inquiry and role switch should probably
also be fixed at some point. However, the controller should return an
error if invalid values are passed to it, so I'm not even sure that it
should be hcid's task to check them. What do you think?

Another thing with the inquiry and periodic inquiry methods is that the
inquiry method allows the caller to specify the num_responses parameter
while the periodic inquiry method has it hardcoded to 100 currently.
Should the parameter also be added to periodic inquiry or should it be
removed from normal inquiry?

The reason why I'm submitting the changes in two patches is that there's
currently a bug[1] in the python dbus bindings which prevents uint16
values from being sent. The fix will probably go to the next dbus
release but we may want to wait for it before applying the change at our
end.

[1] http://lists.freedesktop.org/archives/dbus/2005-November/003733.html

Johan

[-- Attachment #2: dbus-args.patch --]
[-- Type: text/plain, Size: 3320 bytes --]

Index: dbus.c
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/dbus.c,v
retrieving revision 1.48
diff -u -r1.48 dbus.c
--- dbus.c	4 Nov 2005 15:42:24 -0000	1.48
+++ dbus.c	5 Nov 2005 11:05:31 -0000
@@ -1222,15 +1222,19 @@
 	}
 
 	if (handlers) {
-		error = BLUEZ_EDBUS_WRONG_SIGNATURE;
 		for (; handlers->name != NULL; handlers++) {
 			if (strcmp(handlers->name, method) == 0) {
+				ret = DBUS_HANDLER_RESULT_HANDLED;
 				if (strcmp(handlers->signature, signature) == 0) {
 					reply = handlers->handler_func(msg, data);
 					error = 0;
-					ret = DBUS_HANDLER_RESULT_HANDLED;
 					break;
 				}
+				else
+					/* Set the error, but continue looping incase there is
+					 * another method with the same name but a different
+					 * signature */
+					error = BLUEZ_EDBUS_WRONG_SIGNATURE;
 
 			}
 		}
@@ -1392,22 +1396,20 @@
 
 static DBusMessage* handle_inq_req(DBusMessage *msg, void *data)
 {
-	DBusMessageIter iter;
 	DBusMessage *reply = NULL;
 	inquiry_cp cp;
 	evt_cmd_status rp;
 	struct hci_request rq;
 	struct hci_dbus_data *dbus_data = data;
 	int dd = -1;
-	int8_t length;
-	int8_t num_rsp;
+	uint8_t length, num_rsp;
 
-	dbus_message_iter_init(msg, &iter);
-	dbus_message_iter_get_basic(&iter, &length);
-	dbus_message_iter_next(&iter);
-	dbus_message_iter_get_basic(&iter, &num_rsp);
+	dbus_message_get_args(msg, NULL,
+			DBUS_TYPE_BYTE, &length,
+			DBUS_TYPE_BYTE, &num_rsp,
+			DBUS_TYPE_INVALID);
 
-	if ((length <= 0) || (num_rsp <= 0)) {
+	if (length < 0x01 || length > 0x30) {
 		reply = bluez_new_failure_msg(msg, BLUEZ_EDBUS_WRONG_PARAM);
 		goto failed;
 	}
@@ -1486,7 +1488,6 @@
 
 static DBusMessage* handle_role_switch_req(DBusMessage *msg, void *data)
 {
-	DBusMessageIter iter;
 	DBusMessage *reply = NULL;
 	char *str_bdaddr = NULL;
 	struct hci_dbus_data *dbus_data = data;
@@ -1494,10 +1495,10 @@
 	uint8_t role;
 	int dev_id = -1, dd = -1;
 
-	dbus_message_iter_init(msg, &iter);
-	dbus_message_iter_get_basic(&iter, &str_bdaddr);
-	dbus_message_iter_next(&iter);
-	dbus_message_iter_get_basic(&iter, &role);
+	dbus_message_get_args(msg, NULL,
+			DBUS_TYPE_STRING, &str_bdaddr,
+			DBUS_TYPE_BYTE, &role,
+			DBUS_TYPE_INVALID);
 
 	str2ba(str_bdaddr, &bdaddr);
 
@@ -1536,7 +1537,6 @@
 
 static DBusMessage* handle_remote_name_req(DBusMessage *msg, void *data)
 {
-	DBusMessageIter iter;
 	DBusMessage *reply = NULL;
 	struct hci_dbus_data *dbus_data = data;
 	int dd = -1;
@@ -1546,8 +1546,9 @@
 	remote_name_req_cp cp;
 	evt_cmd_status rp;
 
-	dbus_message_iter_init(msg, &iter);
-	dbus_message_iter_get_basic(&iter, &str_bdaddr);
+	dbus_message_get_args(msg, NULL,
+			DBUS_TYPE_STRING, &str_bdaddr,
+			DBUS_TYPE_INVALID);
 
 	str2ba(str_bdaddr, &bdaddr);
 
@@ -1662,7 +1663,6 @@
 	struct hci_request rq;
 	auth_requested_cp cp;
 	evt_cmd_status rp;
-	DBusMessageIter iter;
 	DBusMessage *reply = NULL;
 	char *str_bdaddr = NULL;
 	struct hci_dbus_data *dbus_data = data;
@@ -1671,8 +1671,10 @@
 	int dev_id = -1;
 	int dd = -1;
 
-	dbus_message_iter_init(msg, &iter);
-	dbus_message_iter_get_basic(&iter, &str_bdaddr);
+	dbus_message_get_args(msg, NULL,
+			DBUS_TYPE_STRING, &str_bdaddr,
+			DBUS_TYPE_INVALID);
+
 	str2ba(str_bdaddr, &bdaddr);
 
 	dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);

[-- Attachment #3: dbus-args-uint16.patch --]
[-- Type: text/plain, Size: 2860 bytes --]

Index: dbus-test
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/dbus-test,v
retrieving revision 1.2
diff -u -r1.2 dbus-test
--- dbus-test	3 Nov 2005 14:35:15 -0000	1.2
+++ dbus-test	5 Nov 2005 11:09:05 -0000
@@ -222,7 +222,7 @@
                 length, min, max = self.cmd_args
             self.listen = True
             try:
-                self.ctl.PeriodicInquiry(dbus.Byte(length), dbus.Byte(min), dbus.Byte(max))
+                self.ctl.PeriodicInquiry(dbus.Byte(length), dbus.UInt16(min), dbus.UInt16(max))
             except dbus.DBusException, e:
                 print 'Sending %s failed: %s' % (self.cmd, e)
                 sys.exit(1)
Index: dbus.c
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/dbus.c,v
retrieving revision 1.48
diff -u -r1.48 dbus.c
--- dbus.c	4 Nov 2005 15:42:24 -0000	1.48
+++ dbus.c	5 Nov 2005 11:09:05 -0000
@@ -1299,12 +1299,11 @@
 {
 	write_inquiry_mode_cp inq_mode;
 	periodic_inquiry_cp inq_param;
-	DBusMessageIter iter;
 	DBusMessage *reply = NULL;
 	struct hci_dbus_data *dbus_data = data;
 	uint8_t length;
-	uint8_t max_period;
-	uint8_t min_period;
+	uint16_t max_period;
+	uint16_t min_period;
 	int dd = -1;
 
 	dd = hci_open_dev(dbus_data->dev_id);
@@ -1314,12 +1313,13 @@
 		goto failed;
 	}
 
-	dbus_message_iter_init(msg, &iter);
-	dbus_message_iter_get_basic(&iter, &length);
-	dbus_message_iter_next(&iter);
-	dbus_message_iter_get_basic(&iter, &min_period);
-	dbus_message_iter_next(&iter);
-	dbus_message_iter_get_basic(&iter, &max_period);
+	dbus_message_get_args(msg, NULL,
+			DBUS_TYPE_BYTE, &length,
+			DBUS_TYPE_UINT16, &min_period,
+			DBUS_TYPE_UINT16, &max_period,
+			DBUS_TYPE_INVALID);
+
+	syslog(LOG_DEBUG, "%02X %04X %04X", length, min_period, max_period);
 
 	if (length >= min_period || min_period >= max_period) {
 		reply = bluez_new_failure_msg(msg, BLUEZ_EDBUS_WRONG_PARAM);
@@ -1329,8 +1329,8 @@
 	inq_param.num_rsp = 100;
 	inq_param.length  = length;
 
-	inq_param.max_period = max_period;
-	inq_param.min_period = min_period;
+	inq_param.max_period = htobs(max_period);
+	inq_param.min_period = htobs(min_period);
 
 	/* General/Unlimited Inquiry Access Code (GIAC) */
 	inq_param.lap[0] = 0x33;
Index: dbus.h
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/dbus.h,v
retrieving revision 1.14
diff -u -r1.14 dbus.h
--- dbus.h	4 Nov 2005 15:42:24 -0000	1.14
+++ dbus.h	5 Nov 2005 11:09:06 -0000
@@ -166,8 +166,8 @@
 
 
 #define HCI_PERIODIC_INQ_SIGNATURE			DBUS_TYPE_BYTE_AS_STRING \
-							DBUS_TYPE_BYTE_AS_STRING \
-							DBUS_TYPE_BYTE_AS_STRING \
+							DBUS_TYPE_UINT16_AS_STRING \
+							DBUS_TYPE_UINT16_AS_STRING \
 							__END_SIG__
 
 #define HCI_CANCEL_PERIODIC_INQ_SIGNATURE		__END_SIG__

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Marcel Holtmann]

Hi Johan,

> These patches have the following changes:
> 
> 1. Return the right error if a matching handler is not found in the
> device message handler function. Eduardo said he would fix it in his
> next patch, but I think it's good to get the fix in as soon as possible.
> 
> 2. Change a couple of places which use DBusMessageIter to use
> dbus_message_get_args instead (just one function call and removes the
> need to have the DBusMessageIter variable).
> 
> 3. Use uint16 instead of uint8 for periodic inquiry period length
> parameters (since they are two octets each in the HCI spec).
> 
> 4. Fix checking for the range of inquiry parameters (HCI spec says
> length 0x01-0x30 and num_resp 0x00-0xFF).
> 
> The range checking for periodic inquiry and role switch should probably
> also be fixed at some point. However, the controller should return an
> error if invalid values are passed to it, so I'm not even sure that it
> should be hcid's task to check them. What do you think?
> 
> Another thing with the inquiry and periodic inquiry methods is that the
> inquiry method allows the caller to specify the num_responses parameter
> while the periodic inquiry method has it hardcoded to 100 currently.
> Should the parameter also be added to periodic inquiry or should it be
> removed from normal inquiry?
> 
> The reason why I'm submitting the changes in two patches is that there's
> currently a bug[1] in the python dbus bindings which prevents uint16
> values from being sent. The fix will probably go to the next dbus
> release but we may want to wait for it before applying the change at our
> end.

I decided to take both patches and hopefully the D-Bus Python support
will be fixed soon. However I applied another cleanup after your patches
that fixes some weird coding that ends up in unnecessary deep indented
lines. Please use this style for any loops and make use of labels for
the exit path in functions.

What do you think about using also tabs in dbus-test?

Regards

Marcel




-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Marcel Holtmann]

Hi Johan,

> The range checking for periodic inquiry and role switch should probably
> also be fixed at some point. However, the controller should return an
> error if invalid values are passed to it, so I'm not even sure that it
> should be hcid's task to check them. What do you think?

we should do our own range check, because the intention is to design an
API and not to simply map the Bluetooth HCI to D-Bus. If we have weird
parameters that doesn't matter in real or are only for internal inside
hcid or the kernel then we leave them out. If we wanna combine to HCI
commands into one D-Bus method then we do this to. So please don't try
to stick to the HCI specification. Try to design some logical API for
Bluetooth.

> Another thing with the inquiry and periodic inquiry methods is that the
> inquiry method allows the caller to specify the num_responses parameter
> while the periodic inquiry method has it hardcoded to 100 currently.
> Should the parameter also be added to periodic inquiry or should it be
> removed from normal inquiry?

See my comment above. The question here is if we need this parameter for
any method call or should we make it a configurable option. So we have a
sane default for num_responses and if somebody wants to use a different
one that a second method to change it is needed.

Regards

Marcel




-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Johan Hedberg]

Hi Marcel,

On Sun, Nov 06, 2005, Marcel Holtmann wrote:
> I decided to take both patches and hopefully the D-Bus Python support
> will be fixed soon. However I applied another cleanup after your patches
> that fixes some weird coding that ends up in unnecessary deep indented
> lines. Please use this style for any loops and make use of labels for
> the exit path in functions.

Will do.

> What do you think about using also tabs in dbus-test?

I have nothing fundamental against it. The primary reason why it uses
four spaces for indentation currently is that it's the setting I've
always had in my .vimrc for python code.

Johan


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Marcel Holtmann]

Hi Johan,

> > What do you think about using also tabs in dbus-test?
> 
> I have nothing fundamental against it. The primary reason why it uses
> four spaces for indentation currently is that it's the setting I've
> always had in my .vimrc for python code.

the tabs makes it easy for people to change the visualization of them
from four spaces to eight spaces (or others). This is why I prefer of
having tabs, but since I am not really good in Python, I better not talk
about that coding style ;)

Regards

Marcel




-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Johan Hedberg]

Hi Marcel,

On Sun, Nov 06, 2005, Marcel Holtmann wrote:
> we should do our own range check, because the intention is to design an
> API and not to simply map the Bluetooth HCI to D-Bus. If we have weird
> parameters that doesn't matter in real or are only for internal inside
> hcid or the kernel then we leave them out. If we wanna combine to HCI
> commands into one D-Bus method then we do this to. So please don't try
> to stick to the HCI specification. Try to design some logical API for
> Bluetooth.

Sure. However it would be nice to hear your opinions about e.g. the
level of abstraction we should strive for and if there are areas (e.g.
SDP) which should be abstracted more than others. At least for RFCOMM I
was thinking of doing a similar interface that we have in maemo [1],
where you could directly use identifiers such as "DUN" and "FTP" to
connect to a service without the need to do SDP separately. Of course
there should also be the possibility to use an exact UUID for less
common services.

[1] http://www.maemo.org/platform/docs/osso-gwconnect.html

> > Another thing with the inquiry and periodic inquiry methods is that the
> > inquiry method allows the caller to specify the num_responses parameter
> > while the periodic inquiry method has it hardcoded to 100 currently.
> > Should the parameter also be added to periodic inquiry or should it be
> > removed from normal inquiry?
> 
> See my comment above. The question here is if we need this parameter for
> any method call or should we make it a configurable option. So we have a
> sane default for num_responses and if somebody wants to use a different
> one that a second method to change it is needed.

Is there a reason to have a limit at all, i.e. why not always use the
value 0 (= unlimited number of responses)? Of course we can add a method
for setting another default, but can you give and example of a situation
where that would be needed?

Johan


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Marcel Holtmann]

Hi Johan,

> > we should do our own range check, because the intention is to design an
> > API and not to simply map the Bluetooth HCI to D-Bus. If we have weird
> > parameters that doesn't matter in real or are only for internal inside
> > hcid or the kernel then we leave them out. If we wanna combine to HCI
> > commands into one D-Bus method then we do this to. So please don't try
> > to stick to the HCI specification. Try to design some logical API for
> > Bluetooth.
> 
> Sure. However it would be nice to hear your opinions about e.g. the
> level of abstraction we should strive for and if there are areas (e.g.
> SDP) which should be abstracted more than others. At least for RFCOMM I
> was thinking of doing a similar interface that we have in maemo [1],
> where you could directly use identifiers such as "DUN" and "FTP" to
> connect to a service without the need to do SDP separately. Of course
> there should also be the possibility to use an exact UUID for less
> common services.

I like that idea and since one of my advanced goals is to implement a
full SDP client into bluetoothd, this would work out just fine. Actually
the D-Bus interface should not use any kind of RFCOMM channel or L2CAP
PSM numbers. I consider it more as an API to the Bluetooth profiles and
not the protocols. So in the case of RFCOMM <-> TTY assignments we may
only wanna use bdaddr + uuid as parameters. The UUID then can be a clear
text value like "dun" or "ftp" or a hex value like "0x1111". For the
BD_ADDR it would also make sense if this can be an alias or the remote
name. Maybe changing the parameter name to device would be better.

Since the current SDP library is blocking, we would need some SDP client
code that integrates nicely into an event loop.

> > > Another thing with the inquiry and periodic inquiry methods is that the
> > > inquiry method allows the caller to specify the num_responses parameter
> > > while the periodic inquiry method has it hardcoded to 100 currently.
> > > Should the parameter also be added to periodic inquiry or should it be
> > > removed from normal inquiry?
> > 
> > See my comment above. The question here is if we need this parameter for
> > any method call or should we make it a configurable option. So we have a
> > sane default for num_responses and if somebody wants to use a different
> > one that a second method to change it is needed.
> 
> Is there a reason to have a limit at all, i.e. why not always use the
> value 0 (= unlimited number of responses)? Of course we can add a method
> for setting another default, but can you give and example of a situation
> where that would be needed?

The num_responses and the IAC should be separate setting. It would be
great if this setting is specific to the D-Bus session of the caller. In
this case every D-Bus user can have its own settings. Do you think this
is possible?

For the time (length) of the inquiry I am not sure at the moment.

Regards

Marcel




-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Johan Hedberg]

Hi Marcel,

On Mon, Nov 07, 2005, Marcel Holtmann wrote:
> The num_responses and the IAC should be separate setting. It would be
> great if this setting is specific to the D-Bus session of the caller. In
> this case every D-Bus user can have its own settings. Do you think this
> is possible?

Sure. We could either maintain a table of settings for each D-BUS peer
using the unique name that the bus assigns for each connection, or we
could also do the settings based on the unix user id of the client
application (using the dbus_bus_get_unix_user function).

Johan


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Marcel Holtmann]

Hi Johan,

> > The num_responses and the IAC should be separate setting. It would be
> > great if this setting is specific to the D-Bus session of the caller. In
> > this case every D-Bus user can have its own settings. Do you think this
> > is possible?
> 
> Sure. We could either maintain a table of settings for each D-BUS peer
> using the unique name that the bus assigns for each connection, or we
> could also do the settings based on the unix user id of the client
> application (using the dbus_bus_get_unix_user function).

I prefer the per each D-Bus peer thing, because this gives us the most
flexibility for the applications. A general per user settings would be
nice too, but maybe a little bit too much at the moment. Can you try to
implement it and send me a patch.

Regards

Marcel




-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Claudio Takahasi]

Hi guys,


I was trying understand all discussed things in this email... Sorry if
I am writing irrrelevant thing. :)

More than one client is not a common scenario, but we need handle it!
IMHO, control a per D-Bus client connection will be a little bit hard,
why not support Inquiry, PeriodicInquiry with more than one signature?
In the current implementation is easy make this change.

Use a per client connection identification will be nice if we have a
peer responses instead of using signals. This will be nice when the
kernel cache can be retrieved.

I didn't understand how the client id works.  Could you explain a
little bit more? Every time that a client establish a connection with
the d-bus daemon the connection id changes.
Therefore control client id is useful only for clients that is always
connected with the daemon.

Regards,
Claudio.

On 11/7/05, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Johan,
>
> > > The num_responses and the IAC should be separate setting. It would be
> > > great if this setting is specific to the D-Bus session of the caller.=
 In
> > > this case every D-Bus user can have its own settings. Do you think th=
is
> > > is possible?
> >
> > Sure. We could either maintain a table of settings for each D-BUS peer
> > using the unique name that the bus assigns for each connection, or we
> > could also do the settings based on the unix user id of the client
> > application (using the dbus_bus_get_unix_user function).
>
> I prefer the per each D-Bus peer thing, because this gives us the most
> flexibility for the applications. A general per user settings would be
> nice too, but maybe a little bit too much at the moment. Can you try to
> implement it and send me a patch.
>
> Regards
>
> Marcel
>
>
>
>
> -------------------------------------------------------
> SF.Net email is sponsored by:
> Tame your development challenges with Apache's Geronimo App Server. Downl=
oad
> it for free - -and be entered to win a 42" plasma tv or your very own
> Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
> _______________________________________________
> Bluez-devel mailing list
> Bluez-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/bluez-devel
>


--
---------------------------------------------------------
Claudio Takahasi
Instituto Nokia de Tecnologia - INdT


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Johan Hedberg]

On Mon, Nov 07, 2005, Claudio Takahasi wrote:
> Use a per client connection identification will be nice if we have a
> peer responses instead of using signals. This will be nice when the
> kernel cache can be retrieved.
> 
> I didn't understand how the client id works.  Could you explain a
> little bit more?

The id I was talking about is in practice what you get when you call
dbus_message_get_sender for a received message. The D-BUS spec. calls it
"Unique name of the sending connection".

> Every time that a client establish a connection with the d-bus daemon
> the connection id changes.  Therefore control client id is useful only
> for clients that is always connected with the daemon.

True, and that was also one thing I was worried about when it comes to
the practicality of that solution (the settings wouldn't be persistent
enough). So, I think using the user id might be better in this respect.

However, let me now introduce a third option which we just discussed on
IRC with Claudio: optional D-BUS method parameters. It would be quite
easy to implement the methods so that the client could leave out most
parameters in which case hcid (or bluetoothd) would use some set of
default values. You could e.g. call the Inquiry method without any
parameters and it would just work (kind of the same way that dbus-test
allows doing "./dbus-test Inquiry" in which case it uses length=10 &
num_resp=100 as the default values). If the client insists on using its
own values for the method it would simply include them in the method
call.

In case this variable parameters solution doesn't work out for all
scenarios it could probably happily co-exist with the per-user id
settings solution.

Johan


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Marcel Holtmann]

Hi Johan,

> > Use a per client connection identification will be nice if we have a
> > peer responses instead of using signals. This will be nice when the
> > kernel cache can be retrieved.
> > 
> > I didn't understand how the client id works.  Could you explain a
> > little bit more?
> 
> The id I was talking about is in practice what you get when you call
> dbus_message_get_sender for a received message. The D-BUS spec. calls it
> "Unique name of the sending connection".
> 
> > Every time that a client establish a connection with the d-bus daemon
> > the connection id changes.  Therefore control client id is useful only
> > for clients that is always connected with the daemon.
> 
> True, and that was also one thing I was worried about when it comes to
> the practicality of that solution (the settings wouldn't be persistent
> enough). So, I think using the user id might be better in this respect.

I don't think so, because this options might be used not very often and
so having them per unix user doesn't help. Two different applications
running as them same user might have different needs. One wants to do
inquiry with another IAC, the other with the general IAC. This tends to
be a race condition.

> However, let me now introduce a third option which we just discussed on
> IRC with Claudio: optional D-BUS method parameters. It would be quite
> easy to implement the methods so that the client could leave out most
> parameters in which case hcid (or bluetoothd) would use some set of
> default values. You could e.g. call the Inquiry method without any
> parameters and it would just work (kind of the same way that dbus-test
> allows doing "./dbus-test Inquiry" in which case it uses length=10 &
> num_resp=100 as the default values). If the client insists on using its
> own values for the method it would simply include them in the method
> call.

Let's do this proposal first.

Regards

Marcel




-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Johan Hedberg]

Hi Marcel,

On Mon, Nov 07, 2005, Marcel Holtmann wrote:
> > > Every time that a client establish a connection with the d-bus daemon
> > > the connection id changes.  Therefore control client id is useful only
> > > for clients that is always connected with the daemon.
> > 
> > True, and that was also one thing I was worried about when it comes to
> > the practicality of that solution (the settings wouldn't be persistent
> > enough). So, I think using the user id might be better in this respect.
> 
> I don't think so, because this options might be used not very often and
> so having them per unix user doesn't help. Two different applications
> running as them same user might have different needs. One wants to do
> inquiry with another IAC, the other with the general IAC. This tends to
> be a race condition.

Ok, I see your point. I didn't quite understand the use-case you were
thinking of earlier. One extra requirement that the dbus-id settings has
is that we will have to monitor the lifetime of a D-BUS client (by
listening NameOwnerChanged D-BUS signals) and remove the corresponding
entry from the settings table when an id gets removed from the bus.

> > However, let me now introduce a third option which we just discussed on
> > IRC with Claudio: optional D-BUS method parameters. It would be quite
> > easy to implement the methods so that the client could leave out most
> > parameters in which case hcid (or bluetoothd) would use some set of
> > default values. You could e.g. call the Inquiry method without any
> > parameters and it would just work (kind of the same way that dbus-test
> > allows doing "./dbus-test Inquiry" in which case it uses length=10 &
> > num_resp=100 as the default values). If the client insists on using its
> > own values for the method it would simply include them in the method
> > call.
> 
> Let's do this proposal first.

Ok, I'll begin by doing a patch for Inquiry and PeriodicInquiry
(probably during tomorrow).

Johan


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Marcel Holtmann]

Hi Johan,

> > > > Every time that a client establish a connection with the d-bus daemon
> > > > the connection id changes.  Therefore control client id is useful only
> > > > for clients that is always connected with the daemon.
> > > 
> > > True, and that was also one thing I was worried about when it comes to
> > > the practicality of that solution (the settings wouldn't be persistent
> > > enough). So, I think using the user id might be better in this respect.
> > 
> > I don't think so, because this options might be used not very often and
> > so having them per unix user doesn't help. Two different applications
> > running as them same user might have different needs. One wants to do
> > inquiry with another IAC, the other with the general IAC. This tends to
> > be a race condition.
> 
> Ok, I see your point. I didn't quite understand the use-case you were
> thinking of earlier. One extra requirement that the dbus-id settings has
> is that we will have to monitor the lifetime of a D-BUS client (by
> listening NameOwnerChanged D-BUS signals) and remove the corresponding
> entry from the settings table when an id gets removed from the bus.

this might be too much work for the current interface. Or do you think
it can be easily implemented?

> > > However, let me now introduce a third option which we just discussed on
> > > IRC with Claudio: optional D-BUS method parameters. It would be quite
> > > easy to implement the methods so that the client could leave out most
> > > parameters in which case hcid (or bluetoothd) would use some set of
> > > default values. You could e.g. call the Inquiry method without any
> > > parameters and it would just work (kind of the same way that dbus-test
> > > allows doing "./dbus-test Inquiry" in which case it uses length=10 &
> > > num_resp=100 as the default values). If the client insists on using its
> > > own values for the method it would simply include them in the method
> > > call.
> > 
> > Let's do this proposal first.
> 
> Ok, I'll begin by doing a patch for Inquiry and PeriodicInquiry
> (probably during tomorrow).

This would be great. For the next release I like to have the inquiry (or
better name it device discovery) working. This will include some nice
support for the extended inquiry. So you will get the remote name for
free with a discovery. So is it possible to have different signatures
for signals too?

Regards

Marcel




-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Johan Hedberg]

Hi Marcel,

On Tue, Nov 08, 2005, Marcel Holtmann wrote:
> > Ok, I see your point. I didn't quite understand the use-case you were
> > thinking of earlier. One extra requirement that the dbus-id settings has
> > is that we will have to monitor the lifetime of a D-BUS client (by
> > listening NameOwnerChanged D-BUS signals) and remove the corresponding
> > entry from the settings table when an id gets removed from the bus.
> 
> this might be too much work for the current interface. Or do you think
> it can be easily implemented?

It will require a moderate amount of coding, so it wont be easy at least
in that respect. However, I think that implementing D-BUS client
lifetime monitoring is a feature that we will inevitably have to
implement at some point anyway. It may e.g. be needed for noticing if a
program which requested an RFCOMM connection suddenly died (in which
case hcid/bluetoothd should disconnect that connection). Also, I've
already done it once (for dbus 0.23.4 though) for the D-BUS API we have
in maemo, so there shouldn't come any "supprises" when implementing it.

> > Ok, I'll begin by doing a patch for Inquiry and PeriodicInquiry
> > (probably during tomorrow).
> 
> This would be great. For the next release I like to have the inquiry (or
> better name it device discovery) working. This will include some nice
> support for the extended inquiry. So you will get the remote name for
> free with a discovery. So is it possible to have different signatures
> for signals too?

Sure, and if the new signature differs only by having extra parameters
at the end it should even be backwards compatible (i.e.
dbus_message_get_args call for the old signature works also for the new
signature).

Johan


-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] dbus.c: cleanup/fix method parameter getting

[Marcel Holtmann]

Hi Johan,

> > > Ok, I see your point. I didn't quite understand the use-case you were
> > > thinking of earlier. One extra requirement that the dbus-id settings has
> > > is that we will have to monitor the lifetime of a D-BUS client (by
> > > listening NameOwnerChanged D-BUS signals) and remove the corresponding
> > > entry from the settings table when an id gets removed from the bus.
> > 
> > this might be too much work for the current interface. Or do you think
> > it can be easily implemented?
> 
> It will require a moderate amount of coding, so it wont be easy at least
> in that respect. However, I think that implementing D-BUS client
> lifetime monitoring is a feature that we will inevitably have to
> implement at some point anyway. It may e.g. be needed for noticing if a
> program which requested an RFCOMM connection suddenly died (in which
> case hcid/bluetoothd should disconnect that connection). Also, I've
> already done it once (for dbus 0.23.4 though) for the D-BUS API we have
> in maemo, so there shouldn't come any "supprises" when implementing it.

then lets do it. Or to be more precise, you do it ;)

> > > Ok, I'll begin by doing a patch for Inquiry and PeriodicInquiry
> > > (probably during tomorrow).
> > 
> > This would be great. For the next release I like to have the inquiry (or
> > better name it device discovery) working. This will include some nice
> > support for the extended inquiry. So you will get the remote name for
> > free with a discovery. So is it possible to have different signatures
> > for signals too?
> 
> Sure, and if the new signature differs only by having extra parameters
> at the end it should even be backwards compatible (i.e.
> dbus_message_get_args call for the old signature works also for the new
> signature).

Then I think that I will include the device name in the inquiry response
signal and leave further extensions to a different signature.

Regards

Marcel




-------------------------------------------------------
SF.Net email is sponsored by:
Tame your development challenges with Apache's Geronimo App Server. Download
it for free - -and be entered to win a 42" plasma tv or your very own
Sony(tm)PSP.  Click here to play: http://sourceforge.net/geronimo.php
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel