* btd_adapter_driver
From: Daniel Örstadius @ 2010-08-03 14:38 UTC (permalink / raw)
To: linux-bluetooth
Hi Marcel,
I have a question about the btd_adapter_driver struct.
The Maemo6 MCE plugin (which uses that struct) needs to be notified
when the adapter is powered on or off. One idea is to add a callback
to btd_adapter_driver to be called in adapter.c, but from discussion
with Johan the struct was intended to stay very simple, so that this
might not be an acceptable solution.
What would be a good way to handle it? Could listening to a raw hci
socket inside the driver be an option?
Thanks in advance,
Daniel
^ permalink raw reply
* Re: Enhancements to allow l2cap channel to use either AMP or BR/EDR
From: Tim Monahan-Mitchell @ 2010-08-03 13:40 UTC (permalink / raw)
To: Gustavo F. Padovan
Cc: Peter Krystad, Kevin Hayes, Marcel Holtmann,
tmonahan@codeaurora.org, David Vrabel, Inga Stotland,
linux-bluetooth@vger.kernel.org, rshaffer@codeaurora.org,
johan.hedberg@gmail.com
In-Reply-To: <20100803080719.GD20149@vigoh>
Hi, Gustavo and Marcel,
> Hi Peter,
>
> * Peter Krystad <pkrystad@codeaurora.org> [2010-08-02 18:30:34 -0700]:
>
>>
>> Hi Marcel and Kevin,
>>
>> >> >
>> >> > Agree that it should be done "in background" and that a size
>> >> threshold would be useful. But who evaluates that threshold? The
>> >> bluez kernel components, which essentially implement a transport
>> >> driver, should not be examining objects (files, phonebooks, etc) size
>> >> to see if this threshold is met. Therefore, it would seem Tim's
>> >> suggestion of having the profile send a 'prefer_amp' bit would be
>> >> useful, right?
>> >>
>> >> I would do something like "prefer_amp when over 100kb" or something.
>> >> And
>> >> then the kernel needs to count. Meaning the L2CAP layer could easily
>> >> count this by itself.
>> >
>> > What? Let's say the effect we want is that if the object is greater
>> > than, say, 10 megabytes, then we want to use AMP for the transfer.
>> > With your scheme, you want the kernel to count up to 10 megabytes,
>> > THEN switch over to AMP?
>> > No, you don't, he says rhetorically. :)
>>
>> If the policy was defined as "prefer_amp after n seconds" we would get
>> the desired effect without having to do any counting in the kernel.
>
> In both cases we have to take in account the total size of the file, or
> the estimated time of transfer, so we can avoid to move to AMP when the
> transfer is about to end.
Right! Don't want to switch to AMP just to push those last 100 bytes :)
Since the kernel does not know the total object size to be transferred,
(right?), I've yet to come up with any 'threshold' ideas the kernel can
make. Note that the transfer could be one huge file or many small ones.
--
Tim Monahan-Mitchell
Employee of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* Re: [PATCH][RFC] Fix SDP resolving segfault
From: Manuel Naranjo @ 2010-08-03 13:21 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: Johan Hedberg, BlueZ
In-Reply-To: <4C518373.6000706@aircable.net>
> I will give this a try and let you know. Maybe I can results before the
> Tuesday, but I doubt it.
As I said on IRC it keeps crashing. Here's the log, as you can see the
crash happened in a different place now.
bluetoothd[1842]: src/device.c:btd_device_ref() 0x9411de0: ref=1
bluetoothd[1842]: src/device.c:btd_device_ref() 0x9411de0: ref=2
bluetoothd[1842]: src/device.c:btd_device_ref() 0x9409570: ref=2
bluetoothd[1842]: src/device.c:btd_device_ref() 0x9408b60: ref=2
bluetoothd[1842]: src/adapter.c:create_device() C8:7E:75:D6:B3:B0
bluetoothd[1842]: src/adapter.c:adapter_create_device() C8:7E:75:D6:B3:B0
bluetoothd[1842]: src/device.c:device_create() Creating device
/org/bluez/1842/hci0/dev_C8_7E_75_D6_B3_B0
bluetoothd[1842]: src/device.c:btd_device_ref() 0x93fd258: ref=1
bluetoothd[1842]: src/device.c:btd_device_ref() 0x93fd258: ref=2
bluetoothd[1842]: src/device.c:btd_device_ref() 0x9404dc0: ref=3
bluetoothd[1842]: Discovery session 0x93fb658 with :1.58 activated
bluetoothd[1842]: src/adapter.c:session_ref() 0x93fb658: ref=1
bluetoothd[1842]: src/adapter.c:adapter_get_device() 00:25:47:2D:20:98
bluetoothd[1842]: src/adapter.c:session_unref() 0x93fb658: ref=0
bluetoothd[1842]: src/adapter.c:session_remove() Discovery session
0x93fb658 with :1.58 deactivated
bluetoothd[1842]: src/adapter.c:session_remove() Stopping discovery
bluetoothd[1842]: Stopping discovery
bluetoothd[1842]: /org/bluez/1842/hci0/dev_C8_7E_75_D6_B3_B0: error
updating services: Device or resource busy (16)
bluetoothd[1842]: src/device.c:btd_device_unref() 0x93fd258: ref=1
bluetoothd[1842]: Discovery session 0x93fd4b0 with :1.58 activated
bluetoothd[1842]: src/adapter.c:session_ref() 0x93fd4b0: ref=1
bluetoothd[1842]: /org/bluez/1842/hci0/dev_00_26_E2_3C_5E_05: error
updating services: Host is down (112)
bluetoothd[1842]: src/device.c:btd_device_unref() 0x9404dc0: ref=2
bluetoothd[1842]: src/adapter.c:adapter_get_device() 00:26:E2:3C:5E:05
bluetoothd[1842]: src/adapter.c:session_unref() 0x93fd4b0: ref=0
bluetoothd[1842]: src/adapter.c:session_remove() Discovery session
0x93fd4b0 with :1.58 deactivated
bluetoothd[1842]: src/adapter.c:session_remove() Stopping discovery
bluetoothd[1842]: Stopping discovery
bluetoothd[1842]: /org/bluez/1842/hci0/dev_00_1C_D6_B1_58_1B: error
updating services: Device or resource busy (16)
bluetoothd[1842]: src/device.c:btd_device_unref() 0x9409570: ref=1
bluetoothd[1842]: Discovery session 0x93f9f80 with :1.58 activated
bluetoothd[1842]: src/adapter.c:session_ref() 0x93f9f80: ref=1
bluetoothd[1842]: src/adapter.c:adapter_remove_connection() Removing
temporary device /org/bluez/1842/hci0/dev_00_25_47_2D_20_98
bluetoothd[1842]: src/device.c:device_remove() Removing device
/org/bluez/1842/hci0/dev_00_25_47_2D_20_98
bluetoothd[1842]: src/device.c:btd_device_unref() 0x9411120: ref=1
bluetoothd[1842]: src/device.c:btd_device_unref() 0x9411120: ref=0
bluetoothd[1842]: src/device.c:device_free() 0x9411120
bluetoothd[1842]: src/adapter.c:adapter_get_device() 00:17:D5:65:D0:3D
bluetoothd[1842]: src/adapter.c:session_unref() 0x93f9f80: ref=0
bluetoothd[1842]: src/adapter.c:session_remove() Discovery session
0x93f9f80 with :1.58 deactivated
bluetoothd[1842]: src/adapter.c:session_remove() Stopping discovery
bluetoothd[1842]: Stopping discovery
bluetoothd[1842]: Discovery session 0x9410560 with :1.58 activated
bluetoothd[1842]: src/adapter.c:session_ref() 0x9410560: ref=1
bluetoothd[1842]: src/device.c:device_probe_drivers() Probe drivers
for /org/bluez/1842/hci0/dev_00_17_D5_65_D0_3D
bluetoothd[1842]: serial/manager.c:serial_probe() path
/org/bluez/1842/hci0/dev_00_17_D5_65_D0_3D:
00001105-0000-1000-8000-00805f9b34fb
bluetoothd[1842]: serial/port.c:create_serial_device() Registered
interface org.bluez.Serial on path
/org/bluez/1842/hci0/dev_00_17_D5_65_D0_3D
bluetoothd[1842]: src/device.c:btd_device_unref() 0x9408b60: ref=1
Segmentation fault
--
Manuel Francisco Naranjo
Software Department Argentina
Wireless Cables Inc
www.aircable.net
cel: +5493412010019
skype: naranjomanuelfrancisco
^ permalink raw reply
* Re: Enhancements to allow l2cap channel to use either AMP or BR/EDR
From: David Vrabel @ 2010-08-03 12:59 UTC (permalink / raw)
To: Marcel Holtmann
Cc: tmonahan, Inga Stotland, linux-bluetooth, rshaffer, johan.hedberg
In-Reply-To: <1280775200.12579.30.camel@localhost.localdomain>
Marcel Holtmann wrote:
>
> That said, nothing stops us from allowing to make our AMP polices
> dynamic and allow the application to change it threshold during
> lifetime. For example it starts out as BR/EDR only and then during the
> usage of the link it decides that now AMP preferred would make sense. In
> that sense you would have manual switching to some level.
If the policy is dynamic throughout the lifetime of the connection then
that's okay for best effort links.
I think applications (especially those that stream real-time data at
high rates) will need to know the currently available bandwidth. This
will be useful for even simple file transfer profiles -- if that file
transfer is going to take 100x as long the user probably want to know
about this so they can (for example) turn HS mode on the other device.
This information could be conveyed in some sort of "channel move
complete" event or a more generic "available bandwidth changed" event.
Applications requiring guaranteed links will need some way of supplying
extended flow specifications.
David
--
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park, Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ http://www.csr.com/
Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
^ permalink raw reply
* Re: [PATCH] Notify to device drivers when the SDP records change
From: José Antonio Santos Cadenas @ 2010-08-03 12:43 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <AANLkTin9WnU0v2NqS5cAjRxyhipJCBYX0=KWponiCk14@mail.gmail.com>
El Tuesday 03 August 2010 13:42:15 Luiz Augusto von Dentz escribió:
> Hi,
>
> On Tue, Aug 3, 2010 at 12:12 PM, José Antonio Santos Cadenas
>
> <santoscadenas@gmail.com> wrote:
> > The different with a2dp is that the application will decide the end point
> > to connect to, because there can be more than one matching the source.
> > This end point is represented by a path that is passed to the
> > HealthAgent when is discovered. If the remote record changes it is
> > possible that new end points are available so the application needs
> > something to discover the available end points. That is why I think that
> > we need something like this.
>
> I guess we already suggest you guys to take a look in the
> MediaEndpoint interface, which can be found here:
>
> http://gitorious.org/~vudentz/bluez/vudentzs-clone/commit/2af3d2821f0aaafae
> 12be10522c82dea6a701688.patch
>
> It doesn't expose the remote endpoints it basically do the best
> matching, if the application requesting the connection has local
> endpoint they have priority, and then the 'agent' or in case of audio
> the endpoint will be notified with the remote endpoint capabilities.
>
> Also while looking the health API it seems you guys missed a very
> important point, HealthAgent only knows about the remote services
> since there is no signal for announcing a new HealthApplication nor
> the application knows the about HealthService, so the only way make
> this to work is that the HealthApplication and HealthAgent leaves in
> the same process but this defeats the point of having them separated.
Yes, this only works in the same process. It is a way for receiving callbacks
from the daemon.
> So either we have the applications and services being announced to
> everyone and then anyone can do the matching or we don't announce them
> at all and make bluetoothd do the matching itself, I guess the latter
> is better so we don't have to split the logic in many place (with
> potential code duplication + bugs).
At this point, I have the same opinion, Services can be notified using
signals. But Marcel thought that it will be better and will let the
programmers make less mistakes if only the services related to the application
are notified, that is why this information is send through the agent instead
of sending signals. Of course this can be changed if we think that is better.
>
> Another important thing is that dbus introspection does not have any
> way to notify changes, so it really need to be signal to notify
> applications about new services.
^ permalink raw reply
* Re: [PATCH] Notify to device drivers when the SDP records change
From: José Antonio Santos Cadenas @ 2010-08-03 12:33 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <AANLkTin9WnU0v2NqS5cAjRxyhipJCBYX0=KWponiCk14@mail.gmail.com>
Hi,
El Tuesday 03 August 2010 13:42:15 Luiz Augusto von Dentz escribió:
> Hi,
>
> On Tue, Aug 3, 2010 at 12:12 PM, José Antonio Santos Cadenas
>
> <santoscadenas@gmail.com> wrote:
> > The different with a2dp is that the application will decide the end point
> > to connect to, because there can be more than one matching the source.
> > This end point is represented by a path that is passed to the
> > HealthAgent when is discovered. If the remote record changes it is
> > possible that new end points are available so the application needs
> > something to discover the available end points. That is why I think that
> > we need something like this.
>
> I guess we already suggest you guys to take a look in the
> MediaEndpoint interface, which can be found here:
>
> http://gitorious.org/~vudentz/bluez/vudentzs-clone/commit/2af3d2821f0aaafae
> 12be10522c82dea6a701688.patch
>
> It doesn't expose the remote endpoints it basically do the best
> matching, if the application requesting the connection has local
> endpoint they have priority, and then the 'agent' or in case of audio
> the endpoint will be notified with the remote endpoint capabilities.
>
> Also while looking the health API it seems you guys missed a very
> important point, HealthAgent only knows about the remote services
> since there is no signal for announcing a new HealthApplication nor
> the application knows the about HealthService, so the only way make
> this to work is that the HealthApplication and HealthAgent leaves in
> the same process but this defeats the point of having them separated.
> So either we have the applications and services being announced to
> everyone and then anyone can do the matching or we don't announce them
> at all and make bluetoothd do the matching itself, I guess the latter
> is better so we don't have to split the logic in many place (with
> potential code duplication + bugs).
>
> Another important thing is that dbus introspection does not have any
> way to notify changes, so it really need to be signal to notify
> applications about new services.
Can we have an IRC session to talk about the API and continuing this
discussion? I think that this thread is changing too much from its original
topic.
Regards.
^ permalink raw reply
* Re: [PATCH] Notify to device drivers when the SDP records change
From: Luiz Augusto von Dentz @ 2010-08-03 11:42 UTC (permalink / raw)
To: José Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <201008031112.11556.santoscadenas@gmail.com>
Hi,
On Tue, Aug 3, 2010 at 12:12 PM, José Antonio Santos Cadenas
<santoscadenas@gmail.com> wrote:
> The different with a2dp is that the application will decide the end point to
> connect to, because there can be more than one matching the source. This end
> point is represented by a path that is passed to the HealthAgent when is
> discovered. If the remote record changes it is possible that new end points
> are available so the application needs something to discover the available end
> points. That is why I think that we need something like this.
I guess we already suggest you guys to take a look in the
MediaEndpoint interface, which can be found here:
http://gitorious.org/~vudentz/bluez/vudentzs-clone/commit/2af3d2821f0aaafae12be10522c82dea6a701688.patch
It doesn't expose the remote endpoints it basically do the best
matching, if the application requesting the connection has local
endpoint they have priority, and then the 'agent' or in case of audio
the endpoint will be notified with the remote endpoint capabilities.
Also while looking the health API it seems you guys missed a very
important point, HealthAgent only knows about the remote services
since there is no signal for announcing a new HealthApplication nor
the application knows the about HealthService, so the only way make
this to work is that the HealthApplication and HealthAgent leaves in
the same process but this defeats the point of having them separated.
So either we have the applications and services being announced to
everyone and then anyone can do the matching or we don't announce them
at all and make bluetoothd do the matching itself, I guess the latter
is better so we don't have to split the logic in many place (with
potential code duplication + bugs).
Another important thing is that dbus introspection does not have any
way to notify changes, so it really need to be signal to notify
applications about new services.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH] Fix NULL checking for pbap_get
From: Johan Hedberg @ 2010-08-03 9:54 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1280828987-15562-1-git-send-email-ext-jablonski.radoslaw@nokia.com>
Hi Radek,
On Tue, Aug 03, 2010, Radoslaw Jablonski wrote:
> Fixed NULL checking for name parameter - it shouldn't be there because
> handling for NULL name for some cases is already below. Also passing path
> which is NULL further makes no sense for PBAP so leaving with -EBADR if
> that kind of situation occurs after generating path.
> ---
> plugins/pbap.c | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
Thanks. The patch is now upstream.
Johan
^ permalink raw reply
* [PATCH] Fix NULL checking for pbap_get
From: Radoslaw Jablonski @ 2010-08-03 9:49 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Radoslaw Jablonski
Fixed NULL checking for name parameter - it shouldn't be there because
handling for NULL name for some cases is already below. Also passing path
which is NULL further makes no sense for PBAP so leaving with -EBADR if
that kind of situation occurs after generating path.
---
plugins/pbap.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/plugins/pbap.c b/plugins/pbap.c
index 7422a1e..c008e95 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -555,7 +555,7 @@ static int pbap_get(struct obex_session *os, obex_object_t *obj,
DBG("name %s type %s pbap %p", name, type, pbap);
- if (type == NULL || name == NULL)
+ if (type == NULL)
return -EBADR;
rsize = obex_aparam_read(os, obj, &buffer);
@@ -594,6 +594,9 @@ static int pbap_get(struct obex_session *os, obex_object_t *obj,
} else
return -EBADR;
+ if (path == NULL)
+ return -EBADR;
+
pbap->params = params;
ret = obex_get_stream_start(os, path);
--
1.7.0.4
^ permalink raw reply related
* Re: What is the motivation for conn->power_save
From: Liang Bao @ 2010-08-03 9:32 UTC (permalink / raw)
To: Nick Pelly; +Cc: Andrei Emeltchenko, Marcel Holtmann, linux-bluetooth
In-Reply-To: <AANLkTinIhVG_4CpxLR1p1OqVeNqOzXojY3stnkJJdQAp@mail.gmail.com>
Hi,
2010/7/9 Nick Pelly <npelly@google.com>
>
> On Thu, Jul 8, 2010 at 8:07 AM, Andrei Emeltchenko
> <andrei.emeltchenko.news@gmail.com> wrote:
> > Hi,
> >
> > On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org>=
wrote:
> >> Hi Nick,
> >>
> >>> >>> >> If I understand correctly, conn->power_save prevents the host =
stack
> >>> >>> >> from requesting active mode if it was not the host stack that
> >>> >>> >> requested sniff mode.
> >>> >>> >>
> >>> >>> >> I don't understand the motivation for this. If we have ACL dat=
a to
> >>> >>> >> send, then it seems like a good idea for the host stack to exp=
licitly
> >>> >>> >> request active mode, regardless of the reason that we entered =
sniff
> >>> >>> >> mode.
> >>> >>> >>
> >>> >>> >> We want to enter active mode more aggressively when setting up=
SCO
> >>> >>> >> connections, to avoid a 5 second delay with certain sniff mode=
s. But
> >>> >>> >> the conn->power_save code is getting in the way and doesn't ap=
pear to
> >>> >>> >> be useful in the first place.
> >>> >>> >
> >>> >>> > we have discussed this a few times. And if you lock through the=
code
> >>> >>> > history then you see that initially we just took devices out of=
sniff
> >>> >>> > mode if we had to send data. However with HID devices this fall=
s flat on
> >>> >>> > its face. They need to stay in control of sniff mode if they in=
itiated
> >>> >>> > it. Some of them crash and others just drain the battery. With =
sniff
> >>> >>> > mode you can send small amounts of data even while in sniff and=
for HID
> >>> >>> > that is sometimes used. So the remote side better not interfere=
.
> >>> >>> >
> >>> >>> > What we really need is a socket option where we can control thi=
s on a
> >>> >>> > per socket basis if we take devices out of sniff mode. And one =
extra
> >>> >>> > case might be when we try to establish a SCO channel, because t=
hen it is
> >>> >>> > clearly not an HID device. However even A2DP has this sort of p=
roblems
> >>> >>> > sometimes where the stream setup takes time.
> >>> >>>
> >>> >>> Makes sense. Thanks for the explanation.
> >>> >>
> >>> >> this means you will be working on a patch for this :)
> >>>
> >>> Actually, I want to put a patch together for a socket option to not
> >>> use power_save (so that we *always* exit sniff mode when sending ACL
> >>> data). We're seeing this problem with the Plantronics Voyager 855
> >>> which enters sniff mode during A2DP.
> >>>
> >>> Given that power_save is just for HID devices, my preferred design is
> >>> to disable power_save by default, and have an L2CAP socket option to
> >>> turn on power_save that would be used by HID L2CAP sockets.
> >>> Unfortunately this would require userspace HID code to use the new
> >>> socket option to keep current behavior. But it seems preferable to th=
e
> >>> alternative of having every single other L2CAP socket use a new socke=
t
> >>> option just to disable power_save for the sake of HID.
> >>
> >> actually you still mix up the meaning of the power_save option. From t=
he
> >> stack side, the automatic sniff mode is off by default. You have to
> >> enable via sysfs first. The power_save option is just to control when
> >> sniff mode is activated and the remote enters sniff mode, that we are
> >> not getting out of it if we have ACL data.
> >>
> >> In conclusion, I am fine with a socket option that allows to control
> >> sniff mode (preferable with interval details) and sniff subrate detail=
s
> >> so that we can use them for that ACL link.
> >>
> >> So go ahead and work on this. We can fix userspace easily since a new
> >> socket can be detected easily with newer kernels.
> >
> > We have found that some Nokia BT headsets stop working after idle
> > period when they enter
> > "power save" mode.
> >
> > Do we have any solution for this problem? So far I see good enough
> > patch :-)) below:
> >
> > http://android.git.kernel.org/?p=3Dkernel/common.git;a=3Dblobdiff;f=3Dn=
et/bluetooth/hci_conn.c;h=3Dfa8b412205cd89546b4a325c558c68040eeaf491;hp=3D0=
cf25114a3f576fc2788a549eb96d0087dd39b44;hb=3Dd8488237646920cd71ef43e5f3ae10=
01c6f4cf7b;hpb=3D3f68e5050c5ae559f56d5da9202cb88928d42b36
> >
> > - =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF || !conn->power_save)
> > + =A0 =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF /* || !conn->power_save =
*/)
> >
> > Nick have you done socket option for "power_save"?
>
> No. We'll do it once we need it for HID support.
Can we do this: try to do check against some existing fields in
structure hci_conn, for example device_class and then determine if
conn->power_save shall be used here? By doing this, applications in
user space can be saved from having knowledge how sniff mode works for
different devices.
Alternatively, is it possible to add another flag - I know the
structure is getting big so I agree adding something might not be a
good idea. :)
>
> Nick
> --
> 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 =A0http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: What is the motivation for conn->power_save
From: Liang Bao @ 2010-08-03 9:31 UTC (permalink / raw)
To: Nick Pelly; +Cc: Andrei Emeltchenko, Marcel Holtmann, linux-bluetooth
In-Reply-To: <AANLkTinIhVG_4CpxLR1p1OqVeNqOzXojY3stnkJJdQAp@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5173 bytes --]
2010/7/9 Nick Pelly <npelly@google.com>
> On Thu, Jul 8, 2010 at 8:07 AM, Andrei Emeltchenko
> <andrei.emeltchenko.news@gmail.com> wrote:
> > Hi,
> >
> > On Wed, Jan 13, 2010 at 10:04 AM, Marcel Holtmann <marcel@holtmann.org>
> wrote:
> >> Hi Nick,
> >>
> >>> >>> >> If I understand correctly, conn->power_save prevents the host
> stack
> >>> >>> >> from requesting active mode if it was not the host stack that
> >>> >>> >> requested sniff mode.
> >>> >>> >>
> >>> >>> >> I don't understand the motivation for this. If we have ACL data
> to
> >>> >>> >> send, then it seems like a good idea for the host stack to
> explicitly
> >>> >>> >> request active mode, regardless of the reason that we entered
> sniff
> >>> >>> >> mode.
> >>> >>> >>
> >>> >>> >> We want to enter active mode more aggressively when setting up
> SCO
> >>> >>> >> connections, to avoid a 5 second delay with certain sniff modes.
> But
> >>> >>> >> the conn->power_save code is getting in the way and doesn't
> appear to
> >>> >>> >> be useful in the first place.
> >>> >>> >
> >>> >>> > we have discussed this a few times. And if you lock through the
> code
> >>> >>> > history then you see that initially we just took devices out of
> sniff
> >>> >>> > mode if we had to send data. However with HID devices this falls
> flat on
> >>> >>> > its face. They need to stay in control of sniff mode if they
> initiated
> >>> >>> > it. Some of them crash and others just drain the battery. With
> sniff
> >>> >>> > mode you can send small amounts of data even while in sniff and
> for HID
> >>> >>> > that is sometimes used. So the remote side better not interfere.
> >>> >>> >
> >>> >>> > What we really need is a socket option where we can control this
> on a
> >>> >>> > per socket basis if we take devices out of sniff mode. And one
> extra
> >>> >>> > case might be when we try to establish a SCO channel, because
> then it is
> >>> >>> > clearly not an HID device. However even A2DP has this sort of
> problems
> >>> >>> > sometimes where the stream setup takes time.
> >>> >>>
> >>> >>> Makes sense. Thanks for the explanation.
> >>> >>
> >>> >> this means you will be working on a patch for this :)
> >>>
> >>> Actually, I want to put a patch together for a socket option to not
> >>> use power_save (so that we *always* exit sniff mode when sending ACL
> >>> data). We're seeing this problem with the Plantronics Voyager 855
> >>> which enters sniff mode during A2DP.
> >>>
> >>> Given that power_save is just for HID devices, my preferred design is
> >>> to disable power_save by default, and have an L2CAP socket option to
> >>> turn on power_save that would be used by HID L2CAP sockets.
> >>> Unfortunately this would require userspace HID code to use the new
> >>> socket option to keep current behavior. But it seems preferable to the
> >>> alternative of having every single other L2CAP socket use a new socket
> >>> option just to disable power_save for the sake of HID.
> >>
> >> actually you still mix up the meaning of the power_save option. From the
> >> stack side, the automatic sniff mode is off by default. You have to
> >> enable via sysfs first. The power_save option is just to control when
> >> sniff mode is activated and the remote enters sniff mode, that we are
> >> not getting out of it if we have ACL data.
> >>
> >> In conclusion, I am fine with a socket option that allows to control
> >> sniff mode (preferable with interval details) and sniff subrate details
> >> so that we can use them for that ACL link.
> >>
> >> So go ahead and work on this. We can fix userspace easily since a new
> >> socket can be detected easily with newer kernels.
> >
> > We have found that some Nokia BT headsets stop working after idle
> > period when they enter
> > "power save" mode.
> >
> > Do we have any solution for this problem? So far I see good enough
> > patch :-)) below:
> >
> >
> http://android.git.kernel.org/?p=kernel/common.git;a=blobdiff;f=net/bluetooth/hci_conn.c;h=fa8b412205cd89546b4a325c558c68040eeaf491;hp=0cf25114a3f576fc2788a549eb96d0087dd39b44;hb=d8488237646920cd71ef43e5f3ae1001c6f4cf7b;hpb=3f68e5050c5ae559f56d5da9202cb88928d42b36
> >
> > - if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
> > + if (conn->mode != HCI_CM_SNIFF /* || !conn->power_save */)
> >
> > Nick have you done socket option for "power_save"?
>
> No. We'll do it once we need it for HID support.
>
Can we do this: try to do check against some existing fields in structure
hci_conn, for example device_class and then determine if conn->power_save
shall be used here? By doing this, applications in user space can be saved
from having knowledge how sniff mode works for different devices.
Alternatively, is it possible to add another flag - I know the structure is
getting big so I agree adding something might not be a good idea. :)
>
> Nick
> --
> 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
>
[-- Attachment #2: Type: text/html, Size: 7334 bytes --]
^ permalink raw reply
* Re: [PATCH] Notify to device drivers when the SDP records change
From: José Antonio Santos Cadenas @ 2010-08-03 9:12 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=iNPgNJaUpUK2v=whbWs_nzCpwq5TUbbDKk-7H@mail.gmail.com>
Hi,
El Tuesday 03 August 2010 11:00:07 Luiz Augusto von Dentz escribió:
> Hi,
>
> On Tue, Aug 3, 2010 at 10:30 AM, José Antonio Santos Cadenas
>
> <santoscadenas@gmail.com> wrote:
> > Hi Luiz,
> >
> > El Monday 02 August 2010 16:39:18 Luiz Augusto von Dentz escribió:
> >> Hi,
> >>
> >> On Mon, Jul 26, 2010 at 10:55 AM, Jose Antonio Santos Cadenas
> >>
> >> <santoscadenas@gmail.com> wrote:
> >> > When the remote SDP records that make a driver to be loaded change
> >> > (added, remove, or updated) And the driver is not going to be
> >> > removed, a notification is sent to the device driver. This
> >> > notification is optional annnd does not fail if the driver does
> >> > not implemment it.
> >>
> >> How this is supposed to work?
> >
> > This tries to be a generic way to update the remote services. When we
> > planned the HDP API some weeks ago in Recife, we decided that we need a
> > way to update the services that a device offers (this implies to make
> > some calls to the HealtAgent). Marcel and Johan tell me that it will be
> > better to implement it in a generic way using a generic update services.
> > May be I misunderstood something or this is not the best way to do this,
> > but that's the objective. The objective is to provide an API to the user
> > that notifies them about new health services.
> >
> > Can you see better ways to do this? We are open to any new ideas.
> >
> >> If does seems to me that you guys are
> >> planning to keep polling the services records to update the drivers
> >> which IMO is not a good idea, DiscoverServices is not meant to update
> >> the drivers either it is basically a replacement to sdptool to so
> >> application can do it over dbus.
> >
> > Of course I am not trying to poll remote SDP nor having it cached. Just
> > offering a way to update services for the user.
> >
> >> Also if you take a closer look in the
> >> plugins that use rfcomm channel, which can be changed at any point,
> >> they basically fetch the record when attempting to connect, avdtp also
> >> work in a similar way, we have to do the discover + get capabilities
> >> every time we want to connect to a sink/source since there is no way
> >> to keep this information updated locally.
> >
> > Of course, every time that you are going to connect is when you check the
> > remote record. In the HDP implementation is doing the same way.
> >
> >> In the other hand, this can be useful for LE which we can subscribe to
> >> changes, but for sdp I don't see the point.
> >
> > For HDP is also useful in SDP, because the device can update services and
> > the device driver will not receive any notification. This way we are
> > offering a way to update it when the user wants. This is almost the same
> > than making a service discovery and notify a new UUID discovered, but
> > also notifying the changes in the records.
>
> But you must do the discover every time you want to connect, as you
> state there is no notification for SDP, so updating the drivers
> doesn't guarantee anything. Application should not know about this,
> they basically want to connect and bluetoothd has to figure out which
> sink/source do match for them.
>
> So IMO this has to be done similar to a2dp/avdtp, prior to connect
> discover the endpoints and them proceed with the connection attempt,
> the stored records are just a hint of what the remote device supports.
The different with a2dp is that the application will decide the end point to
connect to, because there can be more than one matching the source. This end
point is represented by a path that is passed to the HealthAgent when is
discovered. If the remote record changes it is possible that new end points
are available so the application needs something to discover the available end
points. That is why I think that we need something like this.
Regards.
^ permalink raw reply
* Re: [PATCH] Notify to device drivers when the SDP records change
From: Luiz Augusto von Dentz @ 2010-08-03 9:00 UTC (permalink / raw)
To: José Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <201008030930.45330.santoscadenas@gmail.com>
Hi,
On Tue, Aug 3, 2010 at 10:30 AM, José Antonio Santos Cadenas
<santoscadenas@gmail.com> wrote:
> Hi Luiz,
>
> El Monday 02 August 2010 16:39:18 Luiz Augusto von Dentz escribió:
>> Hi,
>>
>> On Mon, Jul 26, 2010 at 10:55 AM, Jose Antonio Santos Cadenas
>>
>> <santoscadenas@gmail.com> wrote:
>> > When the remote SDP records that make a driver to be loaded change
>> > (added, remove, or updated) And the driver is not going to be
>> > removed, a notification is sent to the device driver. This
>> > notification is optional annnd does not fail if the driver does
>> > not implemment it.
>>
>> How this is supposed to work?
>
> This tries to be a generic way to update the remote services. When we planned
> the HDP API some weeks ago in Recife, we decided that we need a way to update
> the services that a device offers (this implies to make some calls to the
> HealtAgent). Marcel and Johan tell me that it will be better to implement it
> in a generic way using a generic update services. May be I misunderstood
> something or this is not the best way to do this, but that's the objective.
> The objective is to provide an API to the user that notifies them about new
> health services.
>
> Can you see better ways to do this? We are open to any new ideas.
>
>
>> If does seems to me that you guys are
>> planning to keep polling the services records to update the drivers
>> which IMO is not a good idea, DiscoverServices is not meant to update
>> the drivers either it is basically a replacement to sdptool to so
>> application can do it over dbus.
>
> Of course I am not trying to poll remote SDP nor having it cached. Just
> offering a way to update services for the user.
>
>
>> Also if you take a closer look in the
>> plugins that use rfcomm channel, which can be changed at any point,
>> they basically fetch the record when attempting to connect, avdtp also
>> work in a similar way, we have to do the discover + get capabilities
>> every time we want to connect to a sink/source since there is no way
>> to keep this information updated locally.
>
> Of course, every time that you are going to connect is when you check the
> remote record. In the HDP implementation is doing the same way.
>
>>
>> In the other hand, this can be useful for LE which we can subscribe to
>> changes, but for sdp I don't see the point.
>
> For HDP is also useful in SDP, because the device can update services and the
> device driver will not receive any notification. This way we are offering a
> way to update it when the user wants. This is almost the same than making a
> service discovery and notify a new UUID discovered, but also notifying the
> changes in the records.
But you must do the discover every time you want to connect, as you
state there is no notification for SDP, so updating the drivers
doesn't guarantee anything. Application should not know about this,
they basically want to connect and bluetoothd has to figure out which
sink/source do match for them.
So IMO this has to be done similar to a2dp/avdtp, prior to connect
discover the endpoints and them proceed with the connection attempt,
the stored records are just a hint of what the remote device supports.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH] Add NULL checking for name and path params
From: Johan Hedberg @ 2010-08-03 8:42 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1280823784-14551-1-git-send-email-ext-jablonski.radoslaw@nokia.com>
Hi Radek,
On Tue, Aug 03, 2010, Radoslaw Jablonski wrote:
> Added NULL checking for "name" and "path" parameters in pbap_open_*
> functions. If this variables will be NULL, then setting -EBADR error
> code and ending function. This fixes many obexd crashes.
> ---
> plugins/pbap.c | 15 ++++++++++++++-
> 1 files changed, 14 insertions(+), 1 deletions(-)
Please update your tree against latest upstream. Your patch doesn't
apply anymore (it seems like a new version of the previous one, but that
one got already pushed upstream). Also fix the commit message to explain
what issue in the previous patch this one fixes.
Johan
^ permalink raw reply
* Re: [PATCH 6/6] Support for generating dictionary value of service UUIDs.
From: Johan Hedberg @ 2010-08-03 8:36 UTC (permalink / raw)
To: Inga Stotland; +Cc: linux-bluetooth, rshaffer, marcel
In-Reply-To: <1280576713-1398-7-git-send-email-ingas@codeaurora.org>
Hi Inga,
On Sat, Jul 31, 2010, Inga Stotland wrote:
> Add service UUIDs from EIR to device properties in "Device Found" signal.
> ---
> src/adapter.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> src/adapter.h | 4 +-
> src/dbus-hci.c | 6 ++--
> 3 files changed, 97 insertions(+), 8 deletions(-)
A few more coding style issues:
> + while (len < EIR_DATA_LENGTH) {
> + uint8_t type = eir_data[1];
> + uint8_t field_len = eir_data[0];
Add an empty line here (it's almost always a good thing to have after
variable declarations).
> + if (type == EIR_UUID16_SOME || type == EIR_UUID16_ALL)
> + count += field_len/2;
Space before and after /
> + else if (type == EIR_UUID32_SOME || type == EIR_UUID32_ALL)
> + count += field_len/4;
Same here.
> + else if (type == EIR_UUID128_SOME || type == EIR_UUID128_ALL)
> + count += field_len/16;
And here
> + count = field_len/size;
And here
> + count = field_len/size;
And here
> + val32 = (val32<<8) + data[k];
Space before and after <<
> + count = field_len/size;
Space before and after /
> + service.value.uuid128.data[k] = data[size-k-1];
Space before and after -
> + /* Extract UUIDs from extended inquiry response if any*/
> + if (eir_data != NULL)
> + uuid_count = get_uuid_count_eir(eir_data);
> +
> + if (uuid_count > 0) {
> + uuids = g_new0(char *, uuid_count + 1);
> + get_uuids_eir(uuids, eir_data);
> + }
> +
I don't think these two get_uuids_eir and get_uuid_count_eir functions
are really needed. Could you change it to just one function:
char **get_eir_uuids(const uint8_t *eir_data, size_t *count)
Johan
^ permalink raw reply
* Re: [PATCH] bluetooth: fix not setting security level when creating a rfcomm session
From: Luiz Augusto von Dentz @ 2010-08-03 8:27 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1280822396.12579.80.camel@localhost.localdomain>
Hi Marcel,
On Tue, Aug 3, 2010 at 10:59 AM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Luiz,
>
>> This cause 'No Bonding' to be used if userspace has not yet been paired
>> with remote device since the l2cap socket used to create the rfcomm
>> session does not have any security level set.
>>
>> Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
>> ---
>> =A0net/bluetooth/rfcomm/core.c | =A0 13 ++++++++++---
>> =A01 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
>> index 7dca91b..08f5757 100644
>> --- a/net/bluetooth/rfcomm/core.c
>> +++ b/net/bluetooth/rfcomm/core.c
>> @@ -79,7 +79,10 @@ static void rfcomm_make_uih(struct sk_buff *skb, u8 a=
ddr);
>>
>> =A0static void rfcomm_process_connect(struct rfcomm_session *s);
>>
>> -static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdad=
dr_t *dst, int *err);
>> +static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bdaddr_t *dst,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 u8 sec_level,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int *err);
>> =A0static struct rfcomm_session *rfcomm_session_get(bdaddr_t *src, bdadd=
r_t *dst);
>> =A0static void rfcomm_session_del(struct rfcomm_session *s);
>>
>> @@ -402,7 +405,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, b=
daddr_t *src, bdaddr_t *dst,
>>
>> =A0 =A0 =A0 s =3D rfcomm_session_get(src, dst);
>> =A0 =A0 =A0 if (!s) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 s =3D rfcomm_session_create(src, dst, &err);
>> + =A0 =A0 =A0 =A0 =A0 =A0 s =3D rfcomm_session_create(src, dst, d->sec_l=
evel, &err);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!s)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return err;
>> =A0 =A0 =A0 }
>> @@ -680,7 +683,10 @@ static void rfcomm_session_close(struct rfcomm_sess=
ion *s, int err)
>> =A0 =A0 =A0 rfcomm_session_put(s);
>> =A0}
>>
>> -static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdad=
dr_t *dst, int *err)
>> +static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bdaddr_t *dst,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 u8 sec_level,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int *err)
>> =A0{
>> =A0 =A0 =A0 struct rfcomm_session *s =3D NULL;
>> =A0 =A0 =A0 struct sockaddr_l2 addr;
>> @@ -705,6 +711,7 @@ static struct rfcomm_session *rfcomm_session_create(=
bdaddr_t *src, bdaddr_t *dst
>> =A0 =A0 =A0 sk =3D sock->sk;
>> =A0 =A0 =A0 lock_sock(sk);
>> =A0 =A0 =A0 l2cap_pi(sk)->imtu =3D l2cap_mtu;
>> + =A0 =A0 l2cap_pi(sk)->sec_level =3D sec_level;
>> =A0 =A0 =A0 if (l2cap_ertm)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 l2cap_pi(sk)->mode =3D L2CAP_MODE_ERTM;
>> =A0 =A0 =A0 release_sock(sk);
>
> I see what you are trying to do here, but why is this needed? Shouldn't
> it be already correctly set for PSM 3?
I couldn't find any special check for PSM 3, but I guess it would make
sense to make sure it always set the security level from dlc which
triggered the session, doesn't it? As it is now it first triggers No
Bonding and then when we actually connect the dlc it might have a
different sec_level which might triggers another round of io
capability negotiation, not only this doesn't work always, because
there is a bug in bluetoothd that won't invalidate/remove stored link
keys once we got a new one that is not supposed to be stored (No
Bonding), but IMO much simpler too. Maybe we can do as SDP which seems
to do the check in l2cap.c:l2cap_do_connect, but for rfcomm we
actually have to look in the dlcs to check witch security level should
be use.
--=20
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH 4/6] Fix in logic to write EIR when SDP records are changed.
From: Johan Hedberg @ 2010-08-03 8:23 UTC (permalink / raw)
To: Inga Stotland; +Cc: linux-bluetooth, rshaffer, marcel
In-Reply-To: <1280576713-1398-5-git-send-email-ingas@codeaurora.org>
Hi Inga,
On Sat, Jul 31, 2010, Inga Stotland wrote:
> Whenver SDP service record is added/deleted/modified check for whether
> class of device needs to be updated as well. If the update is
> needed, proceed as before and new EIR will be written subsequently.
> If the class of device is already present, just update EIR and return.
> ---
> src/adapter.c | 14 ++++++++++----
> 1 files changed, 10 insertions(+), 4 deletions(-)
A few minor coding style issues
> - /* If we already have the CoD we want or the cache is enabled or an
> - * existing CoD write is in progress just bail out */
> - if (adapter->current_cod == adapter->wanted_cod ||
> - adapter->cache_enable || adapter->pending_cod)
> + /* If the cache is enabled or an existing CoD write is in progress
> + * just bail out */
> + if(adapter->cache_enable || adapter->pending_cod)
Missing space beteween if and (
> return 0;
Add an empty line here.
> + /* If we already have the CoD we want, update EIR and return */
> + if (adapter->current_cod == adapter->wanted_cod) {
> + update_ext_inquiry_response(adapter);
> + return 0;
> + }
Johan
^ permalink raw reply
* [PATCH] Add NULL checking for name and path params
From: Radoslaw Jablonski @ 2010-08-03 8:23 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Radoslaw Jablonski
Added NULL checking for "name" and "path" parameters in pbap_open_*
functions. If this variables will be NULL, then setting -EBADR error
code and ending function. This fixes many obexd crashes.
---
plugins/pbap.c | 15 ++++++++++++++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/plugins/pbap.c b/plugins/pbap.c
index af4b452..af4a1bc 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -594,6 +594,9 @@ static int pbap_get(struct obex_session *os, obex_object_t *obj,
} else
return -EBADR;
+ if (path == NULL)
+ return -EBADR;
+
pbap->params = params;
ret = obex_get_stream_start(os, path);
@@ -689,6 +692,11 @@ static void *vobject_pull_open(const char *name, int oflag, mode_t mode,
goto fail;
}
+ if (name == NULL) {
+ ret = -EBADR;
+ goto fail;
+ }
+
if (pbap->params->maxlistcount == 0)
cb = phonebook_size_result;
else
@@ -720,6 +728,11 @@ static void *vobject_list_open(const char *name, int oflag, mode_t mode,
goto fail;
}
+ if (name == NULL) {
+ ret = -EBADR;
+ goto fail;
+ }
+
/* PullvCardListing always get the contacts from the cache */
if (pbap->cache.valid) {
@@ -758,7 +771,7 @@ static void *vobject_vcard_open(const char *name, int oflag, mode_t mode,
goto fail;
}
- if (sscanf(name, "%u.vcf", &handle) != 1) {
+ if (name == NULL || sscanf(name, "%u.vcf", &handle) != 1) {
ret = -EBADR;
goto fail;
}
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 3/6] Support for adding UUID128 to extended inquiry response
From: Johan Hedberg @ 2010-08-03 8:18 UTC (permalink / raw)
To: Inga Stotland; +Cc: linux-bluetooth, rshaffer, marcel
In-Reply-To: <1280576713-1398-4-git-send-email-ingas@codeaurora.org>
Hi Inga,
On Sat, Jul 31, 2010, Inga Stotland wrote:
> ---
> src/sdpd-service.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 62 insertions(+), 0 deletions(-)
A couple of coding style comments here:
> + /* Check for duplicates, EIR data is Little Endian */
> + for (i = 0; i < index; i++) {
> + for (k = 0; k < SIZEOF_UUID128; k++) {
> + if (uuid128[i*SIZEOF_UUID128 + k] !=
Space before and after *
> + rec->svclass.value.uuid128.data[SIZEOF_UUID128 - k])
Looks like you're going beyond 80-characters. Maybe you could create an
extra variable and assign rec->svclass.value.uuid.data to it.
> + for (k = 0; k < SIZEOF_UUID128; k++)
> + uuid128[index*SIZEOF_UUID128 + k] =
Space before and after *
> + rec->svclass.value.uuid128.data[SIZEOF_UUID128 - 1 - k];
Again over 80-characters, but if you use the extra variable that I
previously suggested for rec->svclass.value.uuid128.data it should help.
> + ptr[1] = (truncated) ? EIR_UUID128_SOME : EIR_UUID128_ALL;
Since truncated is just a gboolean and not a more complex statement I'd
just leave out the parenthesis around it, but quite a minor thing in the
end.
Johan
^ permalink raw reply
* Re: [PATCH 2/6] Clean up code that generates extended inquiry response.
From: Johan Hedberg @ 2010-08-03 8:12 UTC (permalink / raw)
To: Inga Stotland; +Cc: linux-bluetooth, rshaffer, marcel
In-Reply-To: <1280576713-1398-3-git-send-email-ingas@codeaurora.org>
Hi Inga,
On Sat, Jul 31, 2010, Inga Stotland wrote:
> Minor fix when skipping duplicate UUID16 from EIR.
> ---
> src/sdpd-service.c | 46 ++++++++++++++++++++++++++++++----------------
> 1 files changed, 30 insertions(+), 16 deletions(-)
So your commit message says two things: "Clean up code..." and "Minor
fix..". Cleanups and fixes should be in separate patches. Don't be
afraid of "too small" or trivial patches. As long as a patch contains a
single logically independent change it's fine.
Johan
> + uint16_t uuid16[EIR_DATA_LENGTH/2];
Minor nitpick: space before and after /
Johan
^ permalink raw reply
* Re: Enhancements to allow l2cap channel to use either AMP or BR/EDR
From: Gustavo F. Padovan @ 2010-08-03 8:07 UTC (permalink / raw)
To: Peter Krystad
Cc: Kevin Hayes, Marcel Holtmann, tmonahan@codeaurora.org,
David Vrabel, Inga Stotland, linux-bluetooth@vger.kernel.org,
rshaffer@codeaurora.org, johan.hedberg@gmail.com
In-Reply-To: <3e0b0d621e7c3dcee98b2a16c6e1f23b.squirrel@www.codeaurora.org>
Hi Peter,
* Peter Krystad <pkrystad@codeaurora.org> [2010-08-02 18:30:34 -0700]:
>
> Hi Marcel and Kevin,
>
> >> >
> >> > Agree that it should be done "in background" and that a size
> >> threshold would be useful. But who evaluates that threshold? The
> >> bluez kernel components, which essentially implement a transport
> >> driver, should not be examining objects (files, phonebooks, etc) size
> >> to see if this threshold is met. Therefore, it would seem Tim's
> >> suggestion of having the profile send a 'prefer_amp' bit would be
> >> useful, right?
> >>
> >> I would do something like "prefer_amp when over 100kb" or something.
> >> And
> >> then the kernel needs to count. Meaning the L2CAP layer could easily
> >> count this by itself.
> >
> > What? Let's say the effect we want is that if the object is greater than,
> > say, 10 megabytes, then we want to use AMP for the transfer. With your
> > scheme, you want the kernel to count up to 10 megabytes, THEN switch over
> > to AMP?
> > No, you don't, he says rhetorically. :)
>
> If the policy was defined as "prefer_amp after n seconds" we would get the
> desired effect without having to do any counting in the kernel.
In both cases we have to take in account the total size of the file, or
the estimated time of transfer, so we can avoid to move to AMP when the
transfer is about to end.
--
Gustavo F. Padovan
http://padovan.org
^ permalink raw reply
* Re: [PATCH] bluetooth: fix not setting security level when creating a rfcomm session
From: Marcel Holtmann @ 2010-08-03 7:59 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1280820383-10029-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
> This cause 'No Bonding' to be used if userspace has not yet been paired
> with remote device since the l2cap socket used to create the rfcomm
> session does not have any security level set.
>
> Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
> ---
> net/bluetooth/rfcomm/core.c | 13 ++++++++++---
> 1 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index 7dca91b..08f5757 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -79,7 +79,10 @@ static void rfcomm_make_uih(struct sk_buff *skb, u8 addr);
>
> static void rfcomm_process_connect(struct rfcomm_session *s);
>
> -static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err);
> +static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
> + bdaddr_t *dst,
> + u8 sec_level,
> + int *err);
> static struct rfcomm_session *rfcomm_session_get(bdaddr_t *src, bdaddr_t *dst);
> static void rfcomm_session_del(struct rfcomm_session *s);
>
> @@ -402,7 +405,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
>
> s = rfcomm_session_get(src, dst);
> if (!s) {
> - s = rfcomm_session_create(src, dst, &err);
> + s = rfcomm_session_create(src, dst, d->sec_level, &err);
> if (!s)
> return err;
> }
> @@ -680,7 +683,10 @@ static void rfcomm_session_close(struct rfcomm_session *s, int err)
> rfcomm_session_put(s);
> }
>
> -static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err)
> +static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
> + bdaddr_t *dst,
> + u8 sec_level,
> + int *err)
> {
> struct rfcomm_session *s = NULL;
> struct sockaddr_l2 addr;
> @@ -705,6 +711,7 @@ static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst
> sk = sock->sk;
> lock_sock(sk);
> l2cap_pi(sk)->imtu = l2cap_mtu;
> + l2cap_pi(sk)->sec_level = sec_level;
> if (l2cap_ertm)
> l2cap_pi(sk)->mode = L2CAP_MODE_ERTM;
> release_sock(sk);
I see what you are trying to do here, but why is this needed? Shouldn't
it be already correctly set for PSM 3?
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] Notify to device drivers when the SDP records change
From: José Antonio Santos Cadenas @ 2010-08-03 7:30 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <AANLkTin1khXOxvZTdazRYFbb3VZCf39YFYaZnwa3cbmN@mail.gmail.com>
Hi Luiz,
El Monday 02 August 2010 16:39:18 Luiz Augusto von Dentz escribió:
> Hi,
>
> On Mon, Jul 26, 2010 at 10:55 AM, Jose Antonio Santos Cadenas
>
> <santoscadenas@gmail.com> wrote:
> > When the remote SDP records that make a driver to be loaded change
> > (added, remove, or updated) And the driver is not going to be
> > removed, a notification is sent to the device driver. This
> > notification is optional annnd does not fail if the driver does
> > not implemment it.
>
> How this is supposed to work?
This tries to be a generic way to update the remote services. When we planned
the HDP API some weeks ago in Recife, we decided that we need a way to update
the services that a device offers (this implies to make some calls to the
HealtAgent). Marcel and Johan tell me that it will be better to implement it
in a generic way using a generic update services. May be I misunderstood
something or this is not the best way to do this, but that's the objective.
The objective is to provide an API to the user that notifies them about new
health services.
Can you see better ways to do this? We are open to any new ideas.
> If does seems to me that you guys are
> planning to keep polling the services records to update the drivers
> which IMO is not a good idea, DiscoverServices is not meant to update
> the drivers either it is basically a replacement to sdptool to so
> application can do it over dbus.
Of course I am not trying to poll remote SDP nor having it cached. Just
offering a way to update services for the user.
> Also if you take a closer look in the
> plugins that use rfcomm channel, which can be changed at any point,
> they basically fetch the record when attempting to connect, avdtp also
> work in a similar way, we have to do the discover + get capabilities
> every time we want to connect to a sink/source since there is no way
> to keep this information updated locally.
Of course, every time that you are going to connect is when you check the
remote record. In the HDP implementation is doing the same way.
>
> In the other hand, this can be useful for LE which we can subscribe to
> changes, but for sdp I don't see the point.
For HDP is also useful in SDP, because the device can update services and the
device driver will not receive any notification. This way we are offering a
way to update it when the user wants. This is almost the same than making a
service discovery and notify a new UUID discovered, but also notifying the
changes in the records.
Regards.
Jose.
^ permalink raw reply
* [PATCH] bluetooth: fix not setting security level when creating a rfcomm session
From: Luiz Augusto von Dentz @ 2010-08-03 7:26 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
This cause 'No Bonding' to be used if userspace has not yet been paired
with remote device since the l2cap socket used to create the rfcomm
session does not have any security level set.
Signed-off-by: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
---
net/bluetooth/rfcomm/core.c | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 7dca91b..08f5757 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -79,7 +79,10 @@ static void rfcomm_make_uih(struct sk_buff *skb, u8 addr);
static void rfcomm_process_connect(struct rfcomm_session *s);
-static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err);
+static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
+ bdaddr_t *dst,
+ u8 sec_level,
+ int *err);
static struct rfcomm_session *rfcomm_session_get(bdaddr_t *src, bdaddr_t *dst);
static void rfcomm_session_del(struct rfcomm_session *s);
@@ -402,7 +405,7 @@ static int __rfcomm_dlc_open(struct rfcomm_dlc *d, bdaddr_t *src, bdaddr_t *dst,
s = rfcomm_session_get(src, dst);
if (!s) {
- s = rfcomm_session_create(src, dst, &err);
+ s = rfcomm_session_create(src, dst, d->sec_level, &err);
if (!s)
return err;
}
@@ -680,7 +683,10 @@ static void rfcomm_session_close(struct rfcomm_session *s, int err)
rfcomm_session_put(s);
}
-static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst, int *err)
+static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src,
+ bdaddr_t *dst,
+ u8 sec_level,
+ int *err)
{
struct rfcomm_session *s = NULL;
struct sockaddr_l2 addr;
@@ -705,6 +711,7 @@ static struct rfcomm_session *rfcomm_session_create(bdaddr_t *src, bdaddr_t *dst
sk = sock->sk;
lock_sock(sk);
l2cap_pi(sk)->imtu = l2cap_mtu;
+ l2cap_pi(sk)->sec_level = sec_level;
if (l2cap_ertm)
l2cap_pi(sk)->mode = L2CAP_MODE_ERTM;
release_sock(sk);
--
1.7.0.4
^ permalink raw reply related
* RE: Enhancements to allow l2cap channel to use either AMP or BR/EDR
From: Kevin Hayes @ 2010-08-03 4:47 UTC (permalink / raw)
To: Marcel Holtmann
Cc: tmonahan@codeaurora.org, David Vrabel, Inga Stotland,
linux-bluetooth@vger.kernel.org, rshaffer@codeaurora.org,
johan.hedberg@gmail.com
In-Reply-To: <1280800744.12579.71.camel@localhost.localdomain>
SGkgYWdhaW4gTWFyY2VsLA0KDQo+IEhvd2V2ZXIgd2l0aCBPQkVYIGl0IG1pZ2h0IGJlIHRoYXQg
eW91IHdhbm5hIHJlYWQgdGhlIGZpcnN0IHBhY2thZ2UNCj4gZmlyc3Qgd2l0aCBCUi9FRFIgb25s
eSBhbmQgb25seSBhZnRlciBrbm93aW5nIHRoZSBmaWxlIHNpemUgbWFrZSBhDQo+IGRlY2lzaW9u
IHRvIHByZWZlciBBTVAgb3Igbm90Lg0KDQpFeGFjdGx5LiAgRm9yIE9QUCwgUEJBUCwgQklQLCBG
VFAsIHlvdSBrbm93IHRoZSBmaWxlIHNpemUgcHJldHR5IHF1aWNrbHksIG5vIGNvdW50aW5nIHJl
cXVpcmVkLg0KQW5kIGlmIHlvdSBoYXZlIGEgcG9saWN5IG9mICJ1c2UgQU1QIGlmIG9iamVjdCBi
aWdnZXIgdGhhbiBYIiwgdGhlbiB5b3UgZG9uJ3Qgd2FudCB0byBzcGVuZCBjeWNsZXMgdXNpbmcg
QlIvRURSIGlmIHlvdSBuZWVkIHRvIHhmZXIgYW4gb2JqZWN0IGV2ZW4gb25lIHNpbmdsZSBvY3Rl
dCBiaWdnZXIgdGhhbiBYLiAgQU1QIGVhcmx5LCBBTVAgb2Z0ZW4uICA6KQ0KDQo+IA0KPiBXZSB3
aWxsIHNlZSB3aGF0IHdvcmtzIG91dCBiZXN0Lg0KUG9saWN5IGlzIGEgd29uZGVyZnVsIHRoYW5n
LiAgOikNCg0KCUsrKw0KDQoNCg0K
^ permalink raw reply
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