2008-07-04 21:33:29

by David Stockwell

[permalink] [raw]
Subject: [Bluez-devel] A thought about Adapter.CreatePairedDevice

Currently CreatePairedDevice requires the remote device address, plus
the Object Path of a pairing agent (interface org.bluez.Agent), plus a
string listing the pairing capabilities.

If an agent for the Adapter is already registered, this is redundant at
best, and confusing at worst.

In my mucking around with my Agent (see my other manifesto...), I tried
an experiment to see if it was possible to make a method polymorphic,
and I found that DBus has no objections. It introspects well (although
d-feet does not recognizes only the first definition), and when I send a
message using dbus-send, dbus routes the incoming message to the version
of the Method with a matching signature. Sweet.

So, my suggestion is: in adapter.c in setting up the adapter_methods
table, add an additional method definition sort of like the following:

+++++
/* starting at line 4257 */
static GDBusMethodTable adapter_methods[] = {
...
{ "CreatePairedDevice", "sos", "o", create_paired_device,
G_DBUS_METHOD_FLAG_ASYNC},
{ "CreatePairedDevice", "s", "o", create_paired_device_noagnt,
G_DBUS_METHOD_FLAG_ASYNC},
...
}

+++++

The second one is the version that does not require the object path for
the Agent, nor the capabilities.

Then, add an additional function, right after create_paired_device, to
handle the new version of the message:

+++++

/* starting at line 4011 */
static DBusMessage *create_paired_device(DBusConnection *conn,
DBusMessage *msg, void *data)
{
const gchar *address, *agent_path, *capability;
uint8_t cap;

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_OBJECT_PATH, &agent_path,
DBUS_TYPE_STRING, &capability,
DBUS_TYPE_INVALID) == FALSE)
return invalid_args(msg);

if (check_address(address) < 0)
return invalid_args(msg);

cap = parse_io_capability(capability);
if (cap == IO_CAPABILITY_INVALID)
return invalid_args(msg);

return create_bonding(conn, msg, address, agent_path, cap, data);
}

/* new function to support agent-less CreatePairedDevice */

static DBusMessage *create_paired_device_noagnt(DBusConnection *conn,
DBusMessage *msg, void *data)
{
const gchar *address, *agent_path, *capability;
uint8_t cap;

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_INVALID) == FALSE)
return invalid_args(msg);

if (check_address(address) < 0)
return invalid_args(msg);

return create_bonding(conn, msg, address, NULL, NULL, data);
}

+++++

There would need to be a couple of additional changes in create_bonding
to (a) just use the adapter-wide Agent when the agent_path parameter is
NULL, and (b) throw an error when the agent_path is null, and
adapter->agent is also null.

Any comments? If no major objections, I'll submit a patch against 3.35

David Stockwell
Frequency One


-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel


2008-07-05 17:07:29

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [Bluez-devel] A thought about Adapter.CreatePairedDevice

Hi David,

> Currently CreatePairedDevice requires the remote device address, plus
> the Object Path of a pairing agent (interface org.bluez.Agent), plus a
> string listing the pairing capabilities.
>
> If an agent for the Adapter is already registered, this is redundant at
> best, and confusing at worst.

no it is not. The CreatePairedDevice is mostly used from the UI part
that setups a Bluetooth device (aka wizard). This wants to re-direct the
PIN/passkey dialogs. So this makes total sense. The default agent from
RegisterAgent takes care of all the other cases. Like remote device
connects to us and wants to pair.

> In my mucking around with my Agent (see my other manifesto...), I tried
> an experiment to see if it was possible to make a method polymorphic,
> and I found that DBus has no objections. It introspects well (although
> d-feet does not recognizes only the first definition), and when I send a
> message using dbus-send, dbus routes the incoming message to the version
> of the Method with a matching signature. Sweet.
>
> So, my suggestion is: in adapter.c in setting up the adapter_methods
> table, add an additional method definition sort of like the following:
>
> +++++
> /* starting at line 4257 */
> static GDBusMethodTable adapter_methods[] = {
> ...
> { "CreatePairedDevice", "sos", "o", create_paired_device,
> G_DBUS_METHOD_FLAG_ASYNC},
> { "CreatePairedDevice", "s", "o", create_paired_device_noagnt,
> G_DBUS_METHOD_FLAG_ASYNC},
> ...
> }

While the low-level D-Bus bindings allow this, most high-level bindings
don't and thus we are not doing this.

You can try to give CreatePairedDevice an invalid agent path and then it
should fallback to the default one.

Regards

Marcel



-------------------------------------------------------------------------
Sponsored by: SourceForge.net Community Choice Awards: VOTE NOW!
Studies have shown that voting for your favorite open source project,
along with a healthy diet, reduces your potential for chronic lameness
and boredom. Vote Now at http://www.sourceforge.net/community/cca08
_______________________________________________
Bluez-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel