* Re: Apple Wireless Keyboard connection issue
From: Johan Hedberg @ 2010-11-17 9:05 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: André Kühne, linux-bluetooth
In-Reply-To: <20101117013009.GA15855@vigoh>
[-- Attachment #1: Type: text/plain, Size: 716 bytes --]
Hi Gustavo,
On Tue, Nov 16, 2010, Gustavo F. Padovan wrote:
> * "André Kühne" <andre.kuehne@gmx.net> [2010-11-16 22:27:01 +0100]:
> > I noticed the following on my system: After upgrading to bluez-4.79
> > connecting my Apple Wireless Keyboard does not work anymore. With
> > bluez-4.77 the connection works just fine.
>
> My Microsoft keyboard is not working too. That is probably due to commit
> abe7cd44124a from Johan. It should be fixed soon.
The only real difference that patch makes is the reuse of the HCI socket
inside hciops.c. Maybe that screws up the event filters somehow or
something similar. I don't have a keyboard to verify a fix, but could
you try the attached patch and see if it helps?
Johan
[-- Attachment #2: hciops_encrypt.patch --]
[-- Type: text/x-diff, Size: 1783 bytes --]
diff --git a/plugins/hciops.c b/plugins/hciops.c
index 9d25558..4492983 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -338,31 +338,37 @@ static int hciops_encrypt_link(int index, bdaddr_t *dst, bt_hci_result_t cb,
uint32_t link_mode;
uint16_t handle;
+ dd = hci_open_dev(index);
+ if (dd < 0)
+ return -errno;
+
cr = g_malloc0(sizeof(*cr) + sizeof(struct hci_conn_info));
cr->type = ACL_LINK;
bacpy(&cr->bdaddr, dst);
- err = ioctl(SK(index), HCIGETCONNINFO, cr);
+ err = ioctl(dd, HCIGETCONNINFO, cr);
link_mode = cr->conn_info->link_mode;
handle = cr->conn_info->handle;
g_free(cr);
- if (err < 0)
- return -errno;
+ if (err < 0) {
+ err = -errno;
+ goto fail;
+ }
- if (link_mode & HCI_LM_ENCRYPT)
- return -EALREADY;
+ if (link_mode & HCI_LM_ENCRYPT) {
+ err = -EALREADY;
+ goto fail;
+ }
memset(&cp, 0, sizeof(cp));
cp.handle = htobs(handle);
- if (hci_send_cmd(SK(index), OGF_LINK_CTL, OCF_AUTH_REQUESTED,
- AUTH_REQUESTED_CP_SIZE, &cp) < 0)
- return -errno;
-
- dd = dup(SK(index));
- if (dd < 0)
- return -errno;
+ if (hci_send_cmd(dd, OGF_LINK_CTL, OCF_AUTH_REQUESTED,
+ AUTH_REQUESTED_CP_SIZE, &cp) < 0) {
+ err = -errno;
+ goto fail;
+ }
cmd = g_new0(struct hci_cmd_data, 1);
cmd->handle = handle;
@@ -379,8 +385,7 @@ static int hciops_encrypt_link(int index, bdaddr_t *dst, bt_hci_result_t cb,
if (setsockopt(dd, SOL_HCI, HCI_FILTER, &nf, sizeof(nf)) < 0) {
err = -errno;
g_free(cmd);
- close(dd);
- return -err;
+ goto fail;
}
io = g_io_channel_unix_new(dup(SK(index)));
@@ -391,6 +396,10 @@ static int hciops_encrypt_link(int index, bdaddr_t *dst, bt_hci_result_t cb,
g_io_channel_unref(io);
return 0;
+
+fail:
+ close(dd);
+ return err;
}
/* End async HCI command handling */
^ permalink raw reply related
* Re: [PATCH v5] Bluetooth: btwilink driver
From: Pavan Savoy @ 2010-11-17 5:43 UTC (permalink / raw)
To: Vitaly Wool; +Cc: Gustavo F. Padovan, marcel, linux-bluetooth, linux-kernel
In-Reply-To: <AANLkTi=TxKKxjLdK-6Jwu9oBCiGHXJ8UxrAQH+C82wNK@mail.gmail.com>
On Wed, Nov 17, 2010 at 4:50 AM, Vitaly Wool <vitalywool@gmail.com> wrote:
>>> + =C2=A0 =C2=A0 /* Registration with ST layer is successful,
>>> + =C2=A0 =C2=A0 =C2=A0* hardware is ready to accept commands from HCI c=
ore.
>>> + =C2=A0 =C2=A0 =C2=A0*/
>>> + =C2=A0 =C2=A0 if (test_and_set_bit(HCI_RUNNING, &hdev->flags)) {
>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 clear_bit(HCI_RUNNING, &hde=
v->flags);
>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 err =3D st_unregister(ST_BT=
);
>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (err)
>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
BT_ERR("st_unregister() failed with error %d", err);
>>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hst->st_write =3D NULL;
>>> + =C2=A0 =C2=A0 }
>>
>>
>> What are you trying to do here? test_and_set_bit() result doesn't say
>> nothing about error and you shall put test_and_set_bit should be in the
>> beginning, to know if your device is already opened or not and then
>> clear_bit if some error ocurrs during the function.
>>
>
> Yeap, this piece of code beats me is well. Why is it an error if this
> bit wasn't already set?
Vitaly, Gustavo,
I suppose I never understood HCI_RUNNING flag that way, as in an error
check mechanism to avoid multiple hci0 ups.
What I understood was that HCI_RUNNING suggested as to when hci0 was
ready to be used. With this understanding, I wanted to make sure I
downloaded the firmware for the chip before I proclaim to the world
that the hci0 is ready to be used, as in HCI_RUNNING.
For example, I didn't want my _send_frame to be called before I did
the firmware download - since firmware download takes time - 45kb
send/wait commands :(
But I suppose I now understand - What I would rather do is test_bit in
the beginning of function and do a set_bit at the end of function -
does this make sense ?
> ~Vitaly
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =C2=A0http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v5] Bluetooth: btwilink driver
From: Pavan Savoy @ 2010-11-17 5:34 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: marcel, linux-bluetooth, linux-kernel
In-Reply-To: <20101116225418.GA15101@vigoh>
Gustavo,
On Wed, Nov 17, 2010 at 4:24 AM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> Hi Pavan,
>
> * pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-11-10 08:07:26 -0500]:
>
>> From: Pavan Savoy <pavan_savoy@ti.com>
>>
>> Marcel,
>>
>> Thanks for the comments...
>> This patch contains,
>> v5 comments :-
>> declaration and assiging of variables and private data fixed up.
>> proper casting.
>> removed redundant un-necessary checks in send_frame.
>> HCI_RUNNING fixes in terms of test_and_set/clear bit instead of set and
>> clear.
>> removed redundant checks for hdev, skb being NULL.
>> removed module_param of reset, also WiLink don't need HCI_RESET anyways.
>> removed ti_st_register_dev function and functionality moved to _probe.
>> module_init/exit function names fixed up.
>>
>> stat byte counter increments and tx_complete is similar to hci_ldisc.
>> Also I have not implemented the flush routine, since the functionality
>> which needs to be done in flush routine is done in the underlying driver
>> which is the shared transport driver and moreover the btwilink driver by
>> itself doesn't maintains queue or data relevant to transport, so nothing
>> to do.
>>
>> And Yes, I have verified this driver with multiple up/down reset on
>> hci0.
>> Also I generally test a2dp/ftp to verify large data transfers.
>
> Before re-submit a patch you have to fix all issues reported by the
> reviewer or reply to patch thread why do you think you right and don't
> want to change that. That is not happening with your patches, you are
> not fixing all the stuff before re-submiting and it is not the first
> time. =C2=A0If you do it right we can review it fast and your code goes
> earlier to mainline.
> You should also answer the questions first =C2=A0like "Where is the
> ti_st_proto.write coming from?" You just ignored the
> review and submitted a new patch.
This is the reason, I tend to keep the patch comments in the mail.
I have mentioned here the
1. comments I have taken care of,
2. few comments which I don't understand why it is like that and which
are not taken care of.
Also the question on ti_st_proto.write, I have answered it twice in
mail, once to you and once to marcel, for more clarity I have even
added it in the code comments, Please have a look @ line
>> + /* ti_st_proto.write is filled up by the underlying shared
>> + * transport driver upon registration
>> + */
As to why this function is not EXPORT_SYMBOL and just an function
pointer, Yes I had it as function pointer - But since something like
_read is callback from the protocol driver perspective - to maintain
uniformity I have this as function pointer.
(Note: comments to other drivers which are based on ST driver intended
read/write to be pointers which register/unregister to be EXPORTs).
Ok, apart from this there was an open question as why I am checking
for only 2 error conditions, it is because the underlying driver only
can send those 2 errors and nothing else (st_register has few errors
it can throw...)
I understand my problem with test_and_set_bit, I will correct it, But
I still feel I do nothing critical before test/set so as to return
error if already opened - But will correct it and do it at the
beginning ...
Also what should I be doing regarding the deferred stats update? I
checked up hci_ldisc which is what this code is pretty much based on,
and see that it does pretty much the same thing ....
hci_uart_tx_complete is called after the tty->ops->write correct ?
I also now understand the issue in module_init regarding the
platform_driver registration and will fix that.
Thanks,
Pavan
>>
>> v4 comments :-
>> module init now returns what platform_driver_register returns.
>> type casting of void* private data has been removed
>>
>> v3 comments :-
>> Lizardo,
>> I have taken care of most of the comments you had.
>> Have re-wrote some of the code commenting you've mentioned.
>> Thanks for the comments,
>>
>> The other few like -EPERM for platform driver registration is to keep
>> it similar to other drivers, type casting is maintained just to feel saf=
e
>> and have style similar to other drivers.
>> BT_WILINK in Kconfig is similar to BT_MRVL.
>> I hope those aren't too critical.
>>
>> -- patch description --
>>
>> This is the bluetooth protocol driver for the TI WiLink7 chipsets.
>> Texas Instrument's WiLink chipsets combine wireless technologies
>> like BT, FM, GPS and WLAN onto a single chip.
>>
>> This Bluetooth driver works on top of the TI_ST shared transport
>> line discipline driver which also allows other drivers like
>> FM V4L2 and GPS character driver to make use of the same UART interface.
>>
>> Kconfig and Makefile modifications to enable the Bluetooth
>> driver for Texas Instrument's WiLink 7 chipset.
>>
>> Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
>> ---
>> =C2=A0drivers/bluetooth/Kconfig =C2=A0 =C2=A0| =C2=A0 10 +
>> =C2=A0drivers/bluetooth/Makefile =C2=A0 | =C2=A0 =C2=A01 +
>> =C2=A0drivers/bluetooth/btwilink.c | =C2=A0379 +++++++++++++++++++++++++=
+++++++++++++++++
>> =C2=A03 files changed, 390 insertions(+), 0 deletions(-)
>> =C2=A0create mode 100644 drivers/bluetooth/btwilink.c
>>
>> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
>> index 02deef4..8e0de9a 100644
>> --- a/drivers/bluetooth/Kconfig
>> +++ b/drivers/bluetooth/Kconfig
>> @@ -219,4 +219,14 @@ config BT_ATH3K
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 Say Y here to compile support for "Atheros f=
irmware download driver"
>> =C2=A0 =C2=A0 =C2=A0 =C2=A0 into the kernel or say M to compile it as mo=
dule (ath3k).
>>
>> +config BT_WILINK
>> + =C2=A0 =C2=A0 tristate "Texas Instruments WiLink7 driver"
>> + =C2=A0 =C2=A0 depends on TI_ST
>> + =C2=A0 =C2=A0 help
>> + =C2=A0 =C2=A0 =C2=A0 This enables the Bluetooth driver for Texas Instr=
ument's BT/FM/GPS
>> + =C2=A0 =C2=A0 =C2=A0 combo devices. This makes use of shared transport=
line discipline
>> + =C2=A0 =C2=A0 =C2=A0 core driver to communicate with the BT core of th=
e combo chip.
>> +
>> + =C2=A0 =C2=A0 =C2=A0 Say Y here to compile support for Texas Instrumen=
t's WiLink7 driver
>> + =C2=A0 =C2=A0 =C2=A0 into the kernel or say M to compile it as module.
>> =C2=A0endmenu
>> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
>> index 71bdf13..f4460f4 100644
>> --- a/drivers/bluetooth/Makefile
>> +++ b/drivers/bluetooth/Makefile
>> @@ -18,6 +18,7 @@ obj-$(CONFIG_BT_HCIBTSDIO) =C2=A0+=3D btsdio.o
>> =C2=A0obj-$(CONFIG_BT_ATH3K) =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 +=3D ath3k.o
>> =C2=A0obj-$(CONFIG_BT_MRVL) =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0+=3D btmrvl.o
>> =C2=A0obj-$(CONFIG_BT_MRVL_SDIO) =C2=A0 +=3D btmrvl_sdio.o
>> +obj-$(CONFIG_BT_WILINK) =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0+=3D btwilink.o
>>
>> =C2=A0btmrvl-y =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 :=3D btmrvl_main.o
>> =C2=A0btmrvl-$(CONFIG_DEBUG_FS) =C2=A0 =C2=A0+=3D btmrvl_debugfs.o
>> diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
>> new file mode 100644
>> index 0000000..1b1c4bc
>> --- /dev/null
>> +++ b/drivers/bluetooth/btwilink.c
>> @@ -0,0 +1,379 @@
>> +/*
>> + * =C2=A0Texas Instrument's Bluetooth Driver For Shared Transport.
>> + *
>> + * =C2=A0Bluetooth Driver acts as interface between HCI core and
>> + * =C2=A0TI Shared Transport Layer.
>> + *
>> + * =C2=A0Copyright (C) 2009-2010 Texas Instruments
>> + * =C2=A0Author: Raja Mani <raja_mani@ti.com>
>> + * =C2=A0 Pavan Savoy <pavan_savoy@ti.com>
>> + *
>> + * =C2=A0This program is free software; you can redistribute it and/or =
modify
>> + * =C2=A0it under the terms of the GNU General Public License version 2=
as
>> + * =C2=A0published by the Free Software Foundation.
>> + *
>> + * =C2=A0This program is distributed in the hope that it will be useful=
,
>> + * =C2=A0but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * =C2=A0MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. =C2=A0See=
the
>> + * =C2=A0GNU General Public License for more details.
>> + *
>> + * =C2=A0You should have received a copy of the GNU General Public Lice=
nse
>> + * =C2=A0along with this program; if not, write to the Free Software
>> + * =C2=A0Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA =C2=
=A002111-1307 =C2=A0USA
>> + *
>> + */
>> +
>> +#include <linux/platform_device.h>
>> +#include <net/bluetooth/bluetooth.h>
>> +#include <net/bluetooth/hci_core.h>
>> +
>> +#include <linux/ti_wilink_st.h>
>> +
>> +/* Bluetooth Driver Version */
>> +#define VERSION =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "1.0"
>> +
>> +/* Number of seconds to wait for registration completion
>> + * when ST returns PENDING status.
>> + */
>> +#define BT_REGISTER_TIMEOUT =C2=A0 6000 =C2=A0 /* 6 sec */
>> +
>> +/**
>> + * struct ti_st - driver operation structure
>> + * @hdev: hci device pointer which binds to bt driver
>> + * @reg_status: ST registration callback status
>> + * @st_write: write function provided by the ST driver
>> + * =C2=A0 to be used by the driver during send_frame.
>> + * @wait_reg_completion - completion sync between ti_st_open
>> + * =C2=A0 and ti_st_registration_completion_cb.
>> + */
>> +struct ti_st {
>> + =C2=A0 =C2=A0 struct hci_dev *hdev;
>> + =C2=A0 =C2=A0 char reg_status;
>> + =C2=A0 =C2=A0 long (*st_write) (struct sk_buff *);
>> + =C2=A0 =C2=A0 struct completion wait_reg_completion;
>> +};
>> +
>> +/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
>> +static inline void ti_st_tx_complete(struct ti_st *hst, int pkt_type)
>> +{
>> + =C2=A0 =C2=A0 struct hci_dev *hdev =3D hst->hdev;
>> +
>> + =C2=A0 =C2=A0 /* Update HCI stat counters */
>> + =C2=A0 =C2=A0 switch (pkt_type) {
>> + =C2=A0 =C2=A0 case HCI_COMMAND_PKT:
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hdev->stat.cmd_tx++;
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
>> +
>> + =C2=A0 =C2=A0 case HCI_ACLDATA_PKT:
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hdev->stat.acl_tx++;
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
>> +
>> + =C2=A0 =C2=A0 case HCI_SCODATA_PKT:
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hdev->stat.sco_tx++;
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
>> + =C2=A0 =C2=A0 }
>> +}
>> +
>> +/* ------- Interfaces to Shared Transport ------ */
>> +
>> +/* Called by ST layer to indicate protocol registration completion
>> + * status.ti_st_open() function will wait for signal from this
>> + * API when st_register() function returns ST_PENDING.
>> + */
>> +static void st_registration_completion_cb(void *priv_data, char data)
>> +{
>> + =C2=A0 =C2=A0 struct ti_st *lhst =3D priv_data;
>> +
>> + =C2=A0 =C2=A0 /* Save registration status for use in ti_st_open() */
>> + =C2=A0 =C2=A0 lhst->reg_status =3D data;
>> + =C2=A0 =C2=A0 /* complete the wait in ti_st_open() */
>> + =C2=A0 =C2=A0 complete(&lhst->wait_reg_completion);
>> +}
>> +
>> +/* Called by Shared Transport layer when receive data is
>> + * available */
>> +static long st_receive(void *priv_data, struct sk_buff *skb)
>> +{
>> + =C2=A0 =C2=A0 struct ti_st *lhst =3D priv_data;
>> + =C2=A0 =C2=A0 int err;
>> +
>> + =C2=A0 =C2=A0 if (!skb)
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EFAULT;
>> +
>> + =C2=A0 =C2=A0 if (!lhst) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 kfree_skb(skb);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EFAULT;
>> + =C2=A0 =C2=A0 }
>> +
>> + =C2=A0 =C2=A0 skb->dev =3D (void *) lhst->hdev;
>> +
>> + =C2=A0 =C2=A0 /* Forward skb to HCI core layer */
>> + =C2=A0 =C2=A0 err =3D hci_recv_frame(skb);
>> + =C2=A0 =C2=A0 if (err < 0) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR("Unable to push skb t=
o HCI core(%d)", err);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return err;
>> + =C2=A0 =C2=A0 }
>> +
>> + =C2=A0 =C2=A0 lhst->hdev->stat.byte_rx +=3D skb->len;
>> +
>> + =C2=A0 =C2=A0 return 0;
>> +}
>> +
>> +/* ------- Interfaces to HCI layer ------ */
>> +/* protocol structure registered with shared transport */
>> +static struct st_proto_s ti_st_proto =3D {
>> + =C2=A0 =C2=A0 .type =3D ST_BT,
>> + =C2=A0 =C2=A0 .recv =3D st_receive,
>> + =C2=A0 =C2=A0 .reg_complete_cb =3D st_registration_completion_cb,
>> +};
>> +
>> +/* Called from HCI core to initialize the device */
>> +static int ti_st_open(struct hci_dev *hdev)
>> +{
>> + =C2=A0 =C2=A0 unsigned long timeleft;
>> + =C2=A0 =C2=A0 struct ti_st *hst;
>> + =C2=A0 =C2=A0 int err;
>> +
>> + =C2=A0 =C2=A0 BT_DBG("%s %p", hdev->name, hdev);
>> +
>> + =C2=A0 =C2=A0 /* provide contexts for callbacks from ST */
>> + =C2=A0 =C2=A0 hst =3D hdev->driver_data;
>> + =C2=A0 =C2=A0 ti_st_proto.priv_data =3D hst;
>> +
>> + =C2=A0 =C2=A0 err =3D st_register(&ti_st_proto);
>> + =C2=A0 =C2=A0 if (err =3D=3D -EINPROGRESS) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Prepare wait-for-completi=
on handler data structures.
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* Needed to synchroniz=
e this and
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* st_registration_comp=
letion_cb() functions.
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 init_completion(&hst->wait_r=
eg_completion);
>> +
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Reset ST registration cal=
lback status flag , this value
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* will be updated in t=
i_st_registration_completion_cb()
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* function whenever it=
called from ST driver.
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hst->reg_status =3D -EINPROG=
RESS;
>> +
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* ST is busy with either pr=
otocol registration or firmware
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0* download. Wait until=
the registration callback is called
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0*/
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_DBG(" waiting for registr=
ation completion signal from ST");
>> +
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 timeleft =3D wait_for_comple=
tion_timeout
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
(&hst->wait_reg_completion,
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0msecs_to_jiffies(BT_REGISTER_TIMEOUT));
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!timeleft) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
BT_ERR("Timeout(%d sec),didn't get reg "
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "completion signal =
from ST",
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_REGISTER_TIMEOUT=
/ 1000);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
return -ETIMEDOUT;
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
>> +
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Is ST registration callba=
ck called with ERROR status? */
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (hst->reg_status !=3D 0) =
{
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
BT_ERR("ST registration completed with invalid "
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "status %d", hst->r=
eg_status);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
return -EAGAIN;
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 }
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 err =3D 0;
>> + =C2=A0 =C2=A0 } else if (err =3D=3D -EPERM) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR("st_register failed %=
d", err);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return err;
>> + =C2=A0 =C2=A0 }
>
>
> Again, you are checking for only EPERM and EINPROGRESS. You should check
> for everything. I actually don't undertand why you have a special check
> for EPERM.
>
>> +
>> + =C2=A0 =C2=A0 /* ti_st_proto.write is filled up by the underlying shar=
ed
>> + =C2=A0 =C2=A0 =C2=A0* transport driver upon registration
>> + =C2=A0 =C2=A0 =C2=A0*/
>> + =C2=A0 =C2=A0 hst->st_write =3D ti_st_proto.write;
>
> I do not like that, the underlying should export the write function.
>
>> + =C2=A0 =C2=A0 if (!hst->st_write) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR("undefined ST write f=
unction");
>> +
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Undo registration with ST=
*/
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 err =3D st_unregister(ST_BT)=
;
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (err)
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
BT_ERR("st_unregister() failed with error %d", err);
>> +
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hst->st_write =3D NULL;
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return err;
>> + =C2=A0 =C2=A0 }
>> +
>> + =C2=A0 =C2=A0 /* Registration with ST layer is successful,
>> + =C2=A0 =C2=A0 =C2=A0* hardware is ready to accept commands from HCI co=
re.
>> + =C2=A0 =C2=A0 =C2=A0*/
>> + =C2=A0 =C2=A0 if (test_and_set_bit(HCI_RUNNING, &hdev->flags)) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 clear_bit(HCI_RUNNING, &hdev=
->flags);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 err =3D st_unregister(ST_BT)=
;
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (err)
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
BT_ERR("st_unregister() failed with error %d", err);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hst->st_write =3D NULL;
>> + =C2=A0 =C2=A0 }
>
>
> What are you trying to do here? test_and_set_bit() result doesn't say
> nothing about error and you shall put test_and_set_bit should be in the
> beginning, to know if your device is already opened or not and then
> clear_bit if some error ocurrs during the function.
>
>> +
>> + =C2=A0 =C2=A0 return err;
>> +}
>> +
>> +/* Close device */
>> +static int ti_st_close(struct hci_dev *hdev)
>> +{
>> + =C2=A0 =C2=A0 int err;
>> + =C2=A0 =C2=A0 struct ti_st *hst =3D hdev->driver_data;
>> +
>> + =C2=A0 =C2=A0 /* continue to unregister from transport */
>> + =C2=A0 =C2=A0 err =3D st_unregister(ST_BT);
>> + =C2=A0 =C2=A0 if (err)
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR("st_unregister() fail=
ed with error %d", err);
>> +
>> + =C2=A0 =C2=A0 hst->st_write =3D NULL;
>> +
>> + =C2=A0 =C2=A0 if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return 0;
>
> test_and_clear_bit should come first to check if your device is already
> closed.
>
>> +
>> + =C2=A0 =C2=A0 return err;
>> +}
>> +
>> +static int ti_st_send_frame(struct sk_buff *skb)
>> +{
>> + =C2=A0 =C2=A0 struct hci_dev *hdev;
>> + =C2=A0 =C2=A0 struct ti_st *hst;
>> + =C2=A0 =C2=A0 long len;
>> +
>> + =C2=A0 =C2=A0 hdev =3D (struct hci_dev *)skb->dev;
>> +
>> + =C2=A0 =C2=A0 if (!test_bit(HCI_RUNNING, &hdev->flags))
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EBUSY;
>> +
>> + =C2=A0 =C2=A0 hst =3D hdev->driver_data;
>> +
>> + =C2=A0 =C2=A0 /* Prepend skb with frame type */
>> + =C2=A0 =C2=A0 memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
>> +
>> + =C2=A0 =C2=A0 BT_DBG(" %s: type %d len %d", hdev->name, bt_cb(skb)->pk=
t_type,
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
skb->len);
>> +
>> + =C2=A0 =C2=A0 /* Insert skb to shared transport layer's transmit queue=
.
>> + =C2=A0 =C2=A0 =C2=A0* Freeing skb memory is taken care in shared trans=
port layer,
>> + =C2=A0 =C2=A0 =C2=A0* so don't free skb memory here.
>> + =C2=A0 =C2=A0 =C2=A0*/
>> + =C2=A0 =C2=A0 len =3D hst->st_write(skb);
>> + =C2=A0 =C2=A0 if (len < 0) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 kfree_skb(skb);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR(" ST write failed (%l=
d)", len);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EAGAIN;
>
> Why EAGAIN?
>
>> + =C2=A0 =C2=A0 }
>> +
>> + =C2=A0 =C2=A0 /* ST accepted our skb. So, Go ahead and do rest */
>> + =C2=A0 =C2=A0 hdev->stat.byte_tx +=3D len;
>> + =C2=A0 =C2=A0 ti_st_tx_complete(hst, bt_cb(skb)->pkt_type);
>
> From Marcel in the other thread:
> "What is the reason for this deferred stats update. That code looks
> pretty much hackish to me."
>
>> +
>> + =C2=A0 =C2=A0 return 0;
>> +}
>> +
>> +static void ti_st_destruct(struct hci_dev *hdev)
>> +{
>> + =C2=A0 =C2=A0 BT_DBG("%s", hdev->name);
>> +
>> + =C2=A0 =C2=A0 /* free ti_st memory */
>
> get ride of the comment, it's pointless
>
>> + =C2=A0 =C2=A0 kfree(hdev->driver_data);
>> +
>> + =C2=A0 =C2=A0 return;
>
> No return; here
>
>> +}
>> +
>> +static int bt_ti_probe(struct platform_device *pdev)
>> +{
>> + =C2=A0 =C2=A0 static struct ti_st *hst;
>> + =C2=A0 =C2=A0 struct hci_dev *hdev;
>> + =C2=A0 =C2=A0 int err;
>> +
>> + =C2=A0 =C2=A0 hst =3D kzalloc(sizeof(struct ti_st), GFP_KERNEL);
>> + =C2=A0 =C2=A0 if (!hst)
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -ENOMEM;
>> +
>> + =C2=A0 =C2=A0 /* Expose "hciX" device to user space */
>> + =C2=A0 =C2=A0 hdev =3D hci_alloc_dev();
>> + =C2=A0 =C2=A0 if (!hdev)
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -ENOMEM;
>
> You are leaking hst, if hci_alloc_dev() fails.
>
>> +
>> + =C2=A0 =C2=A0 BT_DBG("hdev %p", hdev);
>> +
>> + =C2=A0 =C2=A0 hst->hdev =3D hdev;
>> + =C2=A0 =C2=A0 hdev->bus =3D HCI_UART;
>> + =C2=A0 =C2=A0 hdev->driver_data =3D hst;
>> + =C2=A0 =C2=A0 hdev->open =3D ti_st_open;
>> + =C2=A0 =C2=A0 hdev->close =3D ti_st_close;
>> + =C2=A0 =C2=A0 hdev->flush =3D NULL;
>> + =C2=A0 =C2=A0 hdev->send =3D ti_st_send_frame;
>> + =C2=A0 =C2=A0 hdev->destruct =3D ti_st_destruct;
>> + =C2=A0 =C2=A0 hdev->owner =3D THIS_MODULE;
>> +
>> + =C2=A0 =C2=A0 err =3D hci_register_dev(hdev);
>> + =C2=A0 =C2=A0 if (err < 0) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR("Can't register HCI d=
evice error %d", err);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hci_free_dev(hdev);
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return err;
>> + =C2=A0 =C2=A0 }
>> +
>> + =C2=A0 =C2=A0 BT_DBG(" HCI device registered (hdev %p)", hdev);
>> +
>> + =C2=A0 =C2=A0 dev_set_drvdata(&pdev->dev, hst);
>> + =C2=A0 =C2=A0 return err;
>> +}
>> +
>> +static int bt_ti_remove(struct platform_device *pdev)
>> +{
>> + =C2=A0 =C2=A0 struct hci_dev *hdev;
>> + =C2=A0 =C2=A0 struct ti_st *hst =3D dev_get_drvdata(&pdev->dev);
>> +
>> + =C2=A0 =C2=A0 if (!hst)
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return -EFAULT;
>> +
>> + =C2=A0 =C2=A0 hdev =3D hst->hdev;
>> + =C2=A0 =C2=A0 ti_st_close(hdev);
>> + =C2=A0 =C2=A0 hci_unregister_dev(hdev);
>> +
>> + =C2=A0 =C2=A0 /* Free HCI device memory */
>> + =C2=A0 =C2=A0 hci_free_dev(hdev);
>> +
>> + =C2=A0 =C2=A0 /* Free driver data memory */
>
> get ride of both commnets here. The name of the funcion is already
> saying that.
>
>> + =C2=A0 =C2=A0 kfree(hst);
>> +
>> + =C2=A0 =C2=A0 dev_set_drvdata(&pdev->dev, NULL);
>> + =C2=A0 =C2=A0 return 0;
>> +}
>> +
>> +static struct platform_driver btwilink_driver =3D {
>> + =C2=A0 =C2=A0 .probe =3D bt_ti_probe,
>> + =C2=A0 =C2=A0 .remove =3D bt_ti_remove,
>> + =C2=A0 =C2=A0 .driver =3D {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .name =3D "btwilink",
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 .owner =3D THIS_MODULE,
>> + =C2=A0 =C2=A0 },
>> +};
>> +
>> +/* ------- Module Init/Exit interfaces ------ */
>> +static int __init btwilink_init(void)
>> +{
>> + =C2=A0 =C2=A0 long ret;
>> +
>> + =C2=A0 =C2=A0 BT_INFO(" Bluetooth Driver for TI WiLink - Version %s", =
VERSION);
>> +
>> + =C2=A0 =C2=A0 ret =3D platform_driver_register(&btwilink_driver);
>> + =C2=A0 =C2=A0 if (ret !=3D 0) {
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 BT_ERR("btwilink platform dr=
iver registration failed");
>> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return ret;
>> + =C2=A0 =C2=A0 }
>> + =C2=A0 =C2=A0 return 0;
>
> Old issue again:
>
> From Marcel: please just do like we do with all other drivers;
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0BT_INFO(...)
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0return platform_driver_register(&btwilink_driv=
er);
>
> --
> Gustavo F. Padovan
> http://profusion.mobi
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at =C2=A0http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Does OBEX authentication works in OPP?
From: liang yu @ 2010-11-17 3:43 UTC (permalink / raw)
To: linux-bluetooth
Hello all:
I am not sure it is the right place to ask the question.
According to the Bluetooth SIG profile OBEX authentication is not used
in OPP. But in source code of Obexd-0.36/plugins/opp.c, the function
opp_chkput invokes manager_request_authorization. Does this mean that
in your implementation it is still checked OBEX authentication in opp?
Doea anybody give me some tips.
Thanks advance.
Best regards
^ permalink raw reply
* Re: Apple Wireless Keyboard connection issue
From: Gustavo F. Padovan @ 2010-11-17 1:30 UTC (permalink / raw)
To: "André Kühne"; +Cc: linux-bluetooth
In-Reply-To: <20101116212701.50060@gmx.net>
* "André Kühne" <andre.kuehne@gmx.net> [2010-11-16 22:27:01 +0100]:
> Hi everyone
>
> I noticed the following on my system: After upgrading to bluez-4.79 connecting my Apple Wireless Keyboard does not work anymore. With bluez-4.77 the connection works just fine.
>
My Microsoft keyboard is not working too. That is probably due to commit
abe7cd44124a from Johan. It should be fixed soon.
--
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* Re: [PATCH v5] Bluetooth: btwilink driver
From: Vitaly Wool @ 2010-11-16 23:20 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: pavan_savoy, marcel, linux-bluetooth, linux-kernel
In-Reply-To: <20101116225418.GA15101@vigoh>
>> + =A0 =A0 /* Registration with ST layer is successful,
>> + =A0 =A0 =A0* hardware is ready to accept commands from HCI core.
>> + =A0 =A0 =A0*/
>> + =A0 =A0 if (test_and_set_bit(HCI_RUNNING, &hdev->flags)) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 clear_bit(HCI_RUNNING, &hdev->flags);
>> + =A0 =A0 =A0 =A0 =A0 =A0 err =3D st_unregister(ST_BT);
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (err)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 BT_ERR("st_unregister() failed=
with error %d", err);
>> + =A0 =A0 =A0 =A0 =A0 =A0 hst->st_write =3D NULL;
>> + =A0 =A0 }
>
>
> What are you trying to do here? test_and_set_bit() result doesn't say
> nothing about error and you shall put test_and_set_bit should be in the
> beginning, to know if your device is already opened or not and then
> clear_bit if some error ocurrs during the function.
>
Yeap, this piece of code beats me is well. Why is it an error if this
bit wasn't already set?
~Vitaly
^ permalink raw reply
* Re: [PATCH v5] Bluetooth: btwilink driver
From: Gustavo F. Padovan @ 2010-11-16 22:54 UTC (permalink / raw)
To: pavan_savoy; +Cc: marcel, linux-bluetooth, linux-kernel
In-Reply-To: <1289394446-14021-1-git-send-email-pavan_savoy@ti.com>
Hi Pavan,
* pavan_savoy@ti.com <pavan_savoy@ti.com> [2010-11-10 08:07:26 -0500]:
> From: Pavan Savoy <pavan_savoy@ti.com>
>
> Marcel,
>
> Thanks for the comments...
> This patch contains,
> v5 comments :-
> declaration and assiging of variables and private data fixed up.
> proper casting.
> removed redundant un-necessary checks in send_frame.
> HCI_RUNNING fixes in terms of test_and_set/clear bit instead of set and
> clear.
> removed redundant checks for hdev, skb being NULL.
> removed module_param of reset, also WiLink don't need HCI_RESET anyways.
> removed ti_st_register_dev function and functionality moved to _probe.
> module_init/exit function names fixed up.
>
> stat byte counter increments and tx_complete is similar to hci_ldisc.
> Also I have not implemented the flush routine, since the functionality
> which needs to be done in flush routine is done in the underlying driver
> which is the shared transport driver and moreover the btwilink driver by
> itself doesn't maintains queue or data relevant to transport, so nothing
> to do.
>
> And Yes, I have verified this driver with multiple up/down reset on
> hci0.
> Also I generally test a2dp/ftp to verify large data transfers.
Before re-submit a patch you have to fix all issues reported by the
reviewer or reply to patch thread why do you think you right and don't
want to change that. That is not happening with your patches, you are
not fixing all the stuff before re-submiting and it is not the first
time. If you do it right we can review it fast and your code goes
earlier to mainline.
You should also answer the questions first like "Where is the
ti_st_proto.write coming from?" You just ignored the
review and submitted a new patch.
>
> Please review and comment.
>
> Thanks,
> Pavan
>
> v4 comments :-
> module init now returns what platform_driver_register returns.
> type casting of void* private data has been removed
>
> v3 comments :-
> Lizardo,
> I have taken care of most of the comments you had.
> Have re-wrote some of the code commenting you've mentioned.
> Thanks for the comments,
>
> The other few like -EPERM for platform driver registration is to keep
> it similar to other drivers, type casting is maintained just to feel safe
> and have style similar to other drivers.
> BT_WILINK in Kconfig is similar to BT_MRVL.
> I hope those aren't too critical.
>
> -- patch description --
>
> This is the bluetooth protocol driver for the TI WiLink7 chipsets.
> Texas Instrument's WiLink chipsets combine wireless technologies
> like BT, FM, GPS and WLAN onto a single chip.
>
> This Bluetooth driver works on top of the TI_ST shared transport
> line discipline driver which also allows other drivers like
> FM V4L2 and GPS character driver to make use of the same UART interface.
>
> Kconfig and Makefile modifications to enable the Bluetooth
> driver for Texas Instrument's WiLink 7 chipset.
>
> Signed-off-by: Pavan Savoy <pavan_savoy@ti.com>
> ---
> drivers/bluetooth/Kconfig | 10 +
> drivers/bluetooth/Makefile | 1 +
> drivers/bluetooth/btwilink.c | 379 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 390 insertions(+), 0 deletions(-)
> create mode 100644 drivers/bluetooth/btwilink.c
>
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 02deef4..8e0de9a 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -219,4 +219,14 @@ config BT_ATH3K
> Say Y here to compile support for "Atheros firmware download driver"
> into the kernel or say M to compile it as module (ath3k).
>
> +config BT_WILINK
> + tristate "Texas Instruments WiLink7 driver"
> + depends on TI_ST
> + help
> + This enables the Bluetooth driver for Texas Instrument's BT/FM/GPS
> + combo devices. This makes use of shared transport line discipline
> + core driver to communicate with the BT core of the combo chip.
> +
> + Say Y here to compile support for Texas Instrument's WiLink7 driver
> + into the kernel or say M to compile it as module.
> endmenu
> diff --git a/drivers/bluetooth/Makefile b/drivers/bluetooth/Makefile
> index 71bdf13..f4460f4 100644
> --- a/drivers/bluetooth/Makefile
> +++ b/drivers/bluetooth/Makefile
> @@ -18,6 +18,7 @@ obj-$(CONFIG_BT_HCIBTSDIO) += btsdio.o
> obj-$(CONFIG_BT_ATH3K) += ath3k.o
> obj-$(CONFIG_BT_MRVL) += btmrvl.o
> obj-$(CONFIG_BT_MRVL_SDIO) += btmrvl_sdio.o
> +obj-$(CONFIG_BT_WILINK) += btwilink.o
>
> btmrvl-y := btmrvl_main.o
> btmrvl-$(CONFIG_DEBUG_FS) += btmrvl_debugfs.o
> diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c
> new file mode 100644
> index 0000000..1b1c4bc
> --- /dev/null
> +++ b/drivers/bluetooth/btwilink.c
> @@ -0,0 +1,379 @@
> +/*
> + * Texas Instrument's Bluetooth Driver For Shared Transport.
> + *
> + * Bluetooth Driver acts as interface between HCI core and
> + * TI Shared Transport Layer.
> + *
> + * Copyright (C) 2009-2010 Texas Instruments
> + * Author: Raja Mani <raja_mani@ti.com>
> + * Pavan Savoy <pavan_savoy@ti.com>
> + *
> + * 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.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
> + *
> + */
> +
> +#include <linux/platform_device.h>
> +#include <net/bluetooth/bluetooth.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +#include <linux/ti_wilink_st.h>
> +
> +/* Bluetooth Driver Version */
> +#define VERSION "1.0"
> +
> +/* Number of seconds to wait for registration completion
> + * when ST returns PENDING status.
> + */
> +#define BT_REGISTER_TIMEOUT 6000 /* 6 sec */
> +
> +/**
> + * struct ti_st - driver operation structure
> + * @hdev: hci device pointer which binds to bt driver
> + * @reg_status: ST registration callback status
> + * @st_write: write function provided by the ST driver
> + * to be used by the driver during send_frame.
> + * @wait_reg_completion - completion sync between ti_st_open
> + * and ti_st_registration_completion_cb.
> + */
> +struct ti_st {
> + struct hci_dev *hdev;
> + char reg_status;
> + long (*st_write) (struct sk_buff *);
> + struct completion wait_reg_completion;
> +};
> +
> +/* Increments HCI counters based on pocket ID (cmd,acl,sco) */
> +static inline void ti_st_tx_complete(struct ti_st *hst, int pkt_type)
> +{
> + struct hci_dev *hdev = hst->hdev;
> +
> + /* Update HCI stat counters */
> + switch (pkt_type) {
> + case HCI_COMMAND_PKT:
> + hdev->stat.cmd_tx++;
> + break;
> +
> + case HCI_ACLDATA_PKT:
> + hdev->stat.acl_tx++;
> + break;
> +
> + case HCI_SCODATA_PKT:
> + hdev->stat.sco_tx++;
> + break;
> + }
> +}
> +
> +/* ------- Interfaces to Shared Transport ------ */
> +
> +/* Called by ST layer to indicate protocol registration completion
> + * status.ti_st_open() function will wait for signal from this
> + * API when st_register() function returns ST_PENDING.
> + */
> +static void st_registration_completion_cb(void *priv_data, char data)
> +{
> + struct ti_st *lhst = priv_data;
> +
> + /* Save registration status for use in ti_st_open() */
> + lhst->reg_status = data;
> + /* complete the wait in ti_st_open() */
> + complete(&lhst->wait_reg_completion);
> +}
> +
> +/* Called by Shared Transport layer when receive data is
> + * available */
> +static long st_receive(void *priv_data, struct sk_buff *skb)
> +{
> + struct ti_st *lhst = priv_data;
> + int err;
> +
> + if (!skb)
> + return -EFAULT;
> +
> + if (!lhst) {
> + kfree_skb(skb);
> + return -EFAULT;
> + }
> +
> + skb->dev = (void *) lhst->hdev;
> +
> + /* Forward skb to HCI core layer */
> + err = hci_recv_frame(skb);
> + if (err < 0) {
> + BT_ERR("Unable to push skb to HCI core(%d)", err);
> + return err;
> + }
> +
> + lhst->hdev->stat.byte_rx += skb->len;
> +
> + return 0;
> +}
> +
> +/* ------- Interfaces to HCI layer ------ */
> +/* protocol structure registered with shared transport */
> +static struct st_proto_s ti_st_proto = {
> + .type = ST_BT,
> + .recv = st_receive,
> + .reg_complete_cb = st_registration_completion_cb,
> +};
> +
> +/* Called from HCI core to initialize the device */
> +static int ti_st_open(struct hci_dev *hdev)
> +{
> + unsigned long timeleft;
> + struct ti_st *hst;
> + int err;
> +
> + BT_DBG("%s %p", hdev->name, hdev);
> +
> + /* provide contexts for callbacks from ST */
> + hst = hdev->driver_data;
> + ti_st_proto.priv_data = hst;
> +
> + err = st_register(&ti_st_proto);
> + if (err == -EINPROGRESS) {
> + /* Prepare wait-for-completion handler data structures.
> + * Needed to synchronize this and
> + * st_registration_completion_cb() functions.
> + */
> + init_completion(&hst->wait_reg_completion);
> +
> + /* Reset ST registration callback status flag , this value
> + * will be updated in ti_st_registration_completion_cb()
> + * function whenever it called from ST driver.
> + */
> + hst->reg_status = -EINPROGRESS;
> +
> + /* ST is busy with either protocol registration or firmware
> + * download. Wait until the registration callback is called
> + */
> + BT_DBG(" waiting for registration completion signal from ST");
> +
> + timeleft = wait_for_completion_timeout
> + (&hst->wait_reg_completion,
> + msecs_to_jiffies(BT_REGISTER_TIMEOUT));
> + if (!timeleft) {
> + BT_ERR("Timeout(%d sec),didn't get reg "
> + "completion signal from ST",
> + BT_REGISTER_TIMEOUT / 1000);
> + return -ETIMEDOUT;
> + }
> +
> + /* Is ST registration callback called with ERROR status? */
> + if (hst->reg_status != 0) {
> + BT_ERR("ST registration completed with invalid "
> + "status %d", hst->reg_status);
> + return -EAGAIN;
> + }
> + err = 0;
> + } else if (err == -EPERM) {
> + BT_ERR("st_register failed %d", err);
> + return err;
> + }
Again, you are checking for only EPERM and EINPROGRESS. You should check
for everything. I actually don't undertand why you have a special check
for EPERM.
> +
> + /* ti_st_proto.write is filled up by the underlying shared
> + * transport driver upon registration
> + */
> + hst->st_write = ti_st_proto.write;
I do not like that, the underlying should export the write function.
> + if (!hst->st_write) {
> + BT_ERR("undefined ST write function");
> +
> + /* Undo registration with ST */
> + err = st_unregister(ST_BT);
> + if (err)
> + BT_ERR("st_unregister() failed with error %d", err);
> +
> + hst->st_write = NULL;
> + return err;
> + }
> +
> + /* Registration with ST layer is successful,
> + * hardware is ready to accept commands from HCI core.
> + */
> + if (test_and_set_bit(HCI_RUNNING, &hdev->flags)) {
> + clear_bit(HCI_RUNNING, &hdev->flags);
> + err = st_unregister(ST_BT);
> + if (err)
> + BT_ERR("st_unregister() failed with error %d", err);
> + hst->st_write = NULL;
> + }
What are you trying to do here? test_and_set_bit() result doesn't say
nothing about error and you shall put test_and_set_bit should be in the
beginning, to know if your device is already opened or not and then
clear_bit if some error ocurrs during the function.
> +
> + return err;
> +}
> +
> +/* Close device */
> +static int ti_st_close(struct hci_dev *hdev)
> +{
> + int err;
> + struct ti_st *hst = hdev->driver_data;
> +
> + /* continue to unregister from transport */
> + err = st_unregister(ST_BT);
> + if (err)
> + BT_ERR("st_unregister() failed with error %d", err);
> +
> + hst->st_write = NULL;
> +
> + if (!test_and_clear_bit(HCI_RUNNING, &hdev->flags))
> + return 0;
test_and_clear_bit should come first to check if your device is already
closed.
> +
> + return err;
> +}
> +
> +static int ti_st_send_frame(struct sk_buff *skb)
> +{
> + struct hci_dev *hdev;
> + struct ti_st *hst;
> + long len;
> +
> + hdev = (struct hci_dev *)skb->dev;
> +
> + if (!test_bit(HCI_RUNNING, &hdev->flags))
> + return -EBUSY;
> +
> + hst = hdev->driver_data;
> +
> + /* Prepend skb with frame type */
> + memcpy(skb_push(skb, 1), &bt_cb(skb)->pkt_type, 1);
> +
> + BT_DBG(" %s: type %d len %d", hdev->name, bt_cb(skb)->pkt_type,
> + skb->len);
> +
> + /* Insert skb to shared transport layer's transmit queue.
> + * Freeing skb memory is taken care in shared transport layer,
> + * so don't free skb memory here.
> + */
> + len = hst->st_write(skb);
> + if (len < 0) {
> + kfree_skb(skb);
> + BT_ERR(" ST write failed (%ld)", len);
> + return -EAGAIN;
Why EAGAIN?
> + }
> +
> + /* ST accepted our skb. So, Go ahead and do rest */
> + hdev->stat.byte_tx += len;
> + ti_st_tx_complete(hst, bt_cb(skb)->pkt_type);
>From Marcel in the other thread:
"What is the reason for this deferred stats update. That code looks
pretty much hackish to me."
> +
> + return 0;
> +}
> +
> +static void ti_st_destruct(struct hci_dev *hdev)
> +{
> + BT_DBG("%s", hdev->name);
> +
> + /* free ti_st memory */
get ride of the comment, it's pointless
> + kfree(hdev->driver_data);
> +
> + return;
No return; here
> +}
> +
> +static int bt_ti_probe(struct platform_device *pdev)
> +{
> + static struct ti_st *hst;
> + struct hci_dev *hdev;
> + int err;
> +
> + hst = kzalloc(sizeof(struct ti_st), GFP_KERNEL);
> + if (!hst)
> + return -ENOMEM;
> +
> + /* Expose "hciX" device to user space */
> + hdev = hci_alloc_dev();
> + if (!hdev)
> + return -ENOMEM;
You are leaking hst, if hci_alloc_dev() fails.
> +
> + BT_DBG("hdev %p", hdev);
> +
> + hst->hdev = hdev;
> + hdev->bus = HCI_UART;
> + hdev->driver_data = hst;
> + hdev->open = ti_st_open;
> + hdev->close = ti_st_close;
> + hdev->flush = NULL;
> + hdev->send = ti_st_send_frame;
> + hdev->destruct = ti_st_destruct;
> + hdev->owner = THIS_MODULE;
> +
> + err = hci_register_dev(hdev);
> + if (err < 0) {
> + BT_ERR("Can't register HCI device error %d", err);
> + hci_free_dev(hdev);
> + return err;
> + }
> +
> + BT_DBG(" HCI device registered (hdev %p)", hdev);
> +
> + dev_set_drvdata(&pdev->dev, hst);
> + return err;
> +}
> +
> +static int bt_ti_remove(struct platform_device *pdev)
> +{
> + struct hci_dev *hdev;
> + struct ti_st *hst = dev_get_drvdata(&pdev->dev);
> +
> + if (!hst)
> + return -EFAULT;
> +
> + hdev = hst->hdev;
> + ti_st_close(hdev);
> + hci_unregister_dev(hdev);
> +
> + /* Free HCI device memory */
> + hci_free_dev(hdev);
> +
> + /* Free driver data memory */
get ride of both commnets here. The name of the funcion is already
saying that.
> + kfree(hst);
> +
> + dev_set_drvdata(&pdev->dev, NULL);
> + return 0;
> +}
> +
> +static struct platform_driver btwilink_driver = {
> + .probe = bt_ti_probe,
> + .remove = bt_ti_remove,
> + .driver = {
> + .name = "btwilink",
> + .owner = THIS_MODULE,
> + },
> +};
> +
> +/* ------- Module Init/Exit interfaces ------ */
> +static int __init btwilink_init(void)
> +{
> + long ret;
> +
> + BT_INFO(" Bluetooth Driver for TI WiLink - Version %s", VERSION);
> +
> + ret = platform_driver_register(&btwilink_driver);
> + if (ret != 0) {
> + BT_ERR("btwilink platform driver registration failed");
> + return ret;
> + }
> + return 0;
Old issue again:
>From Marcel: please just do like we do with all other drivers;
BT_INFO(...)
return platform_driver_register(&btwilink_driver);
--
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* Re: [PATCH 3/3] Bluetooth: Automate remote name requests
From: Gustavo F. Padovan @ 2010-11-16 22:09 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth, Johan Hedberg
In-Reply-To: <1289401913-22982-3-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
* johan.hedberg@gmail.com <johan.hedberg@gmail.com> [2010-11-10 17:11:53 +0200]:
> From: Johan Hedberg <johan.hedberg@nokia.com>
>
> In Bluetooth there are no automatic updates of remote device names when
> they get changed on the remote side. Instead, it is a good idea to do a
> manual name request when a new connection gets created (for whatever
> reason) since at this point it is very cheap (no costly baseband
> connection creation needed just for the sake of the name request).
>
> So far userspace has been responsible for this extra name request but
> tighter control is needed in order not to flood Bluetooth controllers
> with two many commands during connection creation. It has been shown
> that some controllers simply fail to function correctly if they get too
> many (almost) simultaneous commands during connection creation. The
> simplest way to acheive better control of these commands is to move
> their sending completely to the kernel side.
>
> This patch inserts name requests into the sequence of events that the
> kernel performs during connection creation. It does this after the
> remote features have been successfully requested and before any pending
> authentication requests are performed. The code will work sub-optimally
> with userspace versions that still do the name requesting themselves (it
> shouldn't break anything though) so it is recommended to combine this
> with a userspace software version that doesn't have automated name
> requests.
>
> Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
> ---
> net/bluetooth/hci_event.c | 74 ++++++++++++++++++++++++++++++++++----------
> 1 files changed, 57 insertions(+), 17 deletions(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 45569f2..cef970f 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -677,9 +677,8 @@ static void hci_cs_set_conn_encrypt(struct hci_dev *hdev, __u8 status)
> hci_dev_unlock(hdev);
> }
>
> -static int request_outgoing_auth(struct hci_dev *hdev, bdaddr_t *bdaddr)
> +static int outgoing_auth_needed(struct hci_dev *hdev, bdaddr_t *bdaddr)
Can you add a hci_ in the beginning of your functions, just to keep the
coherency with the rest of the code.
> {
> - struct hci_cp_auth_requested cp;
> struct hci_conn *conn;
>
> conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, bdaddr);
> @@ -698,15 +697,43 @@ static int request_outgoing_auth(struct hci_dev *hdev, bdaddr_t *bdaddr)
> conn->sec_level != BT_SECURITY_HIGH)
> return 0;
>
> - cp.handle = __cpu_to_le16(conn->handle);
> - hci_send_cmd(hdev, HCI_OP_AUTH_REQUESTED, sizeof(cp), &cp);
> -
> return 1;
> }
>
> +static int request_auth(struct hci_dev *hdev, bdaddr_t *bdaddr)
> +{
> + struct hci_cp_auth_requested cp;
> + struct hci_conn *conn;
> +
> + conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, bdaddr);
> + if (!conn)
> + return -ENOTCONN;
I'm not happy with have to lookup the hci_conn twice when we can do that
once here. I've noted that always outgoing_auth_needed() returns 1 you do
a request_auth, and always it returns 0 you don't, so I think we can
embed request_auth() inside outgoing_auth_needed() as it was in patch
2/3 and the give a better name to outgoing_auth_needed() which you
reflect the new behavior.
Regards,
--
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* Apple Wireless Keyboard connection issue
From: "André Kühne" @ 2010-11-16 21:27 UTC (permalink / raw)
To: linux-bluetooth
Hi everyone
I noticed the following on my system: After upgrading to bluez-4.79 connecting my Apple Wireless Keyboard does not work anymore. With bluez-4.77 the connection works just fine.
Please let me know if I could be of any help e.g. providing more detailed information.
Best regards
Andre
--
Neu: GMX De-Mail - Einfach wie E-Mail, sicher wie ein Brief!
Jetzt De-Mail-Adresse reservieren: http://portal.gmx.net/de/go/demail
^ permalink raw reply
* [RFC] Discover Primary Service by Service UUID
From: Claudio Takahasi @ 2010-11-16 21:24 UTC (permalink / raw)
To: linux-bluetooth
Hi all,
Before I send an official request-pull I need feedbacks about the
changes in the bt_string2uuid function.
Commit 8774e6c1523707b01efd892c665baeeabad41ab4 of my branch "gatt"
extends the bt_string2uuid to support UUID16, this modification
changes the behavior of the method DiscoverServices(string pattern) in
the device. Now, pattern can be also a hex string value instead of
UUID128 and "friendly" service names only. Is it acceptable?
This change will be required to implement Discover Primary Service by
Service UUID and Characteristic Value Read using UUID.
git request-pull info:
The following changes since commit
8ef71548686b3e9e2152aed46177e6bfca749b09:
Fix typos in adapter documentation (2010-11-16 13:39:43 +0000)
are available in the git repository at:
git://git.infradead.org/users/cktakahasi/bluez.git gatt
Bruna Moreira (1):
Implement Find by Type request encode/decoding
Claudio Takahasi (7):
Add Find By Type Value Response encoding/decoding functions
Implement Find by Type Value Request in the atttribute server
Extend bt_string2uuid to convert hex strings to UUID16
Add an extra parameter in the discovery primary to specify the
UUID
Add uuid command line option on gatttool
Implement Discover Primary Service by Service UUID in the
gatttool
Fix corner cases of Discover Primary Service by UUID in the
server
Makefile.am | 3 +-
attrib/att.c | 121
++++++++++++++++++++++++++++++++++++++++++++++++++-
attrib/att.h | 11 ++++-
attrib/client.c | 8 ++--
attrib/gatt.c | 37 +++++++++++++---
attrib/gatt.h | 4 +-
attrib/gatttool.c | 63 +++++++++++++++++++++++++-
src/attrib-server.c | 75 +++++++++++++++++++++++++++++++
src/glib-helper.c | 21 +++++++++
9 files changed, 323 insertions(+), 20 deletions(-)
Claudio.
^ permalink raw reply
* Re: [PATCHv3 3/7] Add DBus OOB API documentation.
From: Johan Hedberg @ 2010-11-16 16:37 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1289922247-20712-4-git-send-email-szymon.janc@tieto.com>
Hi Szymon,
On Tue, Nov 16, 2010, Szymon Janc wrote:
> +Service org.bluez
> +Interface org.bluez.OOB
> +Object path /org/bluez
I think this should use the same path as the Manager API, which at the
moment is /
> +Methods void RegisterProvider(object provider)
> +
> + This method registers provider for DBus OOB plug-in.
> + When provider is successfully registered plug-in becomes
> + active. Only one provider can be registered at time.
> +
> + Possible errors: org.bluez.Error.AlreadyExists
> +
> + void UnregisterProvider(object provider)
> +
> + This method unregisters provider for DBus OOB plug-in.
> +
> + Possible errors: org.bluez.Error.DoesNotExist
Ok, these methods make sense.
> + array{byte} hash, array{byte} randomizer
> + ReadLocalOobData(object adapter)
Having to pass an adapter path as a parameter seems weird. Wouldn't it
make more sense to have this method in the Adapter path/interface
instead? Also, we could toy around with the thought of putting the two
other methods into the current Manager interface.
One general thing throughout your patches, both the in-code comments and
commit messages: D-Bus is spelled D-Bus, not DBus :)
Johan
^ permalink raw reply
* Re: [PATCHv3 2/7] Add DBus OOB plugin.
From: Johan Hedberg @ 2010-11-16 16:30 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1289922247-20712-3-git-send-email-szymon.janc@tieto.com>
Hi Szymon,
On Tue, Nov 16, 2010, Szymon Janc wrote:
> +#define REQUEST_TIMEOUT (10 * 1000) /* 10 seconds */
Are you sure 10 seconds is enough. What if the provider does user
interaction? In that case I suppose 30 seconds or even a minute (which I
think we use elsewhere for user interaction) would be better.
> +static DBusMessage *register_provider(DBusConnection *conn, DBusMessage *msg,
> + void *data)
> +{
> + const char *path;
> + const char *name;
Just combine these on the same line.
> +static DBusMessage *unregister_provider(DBusConnection *conn, DBusMessage *msg,
> + void *data)
> +{
> + const char *path;
> + const char *name;
Same here.
> + if (!provider || !g_str_equal(provider->path, path)
> + || !g_str_equal(provider->name, name))
> + return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
> + "No such OOB provider registered");
Same thing about line splitting and || as I mentioned in the other
email, i.e. line break comes after || and not before it.
Johan
^ permalink raw reply
* Re: [PATCHv3 5/7] Add approval request for incoming pairing requests with OOB mechanism
From: Johan Hedberg @ 2010-11-16 16:24 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth, marcel
In-Reply-To: <1289922247-20712-6-git-send-email-szymon.janc@tieto.com>
Hi Szymon,
On Tue, Nov 16, 2010, Szymon Janc wrote:
> + if (!device_has_oob_data(device)
> + || btd_event_user_approve(&BDADDR(index), dba))
That should be:
if (!device_has_oob_data(device) ||
btd_event_user_approve(&BDADDR(index), dba) < 0)
Otherwise it looks like btd_event_user_approve might return a boolean.
> + req->msg = dbus_message_new_method_call(agent->name, agent->path,
> + "org.bluez.Agent", "RequestApproval");
I think the name of this method is one of the more important things to
get right before we push this upstream. I'm not so sure of the current
proposal. I have a feeling that it'd be a good idea to have the term
"pair" somewhere explicitly in the name. How about "RequestPairing"? I
know Marcel will have something to say about this too.
Johan
^ permalink raw reply
* Re: [PATCHv3 1/7] Add support for Out of Band (OOB) association model.
From: Johan Hedberg @ 2010-11-16 16:16 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1289922247-20712-2-git-send-email-szymon.janc@tieto.com>
Hi Szymon,
On Tue, Nov 16, 2010, Szymon Janc wrote:
> +gboolean device_get_oob_data(struct btd_device *device, uint8_t *hash,
> + uint8_t *randomizer)
Looks like the continuation line should be indented a bit more. Basicly
indent at least past the ( on the above line and as much as possible as
long as the entire line stays under 79 columns. I think I saw other
places with the same issue so please fix those too.
> +void device_set_oob_data(struct btd_device *device, uint8_t *hash,
> + uint8_t *randomizer);
> +gboolean device_get_oob_data(struct btd_device *device, uint8_t *hash,
> + uint8_t *randomizer);
> +gboolean device_has_oob_data(struct btd_device *device);
I suppose you could just use get_oob_data(dev, NULL, NULL) instead of
having a separate has_oob_data function.
> +gboolean device_request_oob_data(struct btd_device *device, void *cb);
Why is the callback type void* instead of having a well defined
signature?
> +void device_set_local_auth_cap(struct btd_device *device, uint8_t auth,
> + uint8_t cap);
> +void device_get_local_auth_cap(struct btd_device *device, uint8_t *auth,
> + uint8_t *cap);
This local_cap/auth in struct device had me wondering a little bit since
it felt like that should actually be in struct adapter, however this is
really per adapter-device pair data in which case it should be fine,
right?
> +static void btd_event_io_cap_reply(struct btd_device *device)
> +{
> + io_capability_reply_cp cp;
> + int dev;
> + struct btd_adapter *adapter = device_get_adapter(device);
> + uint16_t dev_id = adapter_get_dev_id(adapter);
> +
> + dev = hci_open_dev(dev_id);
That's a no no. Only hciops should use raw HCI sockets anymore. If you
need to do something like this you'll need to add a new adapter_ops
callback and send your HCI command through that.
> {
> struct btd_adapter *adapter;
> struct btd_device *device;
> struct agent *agent = NULL;
> uint8_t agent_cap;
> int err;
> + uint8_t cap;
> + uint8_t auth;
These can be on the same line.
> + if (!plugin || !plugin->local_data_read|| !plugin->plugin_deactivated
> + || !plugin->request_remote_data
> + || active_plugin == plugin)
> + return;
When you split lines the break should be after || && etc. In this case
you could also consider splitting this up into multiple if-statements
for better readability.
Johan
^ permalink raw reply
* [PATCHv3 7/7] simple-agent - add RequestApproval method for OOB pairing
From: Szymon Janc @ 2010-11-16 15:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1289922247-20712-1-git-send-email-szymon.janc@tieto.com>
---
test/simple-agent | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/test/simple-agent b/test/simple-agent
index f2cc3dd..43f03fe 100755
--- a/test/simple-agent
+++ b/test/simple-agent
@@ -69,6 +69,16 @@ class Agent(dbus.service.Object):
raise Rejected("Mode change by user")
@dbus.service.method("org.bluez.Agent",
+ in_signature="o", out_signature="")
+
+ def RequestApproval(self, device):
+ print "RequestApproval (%s)" % (device)
+ approve = raw_input("Approve pairing (yes/no): ")
+ if (approve == "yes"):
+ return
+ raise Rejected("Not approved")
+
+ @dbus.service.method("org.bluez.Agent",
in_signature="", out_signature="")
def Cancel(self):
print "Cancel"
--
1.7.1
^ permalink raw reply related
* [PATCHv3 6/7] Update DBus OOB API with RequestApproval method.
From: Szymon Janc @ 2010-11-16 15:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1289922247-20712-1-git-send-email-szymon.janc@tieto.com>
---
doc/oob-api.txt | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/doc/oob-api.txt b/doc/oob-api.txt
index 051b868..3a89f59 100644
--- a/doc/oob-api.txt
+++ b/doc/oob-api.txt
@@ -62,3 +62,17 @@ Methods void RegisterProvider(object provider)
Possible errors: org.bluez.Error.ReadFailed
org.bluez.Error.NoProvider
org.bluez.Error.InProgress
+
+--------------------------------------------------------------------------------
+
+Service unique name
+Interface org.bluez.Agent
+Object path freely definable
+
+Methods void RequestApproval(object device)
+
+ This method gets called when the service daemon
+ needs to confirm incoming OOB pairing request.
+
+ Possible errors: org.bluez.Error.Rejected
+ org.bluez.Error.Canceled
--
1.7.1
^ permalink raw reply related
* [PATCHv3 5/7] Add approval request for incoming pairing requests with OOB mechanism
From: Szymon Janc @ 2010-11-16 15:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1289922247-20712-1-git-send-email-szymon.janc@tieto.com>
---
plugins/hciops.c | 42 ++++++++++++++++++++++++++++++--------
src/adapter.c | 7 ++++++
src/adapter.h | 3 ++
src/agent.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
src/agent.h | 3 ++
src/device.c | 22 ++++++++++++++++++++
src/device.h | 1 +
src/event.c | 41 +++++++++++++++++++++++++++++++++++++
src/event.h | 1 +
9 files changed, 169 insertions(+), 10 deletions(-)
diff --git a/plugins/hciops.c b/plugins/hciops.c
index 75e6b0c..9546874 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -520,15 +520,8 @@ static void remote_oob_data_request(int index, void *ptr)
adapter = manager_find_adapter(&BDADDR(index));
device = adapter_find_device(adapter, da);
- if (device_has_oob_data(device)) {
- remote_oob_data_reply_cp cp;
-
- bacpy(&cp.bdaddr, dba);
- device_get_oob_data(device,cp.hash,cp.randomizer);
-
- hci_send_cmd(SK(index), OGF_LINK_CTL, OCF_REMOTE_OOB_DATA_REPLY,
- REMOTE_OOB_DATA_REPLY_CP_SIZE, &cp);
- } else
+ if (!device_has_oob_data(device)
+ || btd_event_user_approve(&BDADDR(index), dba))
hci_send_cmd(SK(index),
OGF_LINK_CTL, OCF_REMOTE_OOB_DATA_NEG_REPLY, 6,
ptr);
@@ -2217,6 +2210,36 @@ static int hciops_passkey_reply(int index, bdaddr_t *bdaddr, uint32_t passkey)
return err;
}
+static int hciops_approve_reply(int index, bdaddr_t *bdaddr, gboolean approved)
+{
+ int err;
+ struct btd_adapter *adapter;
+ struct btd_device *device;
+ char da[18];
+
+ ba2str(bdaddr, da);
+ adapter = manager_find_adapter_by_id(index);
+ device = adapter_find_device(adapter, da);
+
+ if (approved) {
+ remote_oob_data_reply_cp cp;
+
+ bacpy(&cp.bdaddr, bdaddr);
+ device_get_oob_data(device,cp.hash,cp.randomizer);
+
+ err = hci_send_cmd(SK(index), OGF_LINK_CTL,
+ OCF_REMOTE_OOB_DATA_REPLY,
+ REMOTE_OOB_DATA_REPLY_CP_SIZE, &cp);
+ } else
+ err = hci_send_cmd(SK(index), OGF_LINK_CTL,
+ OCF_REMOTE_OOB_DATA_NEG_REPLY, 6, bdaddr);
+
+ if (err < 0)
+ err = -errno;
+
+ return err;
+}
+
static int hciops_get_auth_info(int index, bdaddr_t *bdaddr, uint8_t *auth)
{
struct hci_auth_info_req req;
@@ -2365,6 +2388,7 @@ static struct btd_adapter_ops hci_ops = {
.pincode_reply = hciops_pincode_reply,
.confirm_reply = hciops_confirm_reply,
.passkey_reply = hciops_passkey_reply,
+ .approve_reply = hciops_approve_reply,
.get_auth_info = hciops_get_auth_info,
.read_scan_enable = hciops_read_scan_enable,
.read_ssp_mode = hciops_read_ssp_mode,
diff --git a/src/adapter.c b/src/adapter.c
index 39a6514..ab680f3 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3692,6 +3692,13 @@ int btd_adapter_passkey_reply(struct btd_adapter *adapter, bdaddr_t *bdaddr,
return adapter_ops->passkey_reply(adapter->dev_id, bdaddr, passkey);
}
+int btd_adapter_approve_reply(struct btd_adapter *adapter, bdaddr_t *bdaddr,
+ gboolean success)
+{
+ DBG("reply %u", success);
+ return adapter_ops->approve_reply(adapter->dev_id, bdaddr, success);
+}
+
int btd_adapter_get_auth_info(struct btd_adapter *adapter, bdaddr_t *bdaddr,
uint8_t *auth)
{
diff --git a/src/adapter.h b/src/adapter.h
index cc62865..cf66129 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -221,6 +221,7 @@ struct btd_adapter_ops {
int (*pincode_reply) (int index, bdaddr_t *bdaddr, const char *pin);
int (*confirm_reply) (int index, bdaddr_t *bdaddr, gboolean success);
int (*passkey_reply) (int index, bdaddr_t *bdaddr, uint32_t passkey);
+ int (*approve_reply) (int index, bdaddr_t *bdaddr, gboolean success);
int (*get_auth_info) (int index, bdaddr_t *bdaddr, uint8_t *auth);
int (*read_scan_enable) (int index);
int (*read_ssp_mode) (int index);
@@ -272,6 +273,8 @@ int btd_adapter_confirm_reply(struct btd_adapter *adapter, bdaddr_t *bdaddr,
gboolean success);
int btd_adapter_passkey_reply(struct btd_adapter *adapter, bdaddr_t *bdaddr,
uint32_t passkey);
+int btd_adapter_approve_reply(struct btd_adapter *adapter, bdaddr_t *bdaddr,
+ gboolean success);
int btd_adapter_get_auth_info(struct btd_adapter *adapter, bdaddr_t *bdaddr,
uint8_t *auth);
diff --git a/src/agent.c b/src/agent.c
index 2ddfd6e..a4b26b6 100644
--- a/src/agent.c
+++ b/src/agent.c
@@ -55,7 +55,8 @@ typedef enum {
AGENT_REQUEST_CONFIRMATION,
AGENT_REQUEST_PINCODE,
AGENT_REQUEST_AUTHORIZE,
- AGENT_REQUEST_CONFIRM_MODE
+ AGENT_REQUEST_CONFIRM_MODE,
+ AGENT_REQUEST_APPROVAL
} agent_request_type_t;
struct agent {
@@ -693,6 +694,62 @@ failed:
return err;
}
+static int approval_request_new(struct agent_request *req,
+ const char *device_path)
+{
+ struct agent *agent = req->agent;
+
+ req->msg = dbus_message_new_method_call(agent->name, agent->path,
+ "org.bluez.Agent", "RequestApproval");
+ if (req->msg == NULL) {
+ error("Couldn't allocate D-Bus message");
+ return -ENOMEM;
+ }
+
+ dbus_message_append_args(req->msg,
+ DBUS_TYPE_OBJECT_PATH, &device_path,
+ DBUS_TYPE_INVALID);
+
+ if (dbus_connection_send_with_reply(connection, req->msg,
+ &req->call, REQUEST_TIMEOUT) == FALSE) {
+ error("D-Bus send failed");
+ return -EIO;
+ }
+
+ dbus_pending_call_set_notify(req->call, simple_agent_reply, req, NULL);
+
+ return 0;
+}
+
+int agent_request_approval (struct agent *agent, struct btd_device *device,
+ agent_cb cb, void *user_data, GDestroyNotify destroy)
+{
+ struct agent_request *req;
+ const gchar *dev_path = device_get_path(device);
+ int err;
+
+ if (agent->request)
+ return -EBUSY;
+
+ DBG("Calling Agent.RequestApproval: name=%s, path=%s", agent->name,
+ agent->path);
+
+ req = agent_request_new(agent, AGENT_REQUEST_APPROVAL, cb,
+ user_data, destroy);
+
+ err = approval_request_new(req, dev_path);
+ if (err < 0)
+ goto failed;
+
+ agent->request = req;
+
+ return 0;
+
+failed:
+ agent_request_free(req, FALSE);
+ return err;
+}
+
static int request_fallback(struct agent_request *req,
DBusPendingCallNotifyFunction function)
{
diff --git a/src/agent.h b/src/agent.h
index e184250..73dd531 100644
--- a/src/agent.h
+++ b/src/agent.h
@@ -64,6 +64,9 @@ int agent_request_confirmation(struct agent *agent, struct btd_device *device,
int agent_display_passkey(struct agent *agent, struct btd_device *device,
uint32_t passkey);
+int agent_request_approval (struct agent *agent, struct btd_device *device,
+ agent_cb cb, void *user_data, GDestroyNotify destroy);
+
int agent_cancel(struct agent *agent);
gboolean agent_is_busy(struct agent *agent, void *user_data);
diff --git a/src/device.c b/src/device.c
index 24fd44d..a507fe8 100644
--- a/src/device.c
+++ b/src/device.c
@@ -2245,6 +2245,21 @@ static void passkey_cb(struct agent *agent, DBusError *err,
device->authr->agent = NULL;
}
+static void approve_cb(struct agent *agent, DBusError *err, void *data)
+{
+ struct authentication_req *auth = data;
+ struct btd_device *device = auth->device;
+
+ /* No need to reply anything if the authentication already failed */
+ if (auth->cb == NULL)
+ return;
+
+ ((agent_cb) auth->cb)(agent, err, device);
+
+ device->authr->cb = NULL;
+ device->authr->agent = NULL;
+}
+
int device_request_authentication(struct btd_device *device, auth_type_t type,
uint32_t passkey, void *cb)
{
@@ -2283,6 +2298,10 @@ int device_request_authentication(struct btd_device *device, auth_type_t type,
case AUTH_TYPE_NOTIFY:
err = agent_display_passkey(agent, device, passkey);
break;
+ case AUTH_TYPE_APPROVE:
+ err = agent_request_approval (agent, device, approve_cb, auth,
+ NULL);
+ break;
case AUTH_TYPE_AUTO:
err = 0;
break;
@@ -2324,6 +2343,9 @@ static void cancel_authentication(struct authentication_req *auth)
case AUTH_TYPE_PASSKEY:
((agent_passkey_cb) auth->cb)(agent, &err, 0, device);
break;
+ case AUTH_TYPE_APPROVE:
+ ((agent_cb) auth->cb)(agent, &err, device);
+ break;
case AUTH_TYPE_NOTIFY:
case AUTH_TYPE_AUTO:
/* User Notify/Auto doesn't require any reply */
diff --git a/src/device.h b/src/device.h
index b62cdc5..2da3311 100644
--- a/src/device.h
+++ b/src/device.h
@@ -33,6 +33,7 @@ typedef enum {
AUTH_TYPE_CONFIRM,
AUTH_TYPE_NOTIFY,
AUTH_TYPE_AUTO,
+ AUTH_TYPE_APPROVE
} auth_type_t;
struct btd_device *device_create(DBusConnection *conn, struct btd_adapter *adapter,
diff --git a/src/event.c b/src/event.c
index 5a5a288..957a17d 100644
--- a/src/event.c
+++ b/src/event.c
@@ -175,6 +175,18 @@ static void passkey_cb(struct agent *agent, DBusError *err, uint32_t passkey,
btd_adapter_passkey_reply(adapter, &bdaddr, passkey);
}
+static void approve_cb(struct agent *agent, DBusError *err, void *user_data)
+{
+ struct btd_device *device = user_data;
+ struct btd_adapter *adapter = device_get_adapter(device);
+ bdaddr_t bdaddr;
+ gboolean approve = (err == NULL) ? TRUE : FALSE;
+
+ device_get_address(device, &bdaddr);
+
+ btd_adapter_approve_reply(adapter, &bdaddr, approve);
+}
+
int btd_event_user_confirm(bdaddr_t *sba, bdaddr_t *dba, uint32_t passkey)
{
struct btd_adapter *adapter;
@@ -264,6 +276,35 @@ int btd_event_user_notify(bdaddr_t *sba, bdaddr_t *dba, uint32_t passkey)
passkey, NULL);
}
+int btd_event_user_approve(bdaddr_t *sba, bdaddr_t *dba)
+{
+ struct btd_adapter *adapter;
+ struct btd_device *device;
+ struct agent *agent;
+ uint8_t cap;
+
+ if (!get_adapter_and_device(sba, dba, &adapter, &device, TRUE))
+ return -ENODEV;
+
+ agent = device_get_agent(device);
+ if (!agent)
+ return -ENODEV;
+
+ cap = agent_get_io_capability(agent);
+
+ /* If initiator or agent has no input capability approve immediately. */
+ if (device_is_bonding(device, NULL) || cap == 0x00 || cap == 0x03) {
+ bdaddr_t bdaddr;
+
+ device_get_address(device, &bdaddr);
+ btd_adapter_approve_reply(adapter, &bdaddr, TRUE);
+ return 0;
+ }
+
+ return device_request_authentication(device, AUTH_TYPE_APPROVE, 0,
+ approve_cb);
+}
+
void btd_event_bonding_process_complete(bdaddr_t *local, bdaddr_t *peer,
uint8_t status)
{
diff --git a/src/event.h b/src/event.h
index a3e7dda..c1ea2ef 100644
--- a/src/event.h
+++ b/src/event.h
@@ -42,6 +42,7 @@ int btd_event_set_io_cap(bdaddr_t *local, bdaddr_t *remote,
int btd_event_user_confirm(bdaddr_t *sba, bdaddr_t *dba, uint32_t passkey);
int btd_event_user_passkey(bdaddr_t *sba, bdaddr_t *dba);
int btd_event_user_notify(bdaddr_t *sba, bdaddr_t *dba, uint32_t passkey);
+int btd_event_user_approve(bdaddr_t *sba, bdaddr_t *dba);
int btd_event_link_key_notify(bdaddr_t *local, bdaddr_t *peer,
uint8_t *key, uint8_t key_type,
int pin_length, uint8_t old_key_type);
--
1.7.1
^ permalink raw reply related
* [PATCHv3 4/7] Add simple-oobprovider for testing.
From: Szymon Janc @ 2010-11-16 15:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1289922247-20712-1-git-send-email-szymon.janc@tieto.com>
---
test/simple-oobprovider | 61 +++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 61 insertions(+), 0 deletions(-)
create mode 100755 test/simple-oobprovider
diff --git a/test/simple-oobprovider b/test/simple-oobprovider
new file mode 100755
index 0000000..4a1ebd8
--- /dev/null
+++ b/test/simple-oobprovider
@@ -0,0 +1,61 @@
+#!/usr/bin/python
+# Copyright (C) 2010 ST-Ericsson SA
+# Author: Szymon Janc <szymon.janc@tieto.com> for ST-Ericsson
+
+import gobject
+
+import sys
+import dbus
+import dbus.service
+import dbus.mainloop.glib
+
+class NoOobData(dbus.DBusException):
+ _dbus_error_name = "org.bluez.Error.NoData"
+
+
+class Provider(dbus.service.Object):
+
+ remotedata = None
+
+ @dbus.service.method("org.bluez.OOB",
+ in_signature="s", out_signature="ayay")
+
+ def RequestRemoteOobData(self, device):
+ print "RequestRemoteOobData for %s" % (device)
+ if (self.remotedata != None):
+ return self.remotedata
+ raise NoOobData("No OOB data present")
+
+if __name__ == '__main__':
+ dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
+
+ bus = dbus.SystemBus()
+
+ manager = dbus.Interface(bus.get_object("org.bluez", "/"),
+ "org.bluez.Manager")
+
+ adapter_path = manager.DefaultAdapter()
+ adapter = dbus.Interface(bus.get_object("org.bluez",
+ adapter_path), "org.bluez.Adapter")
+
+ oob = dbus.Interface(bus.get_object("org.bluez", "/org/bluez"),
+ "org.bluez.OOB")
+
+ path = "/test/oobprovider"
+ provider = Provider(bus, path)
+
+ mainloop = gobject.MainLoop()
+
+ oob.RegisterProvider(path)
+
+ print "Local data for %s:" % (adapter_path)
+ print oob.ReadLocalOobData(adapter_path)
+
+ provider.remotedata = input("Provide remote data (in python syntax):\n")
+
+ print "You may try pairing now"
+
+ mainloop.run()
+
+ #adapter.UnregisterProvider(path)
+ #print "Provider unregistered"
--
1.7.1
^ permalink raw reply related
* [PATCHv3 3/7] Add DBus OOB API documentation.
From: Szymon Janc @ 2010-11-16 15:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1289922247-20712-1-git-send-email-szymon.janc@tieto.com>
---
Makefile.am | 2 +-
doc/oob-api.txt | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+), 1 deletions(-)
create mode 100644 doc/oob-api.txt
diff --git a/Makefile.am b/Makefile.am
index 1f8f7fb..9098084 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -358,7 +358,7 @@ EXTRA_DIST += doc/manager-api.txt \
doc/service-api.txt doc/agent-api.txt doc/attribute-api.txt \
doc/serial-api.txt doc/network-api.txt \
doc/input-api.txt doc/audio-api.txt doc/control-api.txt \
- doc/hfp-api.txt doc/assigned-numbers.txt
+ doc/hfp-api.txt doc/assigned-numbers.txt doc/oob-api.txt
AM_YFLAGS = -d
diff --git a/doc/oob-api.txt b/doc/oob-api.txt
new file mode 100644
index 0000000..051b868
--- /dev/null
+++ b/doc/oob-api.txt
@@ -0,0 +1,64 @@
+BlueZ D-Bus OOB API description
+*******************************
+
+Copyright (C) 2010 ST-Ericsson SA
+
+Author: Szymon Janc <szymon.janc@tieto.com> for ST-Ericsson
+
+OOB hierarchy
+=================
+
+Service unique name
+Interface org.bluez.OOB
+Object path freely definable
+
+Methods array{byte} hash, array{byte} randomizer
+ RequestRemoteOobData(object device)
+
+ This method gets called when the service daemon needs to
+ get device's hash and randomizer for an OOB
+ authentication. Each array should be 16 bytes long.
+
+ Possible errors: org.bluez.Error.NoData
+
+ void Release()
+
+ This method gets called when DBus plug-in for OOB was
+ deactivated. There is no need to unregister provider,
+ because when this method gets called it has already been
+ unregistered.
+
+--------------------------------------------------------------------------------
+
+Service org.bluez
+Interface org.bluez.OOB
+Object path /org/bluez
+
+Methods void RegisterProvider(object provider)
+
+ This method registers provider for DBus OOB plug-in.
+ When provider is successfully registered plug-in becomes
+ active. Only one provider can be registered at time.
+
+ Possible errors: org.bluez.Error.AlreadyExists
+
+ void UnregisterProvider(object provider)
+
+ This method unregisters provider for DBus OOB plug-in.
+
+ Possible errors: org.bluez.Error.DoesNotExist
+
+
+ array{byte} hash, array{byte} randomizer
+ ReadLocalOobData(object adapter)
+
+ This method reads local OOB data for adapter. Return
+ value is pair of arrays 16 bytes each. Only registered
+ provider should call this method.
+
+ Note: This method will generate and return new local
+ OOB data.
+
+ Possible errors: org.bluez.Error.ReadFailed
+ org.bluez.Error.NoProvider
+ org.bluez.Error.InProgress
--
1.7.1
^ permalink raw reply related
* [PATCHv3 2/7] Add DBus OOB plugin.
From: Szymon Janc @ 2010-11-16 15:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1289922247-20712-1-git-send-email-szymon.janc@tieto.com>
---
Makefile.am | 5 +
acinclude.m4 | 6 +
plugins/dbusoob.c | 353 +++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 364 insertions(+), 0 deletions(-)
create mode 100644 plugins/dbusoob.c
diff --git a/Makefile.am b/Makefile.am
index a61e754..1f8f7fb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -216,6 +216,11 @@ builtin_modules += maemo6
builtin_sources += plugins/maemo6.c
endif
+if DBUSOOBPLUGIN
+builtin_modules += dbusoob
+builtin_sources += plugins/dbusoob.c
+endif
+
sbin_PROGRAMS += src/bluetoothd
src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
diff --git a/acinclude.m4 b/acinclude.m4
index 287f07d..a52d063 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -193,6 +193,7 @@ AC_DEFUN([AC_ARG_BLUEZ], [
configfiles_enable=yes
telephony_driver=dummy
maemo6_enable=no
+ dbusoob_enable=no
AC_ARG_ENABLE(optimization, AC_HELP_STRING([--disable-optimization], [disable code optimization]), [
optimization_enable=${enableval}
@@ -316,6 +317,10 @@ AC_DEFUN([AC_ARG_BLUEZ], [
maemo6_enable=${enableval}
])
+ AC_ARG_ENABLE(dbusoob, AC_HELP_STRING([--enable-dbusoob], [compile with DBUS OOB plugin]), [
+ dbusoob_enable=${enableval}
+ ])
+
AC_ARG_ENABLE(hal, AC_HELP_STRING([--enable-hal], [Use HAL to determine adapter class]), [
hal_enable=${enableval}
])
@@ -372,4 +377,5 @@ AC_DEFUN([AC_ARG_BLUEZ], [
AM_CONDITIONAL(UDEVRULES, test "${udevrules_enable}" = "yes")
AM_CONDITIONAL(CONFIGFILES, test "${configfiles_enable}" = "yes")
AM_CONDITIONAL(MAEMO6PLUGIN, test "${maemo6_enable}" = "yes")
+ AM_CONDITIONAL(DBUSOOBPLUGIN, test "${dbusoob_enable}" = "yes")
])
diff --git a/plugins/dbusoob.c b/plugins/dbusoob.c
new file mode 100644
index 0000000..f347ef6
--- /dev/null
+++ b/plugins/dbusoob.c
@@ -0,0 +1,353 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2010 ST-Ericsson SA
+ *
+ * Author: Szymon Janc <szymon.janc@tieto.com> for ST-Ericsson
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <errno.h>
+#include <gdbus.h>
+
+#include <bluetooth/bluetooth.h>
+#include <bluetooth/hci.h>
+#include <bluetooth/sdp.h>
+
+#include "plugin.h"
+#include "log.h"
+#include "manager.h"
+#include "device.h"
+#include "adapter.h"
+#include "dbus-common.h"
+#include "event.h"
+#include "error.h"
+#include "oob.h"
+
+#define REQUEST_TIMEOUT (10 * 1000) /* 10 seconds */
+#define OOB_INTERFACE "org.bluez.OOB"
+#define OOB_PATH "/org/bluez"
+
+struct oob_provider {
+ char *name;
+ char *path;
+
+ struct btd_adapter *adapter;
+ DBusMessage *msg;
+
+ guint listener_id;
+ gboolean exited;
+};
+
+static struct oob_provider *provider = NULL;
+static DBusConnection *connection = NULL;
+static struct oob_plugin dbusoob;
+
+static void destroy_provider(void)
+{
+ if (!provider->exited)
+ g_dbus_remove_watch(connection, provider->listener_id);
+
+ if (provider->msg)
+ dbus_message_unref(provider->msg);
+
+ g_free(provider->name);
+ g_free(provider->path);
+ g_free(provider);
+ provider = NULL;
+
+ oob_deactivate_plugin(&dbusoob);
+}
+
+static void provider_exited(DBusConnection *conn, void *user_data)
+{
+ DBG("Provider exited without calling Unregister");
+
+ provider->exited = TRUE;
+ destroy_provider();
+}
+
+static void create_provider(const char *path, const char *name)
+{
+ provider = g_new(struct oob_provider, 1);
+ provider->path = g_strdup(path);
+ provider->name = g_strdup(name);
+ provider->adapter = NULL;
+ provider->msg = NULL;
+ provider->exited = FALSE;
+ provider->listener_id = g_dbus_add_disconnect_watch(connection, name,
+ provider_exited, NULL, NULL);
+
+ oob_activate_plugin(&dbusoob);
+}
+
+static void request_remote_data_reply(DBusPendingCall *call, void *data)
+{
+ DBusMessage *msg;
+ DBusError err;
+ struct btd_device *device = data;
+ uint8_t *hash = NULL;
+ uint8_t *randomizer = NULL;
+ int32_t hlen, rlen;
+
+ msg = dbus_pending_call_steal_reply(call);
+
+ dbus_error_init(&err);
+ if (dbus_set_error_from_message(&err, msg)) {
+ error("Provider replied with an error: %s, %s", err.name,
+ err.message);
+ dbus_error_free(&err);
+ goto error;
+ }
+
+ dbus_error_init(&err);
+ if (!dbus_message_get_args(msg, &err,
+ DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE, &hash, &hlen,
+ DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE, &randomizer, &rlen,
+ DBUS_TYPE_INVALID) || hlen != 16 || rlen != 16) {
+ error("RequestRemoteOobData reply signature error: %s, %s",
+ err.name, err.message);
+ dbus_error_free(&err);
+ hash = NULL;
+ randomizer = NULL;
+ }
+
+error:
+ dbus_message_unref(msg);
+ dbus_pending_call_unref(call);
+
+ device_set_oob_data(device, hash, randomizer);
+}
+
+static gboolean request_remote_data(struct btd_device *device)
+{
+ DBusMessage* msg;
+ DBusPendingCall *call = NULL;
+ const gchar *path;
+ gboolean ret = FALSE;
+
+ msg = dbus_message_new_method_call(provider->name, provider->path,
+ OOB_INTERFACE, "RequestRemoteOobData");
+
+ if (!msg) {
+ error("Couldn't allocate D-Bus message");
+ goto error;
+ }
+
+ path = device_get_path(device);
+
+ if (!dbus_message_append_args(msg, DBUS_TYPE_OBJECT_PATH, &path,
+ DBUS_TYPE_INVALID)) {
+ error ("Couldn't append arguments");
+ goto error;
+ }
+
+ if (!dbus_connection_send_with_reply(connection, msg, &call,
+ REQUEST_TIMEOUT)) {
+ error("D-Bus send failed");
+ goto error;
+ }
+
+ if (!dbus_pending_call_set_notify(call, request_remote_data_reply,
+ device, NULL)) {
+ error("Couldn't set reply notification.");
+ dbus_pending_call_unref(call);
+ goto error;
+ }
+
+ ret = TRUE;
+
+error:
+ if (msg)
+ dbus_message_unref(msg);
+
+ return ret;
+}
+
+static void local_data_read(bdaddr_t *ba, uint8_t *hash, uint8_t *randomizer)
+{
+ struct DBusMessage *reply;
+ bdaddr_t addr;
+
+ if (!provider)
+ return;
+
+ adapter_get_address(provider->adapter, &addr);
+ if (bacmp(ba, &addr))
+ return;
+
+ if (hash && randomizer)
+ reply = g_dbus_create_reply(provider->msg,
+ DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE, &hash, 16,
+ DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE, &randomizer, 16,
+ DBUS_TYPE_INVALID);
+ else
+ reply = g_dbus_create_error(provider->msg,
+ ERROR_INTERFACE ".ReadFailed",
+ "Failed to read local OOB data.");
+
+ dbus_message_unref(provider->msg);
+ provider->msg = NULL;
+ provider->adapter = NULL;
+
+ if (!reply) {
+ error("Couldn't allocate D-Bus message");
+ return;
+ }
+
+ if (!g_dbus_send_message(connection, reply))
+ error("D-Bus send failed");
+}
+
+static DBusMessage *read_local_data(DBusConnection *conn, DBusMessage *msg,
+ void *data)
+{
+ const char *name;
+ const char *path;
+
+ if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
+ DBUS_TYPE_INVALID))
+ return NULL;
+
+ name = dbus_message_get_sender(msg);
+ if (!provider || strcmp(provider->name, name) != 0)
+ return g_dbus_create_error(msg, ERROR_INTERFACE ".NoProvider",
+ "Not OOB provider or no provider registered");
+
+ if (provider->msg)
+ return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
+ "Another request in progress.");
+
+ provider->adapter = manager_find_adapter_by_path(path);
+ if (!provider->adapter)
+ return g_dbus_create_error(msg, ERROR_INTERFACE ".ReadFailed",
+ "No adapter with given address found");
+
+ if (btd_adapter_read_local_oob_data(provider->adapter))
+ return g_dbus_create_error(msg, ERROR_INTERFACE ".ReadFailed",
+ "HCI request failed");
+
+ provider->msg = dbus_message_ref(msg);
+ return NULL;
+}
+
+static void plugin_deactivated(void)
+{
+ DBusMessage *msg;
+
+ msg = dbus_message_new_method_call(provider->name, provider->path,
+ OOB_INTERFACE, "Release");
+
+ if (!msg)
+ error("Couldn't allocate D-Bus message");
+ else if (!g_dbus_send_message(connection, msg))
+ error("D-Bus send failed");
+
+ destroy_provider();
+}
+
+static DBusMessage *register_provider(DBusConnection *conn, DBusMessage *msg,
+ void *data)
+{
+ const char *path;
+ const char *name;
+
+ if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
+ DBUS_TYPE_INVALID))
+ return NULL;
+
+ if (provider)
+ return g_dbus_create_error(msg, ERROR_INTERFACE ".AlreadyExists",
+ "OOB provider already registered");
+
+ name = dbus_message_get_sender(msg);
+ create_provider(path, name);
+
+ DBG("OOB provider registered at %s:%s", provider->name, provider->path);
+ return dbus_message_new_method_return(msg);
+}
+
+static DBusMessage *unregister_provider(DBusConnection *conn, DBusMessage *msg,
+ void *data)
+{
+ const char *path;
+ const char *name;
+
+ if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
+ DBUS_TYPE_INVALID))
+ return NULL;
+
+ name = dbus_message_get_sender(msg);
+
+ if (!provider || !g_str_equal(provider->path, path)
+ || !g_str_equal(provider->name, name))
+ return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
+ "No such OOB provider registered");
+
+ DBG("OOB provider (%s:%s) unregistered", provider->name, provider->path);
+
+ destroy_provider();
+
+ return dbus_message_new_method_return(msg);
+}
+
+static GDBusMethodTable oob_methods[] = {
+ { "RegisterProvider", "o", "", register_provider},
+ { "UnregisterProvider", "o", "", unregister_provider},
+ { "ReadLocalOobData", "o", "ayay", read_local_data,
+ G_DBUS_METHOD_FLAG_ASYNC},
+ { }
+};
+
+static gboolean register_on_dbus(void)
+{
+ connection = get_dbus_connection();
+
+ if (!g_dbus_register_interface(connection, OOB_PATH, OOB_INTERFACE,
+ oob_methods, NULL, NULL, NULL, NULL)) {
+ error("OOB interface init failed on path %s", OOB_PATH);
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
+static int dbusoob_init(void)
+{
+ DBG("Setup dbusoob plugin");
+
+ dbusoob.request_remote_data = request_remote_data;
+ dbusoob.local_data_read = local_data_read;
+ dbusoob.plugin_deactivated = plugin_deactivated;
+
+ return register_on_dbus();
+}
+
+static void dbusoob_exit(void)
+{
+ DBG("Cleanup dbusoob plugin");
+ oob_deactivate_plugin(&dbusoob);
+}
+
+BLUETOOTH_PLUGIN_DEFINE(dbusoob, VERSION,
+ BLUETOOTH_PLUGIN_PRIORITY_DEFAULT, dbusoob_init, dbusoob_exit)
--
1.7.1
^ permalink raw reply related
* [PATCHv3 1/7] Add support for Out of Band (OOB) association model.
From: Szymon Janc @ 2010-11-16 15:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
In-Reply-To: <1289922247-20712-1-git-send-email-szymon.janc@tieto.com>
---
Makefile.am | 3 +-
lib/hci.h | 3 ++
plugins/hciops.c | 72 +++++++++++++++++++++++++++++++++++++++++---------
src/adapter.c | 7 ++++-
src/adapter.h | 3 ++
src/device.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/device.h | 12 ++++++++
src/event.c | 75 +++++++++++++++++++++++++++++++++++++++-------------
src/event.h | 3 +-
src/oob.c | 61 +++++++++++++++++++++++++++++++++++++++++++
src/oob.h | 47 +++++++++++++++++++++++++++++++++
11 files changed, 326 insertions(+), 36 deletions(-)
create mode 100644 src/oob.c
create mode 100644 src/oob.h
diff --git a/Makefile.am b/Makefile.am
index da308a7..a61e754 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -238,7 +238,8 @@ src_bluetoothd_SOURCES = $(gdbus_sources) $(builtin_sources) \
src/adapter.h src/adapter.c \
src/device.h src/device.c \
src/dbus-common.c src/dbus-common.h \
- src/event.h src/event.c
+ src/event.h src/event.c \
+ src/oob.c
src_bluetoothd_LDADD = lib/libbluetooth.la @GLIB_LIBS@ @DBUS_LIBS@ \
@CAPNG_LIBS@ -ldl -lrt
src_bluetoothd_LDFLAGS = -Wl,--export-dynamic \
diff --git a/lib/hci.h b/lib/hci.h
index 0cb120f..409abd9 100644
--- a/lib/hci.h
+++ b/lib/hci.h
@@ -524,6 +524,9 @@ typedef struct {
#define OCF_REMOTE_OOB_DATA_NEG_REPLY 0x0033
+#define OOB_DATA_NOT_PRESENT 0x00
+#define OOB_DATA_PRESENT 0x01
+
#define OCF_IO_CAPABILITY_NEG_REPLY 0x0034
typedef struct {
bdaddr_t bdaddr;
diff --git a/plugins/hciops.c b/plugins/hciops.c
index 829011a..75e6b0c 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -3,6 +3,7 @@
* BlueZ - Bluetooth protocol stack for Linux
*
* Copyright (C) 2004-2010 Marcel Holtmann <marcel@holtmann.org>
+ * Copyright (C) 2010 ST-Ericsson SA
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -47,6 +48,7 @@
#include "event.h"
#include "device.h"
#include "manager.h"
+#include "oob.h"
#define HCI_REQ_TIMEOUT 5000
@@ -509,20 +511,41 @@ static void user_passkey_notify(int index, void *ptr)
static void remote_oob_data_request(int index, void *ptr)
{
- hci_send_cmd(SK(index), OGF_LINK_CTL,
- OCF_REMOTE_OOB_DATA_NEG_REPLY, 6, ptr);
+ bdaddr_t *dba = ptr;
+ struct btd_adapter *adapter;
+ struct btd_device *device;
+ char da[18];
+
+ ba2str(dba, da);
+ adapter = manager_find_adapter(&BDADDR(index));
+ device = adapter_find_device(adapter, da);
+
+ if (device_has_oob_data(device)) {
+ remote_oob_data_reply_cp cp;
+
+ bacpy(&cp.bdaddr, dba);
+ device_get_oob_data(device,cp.hash,cp.randomizer);
+
+ hci_send_cmd(SK(index), OGF_LINK_CTL, OCF_REMOTE_OOB_DATA_REPLY,
+ REMOTE_OOB_DATA_REPLY_CP_SIZE, &cp);
+ } else
+ hci_send_cmd(SK(index),
+ OGF_LINK_CTL, OCF_REMOTE_OOB_DATA_NEG_REPLY, 6,
+ ptr);
}
static void io_capa_request(int index, void *ptr)
{
bdaddr_t *dba = ptr;
char sa[18], da[18];
- uint8_t cap, auth;
ba2str(&BDADDR(index), sa); ba2str(dba, da);
info("io_capa_request (sba=%s, dba=%s)", sa, da);
- if (btd_event_get_io_cap(&BDADDR(index), dba, &cap, &auth) < 0) {
+ /* If failed to establish IO capabilities then send negative reply
+ * immediately. Positive reply will be sent when IO capabilities are
+ * established. */
+ if (btd_event_request_io_cap(&BDADDR(index), dba)) {
io_capability_neg_reply_cp cp;
memset(&cp, 0, sizeof(cp));
bacpy(&cp.bdaddr, dba);
@@ -530,15 +553,6 @@ static void io_capa_request(int index, void *ptr)
hci_send_cmd(SK(index), OGF_LINK_CTL,
OCF_IO_CAPABILITY_NEG_REPLY,
IO_CAPABILITY_NEG_REPLY_CP_SIZE, &cp);
- } else {
- io_capability_reply_cp cp;
- memset(&cp, 0, sizeof(cp));
- bacpy(&cp.bdaddr, dba);
- cp.capability = cap;
- cp.oob_data = 0x00;
- cp.authentication = auth;
- hci_send_cmd(SK(index), OGF_LINK_CTL, OCF_IO_CAPABILITY_REPLY,
- IO_CAPABILITY_REPLY_CP_SIZE, &cp);
}
}
@@ -748,6 +762,15 @@ static void read_scan_complete(int index, uint8_t status, void *ptr)
adapter_mode_changed(adapter, rp->enable);
}
+static void read_local_oob_data_complete(bdaddr_t *local, uint8_t status,
+ read_local_oob_data_rp *rp)
+{
+ if (status)
+ oob_local_data_read(local, NULL, NULL);
+ else
+ oob_local_data_read(local, rp->hash, rp->randomizer);
+}
+
static inline void cmd_complete(int index, void *ptr)
{
evt_cmd_complete *evt = ptr;
@@ -808,6 +831,10 @@ static inline void cmd_complete(int index, void *ptr)
ptr += sizeof(evt_cmd_complete);
adapter_update_tx_power(&BDADDR(index), status, ptr);
break;
+ case cmd_opcode_pack(OGF_HOST_CTL, OCF_READ_LOCAL_OOB_DATA):
+ ptr += sizeof(evt_cmd_complete);
+ read_local_oob_data_complete(&BDADDR(index), status, ptr);
+ break;
};
}
@@ -2280,6 +2307,24 @@ static int hciops_get_remote_version(int index, uint16_t handle,
return 0;
}
+static int hciops_read_local_oob_data(int index)
+{
+ int dd;
+ int err = 0;
+
+ dd = hci_open_dev(index);
+ if (dd < 0)
+ return -EIO;
+
+ err = hci_send_cmd(dd, OGF_HOST_CTL, OCF_READ_LOCAL_OOB_DATA, 0, 0);
+ if (err < 0)
+ err = -errno;
+
+ hci_close_dev(dd);
+
+ return err;
+}
+
static struct btd_adapter_ops hci_ops = {
.setup = hciops_setup,
.cleanup = hciops_cleanup,
@@ -2326,6 +2371,7 @@ static struct btd_adapter_ops hci_ops = {
.write_le_host = hciops_write_le_host,
.get_remote_version = hciops_get_remote_version,
.encrypt_link = hciops_encrypt_link,
+ .read_local_oob_data = hciops_read_local_oob_data,
};
static int hciops_init(void)
diff --git a/src/adapter.c b/src/adapter.c
index 31014e5..39a6514 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -3265,7 +3265,7 @@ void adapter_remove_connection(struct btd_adapter *adapter,
/* clean pending HCI cmds */
device_get_address(device, &bdaddr);
- if (device_is_authenticating(device))
+ if (device_is_authenticating(device) || device_has_oob_data(device))
device_cancel_authentication(device, TRUE);
if (device_is_temporary(device)) {
@@ -3738,3 +3738,8 @@ int btd_adapter_encrypt_link(struct btd_adapter *adapter, bdaddr_t *bdaddr,
{
return adapter_ops->encrypt_link(adapter->dev_id, bdaddr, cb, user_data);
}
+
+int btd_adapter_read_local_oob_data(struct btd_adapter *adapter)
+{
+ return adapter_ops->read_local_oob_data(adapter->dev_id);
+}
diff --git a/src/adapter.h b/src/adapter.h
index 8019cfc..cc62865 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -229,6 +229,7 @@ struct btd_adapter_ops {
gboolean delayed);
int (*encrypt_link) (int index, bdaddr_t *bdaddr, bt_hci_result_t cb,
gpointer user_data);
+ int (*read_local_oob_data) (int index);
};
int btd_register_adapter_ops(struct btd_adapter_ops *ops, gboolean priority);
@@ -289,3 +290,5 @@ int btd_adapter_get_remote_version(struct btd_adapter *adapter,
int btd_adapter_encrypt_link(struct btd_adapter *adapter, bdaddr_t *bdaddr,
bt_hci_result_t cb, gpointer user_data);
+
+int btd_adapter_read_local_oob_data(struct btd_adapter *adapter);
diff --git a/src/device.c b/src/device.c
index 7c421e3..24fd44d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -4,6 +4,7 @@
*
* Copyright (C) 2006-2010 Nokia Corporation
* Copyright (C) 2004-2010 Marcel Holtmann <marcel@holtmann.org>
+ * Copyright (C) 2010 ST-Ericsson SA
*
*
* This program is free software; you can redistribute it and/or modify
@@ -59,6 +60,7 @@
#include "sdp-xml.h"
#include "storage.h"
#include "btio.h"
+#include "oob.h"
#define DEFAULT_XML_BUF_SIZE 1024
#define DISCONNECT_TIMER 2
@@ -132,6 +134,9 @@ struct btd_device {
uint8_t cap;
uint8_t auth;
+ uint8_t local_cap;
+ uint8_t local_auth;
+
uint16_t handle; /* Connection handle */
/* Whether were creating a security mode 3 connection */
@@ -149,6 +154,12 @@ struct btd_device {
gboolean has_debug_key;
uint8_t debug_key[16];
+
+ /* For OOB association model */
+ void (*oob_request_cb)(struct btd_device *device);
+ gboolean has_oob_data;
+ uint8_t hash[16];
+ uint8_t randomizer[16];
};
static uint16_t uuid_list[] = {
@@ -829,6 +840,69 @@ static DBusMessage *disconnect(DBusConnection *conn, DBusMessage *msg,
return NULL;
}
+void device_set_oob_data(struct btd_device *device, uint8_t *hash,
+ uint8_t *randomizer)
+{
+ if (!device)
+ return;
+
+ if (hash && randomizer) {
+ memcpy(device->hash, hash, 16);
+ memcpy(device->randomizer, randomizer, 16);
+ device->has_oob_data = TRUE;
+ }
+
+ if (device->oob_request_cb) {
+ device->oob_request_cb(device);
+ device->oob_request_cb = NULL;
+ }
+}
+
+gboolean device_get_oob_data(struct btd_device *device, uint8_t *hash,
+ uint8_t *randomizer)
+{
+ if (!device || !device->has_oob_data)
+ return FALSE;
+
+ memcpy(hash, device->hash, 16);
+ memcpy(randomizer, device->randomizer, 16);
+
+ return TRUE;
+}
+
+gboolean device_has_oob_data(struct btd_device *device)
+{
+ return device && device->has_oob_data;
+}
+
+gboolean device_request_oob_data(struct btd_device *device, void *cb)
+{
+ if (!device)
+ return FALSE;
+
+ device->oob_request_cb = cb;
+ return oob_request_remote_data(device);
+}
+
+void device_set_local_auth_cap(struct btd_device *device, uint8_t auth,
+ uint8_t cap)
+{
+ if (!device)
+ return;
+
+ device->local_auth = auth;
+ device->local_cap = cap;
+}
+void device_get_local_auth_cap(struct btd_device *device, uint8_t *auth,
+ uint8_t *cap)
+{
+ if (!device)
+ return;
+
+ *auth = device->local_auth;
+ *cap = device->local_cap;
+}
+
static GDBusMethodTable device_methods[] = {
{ "GetProperties", "", "a{sv}", get_properties },
{ "SetProperty", "sv", "", set_property },
@@ -2264,6 +2338,8 @@ void device_cancel_authentication(struct btd_device *device, gboolean aborted)
{
struct authentication_req *auth = device->authr;
+ device->has_oob_data = FALSE;
+
if (!auth)
return;
diff --git a/src/device.h b/src/device.h
index b570bd1..b62cdc5 100644
--- a/src/device.h
+++ b/src/device.h
@@ -4,6 +4,7 @@
*
* Copyright (C) 2006-2010 Nokia Corporation
* Copyright (C) 2004-2010 Marcel Holtmann <marcel@holtmann.org>
+ * Copyright (C) 2010 ST-Ericsson SA
*
*
* This program is free software; you can redistribute it and/or modify
@@ -89,6 +90,17 @@ void device_remove_connection(struct btd_device *device, DBusConnection *conn,
gboolean device_has_connection(struct btd_device *device, uint16_t handle);
void device_request_disconnect(struct btd_device *device, DBusMessage *msg);
+void device_set_oob_data(struct btd_device *device, uint8_t *hash,
+ uint8_t *randomizer);
+gboolean device_get_oob_data(struct btd_device *device, uint8_t *hash,
+ uint8_t *randomizer);
+gboolean device_has_oob_data(struct btd_device *device);
+gboolean device_request_oob_data(struct btd_device *device, void *cb);
+void device_set_local_auth_cap(struct btd_device *device, uint8_t auth,
+ uint8_t cap);
+void device_get_local_auth_cap(struct btd_device *device, uint8_t *auth,
+ uint8_t *cap);
+
typedef void (*disconnect_watch) (struct btd_device *device, gboolean removal,
void *user_data);
diff --git a/src/event.c b/src/event.c
index e943c63..5a5a288 100644
--- a/src/event.c
+++ b/src/event.c
@@ -4,6 +4,7 @@
*
* Copyright (C) 2006-2010 Nokia Corporation
* Copyright (C) 2004-2010 Marcel Holtmann <marcel@holtmann.org>
+ * Copyright (C) 2010 ST-Ericsson SA
*
*
* This program is free software; you can redistribute it and/or modify
@@ -274,7 +275,7 @@ void btd_event_bonding_process_complete(bdaddr_t *local, bdaddr_t *peer,
if (!get_adapter_and_device(local, peer, &adapter, &device, TRUE))
return;
- if (!device_is_authenticating(device)) {
+ if (!device_is_authenticating(device) && !device_has_oob_data(device)) {
/* This means that there was no pending PIN or SSP token
* request from the controller, i.e. this is not a new
* pairing */
@@ -756,26 +757,56 @@ void btd_event_returned_link_key(bdaddr_t *local, bdaddr_t *peer)
device_set_paired(device, TRUE);
}
-int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
- uint8_t *cap, uint8_t *auth)
+static void btd_event_io_cap_reply(struct btd_device *device)
+{
+ io_capability_reply_cp cp;
+ int dev;
+ struct btd_adapter *adapter = device_get_adapter(device);
+ uint16_t dev_id = adapter_get_dev_id(adapter);
+
+ dev = hci_open_dev(dev_id);
+ if (dev < 0) {
+ error("hci_open_dev(%d): %s (%d)", dev_id,
+ strerror(errno), errno);
+ return;
+ }
+
+ memset(&cp, 0, sizeof(cp));
+ device_get_address(device, &cp.bdaddr);
+ device_get_local_auth_cap(device, &cp.authentication, &cp.capability);
+ cp.oob_data = device_has_oob_data(device)
+ ? OOB_DATA_PRESENT : OOB_DATA_NOT_PRESENT;
+
+ DBG("final capabilities reply is cap=0x%02x, auth=0x%02x, oob=0x%02x",
+ cp.capability, cp.authentication, cp.oob_data);
+
+ hci_send_cmd(dev, OGF_LINK_CTL, OCF_IO_CAPABILITY_REPLY,
+ IO_CAPABILITY_REPLY_CP_SIZE, &cp);
+
+ hci_close_dev(dev);
+}
+
+int btd_event_request_io_cap(bdaddr_t *local, bdaddr_t *remote)
{
struct btd_adapter *adapter;
struct btd_device *device;
struct agent *agent = NULL;
uint8_t agent_cap;
int err;
+ uint8_t cap;
+ uint8_t auth;
if (!get_adapter_and_device(local, remote, &adapter, &device, TRUE))
return -ENODEV;
- err = btd_adapter_get_auth_info(adapter, remote, auth);
+ err = btd_adapter_get_auth_info(adapter, remote, &auth);
if (err < 0)
return err;
- DBG("initial authentication requirement is 0x%02x", *auth);
+ DBG("initial authentication requirement is 0x%02x", auth);
- if (*auth == 0xff)
- *auth = device_get_auth(device);
+ if (auth == 0xff)
+ auth = device_get_auth(device);
/* Check if the adapter is not pairable and if there isn't a bonding
* in progress */
@@ -784,11 +815,11 @@ int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
if (device_get_auth(device) < 0x02) {
DBG("Allowing no bonding in non-bondable mode");
/* No input, no output */
- *cap = 0x03;
+ cap = 0x03;
/* Kernel defaults to general bonding and so
* overwrite for this special case. Otherwise
* non-pairable test cases will fail. */
- *auth = 0x00;
+ auth = 0x00;
goto done;
}
return -EPERM;
@@ -804,13 +835,13 @@ int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
}
/* No agent available, and no bonding case */
- if (*auth == 0x00 || *auth == 0x04) {
+ if (auth == 0x00 || auth == 0x04) {
DBG("Allowing no bonding without agent");
/* No input, no output */
- *cap = 0x03;
+ cap = 0x03;
/* If kernel defaults to general bonding, set it
* back to no bonding */
- *auth = 0x00;
+ auth = 0x00;
goto done;
}
@@ -820,7 +851,7 @@ int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
agent_cap = agent_get_io_capability(agent);
- if (*auth == 0x00 || *auth == 0x04) {
+ if (auth == 0x00 || auth == 0x04) {
/* If remote requests dedicated bonding follow that lead */
if (device_get_auth(device) == 0x02 ||
device_get_auth(device) == 0x03) {
@@ -829,9 +860,9 @@ int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
* then require it, otherwise don't */
if (device_get_cap(device) == 0x03 ||
agent_cap == 0x03)
- *auth = 0x02;
+ auth = 0x02;
else
- *auth = 0x03;
+ auth = 0x03;
}
/* If remote indicates no bonding then follow that. This
@@ -839,7 +870,7 @@ int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
* as default. */
if (device_get_auth(device) == 0x00 ||
device_get_auth(device) == 0x01)
- *auth = 0x00;
+ auth = 0x00;
/* If remote requires MITM then also require it, unless
* our IO capability is NoInputNoOutput (so some
@@ -847,13 +878,19 @@ int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
if (device_get_auth(device) != 0xff &&
(device_get_auth(device) & 0x01) &&
agent_cap != 0x03)
- *auth |= 0x01;
+ auth |= 0x01;
}
- *cap = agent_get_io_capability(agent);
+ cap = agent_get_io_capability(agent);
done:
- DBG("final authentication requirement is 0x%02x", *auth);
+ DBG("final authentication requirement is 0x%02x", auth);
+
+ device_set_local_auth_cap(device, auth, cap);
+
+ /* If failed to request remote OOB data then reply immediately. */
+ if (!device_request_oob_data(device, btd_event_io_cap_reply))
+ btd_event_io_cap_reply(device);
return 0;
}
diff --git a/src/event.h b/src/event.h
index 4a7b9c9..a3e7dda 100644
--- a/src/event.h
+++ b/src/event.h
@@ -36,8 +36,7 @@ void btd_event_le_set_scan_enable_complete(bdaddr_t *local, uint8_t status);
void btd_event_write_simple_pairing_mode_complete(bdaddr_t *local);
void btd_event_read_simple_pairing_mode_complete(bdaddr_t *local, void *ptr);
void btd_event_returned_link_key(bdaddr_t *local, bdaddr_t *peer);
-int btd_event_get_io_cap(bdaddr_t *local, bdaddr_t *remote,
- uint8_t *cap, uint8_t *auth);
+int btd_event_request_io_cap(bdaddr_t *local, bdaddr_t *remote);
int btd_event_set_io_cap(bdaddr_t *local, bdaddr_t *remote,
uint8_t cap, uint8_t auth);
int btd_event_user_confirm(bdaddr_t *sba, bdaddr_t *dba, uint32_t passkey);
diff --git a/src/oob.c b/src/oob.c
new file mode 100644
index 0000000..4b8b01d
--- /dev/null
+++ b/src/oob.c
@@ -0,0 +1,61 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2010 ST-Ericsson SA
+ *
+ * Author: Szymon Janc <szymon.janc@tieto.com> for ST-Ericsson
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#include <glib.h>
+#include "manager.h"
+#include "adapter.h"
+#include "oob.h"
+
+static struct oob_plugin *active_plugin = NULL;
+
+void oob_activate_plugin(struct oob_plugin *plugin)
+{
+ if (!plugin || !plugin->local_data_read|| !plugin->plugin_deactivated
+ || !plugin->request_remote_data
+ || active_plugin == plugin)
+ return;
+
+ if (active_plugin)
+ active_plugin->plugin_deactivated();
+
+ active_plugin = plugin;
+}
+
+void oob_deactivate_plugin(struct oob_plugin *plugin)
+{
+ if (active_plugin == plugin)
+ active_plugin = NULL;
+}
+
+gboolean oob_request_remote_data(struct btd_device *device)
+{
+ return active_plugin && active_plugin->request_remote_data(device);
+}
+
+void oob_local_data_read(bdaddr_t *ba, uint8_t *hash, uint8_t *randomizer)
+{
+ if (active_plugin)
+ active_plugin->local_data_read(ba, hash, randomizer);
+}
diff --git a/src/oob.h b/src/oob.h
new file mode 100644
index 0000000..d0d22e2
--- /dev/null
+++ b/src/oob.h
@@ -0,0 +1,47 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2010 ST-Ericsson SA
+ *
+ * Author: Szymon Janc <szymon.janc@tieto.com> for ST-Ericsson
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+struct oob_plugin
+{
+ /* If request was successfully send this functions should return TRUE.
+ * Function should not block for too long. */
+ gboolean (*request_remote_data)(struct btd_device *device);
+
+ /* New local OOB data was read. If corresponding HCI command failed,
+ * hash and randomizer are NULL */
+ void (*local_data_read)(bdaddr_t *ba, uint8_t *hash,
+ uint8_t *randomizer);
+
+ /* Plug-in was deactivated (called only for active plug-in). */
+ void (*plugin_deactivated)(void);
+};
+
+/* These functions are called by OOB plug-in. */
+void oob_activate_plugin(struct oob_plugin *plugin);
+void oob_deactivate_plugin(struct oob_plugin *plugin);
+
+/* These functions are called from stack to interact with OOB plug-in. */
+gboolean oob_request_remote_data(struct btd_device *device);
+void oob_local_data_read(bdaddr_t *ba, uint8_t *hash, uint8_t *randomizer);
--
1.7.1
^ permalink raw reply related
* [PATCHv3 0/7] Support for out of band association model
From: Szymon Janc @ 2010-11-16 15:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Szymon Janc
Changes since v2:
- DBus provider Deactive() changed to Release() to be consistent with
Agent API
- DBus UpdateLocalOobData() changed to ReadLocalOobData() and explanation
note in documentation added
- renamed all update local oob data functions to read equivalents to keep
names consistent (with hci command) in call chain
Szymon Janc (7):
Add support for Out of Band (OOB) association model.
Add DBus OOB plugin.
Add DBus OOB API documentation.
Add simple-oobprovider for testing.
Add approval request for incoming pairing requests with OOB mechanism
Update DBus OOB API with RequestApproval method.
simple-agent - add RequestApproval method for OOB pairing
Makefile.am | 10 +-
acinclude.m4 | 6 +
doc/oob-api.txt | 78 +++++++++++
lib/hci.h | 3 +
plugins/dbusoob.c | 353 +++++++++++++++++++++++++++++++++++++++++++++++
plugins/hciops.c | 96 +++++++++++--
src/adapter.c | 14 ++-
src/adapter.h | 6 +
src/agent.c | 59 ++++++++-
src/agent.h | 3 +
src/device.c | 98 +++++++++++++
src/device.h | 13 ++
src/event.c | 116 +++++++++++++---
src/event.h | 4 +-
src/oob.c | 61 ++++++++
src/oob.h | 47 +++++++
test/simple-agent | 10 ++
test/simple-oobprovider | 61 ++++++++
18 files changed, 1000 insertions(+), 38 deletions(-)
create mode 100644 doc/oob-api.txt
create mode 100644 plugins/dbusoob.c
create mode 100644 src/oob.c
create mode 100644 src/oob.h
create mode 100755 test/simple-oobprovider
^ permalink raw reply
* Re: [PATCH] Adding a new option to specify security level for gatttool
From: Johan Hedberg @ 2010-11-16 15:36 UTC (permalink / raw)
To: Sheldon Demario; +Cc: linux-bluetooth
In-Reply-To: <1289913613-3717-1-git-send-email-sheldon.demario@openbossa.org>
Hi Sheldon,
On Tue, Nov 16, 2010, Sheldon Demario wrote:
> ---
> TODO | 6 ------
> attrib/gatttool.c | 15 +++++++++++++--
> 2 files changed, 13 insertions(+), 8 deletions(-)
In general the patch seems fine, but:
> + if (strncmp(opt_sec_level, "medium", 6) == 0)
> + sec_level = BT_IO_SEC_MEDIUM;
> + else if (strncmp(opt_sec_level, "high", 4) == 0)
> + sec_level = BT_IO_SEC_HIGH;
> + else
> + sec_level = BT_IO_SEC_LOW;
Why do you use strncmp instead of strcmp (or even g_str_equal)? I don't
think there's any need to map e.g. "mediumfoobar" to BT_IO_SEC_MEDIUM ;)
Johan
^ permalink raw reply
* Re: [PATCH v2 2/2] Split pull_contacts to smaller functions
From: Johan Hedberg @ 2010-11-16 15:22 UTC (permalink / raw)
To: Bartosz Szatkowski; +Cc: linux-bluetooth
In-Reply-To: <1289915995-2749-1-git-send-email-bulislaw@linux.com>
Hi Bartosz,
On Tue, Nov 16, 2010, Bartosz Szatkowski wrote:
> +static void contact_init(struct phonebook_contact **contact, char **reply)
> +{
> + if (*contact == NULL)
> + *contact = g_new0(struct phonebook_contact, 1);
Could you do this initialization before calling this function and pass
just a simple pointer to it. I think it'd be a cleaner approach.
> +static void contact_add_numbers(struct phonebook_contact **contact, char **reply)
Over 79 column line. Please split it.
> + if ((g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_CELL_NUMBER]) != 0) &&
> + (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_WORK_NUMBER]) != 0) &&
> + (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_HOME_NUMBER]) != 0))
> + add_phone_number(*contact, reply[COL_OTHER_NUMBER], TEL_TYPE_OTHER);
> +}
Same here. However, I think you can make it considerably more readable
with something like:
if (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_CELL_NUMBER]) == 0)
return;
if (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_WORK_NUMBER]) == 0)
return;
if (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_HOME_NUMBER]) == 0)
return;
add_phone_number(*contact, reply[COL_OTHER_NUMBER], TEL_TYPE_OTHER);
> +static void contact_add_emails(struct phonebook_contact **contact, char **reply)
Over 79 column line.
> +{
> + add_email(*contact, reply[COL_HOME_EMAIL], EMAIL_TYPE_HOME);
> + add_email(*contact, reply[COL_WORK_EMAIL], EMAIL_TYPE_WORK);
> + add_email(*contact, reply[COL_OTHER_EMAIL], EMAIL_TYPE_OTHER);
> +}
Why does the function take a pointer to pointer when a simple pointer
would be enough?
> +static void contact_add_addresses(struct phonebook_contact **contact, char **reply)
Over 79 columns and unnecessary ** pointer again.
> + home_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
> + reply[COL_HOME_ADDR_POBOX], reply[COL_HOME_ADDR_EXT],
> + reply[COL_HOME_ADDR_STREET], reply[COL_HOME_ADDR_LOCALITY],
> + reply[COL_HOME_ADDR_REGION], reply[COL_HOME_ADDR_CODE],
> + reply[COL_HOME_ADDR_COUNTRY]);
> +
> + work_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
> + reply[COL_WORK_ADDR_POBOX], reply[COL_WORK_ADDR_EXT],
> + reply[COL_WORK_ADDR_STREET], reply[COL_WORK_ADDR_LOCALITY],
> + reply[COL_WORK_ADDR_REGION], reply[COL_WORK_ADDR_CODE],
> + reply[COL_WORK_ADDR_COUNTRY]);
> +
> + other_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
> + reply[COL_OTHER_ADDR_POBOX], reply[COL_OTHER_ADDR_EXT],
> + reply[COL_OTHER_ADDR_STREET], reply[COL_OTHER_ADDR_LOCALITY],
> + reply[COL_OTHER_ADDR_REGION], reply[COL_OTHER_ADDR_CODE],
> + reply[COL_OTHER_ADDR_COUNTRY]);
All of these are over 79 columns.
> +static void contact_add_organization(struct phonebook_contact **contact, char **reply)
Same here. And a simple * pointer is enough.
Johan
^ permalink raw reply
* [PATCH v2 2/2] Split pull_contacts to smaller functions
From: Bartosz Szatkowski @ 2010-11-16 13:59 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Bartosz Szatkowski
Parts of pull_contact responsible for filling contact fields moved
to new functions.
---
plugins/phonebook-tracker.c | 159 ++++++++++++++++++++++++------------------
1 files changed, 91 insertions(+), 68 deletions(-)
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 71ed2f0..e7de001 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -1395,6 +1395,92 @@ static void add_affiliation(char **field, const char *value)
*field = g_strdup(value);
}
+static void contact_init(struct phonebook_contact **contact, char **reply)
+{
+ if (*contact == NULL)
+ *contact = g_new0(struct phonebook_contact, 1);
+
+ (*contact)->fullname = g_strdup(reply[COL_FULL_NAME]);
+ (*contact)->family = g_strdup(reply[COL_FAMILY_NAME]);
+ (*contact)->given = g_strdup(reply[COL_GIVEN_NAME]);
+ (*contact)->additional = g_strdup(reply[COL_ADDITIONAL_NAME]);
+ (*contact)->prefix = g_strdup(reply[COL_NAME_PREFIX]);
+ (*contact)->suffix = g_strdup(reply[COL_NAME_SUFFIX]);
+ (*contact)->birthday = g_strdup(reply[COL_BIRTH_DATE]);
+ (*contact)->nickname = g_strdup(reply[COL_NICKNAME]);
+ (*contact)->website = g_strdup(reply[COL_URL]);
+ (*contact)->photo = g_strdup(reply[COL_PHOTO]);
+ (*contact)->company = g_strdup(reply[COL_ORG_NAME]);
+ (*contact)->department = g_strdup(reply[COL_ORG_DEPARTMENT]);
+ (*contact)->role = g_strdup(reply[COL_ORG_ROLE]);
+ (*contact)->uid = g_strdup(reply[COL_UID]);
+ (*contact)->title = g_strdup(reply[COL_TITLE]);
+
+ set_call_type(*contact, reply[COL_DATE], reply[COL_SENT],
+ reply[COL_ANSWERED]);
+}
+
+static void contact_add_numbers(struct phonebook_contact **contact, char **reply)
+{
+ add_phone_number(*contact, reply[COL_HOME_NUMBER], TEL_TYPE_HOME);
+ add_phone_number(*contact, reply[COL_WORK_NUMBER], TEL_TYPE_WORK);
+ add_phone_number(*contact, reply[COL_FAX_NUMBER], TEL_TYPE_FAX);
+ add_phone_number(*contact, reply[COL_CELL_NUMBER], TEL_TYPE_MOBILE);
+ if ((g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_CELL_NUMBER]) != 0) &&
+ (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_WORK_NUMBER]) != 0) &&
+ (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_HOME_NUMBER]) != 0))
+ add_phone_number(*contact, reply[COL_OTHER_NUMBER], TEL_TYPE_OTHER);
+}
+
+static void contact_add_emails(struct phonebook_contact **contact, char **reply)
+{
+ add_email(*contact, reply[COL_HOME_EMAIL], EMAIL_TYPE_HOME);
+ add_email(*contact, reply[COL_WORK_EMAIL], EMAIL_TYPE_WORK);
+ add_email(*contact, reply[COL_OTHER_EMAIL], EMAIL_TYPE_OTHER);
+}
+
+static void contact_add_addresses(struct phonebook_contact **contact, char **reply)
+{
+
+ char *home_addr, *work_addr, *other_addr;
+
+ home_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
+ reply[COL_HOME_ADDR_POBOX], reply[COL_HOME_ADDR_EXT],
+ reply[COL_HOME_ADDR_STREET], reply[COL_HOME_ADDR_LOCALITY],
+ reply[COL_HOME_ADDR_REGION], reply[COL_HOME_ADDR_CODE],
+ reply[COL_HOME_ADDR_COUNTRY]);
+
+ work_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
+ reply[COL_WORK_ADDR_POBOX], reply[COL_WORK_ADDR_EXT],
+ reply[COL_WORK_ADDR_STREET], reply[COL_WORK_ADDR_LOCALITY],
+ reply[COL_WORK_ADDR_REGION], reply[COL_WORK_ADDR_CODE],
+ reply[COL_WORK_ADDR_COUNTRY]);
+
+ other_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
+ reply[COL_OTHER_ADDR_POBOX], reply[COL_OTHER_ADDR_EXT],
+ reply[COL_OTHER_ADDR_STREET], reply[COL_OTHER_ADDR_LOCALITY],
+ reply[COL_OTHER_ADDR_REGION], reply[COL_OTHER_ADDR_CODE],
+ reply[COL_OTHER_ADDR_COUNTRY]);
+
+ add_address(*contact, home_addr, ADDR_TYPE_HOME);
+ add_address(*contact, work_addr, ADDR_TYPE_WORK);
+ add_address(*contact, other_addr, ADDR_TYPE_OTHER);
+
+ g_free(home_addr);
+ g_free(work_addr);
+ g_free(other_addr);
+}
+
+static void contact_add_organization(struct phonebook_contact **contact, char **reply)
+{
+ /* Adding fields connected by nco:hasAffiliation - they may be in
+ * separate replies */
+ add_affiliation(&(*contact)->title, reply[COL_TITLE]);
+ add_affiliation(&(*contact)->company, reply[COL_ORG_NAME]);
+ add_affiliation(&(*contact)->department, reply[COL_ORG_DEPARTMENT]);
+ add_affiliation(&(*contact)->role, reply[COL_ORG_ROLE]);
+}
+
static void pull_contacts(char **reply, int num_fields, void *user_data)
{
struct phonebook_data *data = user_data;
@@ -1404,7 +1490,6 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
GString *vcards;
int last_index, i;
gboolean cdata_present = FALSE;
- char *home_addr, *work_addr, *other_addr;
static char *temp_id = NULL;
if (num_fields < 0) {
@@ -1455,75 +1540,13 @@ static void pull_contacts(char **reply, int num_fields, void *user_data)
return;
add_entry:
- contact = g_new0(struct phonebook_contact, 1);
- contact->fullname = g_strdup(reply[COL_FULL_NAME]);
- contact->family = g_strdup(reply[COL_FAMILY_NAME]);
- contact->given = g_strdup(reply[COL_GIVEN_NAME]);
- contact->additional = g_strdup(reply[COL_ADDITIONAL_NAME]);
- contact->prefix = g_strdup(reply[COL_NAME_PREFIX]);
- contact->suffix = g_strdup(reply[COL_NAME_SUFFIX]);
- contact->birthday = g_strdup(reply[COL_BIRTH_DATE]);
- contact->nickname = g_strdup(reply[COL_NICKNAME]);
- contact->website = g_strdup(reply[COL_URL]);
- contact->photo = g_strdup(reply[COL_PHOTO]);
- contact->company = g_strdup(reply[COL_ORG_NAME]);
- contact->department = g_strdup(reply[COL_ORG_DEPARTMENT]);
- contact->role = g_strdup(reply[COL_ORG_ROLE]);
- contact->uid = g_strdup(reply[COL_UID]);
- contact->title = g_strdup(reply[COL_TITLE]);
-
- set_call_type(contact, reply[COL_DATE], reply[COL_SENT],
- reply[COL_ANSWERED]);
+ contact_init(&contact, reply);
add_numbers:
- /* Adding phone numbers to contact struct */
- add_phone_number(contact, reply[COL_HOME_NUMBER], TEL_TYPE_HOME);
- add_phone_number(contact, reply[COL_WORK_NUMBER], TEL_TYPE_WORK);
- add_phone_number(contact, reply[COL_FAX_NUMBER], TEL_TYPE_FAX);
- add_phone_number(contact, reply[COL_CELL_NUMBER], TEL_TYPE_MOBILE);
- if ((g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_CELL_NUMBER]) != 0) &&
- (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_WORK_NUMBER]) != 0) &&
- (g_strcmp0(reply[COL_OTHER_NUMBER], reply[COL_HOME_NUMBER]) != 0))
- add_phone_number(contact, reply[COL_OTHER_NUMBER], TEL_TYPE_OTHER);
-
- /* Adding emails */
- add_email(contact, reply[COL_HOME_EMAIL], EMAIL_TYPE_HOME);
- add_email(contact, reply[COL_WORK_EMAIL], EMAIL_TYPE_WORK);
- add_email(contact, reply[COL_OTHER_EMAIL], EMAIL_TYPE_OTHER);
-
- /* Adding addresses */
- home_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
- reply[COL_HOME_ADDR_POBOX], reply[COL_HOME_ADDR_EXT],
- reply[COL_HOME_ADDR_STREET], reply[COL_HOME_ADDR_LOCALITY],
- reply[COL_HOME_ADDR_REGION], reply[COL_HOME_ADDR_CODE],
- reply[COL_HOME_ADDR_COUNTRY]);
-
- work_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
- reply[COL_WORK_ADDR_POBOX], reply[COL_WORK_ADDR_EXT],
- reply[COL_WORK_ADDR_STREET], reply[COL_WORK_ADDR_LOCALITY],
- reply[COL_WORK_ADDR_REGION], reply[COL_WORK_ADDR_CODE],
- reply[COL_WORK_ADDR_COUNTRY]);
-
- other_addr = g_strdup_printf("%s;%s;%s;%s;%s;%s;%s",
- reply[COL_OTHER_ADDR_POBOX], reply[COL_OTHER_ADDR_EXT],
- reply[COL_OTHER_ADDR_STREET], reply[COL_OTHER_ADDR_LOCALITY],
- reply[COL_OTHER_ADDR_REGION], reply[COL_OTHER_ADDR_CODE],
- reply[COL_OTHER_ADDR_COUNTRY]);
-
- add_address(contact, home_addr, ADDR_TYPE_HOME);
- add_address(contact, work_addr, ADDR_TYPE_WORK);
- add_address(contact, other_addr, ADDR_TYPE_OTHER);
-
- g_free(home_addr);
- g_free(work_addr);
- g_free(other_addr);
-
- /* Adding fields connected by nco:hasAffiliation - they may be in
- * separate replies */
- add_affiliation(&contact->title, reply[COL_TITLE]);
- add_affiliation(&contact->company, reply[COL_ORG_NAME]);
- add_affiliation(&contact->department, reply[COL_ORG_DEPARTMENT]);
- add_affiliation(&contact->role, reply[COL_ORG_ROLE]);
+ contact_add_numbers(&contact, reply);
+ contact_add_emails(&contact, reply);
+ contact_add_addresses(&contact, reply);
+ contact_add_organization(&contact, reply);
DBG("contact %p", contact);
--
1.7.0.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox