* Re: [PATCH 00/32] Health device profile (HDP)
From: Gustavo F. Padovan @ 2010-06-02 20:48 UTC (permalink / raw)
To: Jose Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <1275484768-25838-1-git-send-email-santoscadenas@gmail.com>
Hi Jose,
* Jose Antonio Santos Cadenas <santoscadenas@gmail.com> [2010-06-02 15:18:56 +0200]:
> Next patches are aplied over the MCAP patches sended to thie mailing list by
> Santiago Carot. This a first patches manage the creation of links beetween
> instances and begins the support for creating data chanels, but doesn't
> finish it yet.
For debug use the DBG() function instead of debug(). Change your patches
to match this and use the new dynamic debug feature.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [PATCH v2] frame reassembly implementation for data stream
From: Gustavo F. Padovan @ 2010-06-02 16:44 UTC (permalink / raw)
To: Suraj
Cc: Suraj Sumangala, Marcel Holtmann, linux-bluetooth@vger.kernel.org,
Luis Rodriguez, Jothikumar Mothilal
In-Reply-To: <4C0684DA.3000306@Atheros.com>
Hi Suraj,
* Suraj <suraj@Atheros.com> [2010-06-02 21:50:42 +0530]:
> Hi Gustavo,
>
> On 6/2/2010 9:41 PM, Gustavo F. Padovan wrote:
> >Hi Suraj,
> >
> >* Marcel Holtmann<marcel@holtmann.org> [2010-06-02 08:02:35 -0700]:
> >
> >>Hi Suraj,
> >>
> >>>Implemented hci_recv_stream_fragment to reassemble HCI packets received from a data stream.
> >>>
> >>>Signed-off-by: suraj<suraj@Atheros.com>
> >>
> >>please fix your signed-off-by line. This is not proper.
> >>
> >>>---
> >>> include/net/bluetooth/hci_core.h | 1 +
> >>> net/bluetooth/hci_core.c | 98 ++++++++++++++++++++++++++++++++++++++
> >>> 2 files changed, 99 insertions(+), 0 deletions(-)
> >>>
> >>>diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> >>>index e42f6ed..6f33f11 100644
> >>>--- a/include/net/bluetooth/hci_core.h
> >>>+++ b/include/net/bluetooth/hci_core.h
> >>>@@ -428,6 +428,7 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
> >>>
> >>> int hci_recv_frame(struct sk_buff *skb);
> >>> int hci_recv_fragment(struct hci_dev *hdev, int type, void *data, int count);
> >>>+int hci_recv_stream_fragment(struct hci_dev *hdev, void *data, int count);
> >>>
> >>> int hci_register_sysfs(struct hci_dev *hdev);
> >>> void hci_unregister_sysfs(struct hci_dev *hdev);
> >>>diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> >>>index 5e83f8e..ac9ccf7 100644
> >>>--- a/net/bluetooth/hci_core.c
> >>>+++ b/net/bluetooth/hci_core.c
> >>>@@ -1033,6 +1033,104 @@ EXPORT_SYMBOL(hci_recv_frame);
> >>> /* Receive packet type fragment */
> >>> #define __reassembly(hdev, type) ((hdev)->reassembly[(type) - 2])
> >>>
> >>>+#define __get_max_rx_size(type) \
> >>>+ (((type) == HCI_ACLDATA_PKT) ? \
> >>>+ HCI_MAX_FRAME_SIZE : \
> >>>+ ((type) == HCI_EVENT_PKT) ? HCI_MAX_EVENT_SIZE :\
> >>>+ HCI_MAX_SCO_SIZE)
> >>>+
> >>>+#define __get_header_len(type) \
> >>>+ (((type) == HCI_ACLDATA_PKT) ? \
> >>>+ HCI_ACL_HDR_SIZE : \
> >>>+ ((type) == HCI_EVENT_PKT) ? HCI_EVENT_HDR_SIZE :\
> >>>+ HCI_SCO_HDR_SIZE)
> >>
> >>This is total hackish code. Who do you think is able to read this?
> >
> >A switch sounds a way better for both macros, change that to a function
> >and use switch to compare.
>
> Sure, I guess I was trying to be too clever there. This call is
> called only once. So, wouldn't it better to put the switch case
> directly inline rather than writing a function?
> >
It is your choice, both a inline function or the switch directly should be
good.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [PATCH v2] frame reassembly implementation for data stream
From: Suraj @ 2010-06-02 16:20 UTC (permalink / raw)
To: Gustavo F. Padovan
Cc: Suraj Sumangala, Marcel Holtmann, linux-bluetooth@vger.kernel.org,
Luis Rodriguez, Jothikumar Mothilal
In-Reply-To: <20100602161117.GA16657@vigoh>
Hi Gustavo,
On 6/2/2010 9:41 PM, Gustavo F. Padovan wrote:
> Hi Suraj,
>
> * Marcel Holtmann<marcel@holtmann.org> [2010-06-02 08:02:35 -0700]:
>
>> Hi Suraj,
>>
>>> Implemented hci_recv_stream_fragment to reassemble HCI packets received from a data stream.
>>>
>>> Signed-off-by: suraj<suraj@Atheros.com>
>>
>> please fix your signed-off-by line. This is not proper.
>>
>>> ---
>>> include/net/bluetooth/hci_core.h | 1 +
>>> net/bluetooth/hci_core.c | 98 ++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 99 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>>> index e42f6ed..6f33f11 100644
>>> --- a/include/net/bluetooth/hci_core.h
>>> +++ b/include/net/bluetooth/hci_core.h
>>> @@ -428,6 +428,7 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
>>>
>>> int hci_recv_frame(struct sk_buff *skb);
>>> int hci_recv_fragment(struct hci_dev *hdev, int type, void *data, int count);
>>> +int hci_recv_stream_fragment(struct hci_dev *hdev, void *data, int count);
>>>
>>> int hci_register_sysfs(struct hci_dev *hdev);
>>> void hci_unregister_sysfs(struct hci_dev *hdev);
>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>>> index 5e83f8e..ac9ccf7 100644
>>> --- a/net/bluetooth/hci_core.c
>>> +++ b/net/bluetooth/hci_core.c
>>> @@ -1033,6 +1033,104 @@ EXPORT_SYMBOL(hci_recv_frame);
>>> /* Receive packet type fragment */
>>> #define __reassembly(hdev, type) ((hdev)->reassembly[(type) - 2])
>>>
>>> +#define __get_max_rx_size(type) \
>>> + (((type) == HCI_ACLDATA_PKT) ? \
>>> + HCI_MAX_FRAME_SIZE : \
>>> + ((type) == HCI_EVENT_PKT) ? HCI_MAX_EVENT_SIZE :\
>>> + HCI_MAX_SCO_SIZE)
>>> +
>>> +#define __get_header_len(type) \
>>> + (((type) == HCI_ACLDATA_PKT) ? \
>>> + HCI_ACL_HDR_SIZE : \
>>> + ((type) == HCI_EVENT_PKT) ? HCI_EVENT_HDR_SIZE :\
>>> + HCI_SCO_HDR_SIZE)
>>
>> This is total hackish code. Who do you think is able to read this?
>
> A switch sounds a way better for both macros, change that to a function
> and use switch to compare.
Sure, I guess I was trying to be too clever there. This call is called
only once. So, wouldn't it better to put the switch case directly inline
rather than writing a function?
>
>>
>>> +int hci_recv_stream_fragment(struct hci_dev *hdev, void *data, int count)
>>> +{
>>> + int type;
>>> +
>>> + while (count) {
>>> + struct sk_buff *skb = __reassembly(hdev, HCI_ACLDATA_PKT);
>>> +
>>> + struct { int expect; int pkt_type; } *scb;
>>> + int len = 0;
>>> +
>>> + if (!skb) {
>>> + struct { char type; } *pkt;
>>> +
>>> + /* Start of the frame */
>>> + pkt = data;
>>> + type = pkt->type;
>>> +
>>> + if (type< HCI_ACLDATA_PKT || type> HCI_EVENT_PKT)
>>> + return -EILSEQ;
>>> +
>>> + len = __get_max_rx_size(type);
>>> +
>>> + skb = bt_skb_alloc(len, GFP_ATOMIC);
>>> + if (!skb)
>>> + return -ENOMEM;
>>> +
>>> + scb = (void *) skb->cb;
>>> + scb->expect = __get_header_len(type);
>>> + scb->pkt_type = type;
>>> +
>>> + skb->dev = (void *) hdev;
>>> + __reassembly(hdev, HCI_ACLDATA_PKT) = skb;
>>> +
>>> + data++;
>>> + count--;
>>> +
>>> + continue;
>>> + } else {
>>> + scb = (void *) skb->cb;
>>> + len = min(scb->expect, count);
>>> + type = scb->pkt_type;
>>> +
>>> + memcpy(skb_put(skb, len), data, len);
>>> +
>>> + count -= len;
>>> + data += len;
>>> + scb->expect -= len;
>>> + }
>>> +
>>> + switch (type) {
>>> + case HCI_EVENT_PKT:
>>> + if (skb->len == HCI_EVENT_HDR_SIZE) {
>>> + struct hci_event_hdr *h = hci_event_hdr(skb);
>>> + scb->expect = h->plen;
>>> + }
>>> + break;
>>> +
>>> + case HCI_ACLDATA_PKT:
>>> + if (skb->len == HCI_ACL_HDR_SIZE) {
>>> + struct hci_acl_hdr *h = hci_acl_hdr(skb);
>>> + scb->expect = __le16_to_cpu(h->dlen);
>>> + }
>>> + break;
>>> +
>>> + case HCI_SCODATA_PKT:
>>> + if (skb->len == HCI_SCO_HDR_SIZE) {
>>> + struct hci_sco_hdr *h = hci_sco_hdr(skb);
>>> + scb->expect = h->dlen;
>>> + }
>>> + break;
>>> + }
>>> +
>>> + if (scb->expect == 0) {
>>> + /* Complete frame */
>>> +
>>> + __reassembly(hdev, HCI_ACLDATA_PKT) = NULL;
>>> +
>>> + bt_cb(skb)->pkt_type = type;
>>> + hci_recv_frame(skb);
>>> + }
>>> +
>>> + }
>>> + return 0;
>>> +}
>>
>> I don't like this implementation at all. The biggest problem is that you
>> are misusing __reassembly(hdev, HCI_ACLDATA_PKT) for getting your SKB. I
>> don't wanna intermix this. I am also missing checks for the packet
>> length matching or when packets are too big or the header size is not
>> matching up.
>>
>> So in theory both functions do exactly the same. Only minor exception is
>> that one knows the packet type up-front, the other has to read it from
>> the stream as a 1-byte header. I don't wanna maintain two functions that
>> do exactly the same.
>>
>> Creating an internal helper function that can maintain the current state
>> of the reassembly sounds a lot better. Then re-use that function and
>> ensure that the reassembly logic is inside the helper.
>>
>> Regards
>>
>> Marcel
>>
>>
>> --
>> 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 http://vger.kernel.org/majordomo-info.htm
>
Regards
Suraj
^ permalink raw reply
* Re: [PATCH v2] frame reassembly implementation for data stream
From: Gustavo F. Padovan @ 2010-06-02 16:11 UTC (permalink / raw)
To: suraj; +Cc: Marcel Holtmann, linux-bluetooth, Luis.Rodriguez,
Jothikumar.Mothilal
In-Reply-To: <1275490955.2182.21.camel@localhost.localdomain>
Hi Suraj,
* Marcel Holtmann <marcel@holtmann.org> [2010-06-02 08:02:35 -0700]:
> Hi Suraj,
>
> > Implemented hci_recv_stream_fragment to reassemble HCI packets received from a data stream.
> >
> > Signed-off-by: suraj <suraj@Atheros.com>
>
> please fix your signed-off-by line. This is not proper.
>
> > ---
> > include/net/bluetooth/hci_core.h | 1 +
> > net/bluetooth/hci_core.c | 98 ++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 99 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> > index e42f6ed..6f33f11 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -428,6 +428,7 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
> >
> > int hci_recv_frame(struct sk_buff *skb);
> > int hci_recv_fragment(struct hci_dev *hdev, int type, void *data, int count);
> > +int hci_recv_stream_fragment(struct hci_dev *hdev, void *data, int count);
> >
> > int hci_register_sysfs(struct hci_dev *hdev);
> > void hci_unregister_sysfs(struct hci_dev *hdev);
> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> > index 5e83f8e..ac9ccf7 100644
> > --- a/net/bluetooth/hci_core.c
> > +++ b/net/bluetooth/hci_core.c
> > @@ -1033,6 +1033,104 @@ EXPORT_SYMBOL(hci_recv_frame);
> > /* Receive packet type fragment */
> > #define __reassembly(hdev, type) ((hdev)->reassembly[(type) - 2])
> >
> > +#define __get_max_rx_size(type) \
> > + (((type) == HCI_ACLDATA_PKT) ? \
> > + HCI_MAX_FRAME_SIZE : \
> > + ((type) == HCI_EVENT_PKT) ? HCI_MAX_EVENT_SIZE :\
> > + HCI_MAX_SCO_SIZE)
> > +
> > +#define __get_header_len(type) \
> > + (((type) == HCI_ACLDATA_PKT) ? \
> > + HCI_ACL_HDR_SIZE : \
> > + ((type) == HCI_EVENT_PKT) ? HCI_EVENT_HDR_SIZE :\
> > + HCI_SCO_HDR_SIZE)
>
> This is total hackish code. Who do you think is able to read this?
A switch sounds a way better for both macros, change that to a function
and use switch to compare.
>
> > +int hci_recv_stream_fragment(struct hci_dev *hdev, void *data, int count)
> > +{
> > + int type;
> > +
> > + while (count) {
> > + struct sk_buff *skb = __reassembly(hdev, HCI_ACLDATA_PKT);
> > +
> > + struct { int expect; int pkt_type; } *scb;
> > + int len = 0;
> > +
> > + if (!skb) {
> > + struct { char type; } *pkt;
> > +
> > + /* Start of the frame */
> > + pkt = data;
> > + type = pkt->type;
> > +
> > + if (type < HCI_ACLDATA_PKT || type > HCI_EVENT_PKT)
> > + return -EILSEQ;
> > +
> > + len = __get_max_rx_size(type);
> > +
> > + skb = bt_skb_alloc(len, GFP_ATOMIC);
> > + if (!skb)
> > + return -ENOMEM;
> > +
> > + scb = (void *) skb->cb;
> > + scb->expect = __get_header_len(type);
> > + scb->pkt_type = type;
> > +
> > + skb->dev = (void *) hdev;
> > + __reassembly(hdev, HCI_ACLDATA_PKT) = skb;
> > +
> > + data++;
> > + count--;
> > +
> > + continue;
> > + } else {
> > + scb = (void *) skb->cb;
> > + len = min(scb->expect, count);
> > + type = scb->pkt_type;
> > +
> > + memcpy(skb_put(skb, len), data, len);
> > +
> > + count -= len;
> > + data += len;
> > + scb->expect -= len;
> > + }
> > +
> > + switch (type) {
> > + case HCI_EVENT_PKT:
> > + if (skb->len == HCI_EVENT_HDR_SIZE) {
> > + struct hci_event_hdr *h = hci_event_hdr(skb);
> > + scb->expect = h->plen;
> > + }
> > + break;
> > +
> > + case HCI_ACLDATA_PKT:
> > + if (skb->len == HCI_ACL_HDR_SIZE) {
> > + struct hci_acl_hdr *h = hci_acl_hdr(skb);
> > + scb->expect = __le16_to_cpu(h->dlen);
> > + }
> > + break;
> > +
> > + case HCI_SCODATA_PKT:
> > + if (skb->len == HCI_SCO_HDR_SIZE) {
> > + struct hci_sco_hdr *h = hci_sco_hdr(skb);
> > + scb->expect = h->dlen;
> > + }
> > + break;
> > + }
> > +
> > + if (scb->expect == 0) {
> > + /* Complete frame */
> > +
> > + __reassembly(hdev, HCI_ACLDATA_PKT) = NULL;
> > +
> > + bt_cb(skb)->pkt_type = type;
> > + hci_recv_frame(skb);
> > + }
> > +
> > + }
> > + return 0;
> > +}
>
> I don't like this implementation at all. The biggest problem is that you
> are misusing __reassembly(hdev, HCI_ACLDATA_PKT) for getting your SKB. I
> don't wanna intermix this. I am also missing checks for the packet
> length matching or when packets are too big or the header size is not
> matching up.
>
> So in theory both functions do exactly the same. Only minor exception is
> that one knows the packet type up-front, the other has to read it from
> the stream as a 1-byte header. I don't wanna maintain two functions that
> do exactly the same.
>
> Creating an internal helper function that can maintain the current state
> of the reassembly sounds a lot better. Then re-use that function and
> ensure that the reassembly logic is inside the helper.
>
> Regards
>
> Marcel
>
>
> --
> 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 http://vger.kernel.org/majordomo-info.html
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [PATCH v2] frame reassembly implementation for data stream
From: Suraj @ 2010-06-02 16:10 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Suraj Sumangala, linux-bluetooth@vger.kernel.org, Luis Rodriguez,
Jothikumar Mothilal
In-Reply-To: <1275490955.2182.21.camel@localhost.localdomain>
Hi Marcel,
On 6/2/2010 8:32 PM, Marcel Holtmann wrote:
> Hi Suraj,
>
>> Implemented hci_recv_stream_fragment to reassemble HCI packets received from a data stream.
>>
>> Signed-off-by: suraj<suraj@Atheros.com>
>
> please fix your signed-off-by line. This is not proper.
>
>> ---
>> include/net/bluetooth/hci_core.h | 1 +
>> net/bluetooth/hci_core.c | 98 ++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 99 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
>> index e42f6ed..6f33f11 100644
>> --- a/include/net/bluetooth/hci_core.h
>> +++ b/include/net/bluetooth/hci_core.h
>> @@ -428,6 +428,7 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
>>
>> int hci_recv_frame(struct sk_buff *skb);
>> int hci_recv_fragment(struct hci_dev *hdev, int type, void *data, int count);
>> +int hci_recv_stream_fragment(struct hci_dev *hdev, void *data, int count);
>>
>> int hci_register_sysfs(struct hci_dev *hdev);
>> void hci_unregister_sysfs(struct hci_dev *hdev);
>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
>> index 5e83f8e..ac9ccf7 100644
>> --- a/net/bluetooth/hci_core.c
>> +++ b/net/bluetooth/hci_core.c
>> @@ -1033,6 +1033,104 @@ EXPORT_SYMBOL(hci_recv_frame);
>> /* Receive packet type fragment */
>> #define __reassembly(hdev, type) ((hdev)->reassembly[(type) - 2])
>>
>> +#define __get_max_rx_size(type) \
>> + (((type) == HCI_ACLDATA_PKT) ? \
>> + HCI_MAX_FRAME_SIZE : \
>> + ((type) == HCI_EVENT_PKT) ? HCI_MAX_EVENT_SIZE :\
>> + HCI_MAX_SCO_SIZE)
>> +
>> +#define __get_header_len(type) \
>> + (((type) == HCI_ACLDATA_PKT) ? \
>> + HCI_ACL_HDR_SIZE : \
>> + ((type) == HCI_EVENT_PKT) ? HCI_EVENT_HDR_SIZE :\
>> + HCI_SCO_HDR_SIZE)
>
> This is total hackish code. Who do you think is able to read this?
Will change it, thought this could make the actual function bit easier
to read.
>
>> +int hci_recv_stream_fragment(struct hci_dev *hdev, void *data, int count)
>> +{
>> + int type;
>> +
>> + while (count) {
>> + struct sk_buff *skb = __reassembly(hdev, HCI_ACLDATA_PKT);
>> +
>> + struct { int expect; int pkt_type; } *scb;
>> + int len = 0;
>> +
>> + if (!skb) {
>> + struct { char type; } *pkt;
>> +
>> + /* Start of the frame */
>> + pkt = data;
>> + type = pkt->type;
>> +
>> + if (type< HCI_ACLDATA_PKT || type> HCI_EVENT_PKT)
>> + return -EILSEQ;
>> +
>> + len = __get_max_rx_size(type);
>> +
>> + skb = bt_skb_alloc(len, GFP_ATOMIC);
>> + if (!skb)
>> + return -ENOMEM;
>> +
>> + scb = (void *) skb->cb;
>> + scb->expect = __get_header_len(type);
>> + scb->pkt_type = type;
>> +
>> + skb->dev = (void *) hdev;
>> + __reassembly(hdev, HCI_ACLDATA_PKT) = skb;
>> +
>> + data++;
>> + count--;
>> +
>> + continue;
>> + } else {
>> + scb = (void *) skb->cb;
>> + len = min(scb->expect, count);
>> + type = scb->pkt_type;
>> +
>> + memcpy(skb_put(skb, len), data, len);
>> +
>> + count -= len;
>> + data += len;
>> + scb->expect -= len;
>> + }
>> +
>> + switch (type) {
>> + case HCI_EVENT_PKT:
>> + if (skb->len == HCI_EVENT_HDR_SIZE) {
>> + struct hci_event_hdr *h = hci_event_hdr(skb);
This is a major difference. hci_recv_fragment makes a critical
assumption that we will get the full packet header in one shot, If not
the whole reassembly fails.
For a stream, we can not assume that. You count receive data 1 byte at
at time. compare the detail of both function.
>> + scb->expect = h->plen;
>> + }
>> + break;
>> +
>> + case HCI_ACLDATA_PKT:
>> + if (skb->len == HCI_ACL_HDR_SIZE) {
>> + struct hci_acl_hdr *h = hci_acl_hdr(skb);
>> + scb->expect = __le16_to_cpu(h->dlen);
>> + }
>> + break;
>> +
>> + case HCI_SCODATA_PKT:
>> + if (skb->len == HCI_SCO_HDR_SIZE) {
>> + struct hci_sco_hdr *h = hci_sco_hdr(skb);
>> + scb->expect = h->dlen;
>> + }
>> + break;
>> + }
>> +
>> + if (scb->expect == 0) {
>> + /* Complete frame */
>> +
>> + __reassembly(hdev, HCI_ACLDATA_PKT) = NULL;
>> +
>> + bt_cb(skb)->pkt_type = type;
>> + hci_recv_frame(skb);
>> + }
>> +
>> + }
>> + return 0;
>> +}
>
> I don't like this implementation at all. The biggest problem is that you
> are misusing __reassembly(hdev, HCI_ACLDATA_PKT) for getting your SKB. I
> don't wanna intermix this.
The reason why I thought about reusing this is because it might save
another entry in the structure.
I am also missing checks for the packet
> length matching or when packets are too big or the header size is not
> matching up.
We are dealing with a stream, not packets.
A packet can never be too big. Because, we reassemble only that number
of bytes mentioned in packet header. The rest are part of the next
frame.
The only option is to read the header, find length and reassemble until
we receive the number of bytes mentioned in the header.
The rest has to be assumed as the beginning of next frame and start
another cycle.
I guess even hci_recv_fragment works the same way.
>
> So in theory both functions do exactly the same.
Only minor exception is
> that one knows the packet type up-front, the other has to read it from
> the stream as a 1-byte header. I don't wanna maintain two functions that
> do exactly the same.
Actually both only looks similar, but work differently. Just check the
details.
There are certain assumptions taken by hci_recv_fragment() that need to
be avoided.
that is we receive the packet header in one call. This can not be
assumed for a stream.
>
> Creating an internal helper function that can maintain the current state
> of the reassembly sounds a lot better. Then re-use that function and
> ensure that the reassembly logic is inside the helper.
>
> Regards
>
> Marcel
>
>
I will work to get a better implementation. Please go through both the
implementation and let me know your comments.
Regards
Suraj
^ permalink raw reply
* Re: [PATCH v2] frame reassembly implementation for data stream
From: Marcel Holtmann @ 2010-06-02 15:02 UTC (permalink / raw)
To: suraj; +Cc: linux-bluetooth, Luis.Rodriguez, Jothikumar.Mothilal
In-Reply-To: <1275467080.13946.3.camel@atheros013-desktop>
Hi Suraj,
> Implemented hci_recv_stream_fragment to reassemble HCI packets received from a data stream.
>
> Signed-off-by: suraj <suraj@Atheros.com>
please fix your signed-off-by line. This is not proper.
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_core.c | 98 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 99 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index e42f6ed..6f33f11 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -428,6 +428,7 @@ void hci_event_packet(struct hci_dev *hdev, struct sk_buff *skb);
>
> int hci_recv_frame(struct sk_buff *skb);
> int hci_recv_fragment(struct hci_dev *hdev, int type, void *data, int count);
> +int hci_recv_stream_fragment(struct hci_dev *hdev, void *data, int count);
>
> int hci_register_sysfs(struct hci_dev *hdev);
> void hci_unregister_sysfs(struct hci_dev *hdev);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 5e83f8e..ac9ccf7 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -1033,6 +1033,104 @@ EXPORT_SYMBOL(hci_recv_frame);
> /* Receive packet type fragment */
> #define __reassembly(hdev, type) ((hdev)->reassembly[(type) - 2])
>
> +#define __get_max_rx_size(type) \
> + (((type) == HCI_ACLDATA_PKT) ? \
> + HCI_MAX_FRAME_SIZE : \
> + ((type) == HCI_EVENT_PKT) ? HCI_MAX_EVENT_SIZE :\
> + HCI_MAX_SCO_SIZE)
> +
> +#define __get_header_len(type) \
> + (((type) == HCI_ACLDATA_PKT) ? \
> + HCI_ACL_HDR_SIZE : \
> + ((type) == HCI_EVENT_PKT) ? HCI_EVENT_HDR_SIZE :\
> + HCI_SCO_HDR_SIZE)
This is total hackish code. Who do you think is able to read this?
> +int hci_recv_stream_fragment(struct hci_dev *hdev, void *data, int count)
> +{
> + int type;
> +
> + while (count) {
> + struct sk_buff *skb = __reassembly(hdev, HCI_ACLDATA_PKT);
> +
> + struct { int expect; int pkt_type; } *scb;
> + int len = 0;
> +
> + if (!skb) {
> + struct { char type; } *pkt;
> +
> + /* Start of the frame */
> + pkt = data;
> + type = pkt->type;
> +
> + if (type < HCI_ACLDATA_PKT || type > HCI_EVENT_PKT)
> + return -EILSEQ;
> +
> + len = __get_max_rx_size(type);
> +
> + skb = bt_skb_alloc(len, GFP_ATOMIC);
> + if (!skb)
> + return -ENOMEM;
> +
> + scb = (void *) skb->cb;
> + scb->expect = __get_header_len(type);
> + scb->pkt_type = type;
> +
> + skb->dev = (void *) hdev;
> + __reassembly(hdev, HCI_ACLDATA_PKT) = skb;
> +
> + data++;
> + count--;
> +
> + continue;
> + } else {
> + scb = (void *) skb->cb;
> + len = min(scb->expect, count);
> + type = scb->pkt_type;
> +
> + memcpy(skb_put(skb, len), data, len);
> +
> + count -= len;
> + data += len;
> + scb->expect -= len;
> + }
> +
> + switch (type) {
> + case HCI_EVENT_PKT:
> + if (skb->len == HCI_EVENT_HDR_SIZE) {
> + struct hci_event_hdr *h = hci_event_hdr(skb);
> + scb->expect = h->plen;
> + }
> + break;
> +
> + case HCI_ACLDATA_PKT:
> + if (skb->len == HCI_ACL_HDR_SIZE) {
> + struct hci_acl_hdr *h = hci_acl_hdr(skb);
> + scb->expect = __le16_to_cpu(h->dlen);
> + }
> + break;
> +
> + case HCI_SCODATA_PKT:
> + if (skb->len == HCI_SCO_HDR_SIZE) {
> + struct hci_sco_hdr *h = hci_sco_hdr(skb);
> + scb->expect = h->dlen;
> + }
> + break;
> + }
> +
> + if (scb->expect == 0) {
> + /* Complete frame */
> +
> + __reassembly(hdev, HCI_ACLDATA_PKT) = NULL;
> +
> + bt_cb(skb)->pkt_type = type;
> + hci_recv_frame(skb);
> + }
> +
> + }
> + return 0;
> +}
I don't like this implementation at all. The biggest problem is that you
are misusing __reassembly(hdev, HCI_ACLDATA_PKT) for getting your SKB. I
don't wanna intermix this. I am also missing checks for the packet
length matching or when packets are too big or the header size is not
matching up.
So in theory both functions do exactly the same. Only minor exception is
that one knows the packet type up-front, the other has to read it from
the stream as a 1-byte header. I don't wanna maintain two functions that
do exactly the same.
Creating an internal helper function that can maintain the current state
of the reassembly sounds a lot better. Then re-use that function and
ensure that the reassembly logic is inside the helper.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 00/32] Health device profile (HDP)
From: José Antonio Santos Cadenas @ 2010-06-02 14:44 UTC (permalink / raw)
To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <20100602135159.GA4178@jh-x301>
Hi,
El Wednesday 02 June 2010 15:51:59 Johan Hedberg escribió:
> Hi,
>
> On Wed, Jun 02, 2010, Jose Antonio Santos Cadenas wrote:
> > Next patches are aplied over the MCAP patches sended to thie mailing list
> > by Santiago Carot. This a first patches manage the creation of links
> > beetween instances and begins the support for creating data chanels, but
> > doesn't finish it yet.
>
> Could you give the address of your public git tree, please. It'd be
> easier to deal with than this huge amount of patches on the mailing
> list.
We are using gitorious for developing. All the sent patches (mcap and hdp) are
applied on this branch:
http://gitorious.org/bluez-mcap-hdp/bluez-mcap-hdp/commits/hdp_mcap_sent
Clone url:
git://gitorious.org/bluez-mcap-hdp/bluez-mcap-hdp.git
Branch:
hdp_mcap_sent
>
> Johan
Regards
Jose.
^ permalink raw reply
* Re: [PATCH 00/32] Health device profile (HDP)
From: Johan Hedberg @ 2010-06-02 13:51 UTC (permalink / raw)
To: Jose Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <1275484768-25838-1-git-send-email-santoscadenas@gmail.com>
Hi,
On Wed, Jun 02, 2010, Jose Antonio Santos Cadenas wrote:
> Next patches are aplied over the MCAP patches sended to thie mailing list by
> Santiago Carot. This a first patches manage the creation of links beetween
> instances and begins the support for creating data chanels, but doesn't
> finish it yet.
Could you give the address of your public git tree, please. It'd be
easier to deal with than this huge amount of patches on the mailing
list.
Johan
^ permalink raw reply
* [PATCH 32/32] Add support for mcl reconnections
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-32-git-send-email-santoscadenas@gmail.com>
---
health/hdp.c | 20 ++++++++++++--------
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index 9678baa..7a726e0 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -56,6 +56,7 @@ struct hdp_connection_cb {
struct hdp_instance *hdpi;
uint32_t rem_id;
DBusMessage *msg;
+ struct hdp_link *hdpl;
};
static struct hdp_adapter *find_adapter(GSList *list,
@@ -554,11 +555,15 @@ static void hdp_mcl_connect_cb(struct mcap_mcl *mcl, GError *err, void *data)
if (err)
goto fail;
- hdpl = create_health_link(hdpi, mcl, &gerr);
- if (gerr)
- goto fail;
+ if (cb_data->hdpl)
+ hdpl = cb_data->hdpl;
+ else {
+ hdpl = create_health_link(hdpi, mcl, &gerr);
+ if (gerr)
+ goto fail;
+ hdpi->hlink = g_slist_prepend(hdpi->hlink, hdpl);
+ }
- hdpi->hlink = g_slist_prepend(hdpi->hlink, hdpl);
reply = g_dbus_create_reply(msg, DBUS_TYPE_OBJECT_PATH, &hdpl->path,
DBUS_TYPE_INVALID);
g_dbus_send_message(device->conn, reply);
@@ -664,10 +669,7 @@ static DBusMessage *hdp_connect(DBusConnection *conn,
hdpi = l->data;
hdpl = get_health_link(hdpi, device);
if (hdpl) {
- if (hdpl->closed) {
- debug("Need a reconection");
- /* TODO: Reconnect */;
- } else
+ if (!hdpl->closed)
return g_dbus_create_reply(msg, DBUS_TYPE_OBJECT_PATH,
&hdpl->path, DBUS_TYPE_INVALID);
}
@@ -677,6 +679,8 @@ static DBusMessage *hdp_connect(DBusConnection *conn,
cb_data->hdpi = hdpi;
cb_data->rem_id = rid;
cb_data->msg = dbus_message_ref(msg);
+ /* Used for reconnections */
+ cb_data->hdpl = hdpl;
adapter = device->hdp_adapter->btd_adapter;
adapter_get_address(adapter, &src);
--
1.6.3.3
^ permalink raw reply related
* [PATCH 31/32] Send MCL disconnect callback to agents
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: José Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-31-git-send-email-santoscadenas@gmail.com>
From: José Antonio Santos Cadenas <santoscadenas@gmail.com>
Whenever a MCL is discoinnected we shall notify to agents using
D-Bus callback.
---
health/hdp.c | 82 +++++++++++++++++++++++++++++++++++++--------------------
1 files changed, 53 insertions(+), 29 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index f7146eb..9678baa 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -393,10 +393,46 @@ static uint8_t hdp_mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
return MCAP_REQUEST_NOT_SUPPORTED;
}
+static void health_link_free(struct hdp_link *hdpl)
+{
+ /*TODO: Release structures related with Data Channels */
+
+ if (hdpl->mcl) {
+ mcap_close_mcl(hdpl->mcl, FALSE);
+ mcap_mcl_unref(hdpl->mcl);
+ }
+
+ if (hdpl->path)
+ g_free(hdpl->path);
+
+ if (hdpl->ndc)
+ g_free(hdpl->ndc);
+
+ g_free(hdpl);
+}
+
+static gboolean agent_mcl_disconnect_msg(struct hdp_link *hdpl)
+{
+ struct hdp_instance *hdpi = hdpl->hdpi;
+ DBusMessage* message;
+
+ message = dbus_message_new_method_call(hdpi->aname, hdpi->apath,
+ HEALTH_AGENT, "LinkDisconnected");
+ if (!message) {
+ error("Couldn't allocate D-Bus message");
+ return FALSE;
+ }
+
+ dbus_message_append_args(message, DBUS_TYPE_OBJECT_PATH, &hdpl->path,
+ DBUS_TYPE_INVALID);
+ return g_dbus_send_message(hdpi->adapter->conn, message);
+}
+
static uint8_t hdp_mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
uint16_t mdlid, uint8_t *conf, void *data)
{
struct hdp_link *hdpl = data;
+ struct hdp_instance *hdpi;
struct hdp_supp_fts *f;
struct hdp_dc *dc;
uint8_t new_conf;
@@ -418,7 +454,7 @@ static uint8_t hdp_mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
new_conf = HDP_RELIABLE_DC;
break;
case HDP_STREAMING_DC:
- if (g_slist_length(hdpl->channels) == 0)
+ if (!hdpl->channels)
return MCAP_CONFIGURATION_REJECTED;
case HDP_RELIABLE_DC:
if (f->role == HDP_SOURCE)
@@ -427,13 +463,16 @@ static uint8_t hdp_mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
break;
default:
/* Special case defined in HDP spec 3.4. When an invalid
- * configuration is received we need close the MCL when
+ * configuration is received we shall close the MCL when
* we are still processing the callback. When MCL is
* closed in a callback the returned value won't be
* proccesed in MCAP */
- /* TODO: Send MCL disconnection to agent */
- g_dbus_unregister_interface(hdpl->hdpi->adapter->conn,
- hdpl->path, HEALTH_LINK);
+ hdpi = hdpl->hdpi;
+ hdpi->hlink = g_slist_remove(hdpi->hlink, hdpl);
+ agent_mcl_disconnect_msg(hdpl);
+ if (!g_dbus_unregister_interface(hdpi->adapter->conn,
+ hdpl->path, HEALTH_LINK))
+ health_link_free(hdpl);
return MCAP_CONFIGURATION_REJECTED; /* not processed */
}
*conf = new_conf;
@@ -447,25 +486,6 @@ static uint8_t hdp_mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
return MCAP_SUCCESS;
}
-static void health_link_free(struct hdp_link *hdpl)
-{
- /*TODO: Release structures related with Data Channels */
-
- if (hdpl->mcl) {
- mcap_close_mcl(hdpl->mcl, FALSE);
- mcap_mcl_unref(hdpl->mcl);
- }
-
- if (hdpl->path)
- g_free(hdpl->path);
-
- if (hdpl->ndc)
- g_free(hdpl->ndc);
-
- hdpl->hdpi->hlink = g_slist_remove(hdpl->hdpi->hlink, hdpl);
- g_free(hdpl);
-}
-
static void health_link_path_unregister(void *data)
{
struct hdp_link *hdpl = data;
@@ -696,8 +716,11 @@ static DBusMessage *hdp_disconnect(DBusConnection *conn,
if (cache) {
mcap_close_mcl(hdpl->mcl, cache);
hdpl->closed = TRUE;
- } else
- g_dbus_unregister_interface(conn, hdpl->path, HEALTH_LINK);
+ } else {
+ hdpl->hdpi->hlink = g_slist_remove(hdpl->hdpi->hlink, hdpl);
+ if (!g_dbus_unregister_interface(conn, hdpl->path, HEALTH_LINK))
+ health_link_free(hdpl);
+ }
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
}
@@ -788,7 +811,6 @@ static void mcl_connected(struct mcap_mcl *mcl, gpointer data)
}
error("D-Bus send failed");
health_link_free(hdpl);
- dbus_message_unref(message);
}
static void mcl_reconnected(struct mcap_mcl *mcl, gpointer data)
@@ -837,8 +859,10 @@ static void mcl_uncached(struct mcap_mcl *mcl, gpointer data)
hdpl = l->data;
hdpi->hlink = g_slist_remove(hdpi->hlink, hdpl);
- g_dbus_unregister_interface(hdpi->adapter->conn, hdpl->path,
- HEALTH_LINK);
+ agent_mcl_disconnect_msg(hdpl);
+ if (!g_dbus_unregister_interface(hdpi->adapter->conn, hdpl->path,
+ HEALTH_LINK))
+ health_link_free(hdpl);
}
static DBusMessage *hdp_create_instance(DBusConnection *conn,
--
1.6.3.3
^ permalink raw reply related
* [PATCH 30/32] Add a test that creates a simple health agent
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-30-git-send-email-santoscadenas@gmail.com>
---
test/test-health | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 61 insertions(+), 0 deletions(-)
create mode 100755 test/test-health
diff --git a/test/test-health b/test/test-health
new file mode 100755
index 0000000..2129923
--- /dev/null
+++ b/test/test-health
@@ -0,0 +1,61 @@
+#!/usr/bin/python
+# -*- coding: utf-8 -*-
+
+import dbus
+import dbus.service
+import gobject
+from dbus.mainloop.glib import DBusGMainLoop
+
+agent_iface = 'org.bluez.HealthAgent'
+obj_path = "/org/bluez/test/health/agent"
+
+DBusGMainLoop(set_as_default=True)
+loop = gobject.MainLoop()
+
+bus = dbus.SystemBus()
+manager = dbus.Interface(bus.get_object("org.bluez", "/"),
+ "org.bluez.Manager")
+
+hdp = dbus.Interface(bus.get_object("org.bluez", manager.DefaultAdapter()),
+ "org.bluez.HealthAdapter")
+
+class HDP(dbus.service.Object):
+ def __init__(self, bus, obj_path):
+ self.bus = bus
+ dbus.service.Object.__init__(self, self.bus, obj_path)
+ @dbus.service.method(agent_iface, in_signature='o', out_signature='',
+ sender_keyword='sender')
+ def LinkConnected(self, path, sender=None):
+ print "Connected new link %s: (sender %s)" % (path, sender)
+ @dbus.service.method(agent_iface, in_signature='o', out_signature='',
+ sender_keyword='sender')
+ def LinkDisconnected(self, path, sender=None):
+ print "Link %s: disconnected (sender: %s)" % (path, sender)
+
+session_id = hdp.CreateInstance(dbus.ObjectPath(obj_path),
+ {"data_spec": dbus.Byte(1, variant_level=1),
+ "end_points":dbus.Array([{ "role": dbus.String("sink", variant_level=1),
+ "specs": dbus.Array([{
+ "data_type":
+ dbus.UInt16(4100, variant_level =1),
+ "description":
+ dbus.String("Oximeter",
+ variant_level = 1),
+ },{
+ "data_type":
+ dbus.UInt16(4103, variant_level =1),
+ "description":
+ dbus.String("Blood pressure",
+ variant_level = 1),
+ }
+ ], variant_level=1),
+ },
+ ], variant_level=1)})
+
+hdp_obj = HDP(bus, obj_path);
+
+try:
+ print "Waiting for connections, push Ctrl+C to stop"
+ loop.run()
+except:
+ print "Loop interrupted, closing session"
--
1.6.3.3
^ permalink raw reply related
* [PATCH 29/32] Call the agent when a new Health Link is created
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-29-git-send-email-santoscadenas@gmail.com>
---
health/hdp.c | 27 ++++++++++++++++++++++++---
src/bluetooth.conf | 1 +
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index e99e00a..f7146eb 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -760,14 +760,35 @@ static void mcl_connected(struct mcap_mcl *mcl, gpointer data)
{
struct hdp_instance *hdpi = data;
struct hdp_link *hdpl;
+ DBusMessage* message;
GError *err = NULL;
debug("mcl_connected");
+
+ message = dbus_message_new_method_call(hdpi->aname, hdpi->apath,
+ HEALTH_AGENT, "LinkConnected");
+ if (!message) {
+ error("Couldn't allocate D-Bus message");
+ return;
+ }
+
hdpl = create_health_link(hdpi, mcl, &err);
- if (err)
+ if (err) {
+ dbus_message_unref(message);
return;
- /* TODO: Send the notification to the Agent */
- hdpi->hlink = g_slist_prepend(hdpi->hlink, hdpl);
+ }
+
+ dbus_message_append_args(message,
+ DBUS_TYPE_OBJECT_PATH, &hdpl->path,
+ DBUS_TYPE_INVALID);
+
+ if (g_dbus_send_message(hdpi->adapter->conn, message)) {
+ hdpi->hlink = g_slist_prepend(hdpi->hlink, hdpl);
+ return;
+ }
+ error("D-Bus send failed");
+ health_link_free(hdpl);
+ dbus_message_unref(message);
}
static void mcl_reconnected(struct mcap_mcl *mcl, gpointer data)
diff --git a/src/bluetooth.conf b/src/bluetooth.conf
index 56e7a83..4471da5 100644
--- a/src/bluetooth.conf
+++ b/src/bluetooth.conf
@@ -12,6 +12,7 @@
<allow send_destination="org.bluez"/>
<allow send_interface="org.bluez.Agent"/>
<allow send_interface="org.bluez.HandsfreeAgent"/>
+ <allow send_interface="org.bluez.HealthAgent"/>
</policy>
<policy at_console="true">
--
1.6.3.3
^ permalink raw reply related
* [PATCH 28/32] Changed HEALTH_MANAGER_INTERFACE to HEALTH_MANAGER
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-28-git-send-email-santoscadenas@gmail.com>
---
health/hdp.c | 6 +++---
health/hdp_types.h | 7 ++++---
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index cd344d0..e99e00a 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -964,11 +964,11 @@ int hdp_adapter_register(DBusConnection *conn, struct btd_adapter *btd_adapter)
adapter = g_new0(struct hdp_adapter, 1);
debug("HDP init");
- if (!g_dbus_register_interface(conn, path, HEALTH_MANAGER_INTERFACE,
+ if (!g_dbus_register_interface(conn, path, HEALTH_MANAGER,
hdp_methods, NULL, NULL,
adapter, hdp_path_unregister)) {
error("Failed to register %s interface to %s",
- HEALTH_MANAGER_INTERFACE, path);
+ HEALTH_MANAGER, path);
g_free(adapter);
return -1;
}
@@ -988,7 +988,7 @@ void hdp_adapter_unregister(struct btd_adapter *btd_adapter)
g_dbus_unregister_interface(adapter->conn,
adapter_get_path(btd_adapter),
- HEALTH_MANAGER_INTERFACE);
+ HEALTH_MANAGER);
dbus_connection_unref(adapter->conn);
btd_adapter_unref(adapter->btd_adapter);
adapters = g_slist_remove(adapters, adapter);
diff --git a/health/hdp_types.h b/health/hdp_types.h
index abbfbf3..aff116e 100644
--- a/health/hdp_types.h
+++ b/health/hdp_types.h
@@ -42,9 +42,10 @@
#define HDP_ERROR g_quark_from_static_string("hdp-error-quark")
-#define HEALTH_MANAGER_INTERFACE "org.bluez.HealthAdapter"
-#define HEALTH_DEVICE "org.bluez.HealthDevice"
-#define HEALTH_LINK "org.bluez.HealthLink"
+#define HEALTH_MANAGER "org.bluez.HealthAdapter"
+#define HEALTH_DEVICE "org.bluez.HealthDevice"
+#define HEALTH_LINK "org.bluez.HealthLink"
+#define HEALTH_AGENT "org.bluez.HealthAgent"
#define HDP_NO_PREFERENCE_DC 0x00
#define HDP_RELIABLE_DC 0x01
--
1.6.3.3
^ permalink raw reply related
* [PATCH 27/32] Change function name when retreiving remote SDP records
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-27-git-send-email-santoscadenas@gmail.com>
---
health/hdp.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index 1d626d7..cd344d0 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -266,7 +266,7 @@ static void fill_up_health_instances(DBusMessageIter *dict, gpointer data)
}
}
-static void sink_health_instances(sdp_list_t *recs, int err, gpointer user_data)
+static void health_records_found(sdp_list_t *recs, int err, gpointer user_data)
{
struct instances_aux *cb_data = user_data;
DBusMessage *msg = cb_data->msg;
@@ -314,7 +314,7 @@ static DBusMessage *get_health_instances(DBusConnection *conn,
bt_string2uuid(&uuid, HDP_UUID);
- if (bt_search_service(&src, &dst, &uuid, sink_health_instances,
+ if (bt_search_service(&src, &dst, &uuid, health_records_found,
cb_data, NULL) == 0)
return NULL;
--
1.6.3.3
^ permalink raw reply related
* [PATCH 26/32] Implement connect data channel callback
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-26-git-send-email-santoscadenas@gmail.com>
---
health/hdp.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++----
health/hdp_types.h | 5 ++++
2 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index a38cbe0..1d626d7 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -107,6 +107,13 @@ static struct hdp_link *find_health_link(struct hdp_adapter *adapter,
return NULL;
}
+static int hdp_supp_fts_mdepcmp(gconstpointer ft, gconstpointer mdepid)
+{
+ const struct hdp_supp_fts *fts = ft;
+
+ return bcmp(&fts->mdepid, mdepid, sizeof(uint8_t));
+}
+
static int hdp_instance_idcmp(gconstpointer instance, gconstpointer p)
{
const struct hdp_instance *hdpi = instance;
@@ -379,19 +386,65 @@ static void hdp_mcap_mdl_closed_cb(struct mcap_mdl *mdl, void *data)
debug("TODO: Incomplete callback, mdl closed");
}
-static uint8_t hdp_mcap_mdl_conn_req_cb(struct mcap_mdl *mdl, void *data)
+static uint8_t hdp_mcap_mdl_reconn_req_cb(struct mcap_mdl *mdl, void *data)
{
/* struct hdp_link *hdpl = data; */
debug("TODO: Incomplete callback, mdl connection request");
return MCAP_REQUEST_NOT_SUPPORTED;
}
-static uint8_t hdp_mcap_mdl_reconn_req_cb(struct mcap_mcl *mcl,
- uint8_t mdepid, uint16_t mdlid,
- uint8_t *conf, void *data)
+static uint8_t hdp_mcap_mdl_conn_req_cb(struct mcap_mcl *mcl, uint8_t mdepid,
+ uint16_t mdlid, uint8_t *conf, void *data)
{
- debug("TODO: Incomplete callback, mdl reconnection request");
- return MCAP_REQUEST_NOT_SUPPORTED;
+ struct hdp_link *hdpl = data;
+ struct hdp_supp_fts *f;
+ struct hdp_dc *dc;
+ uint8_t new_conf;
+ GSList *l;
+
+ l = g_slist_find_custom(hdpl->hdpi->config->supp_fts, &mdepid,
+ hdp_supp_fts_mdepcmp);
+ if (!l)
+ return MCAP_INVALID_MDEP;
+ f = l->data;
+
+ /* Check if is the first dc if so,
+ * only reliable configuration is allowed */
+ switch(*conf) {
+ case HDP_NO_PREFERENCE_DC:
+ if (f->role == HDP_SINK)
+ return MCAP_CONFIGURATION_REJECTED;
+ else
+ new_conf = HDP_RELIABLE_DC;
+ break;
+ case HDP_STREAMING_DC:
+ if (g_slist_length(hdpl->channels) == 0)
+ return MCAP_CONFIGURATION_REJECTED;
+ case HDP_RELIABLE_DC:
+ if (f->role == HDP_SOURCE)
+ return MCAP_CONFIGURATION_REJECTED;
+ new_conf = *conf;
+ break;
+ default:
+ /* Special case defined in HDP spec 3.4. When an invalid
+ * configuration is received we need close the MCL when
+ * we are still processing the callback. When MCL is
+ * closed in a callback the returned value won't be
+ * proccesed in MCAP */
+ /* TODO: Send MCL disconnection to agent */
+ g_dbus_unregister_interface(hdpl->hdpi->adapter->conn,
+ hdpl->path, HEALTH_LINK);
+ return MCAP_CONFIGURATION_REJECTED; /* not processed */
+ }
+ *conf = new_conf;
+
+ dc = g_new0(struct hdp_dc, 1);
+ dc->hdpl = hdpl;
+ dc->conf = *conf;
+ dc->mdlid = mdlid;
+
+ hdpl->ndc = dc;
+ return MCAP_SUCCESS;
}
static void health_link_free(struct hdp_link *hdpl)
@@ -406,6 +459,9 @@ static void health_link_free(struct hdp_link *hdpl)
if (hdpl->path)
g_free(hdpl->path);
+ if (hdpl->ndc)
+ g_free(hdpl->ndc);
+
hdpl->hdpi->hlink = g_slist_remove(hdpl->hdpi->hlink, hdpl);
g_free(hdpl);
}
diff --git a/health/hdp_types.h b/health/hdp_types.h
index 6d41e2b..abbfbf3 100644
--- a/health/hdp_types.h
+++ b/health/hdp_types.h
@@ -46,6 +46,10 @@
#define HEALTH_DEVICE "org.bluez.HealthDevice"
#define HEALTH_LINK "org.bluez.HealthLink"
+#define HDP_NO_PREFERENCE_DC 0x00
+#define HDP_RELIABLE_DC 0x01
+#define HDP_STREAMING_DC 0x02
+
typedef enum {
HDP_DIC_PARSE_ERROR,
HDP_DIC_ENTRY_PARSE_ERROR,
@@ -113,6 +117,7 @@ struct hdp_link {
GSList *channels; /* Data channels */
char *path; /* HDP link path */
uint32_t id; /* Health link id */
+ struct hdp_dc *ndc; /* Data channel negotiated */
};
struct hdp_dc {
--
1.6.3.3
^ permalink raw reply related
* [PATCH 25/32] Create new structure to manage data channels in HDP
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-25-git-send-email-santoscadenas@gmail.com>
---
health/hdp_types.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/health/hdp_types.h b/health/hdp_types.h
index 5db8e0d..6d41e2b 100644
--- a/health/hdp_types.h
+++ b/health/hdp_types.h
@@ -115,6 +115,13 @@ struct hdp_link {
uint32_t id; /* Health link id */
};
+struct hdp_dc {
+ struct hdp_link *hdpl; /* Health link */
+ struct mcap_mdl *mdl; /* MCAP MDL structure */
+ uint8_t conf; /* Requested conf */
+ uint16_t mdlid; /* MDL id */
+};
+
struct hdp_device {
DBusConnection *conn; /* For name listener handling */
struct btd_device *dev; /* Device reference */
--
1.6.3.3
^ permalink raw reply related
* [PATCH 24/32] Disconnect health link petition
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-24-git-send-email-santoscadenas@gmail.com>
---
health/hdp.c | 28 +++++++++++++++-------------
1 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index 25a644c..a38cbe0 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -401,13 +401,10 @@ static void health_link_free(struct hdp_link *hdpl)
if (hdpl->mcl) {
mcap_close_mcl(hdpl->mcl, FALSE);
mcap_mcl_unref(hdpl->mcl);
- hdpl->mcl = NULL;
}
- if (hdpl->path) {
+ if (hdpl->path)
g_free(hdpl->path);
- hdpl->path = NULL;
- }
hdpl->hdpi->hlink = g_slist_remove(hdpl->hdpi->hlink, hdpl);
g_free(hdpl);
@@ -623,32 +620,37 @@ static DBusMessage *hdp_disconnect(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
struct hdp_device *device = user_data;
- struct hdp_link *hlink;
+ struct hdp_link *hdpl;
const char *path;
- const gboolean *del;
+ gboolean cache;
if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
- DBUS_TYPE_BOOLEAN, &del,
- DBUS_TYPE_INVALID)){
+ DBUS_TYPE_BOOLEAN, &cache,
+ DBUS_TYPE_INVALID)){
return g_dbus_create_error(msg,
ERROR_INTERFACE ".InvalidArguments",
"Invalid arguments in method call");
}
- hlink = find_health_link(device->hdp_adapter, path);
- if (!hlink)
+ hdpl = find_health_link(device->hdp_adapter, path);
+ if (!hdpl)
return g_dbus_create_error(msg, ERROR_INTERFACE ".HealthError",
"Health link does not found");
- return g_dbus_create_error(msg, ERROR_INTERFACE ".HealthError",
- "Not yet implemented");
+ if (cache) {
+ mcap_close_mcl(hdpl->mcl, cache);
+ hdpl->closed = TRUE;
+ } else
+ g_dbus_unregister_interface(conn, hdpl->path, HEALTH_LINK);
+
+ return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
}
static GDBusMethodTable device_methods[] = {
{ "GetHealthInstances", "", "aa{sv}", get_health_instances,
G_DBUS_METHOD_FLAG_ASYNC },
{ "Connect", "uu", "o", hdp_connect, G_DBUS_METHOD_FLAG_ASYNC },
- { "Diconnect", "ob", "", hdp_disconnect },
+ { "Disconnect", "ob", "", hdp_disconnect },
{ NULL }
};
--
1.6.3.3
^ permalink raw reply related
* [PATCH 23/32] Avoid multiple links with the same device
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-23-git-send-email-santoscadenas@gmail.com>
---
health/hdp.c | 42 ++++++++++++++++++++++++++++++++++++++++--
1 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index 10621c4..25a644c 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -117,7 +117,8 @@ static int hdp_instance_idcmp(gconstpointer instance, gconstpointer p)
return -1;
}
-static int hdp_link_mcl_cmp(gconstpointer link, gconstpointer mcl){
+static int hdp_link_mcl_cmp(gconstpointer link, gconstpointer mcl)
+{
const struct hdp_link *hdpl = link;
if (hdpl->mcl == mcl)
@@ -125,6 +126,17 @@ static int hdp_link_mcl_cmp(gconstpointer link, gconstpointer mcl){
return -1;
}
+static int hdp_link_addr_cmp(gconstpointer link, gconstpointer addr)
+{
+ const struct hdp_link *hdpl = link;
+ const bdaddr_t *dst;
+ bdaddr_t mcl_addr;
+
+ dst = addr;
+ mcl_addr = mcap_mcl_get_addr(hdpl->mcl);
+ return bacmp(addr, &mcl_addr);
+}
+
static void set_health_link_path(struct hdp_link *hdpl)
{
char path[MAX_PATH_LENGTH + 1];
@@ -534,12 +546,27 @@ fail:
g_dbus_send_message(device->conn, reply);
}
+static struct hdp_link *get_health_link(struct hdp_instance *hdpi,
+ struct hdp_device *device)
+{
+ bdaddr_t dst;
+ GSList *l;
+
+ device_get_address(device->dev, &dst);
+ l = g_slist_find_custom(hdpi->hlink, &dst, hdp_link_addr_cmp);
+ if (l)
+ return l->data;
+ return NULL;
+}
+
static DBusMessage *hdp_connect(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
struct hdp_device *device = user_data;
struct hdp_connection_cb *cb_data;
struct btd_adapter *adapter;
+ struct hdp_instance *hdpi;
+ struct hdp_link *hdpl;
bdaddr_t src, dst;
uint32_t lid, rid;
uuid_t uuid;
@@ -561,9 +588,20 @@ static DBusMessage *hdp_connect(DBusConnection *conn,
ERROR_INTERFACE ".InvalidArguments",
"Invalid local health instance id");
+ hdpi = l->data;
+ hdpl = get_health_link(hdpi, device);
+ if (hdpl) {
+ if (hdpl->closed) {
+ debug("Need a reconection");
+ /* TODO: Reconnect */;
+ } else
+ return g_dbus_create_reply(msg, DBUS_TYPE_OBJECT_PATH,
+ &hdpl->path, DBUS_TYPE_INVALID);
+ }
+
cb_data = g_new0(struct hdp_connection_cb, 1);
cb_data->device = device;
- cb_data->hdpi = l->data;
+ cb_data->hdpi = hdpi;
cb_data->rem_id = rid;
cb_data->msg = dbus_message_ref(msg);
--
1.6.3.3
^ permalink raw reply related
* [PATCH 22/32] Initial work for disconnecting health links
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-22-git-send-email-santoscadenas@gmail.com>
---
health/hdp.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 45 insertions(+), 0 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index cb05324..10621c4 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -88,6 +88,25 @@ static struct hdp_device *find_device(GSList *devices, struct btd_device *dev)
return NULL;
}
+static struct hdp_link *find_health_link(struct hdp_adapter *adapter,
+ const char *link_path)
+{
+ struct hdp_instance *hdpi;
+ struct hdp_link *hdpl;
+ GSList *l, *ll;
+
+ for (l = adapter->instances; l; l = l->next) {
+ hdpi = l->data;
+ for (ll = hdpi->hlink; ll; ll = ll->next) {
+ hdpl = ll->data;
+ if (!strcmp(hdpl->path, link_path))
+ return hdpl;
+ }
+ }
+
+ return NULL;
+}
+
static int hdp_instance_idcmp(gconstpointer instance, gconstpointer p)
{
const struct hdp_instance *hdpi = instance;
@@ -562,10 +581,36 @@ static DBusMessage *hdp_connect(DBusConnection *conn,
"Error getting remote information");
}
+static DBusMessage *hdp_disconnect(DBusConnection *conn,
+ DBusMessage *msg, void *user_data)
+{
+ struct hdp_device *device = user_data;
+ struct hdp_link *hlink;
+ const char *path;
+ const gboolean *del;
+
+ if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
+ DBUS_TYPE_BOOLEAN, &del,
+ DBUS_TYPE_INVALID)){
+ return g_dbus_create_error(msg,
+ ERROR_INTERFACE ".InvalidArguments",
+ "Invalid arguments in method call");
+ }
+
+ hlink = find_health_link(device->hdp_adapter, path);
+ if (!hlink)
+ return g_dbus_create_error(msg, ERROR_INTERFACE ".HealthError",
+ "Health link does not found");
+
+ return g_dbus_create_error(msg, ERROR_INTERFACE ".HealthError",
+ "Not yet implemented");
+}
+
static GDBusMethodTable device_methods[] = {
{ "GetHealthInstances", "", "aa{sv}", get_health_instances,
G_DBUS_METHOD_FLAG_ASYNC },
{ "Connect", "uu", "o", hdp_connect, G_DBUS_METHOD_FLAG_ASYNC },
+ { "Diconnect", "ob", "", hdp_disconnect },
{ NULL }
};
--
1.6.3.3
^ permalink raw reply related
* [PATCH 21/32] Manage mcap disconnections and reconnections
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-21-git-send-email-santoscadenas@gmail.com>
---
health/hdp.c | 58 ++++++++++++++++++++++++++++++++++++++++++---------
health/hdp_types.h | 5 ++++
health/hdp_util.c | 19 ++++++++++++----
3 files changed, 66 insertions(+), 16 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index 19ff2f9..cb05324 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -37,10 +37,6 @@
#include "../src/dbus-common.h"
-#define HEALTH_MANAGER_INTERFACE "org.bluez.HealthAdapter"
-#define HEALTH_DEVICE "org.bluez.HealthDevice"
-#define HEALTH_LINK "org.bluez.HealthLink"
-
static GSList *adapters = NULL;
static GSList *devices = NULL;
@@ -102,6 +98,14 @@ static int hdp_instance_idcmp(gconstpointer instance, gconstpointer p)
return -1;
}
+static int hdp_link_mcl_cmp(gconstpointer link, gconstpointer mcl){
+ const struct hdp_link *hdpl = link;
+
+ if (hdpl->mcl == mcl)
+ return 0;
+ return -1;
+}
+
static void set_health_link_path(struct hdp_link *hdpl)
{
char path[MAX_PATH_LENGTH + 1];
@@ -617,7 +621,7 @@ static void mcl_connected(struct mcap_mcl *mcl, gpointer data)
struct hdp_link *hdpl;
GError *err = NULL;
- debug("TODO: implement mcl_connected");
+ debug("mcl_connected");
hdpl = create_health_link(hdpi, mcl, &err);
if (err)
return;
@@ -627,20 +631,52 @@ static void mcl_connected(struct mcap_mcl *mcl, gpointer data)
static void mcl_reconnected(struct mcap_mcl *mcl, gpointer data)
{
- /* struct hdp_instance *hdpi = data; */
- debug("TODO: implement mcl_reconnected");
+ struct hdp_instance *hdpi = data;
+ struct hdp_link *hdpl;
+ GSList *l;
+
+ debug("mcl_reconnected");
+ l = g_slist_find_custom(hdpi->hlink, mcl, hdp_link_mcl_cmp);
+ if (l) {
+ hdpl = l->data;
+ hdpl->closed = FALSE;
+ return;
+ }
+
+ mcl_connected(mcl, data);
}
static void mcl_disconnected(struct mcap_mcl *mcl, gpointer data)
{
- /* struct hdp_instance *hdpi = data; */
- debug("TODO: implement mcl_disconnected");
+ struct hdp_instance *hdpi = data;
+ struct hdp_link *hdpl;
+ GSList *l;
+
+ debug("mcl_disconnected");
+ l = g_slist_find_custom(hdpi->hlink, mcl, hdp_link_mcl_cmp);
+ if (!l)
+ return;
+
+ hdpl = l->data;
+ hdpl->closed = TRUE;
}
static void mcl_uncached(struct mcap_mcl *mcl, gpointer data)
{
- /* struct hdp_instance *hdpi = data; */
- debug("TODO: implement mcl_uncached");
+ struct hdp_instance *hdpi = data;
+ struct hdp_link *hdpl;
+ GSList *l;
+
+ debug("mcl_uncached");
+ l = g_slist_find_custom(hdpi->hlink, mcl, hdp_link_mcl_cmp);
+ if (!l)
+ return;
+
+ hdpl = l->data;
+
+ hdpi->hlink = g_slist_remove(hdpi->hlink, hdpl);
+ g_dbus_unregister_interface(hdpi->adapter->conn, hdpl->path,
+ HEALTH_LINK);
}
static DBusMessage *hdp_create_instance(DBusConnection *conn,
diff --git a/health/hdp_types.h b/health/hdp_types.h
index 79419a1..5db8e0d 100644
--- a/health/hdp_types.h
+++ b/health/hdp_types.h
@@ -42,6 +42,10 @@
#define HDP_ERROR g_quark_from_static_string("hdp-error-quark")
+#define HEALTH_MANAGER_INTERFACE "org.bluez.HealthAdapter"
+#define HEALTH_DEVICE "org.bluez.HealthDevice"
+#define HEALTH_LINK "org.bluez.HealthLink"
+
typedef enum {
HDP_DIC_PARSE_ERROR,
HDP_DIC_ENTRY_PARSE_ERROR,
@@ -105,6 +109,7 @@ struct hdp_instance {
struct hdp_link {
struct hdp_instance *hdpi; /* HDP session */
struct mcap_mcl *mcl; /* MCAP mcl */
+ gboolean closed; /* MCL is closed but cached */
GSList *channels; /* Data channels */
char *path; /* HDP link path */
uint32_t id; /* Health link id */
diff --git a/health/hdp_util.c b/health/hdp_util.c
index c54f55c..72d06df 100644
--- a/health/hdp_util.c
+++ b/health/hdp_util.c
@@ -109,15 +109,24 @@ static void free_config(struct hdp_config *config)
g_free(config);
}
+static void hdp_link_unregister(gpointer link, gpointer data)
+{
+ struct hdp_link *hdpl = link;
+ struct hdp_instance *hdpi = hdpl->hdpi;
+
+ hdpi->hlink = g_slist_remove(hdpi->hlink, hdpl);
+ g_dbus_unregister_interface(hdpi->adapter->conn, hdpl->path,
+ HEALTH_LINK);
+}
+
void hdp_instance_free(struct hdp_instance *hdpi)
{
debug("HDP instance %d is deleted", hdpi->id);
/* TODO: Complete this part */
- /*
- g_slist_foreach(hdpi->devices, hdp_device_unregister, NULL);
- g_slist_free(hdpi->devices);
- hdpi->devices = NULL;
- */
+
+ g_slist_foreach(hdpi->hlink, hdp_link_unregister, NULL);
+ g_slist_free(hdpi->hlink);
+ hdpi->hlink = NULL;
if (hdpi->dbus_watcher)
g_dbus_remove_watch(hdpi->adapter->conn, hdpi->dbus_watcher);
--
1.6.3.3
^ permalink raw reply related
* [PATCH 20/32] Release health link resources
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-20-git-send-email-santoscadenas@gmail.com>
---
health/hdp.c | 27 ++++++++++++++++++++++++---
1 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index 70ffde5..19ff2f9 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -359,10 +359,32 @@ static uint8_t hdp_mcap_mdl_reconn_req_cb(struct mcap_mcl *mcl,
return MCAP_REQUEST_NOT_SUPPORTED;
}
+static void health_link_free(struct hdp_link *hdpl)
+{
+ /*TODO: Release structures related with Data Channels */
+
+ if (hdpl->mcl) {
+ mcap_close_mcl(hdpl->mcl, FALSE);
+ mcap_mcl_unref(hdpl->mcl);
+ hdpl->mcl = NULL;
+ }
+
+ if (hdpl->path) {
+ g_free(hdpl->path);
+ hdpl->path = NULL;
+ }
+
+ hdpl->hdpi->hlink = g_slist_remove(hdpl->hdpi->hlink, hdpl);
+ g_free(hdpl);
+}
+
static void health_link_path_unregister(void *data)
{
- /* struct hdp_link *hdpl = data */
- /* TODO: Unregister hdp_link*/
+ struct hdp_link *hdpl = data;
+
+ debug("Unregistered interface %s on path %s", HEALTH_LINK, hdpl->path);
+
+ health_link_free(hdpl);
}
static GDBusMethodTable health_link_methods[] = {
@@ -399,7 +421,6 @@ static struct hdp_link *create_health_link(struct hdp_instance *hdpi,
g_set_error(err, HDP_ERROR, HDP_UNSPECIFIED_ERROR,
"Cant register the health link in the bus");
- /* TODO create a function to free health link correctly */
/* MCAP will close the MCL and won't cache it if we didn't
* increase the MCL reference counter during the callback. */
mcap_mcl_unref(hdpl->mcl);
--
1.6.3.3
^ permalink raw reply related
* [PATCH 19/32] Remove hdp_device pointer from health link struct
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-19-git-send-email-santoscadenas@gmail.com>
---
health/hdp.c | 6 +++---
health/hdp_types.h | 1 -
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index 0a98118..70ffde5 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -391,9 +391,9 @@ static struct hdp_link *create_health_link(struct hdp_instance *hdpi,
if (*err)
return NULL;
- if (g_dbus_register_interface(hdpl->dev->conn, hdpl->path, HEALTH_LINK,
- health_link_methods, NULL, NULL,
- hdpl, health_link_path_unregister))
+ if (g_dbus_register_interface(hdpl->hdpi->adapter->conn, hdpl->path,
+ HEALTH_LINK, health_link_methods,
+ NULL, NULL, hdpl, health_link_path_unregister))
return hdpl;
g_set_error(err, HDP_ERROR, HDP_UNSPECIFIED_ERROR,
diff --git a/health/hdp_types.h b/health/hdp_types.h
index 05bfbfe..79419a1 100644
--- a/health/hdp_types.h
+++ b/health/hdp_types.h
@@ -104,7 +104,6 @@ struct hdp_instance {
struct hdp_link {
struct hdp_instance *hdpi; /* HDP session */
- struct hdp_device *dev; /* Health Device */
struct mcap_mcl *mcl; /* MCAP mcl */
GSList *channels; /* Data channels */
char *path; /* HDP link path */
--
1.6.3.3
^ permalink raw reply related
* [PATCH 18/32] Unify the creation of health links
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-18-git-send-email-santoscadenas@gmail.com>
---
health/hdp.c | 82 ++++++++++++++++++++++++++++++++++++---------------------
1 files changed, 52 insertions(+), 30 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index 7f25144..0a98118 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -369,28 +369,17 @@ static GDBusMethodTable health_link_methods[] = {
{ NULL }
};
-static void hdp_mcl_connect_cb(struct mcap_mcl *mcl, GError *err, void *data)
+static struct hdp_link *create_health_link(struct hdp_instance *hdpi,
+ struct mcap_mcl *mcl, GError **err)
{
- struct hdp_connection_cb *cb_data = data;
- struct hdp_device *device = cb_data->device;
- struct hdp_instance *hdpi = cb_data->hdpi;
- DBusMessage *msg = cb_data->msg;
- struct hdp_link *hdpl = NULL;
- GError *cberr = NULL;
- DBusMessage *reply;
-
- g_free(cb_data);
-
- if (err)
- goto fail;
+ struct hdp_link *hdpl;
hdpl = g_new0(struct hdp_link, 1);
hdpl->hdpi = hdpi;
- hdpl->dev = device;
hdpl->mcl = mcap_mcl_ref(mcl);
set_health_link_path(hdpl);
- mcap_mcl_set_cb(mcl, &cberr, hdpl,
+ mcap_mcl_set_cb(mcl, err, hdpl,
MCAP_MDL_CB_CONNECTED, hdp_mcap_mdl_connected_cb,
MCAP_MDL_CB_CLOSED, hdp_mcap_mdl_closed_cb,
MCAP_MDL_CB_DELETED, hdp_mcap_mdl_deleted_cb,
@@ -399,16 +388,46 @@ static void hdp_mcl_connect_cb(struct mcap_mcl *mcl, GError *err, void *data)
MCAP_MDL_CB_REMOTE_RECONN_REQ, hdp_mcap_mdl_reconn_req_cb,
MCAP_MDL_CB_INVALID);
- if (cberr)
- goto fail;
+ if (*err)
+ return NULL;
- if (!g_dbus_register_interface(hdpl->dev->conn, hdpl->path, HEALTH_LINK,
+ if (g_dbus_register_interface(hdpl->dev->conn, hdpl->path, HEALTH_LINK,
health_link_methods, NULL, NULL,
- hdpl, health_link_path_unregister)) {
- error("D-Bus failed to register %s interface to %s",
+ hdpl, health_link_path_unregister))
+ return hdpl;
+
+ g_set_error(err, HDP_ERROR, HDP_UNSPECIFIED_ERROR,
+ "Cant register the health link in the bus");
+
+ /* TODO create a function to free health link correctly */
+ /* MCAP will close the MCL and won't cache it if we didn't
+ * increase the MCL reference counter during the callback. */
+ mcap_mcl_unref(hdpl->mcl);
+ g_free(hdpl->path);
+ g_free(hdpl);
+ error("D-Bus failed to register %s interface to %s",
HEALTH_LINK, hdpl->path);
+ return NULL;
+}
+
+static void hdp_mcl_connect_cb(struct mcap_mcl *mcl, GError *err, void *data)
+{
+ struct hdp_connection_cb *cb_data = data;
+ struct hdp_device *device = cb_data->device;
+ struct hdp_instance *hdpi = cb_data->hdpi;
+ DBusMessage *msg = cb_data->msg;
+ struct hdp_link *hdpl = NULL;
+ GError *gerr = NULL;
+ DBusMessage *reply;
+
+ g_free(cb_data);
+
+ if (err)
+ goto fail;
+
+ hdpl = create_health_link(hdpi, mcl, &gerr);
+ if (gerr)
goto fail;
- }
hdpi->hlink = g_slist_prepend(hdpi->hlink, hdpl);
reply = g_dbus_create_reply(msg, DBUS_TYPE_OBJECT_PATH, &hdpl->path,
@@ -417,14 +436,9 @@ static void hdp_mcl_connect_cb(struct mcap_mcl *mcl, GError *err, void *data)
return;
fail:
reply = g_dbus_create_error(msg, ERROR_INTERFACE ".HdpError",
- (err ? err->message : cberr->message));
- if (cberr) {
- /* MCAP will close the MCL and won't cache it if we didn't
- * increase the MCL reference counter during the callback. */
- mcap_mcl_unref(hdpl->mcl);
- g_free(hdpl->path);
- g_free(hdpl);
- g_error_free(cberr);
+ (err ? err->message : gerr->message));
+ if (gerr) {
+ g_error_free(gerr);
}
g_dbus_send_message(device->conn, reply);
@@ -578,8 +592,16 @@ static void client_disconnected(DBusConnection *connection, void *user_data)
static void mcl_connected(struct mcap_mcl *mcl, gpointer data)
{
- /* struct hdp_instance *hdpi = data; */
+ struct hdp_instance *hdpi = data;
+ struct hdp_link *hdpl;
+ GError *err = NULL;
+
debug("TODO: implement mcl_connected");
+ hdpl = create_health_link(hdpi, mcl, &err);
+ if (err)
+ return;
+ /* TODO: Send the notification to the Agent */
+ hdpi->hlink = g_slist_prepend(hdpi->hlink, hdpl);
}
static void mcl_reconnected(struct mcap_mcl *mcl, gpointer data)
--
1.6.3.3
^ permalink raw reply related
* [PATCH 17/32] Analize remote record looking for psm
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-17-git-send-email-santoscadenas@gmail.com>
---
health/hdp.c | 1 +
health/hdp_util.c | 52 +++++++++++++++++++++++++++++++++++++---------------
2 files changed, 38 insertions(+), 15 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index 88e7235..7f25144 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -462,6 +462,7 @@ static void connect_health_instance(sdp_list_t *recs, int err, gpointer data)
goto fail;
}
+ info("psm = 0x%x, version = 0x%x", ccpsm, version);
device_get_address(device->dev, &dst);
mcap_create_mcl(hdpi->mi, &dst, ccpsm, &gerr, hdp_mcl_connect_cb,
cb_data);
diff --git a/health/hdp_util.c b/health/hdp_util.c
index 5ba61dc..c54f55c 100644
--- a/health/hdp_util.c
+++ b/health/hdp_util.c
@@ -884,29 +884,51 @@ gboolean hdp_get_data_exchange_spec(const sdp_record_t *rec, guint8 *val)
return TRUE;
}
+static gboolean get_prot_desc_entry(sdp_data_t *entry, int type, guint16 *val)
+{
+ sdp_data_t *iter;
+ int proto;
+
+ if (!entry || (entry->dtd != SDP_SEQ8))
+ return FALSE;
+
+ iter = entry->val.dataseq;
+ if (!(iter->dtd & SDP_UUID_UNSPEC))
+ return FALSE;
+
+ proto = sdp_uuid_to_proto(&iter->val.uuid);
+ if (proto != type)
+ return FALSE;
+
+ iter = iter->next;
+ if (iter->dtd != SDP_UINT16)
+ return FALSE;
+ if (val) {
+ *val = iter->val.uint16;
+ }
+ return TRUE;
+}
+
gboolean hdp_get_prot_desc_list(const sdp_record_t *rec, guint16 *psm,
guint16 *version)
{
- if (!(psm || version))
+ sdp_data_t *pdl, *p0, *p1;
+
+ if (!psm && !version)
return TRUE;
- /* TODO:
- sdp_data_t *pdl, *l;
+ pdl = sdp_data_get(rec, SDP_ATTR_PROTO_DESC_LIST);
+ if (pdl->dtd != SDP_SEQ8)
+ return FALSE;
+
+ p0 = pdl->val.dataseq;
- exspec = sdp_data_get(rec, SDP_ATTR_PROTO_DESC_LIST);
- if (exspec->dtd != SDP_SEQ8)
+ if (!get_prot_desc_entry(p0, L2CAP_UUID, psm))
+ return FALSE;
+ p1 = p0->next;
+ if (!get_prot_desc_entry(p1, MCAP_CTRL_UUID, version))
return FALSE;
- for (l = pdl->val.dataseq; l; l = l->next) {
- if (l->dtd != SDP_SEQ8)
- continue;
- epl = get_feature(epl, l->val.dataseq);
- }
- */
- if (psm)
- *psm = 0x1001;
- if (version)
- *version = 0x0100;
return TRUE;
}
--
1.6.3.3
^ permalink raw reply related
* [PATCH 16/32] Register Health link int the bus when MCL is connected
From: Jose Antonio Santos Cadenas @ 2010-06-02 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
In-Reply-To: <1275484768-25838-16-git-send-email-santoscadenas@gmail.com>
---
health/hdp.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++-----
health/hdp_types.h | 12 +++++++++-
2 files changed, 68 insertions(+), 8 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index 0d3265f..88e7235 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -37,8 +37,9 @@
#include "../src/dbus-common.h"
-#define HEALTH_MANAGER_INTERFACE "org.bluez.HealthAdapter"
-#define HEALTH_DEVICE "org.bluez.HealthDevice"
+#define HEALTH_MANAGER_INTERFACE "org.bluez.HealthAdapter"
+#define HEALTH_DEVICE "org.bluez.HealthDevice"
+#define HEALTH_LINK "org.bluez.HealthLink"
static GSList *adapters = NULL;
static GSList *devices = NULL;
@@ -101,6 +102,18 @@ static int hdp_instance_idcmp(gconstpointer instance, gconstpointer p)
return -1;
}
+static void set_health_link_path(struct hdp_link *hdpl)
+{
+ char path[MAX_PATH_LENGTH + 1];
+
+ hdpl->id = hdpl->hdpi->hlc++;
+ snprintf(path, MAX_PATH_LENGTH, "%s/health_link_%d_%d",
+ adapter_get_path(hdpl->hdpi->adapter->btd_adapter),
+ hdpl->hdpi->id, hdpl->id);
+
+ hdpl->path = g_strdup(path);
+}
+
static void fill_up_one_spec(DBusMessageIter *dict, gpointer data)
{
struct hdp_feature *feature = data;
@@ -346,12 +359,23 @@ static uint8_t hdp_mcap_mdl_reconn_req_cb(struct mcap_mcl *mcl,
return MCAP_REQUEST_NOT_SUPPORTED;
}
+static void health_link_path_unregister(void *data)
+{
+ /* struct hdp_link *hdpl = data */
+ /* TODO: Unregister hdp_link*/
+}
+
+static GDBusMethodTable health_link_methods[] = {
+ { NULL }
+};
+
static void hdp_mcl_connect_cb(struct mcap_mcl *mcl, GError *err, void *data)
{
struct hdp_connection_cb *cb_data = data;
struct hdp_device *device = cb_data->device;
- /* struct hdp_instance *hdpi = cb_data->hdpi; */
+ struct hdp_instance *hdpi = cb_data->hdpi;
DBusMessage *msg = cb_data->msg;
+ struct hdp_link *hdpl = NULL;
GError *cberr = NULL;
DBusMessage *reply;
@@ -360,8 +384,13 @@ static void hdp_mcl_connect_cb(struct mcap_mcl *mcl, GError *err, void *data)
if (err)
goto fail;
- /* Create and Register HealthLink interface */
- mcap_mcl_set_cb(mcl, &cberr, NULL /*health_link*/,
+ hdpl = g_new0(struct hdp_link, 1);
+ hdpl->hdpi = hdpi;
+ hdpl->dev = device;
+ hdpl->mcl = mcap_mcl_ref(mcl);
+ set_health_link_path(hdpl);
+
+ mcap_mcl_set_cb(mcl, &cberr, hdpl,
MCAP_MDL_CB_CONNECTED, hdp_mcap_mdl_connected_cb,
MCAP_MDL_CB_CLOSED, hdp_mcap_mdl_closed_cb,
MCAP_MDL_CB_DELETED, hdp_mcap_mdl_deleted_cb,
@@ -369,14 +398,35 @@ static void hdp_mcl_connect_cb(struct mcap_mcl *mcl, GError *err, void *data)
MCAP_MDL_CB_REMOTE_CONN_REQ, hdp_mcap_mdl_conn_req_cb,
MCAP_MDL_CB_REMOTE_RECONN_REQ, hdp_mcap_mdl_reconn_req_cb,
MCAP_MDL_CB_INVALID);
+
if (cberr)
goto fail;
+
+ if (!g_dbus_register_interface(hdpl->dev->conn, hdpl->path, HEALTH_LINK,
+ health_link_methods, NULL, NULL,
+ hdpl, health_link_path_unregister)) {
+ error("D-Bus failed to register %s interface to %s",
+ HEALTH_LINK, hdpl->path);
+ goto fail;
+ }
+
+ hdpi->hlink = g_slist_prepend(hdpi->hlink, hdpl);
+ reply = g_dbus_create_reply(msg, DBUS_TYPE_OBJECT_PATH, &hdpl->path,
+ DBUS_TYPE_INVALID);
+ g_dbus_send_message(device->conn, reply);
return;
fail:
reply = g_dbus_create_error(msg, ERROR_INTERFACE ".HdpError",
(err ? err->message : cberr->message));
- if (!cberr)
+ if (cberr) {
+ /* MCAP will close the MCL and won't cache it if we didn't
+ * increase the MCL reference counter during the callback. */
+ mcap_mcl_unref(hdpl->mcl);
+ g_free(hdpl->path);
+ g_free(hdpl);
g_error_free(cberr);
+ }
+
g_dbus_send_message(device->conn, reply);
}
@@ -421,8 +471,8 @@ static void connect_health_instance(sdp_list_t *recs, int err, gpointer data)
reply = g_dbus_create_error(msg, ERROR_INTERFACE ".HealthError",
"Error getting remote protocol descriptor list");
fail:
- g_dbus_send_message(device->conn, reply);
g_free(cb_data);
+ g_dbus_send_message(device->conn, reply);
}
static DBusMessage *hdp_connect(DBusConnection *conn,
diff --git a/health/hdp_types.h b/health/hdp_types.h
index 3bab4ea..05bfbfe 100644
--- a/health/hdp_types.h
+++ b/health/hdp_types.h
@@ -99,10 +99,20 @@ struct hdp_instance {
struct hdp_config *config; /* Configuration */
uint32_t sdp_handler; /* SDP record handler */
guint dbus_watcher; /* Client D-Bus conn watcher */
+ uint16_t hlc; /* Health link id. counter */
+};
+
+struct hdp_link {
+ struct hdp_instance *hdpi; /* HDP session */
+ struct hdp_device *dev; /* Health Device */
+ struct mcap_mcl *mcl; /* MCAP mcl */
+ GSList *channels; /* Data channels */
+ char *path; /* HDP link path */
+ uint32_t id; /* Health link id */
};
struct hdp_device {
- DBusConnection *conn; /* for name listener handling */
+ DBusConnection *conn; /* For name listener handling */
struct btd_device *dev; /* Device reference */
struct hdp_adapter *hdp_adapter; /* hdp_adapater */
};
--
1.6.3.3
^ 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