Linux bluetooth development
 help / color / mirror / Atom feed
* Re: [PATCH hcidump] amp: Decode Read Data Block Size HCI cmd
From: Johan Hedberg @ 2012-09-28 11:11 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1346331979-3837-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Thu, Aug 30, 2012, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Decode Read Data Block Size HCI command and command status.
> 
>  < HCI Command: Read Data Block Size (0x04|0x000a) plen 0
>  > HCI Event: Command Complete (0x0e) plen 10
>     Read Data Block Size (0x04|0x000a) ncmd 1
>     status 0x00
>     Max ACL 1492 Block len 1492 Num blocks 4
> ---
>  lib/hci.h    |    8 ++++++++
>  parser/hci.c |   24 +++++++++++++++++++++++-
>  2 files changed, 31 insertions(+), 1 deletion(-)

Applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH hcidump] amp: Decode Num Completed Data Blocks
From: Johan Hedberg @ 2012-09-28 11:10 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1346398640-8423-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

On Fri, Aug 31, 2012, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Adds decoding Number Of Completed Data Blocks Event
> 
> > HCI Event: Number Of Completed Data Blocks (0x48) plen 9
>     Total num blocks 4 Num handles 1
>       Handle 0x0001: Num complt pkts 1 Num complt blks 1
> ---
>  lib/hci.h    |   10 ++++++++++
>  parser/hci.c |   22 ++++++++++++++++++++++
>  2 files changed, 32 insertions(+)

Applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH 07/14] thermometer: Reformat MeasurementReceived description
From: Santiago Carot @ 2012-09-28 11:09 UTC (permalink / raw)
  To: Andrzej Kaczmarek, linux-bluetooth
In-Reply-To: <20120928104318.GC25415@x220>

Hi johan,

2012/9/28 Johan Hedberg <johan.hedberg@gmail.com>:
> Hi Andrzej,
>
> On Tue, Sep 25, 2012, Andrzej Kaczmarek wrote:
>> +                     Measurement:
>> +
>> +                             int16 Exponent:
>> +                             int32 Mantissa:
>> +
>> +                                     Measurement value is calculated as
>> +                                     (Mantissa) x (10^Exponent)
>> +
>> +                                     In case of invalid or missing data,
>> +                                     Exponent is set to 0 and Mantissa is
>> +                                     set to 2^23-1 (0x7FFFFF hexadecimal).
>
> Why aren't we using the D-Bus DOUBLE type for this? For invalid/missing
> data just leave out these values from the dict. No need to try to
> brute-force a 1:1 mapping from the protocol to D-Bus.
>

We don't use double type here because these information is valuable
for upper profiles implementing a IEE11073-20601 protocol layer. This
fact is scarcely commented in HTP and better explained in former
specification where there is a need to pack both values (mantissa and
exponent) in special kind of APDUs. Of course we could provide the
double value here, but there will remain the need to know when an
special case occurs: NRes, NaN,INFINITY, -INFINITY.

Furthermore, providing a double value here will force to upper
profiles to do extra stuff to calculate the mantissa and the exponent
values in order to create APDUs, beside they will have to deal with
impreccisions derivated to comparate float point numbers and so it
might make difficult to check when a special case has happened.

these are, as far as I remember, the reasons which we did it in this
way. We were already talking about this fact in the mailing list in
the past but I don't find the thread right now. sorry

^ permalink raw reply

* Re: [PATCH v3 BlueZ 1/4] gatt: Add support for find included services
From: Johan Hedberg @ 2012-09-28 11:08 UTC (permalink / raw)
  To: Vinicius Costa Gomes; +Cc: linux-bluetooth
In-Reply-To: <1348526019-28402-2-git-send-email-vinicius.gomes@openbossa.org>

Hi Vinicius,

On Mon, Sep 24, 2012, Vinicius Costa Gomes wrote:
> Some services like HID over LE can reference another service using
> included services.
> 
> See Vol 3, Part G, section 2.6.3 of Core specification for more
> details.
> ---
>  attrib/gatt.c | 194 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  attrib/gatt.h |   9 +++
>  2 files changed, 203 insertions(+)

This one doesn't apply:

Applying: gatt: Add support for find included services
WARNING:LONG_LINE: line over 80 characters
#107: FILE: attrib/gatt.c:335:
+					resolve_included_uuid_cb, resolve, NULL);

Btw, if you guys are wondering why the stricter coding style checks,
I've added the following to .git/hooks/pre-{commit,applypatch}:

git diff --cached | ~/src/linux/scripts/checkpatch.pl --no-signoff --ignore INITIALISED_STATIC,NEW_TYPEDEFS,VOLATILE --show-types --mailback -

You might want to do the same (with the correct path to checkpatch.pl)).
And thanks to Luiz for the idea (he's using the same when applying
patches).

Johan

^ permalink raw reply

* Re: [PATCH v4 01/10] battery: Add generic device battery documentation
From: Johan Hedberg @ 2012-09-28 10:59 UTC (permalink / raw)
  To: chen.ganir; +Cc: linux-bluetooth
In-Reply-To: <1348470919-25567-2-git-send-email-chen.ganir@ti.com>

Hi Chen,

On Mon, Sep 24, 2012, chen.ganir@ti.com wrote:
> +BlueZ D-Bus Battery API description
> +****************************************

Please keep the "underline" consistent with the length of the text above
it.

> +	Texas Instruments, Inc. <chen.ganir@ti.com>

What's this supposed to be? If you want a copyright statement here then
make it of the proper format. Author info is not needed as we have the
AUTHORS file and the commit history.

> +Device Battery hierarchy
> +=====================================

Same thing with the consistency of the "underline"

> +Service		org.bluez
> +Interface	org.bluez.Battery
> +Object path	[variable prefix]/{hci0,..}/dev_XX_XX_XX_XX_XX_XX/BATTYYYY
> +YYYY is numeric value between 0 and 9999.

I don't think you need a separate comment for the "YYY". We don't have
that for the "XX" stuff either.

> +		array{object} Batteries [readonly]

I guess you should mark this with [experimental] since it'll be removed
with the advent of ObjectManager.

Johan

^ permalink raw reply

* Re: [RFC] Convert storage to use per-remote device directories
From: Anderson Lizardo @ 2012-09-28 10:56 UTC (permalink / raw)
  To: Frederic Danis; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <50642389.40107@intel.com>

Hi Frederic,

On Thu, Sep 27, 2012 at 5:59 AM, Frederic Danis
<frederic.danis@intel.com> wrote:
> Hi everyone,
>
> Here is my proposal for new storage directory structure using ini-file
> format.

Do you know if the INI parser in glib allows for multiple groups with
same name? In LE we can have services with same UUID (not sure if this
is valid for BR/EDR also), and they are differentiated by other means
(e.g. a "Description" descriptor with different value, besides the
different handle).

Regarding LE, we have two kinds of storage needs: server profiles, and
client profiles.

For server profiles, we need to store the attribute database.
Currently we don't do this, but we need to implement it, or have
ServiceChanged characteristic notifying that the service ranges have
changed every time BlueZ restarts (which adds overhead as service
discovery is triggered on the client side). I suggest we store the
attribute database into a "attribute_db.conf" file containing the
attribute name/value/uuid triples. Additionally, we also need to store
the Client Characteristic Configuration descriptor values (which are
specific to each bonded device), currently stored into a "ccc" file.
We could have one group just for them into attribute_db.conf.

For client profiles, I think the storage needs are specific to each
profile. From memory, only the generic GATT client and the GAP plugin
use storage currently (other profiles eventually need to use storage
to avoid full service discovery every time a bonded device
re-connects). The generic GATT client stores service/characteristics
handles/uuids ("primaries" and "characteristics" files), and the GAP
plugin stores appearance data ("appearances" file).

Johan, what do you think?

Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

^ permalink raw reply

* Re: [PATCH 07/14] thermometer: Reformat MeasurementReceived description
From: Johan Hedberg @ 2012-09-28 10:43 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1348584763-22824-8-git-send-email-andrzej.kaczmarek@tieto.com>

Hi Andrzej,

On Tue, Sep 25, 2012, Andrzej Kaczmarek wrote:
> +			Measurement:
> +
> +				int16 Exponent:
> +				int32 Mantissa:
> +
> +					Measurement value is calculated as
> +					(Mantissa) x (10^Exponent)
> +
> +					In case of invalid or missing data,
> +					Exponent is set to 0 and Mantissa is
> +					set to 2^23-1 (0x7FFFFF hexadecimal).

Why aren't we using the D-Bus DOUBLE type for this? For invalid/missing
data just leave out these values from the dict. No need to try to
brute-force a 1:1 mapping from the protocol to D-Bus.

> +
> +				string Unit:
> +
> +					Possible values: "Celsius" or "Fahrenheit"

String properties should be lower-case. Please fix (both doc and
implementation).

> +				string Type (optional):
> +
> +					Only present if measurement type is known.
> +
> +					Possible values: "Armpit", "Body", "Ear",
> +						"Finger", "Intestines", "Mouth",
> +						"Rectum", "Toe", "Tympanum"

Same here.

> +				string Measurement:
> +
> +					Possible values: "Final" or "Intermediate"

And here.

Johan

^ permalink raw reply

* Re: [PATCH 05/14] thermometer: Update API document
From: Johan Hedberg @ 2012-09-28 10:40 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1348584763-22824-6-git-send-email-andrzej.kaczmarek@tieto.com>

Hi Andrzej,

On Tue, Sep 25, 2012, Andrzej Kaczmarek wrote:
> This patch updates Thermometer API document to reflect changes in
> Thermometer interface and introduction of ThermometerManager
> interface.
> ---
>  doc/thermometer-api.txt | 59 +++++++++++++++++++++++++------------------------
>  1 file changed, 30 insertions(+), 29 deletions(-)
> 
> diff --git a/doc/thermometer-api.txt b/doc/thermometer-api.txt
> index 2271270..b0b0fe2 100644
> --- a/doc/thermometer-api.txt
> +++ b/doc/thermometer-api.txt
> @@ -3,28 +3,14 @@ BlueZ D-Bus Thermometer API description
>  
>  	Santiago Carot-Nemesio <sancane@gmail.com>
>  
> -Health Thermometer Profile hierarchy
> -=====================================

Could you please fix minor formatting inconsistencies like the
underlines (====) being different length than the text above it.

> -Service		org.bluez
> -Interface	org.bluez.Thermometer
> -Object path	[variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
> -
> -
> -Methods		void SetProperty(string name, variant value)
> -
> -			Changes the value of the specified property. Only
> -			read-write properties can be changed. On success
> -			this will emit a PropertyChanged signal.
> +Health Thermometer Manager hierarchy
> +============================
>  
> -			Possible Errors: org.bluez.Error.InvalidArguments
> +Service 	org.bluez

Theres some weird space-tab stuff going on with the line above please
fix.

Johan

^ permalink raw reply

* Re: [PATCH v8 00/13] Heart Rate Profile plugin
From: Johan Hedberg @ 2012-09-28 10:25 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1348583267-16816-1-git-send-email-andrzej.kaczmarek@tieto.com>

Hi Andrzej,

On Tue, Sep 25, 2012, Andrzej Kaczmarek wrote:
> Changes since v7:
> - rebased to latest upstream master (btd_profile changes)
> 
> 
> Andrzej Kaczmarek (6):
>   heartrate: Add attio callbacks
>   heartrate: Discover HRS characteristics
>   heartrate: Discover Heart Rate Measurement CCC
>   heartrate: Add support to enable notifications
>   heartrate: Process measurement notifications
>   heartrate: Add support to reset Energy Expended
> 
> Rafal Garbat (6):
>   heartrate: Add initial HRP client support
>   heartrate: Read Body Sensor Location characteristics
>   heartrate: Add HeartRateManager interface
>   heartrate: Add GetProperties method
>   heartrate: Add HeartRateWatcher interface to default policy
>   heartrate: Add test script
> 
> Santiago Carot-Nemesio (1):
>   Heart Rate Profile (HRP) client API
> 
>  Makefile.am                    |   9 +-
>  Makefile.tools                 |   3 +-
>  doc/heartrate-api.txt          |  84 +++++
>  lib/uuid.h                     |   5 +
>  profiles/heartrate/heartrate.c | 826 +++++++++++++++++++++++++++++++++++++++++
>  profiles/heartrate/heartrate.h |  27 ++
>  profiles/heartrate/main.c      |  52 +++
>  profiles/heartrate/manager.c   |  97 +++++
>  profiles/heartrate/manager.h   |  24 ++
>  src/bluetooth.conf             |   1 +
>  test/test-heartrate            | 103 +++++
>  11 files changed, 1228 insertions(+), 3 deletions(-)
>  create mode 100644 doc/heartrate-api.txt
>  create mode 100644 profiles/heartrate/heartrate.c
>  create mode 100644 profiles/heartrate/heartrate.h
>  create mode 100644 profiles/heartrate/main.c
>  create mode 100644 profiles/heartrate/manager.c
>  create mode 100644 profiles/heartrate/manager.h
>  create mode 100755 test/test-heartrate

All patches in this set have been applied. Thanks.

One notable change I did when applying was to the sensor location API.
The convention is to use lower case for string property values so I
change the API and code to follow this.

Johan

^ permalink raw reply

* Re: [PATCH BlueZ v2 0/8] Scan Parameters Plugin
From: Johan Hedberg @ 2012-09-28 10:05 UTC (permalink / raw)
  To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1348586802-32613-1-git-send-email-claudio.takahasi@openbossa.org>

Hi Claudio,

On Tue, Sep 25, 2012, Claudio Takahasi wrote:
> This patch series implements a new GATT based service/plugin for Scan
> Parameters: Optional service for HID devices.
> 
> This service enables a GATT Client to store the LE scan parameters on
> a GATT Server device, allowing the GATT Server to use this information
> to adjust the scanning settings to optimize power consumption and/or
> reconnection latency.
> 
> v1 changes: Using new profile abstraction (replaces btd_device drivers)
> v2 changes: Add profile parameter to device probe function
> 
> Claudio Takahasi (8):
>   scan: Add plugin skeleton
>   scan: Register profile
>   scan: Add ATTIO callbacks registration
>   scan: Add write scan interval window
>   scan: Enable Scan Refresh notification
>   scan: Register notification handler
>   scan: Write parameters when requested
>   scan: Avoid discover if scan handle is known
> 
>  Makefile.am                  |   8 +-
>  profiles/scanparam/main.c    |  55 +++++++++
>  profiles/scanparam/manager.c |  92 +++++++++++++++
>  profiles/scanparam/manager.h |  26 ++++
>  profiles/scanparam/scan.c    | 274 +++++++++++++++++++++++++++++++++++++++++++
>  profiles/scanparam/scan.h    |  26 ++++
>  6 files changed, 479 insertions(+), 2 deletions(-)
>  create mode 100644 profiles/scanparam/main.c
>  create mode 100644 profiles/scanparam/manager.c
>  create mode 100644 profiles/scanparam/manager.h
>  create mode 100644 profiles/scanparam/scan.c
>  create mode 100644 profiles/scanparam/scan.h

All patches in this set have been applied. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH BlueZ v3 4/6] hog: Create a FIFO for dummy suspend
From: Johan Hedberg @ 2012-09-28  9:58 UTC (permalink / raw)
  To: Claudio Takahasi; +Cc: linux-bluetooth
In-Reply-To: <1348587260-431-5-git-send-email-claudio.takahasi@openbossa.org>

Hi Claudio,

On Tue, Sep 25, 2012, Claudio Takahasi wrote:
> This patch creates a FIFO on "/tmp/hogsuspend" to allow the users to
> test the HoG suspend(HID Control Point) when the dummy back-end is
> enabled.
> ---
>  profiles/input/suspend-dummy.c | 102 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 101 insertions(+), 1 deletion(-)

Patches 1-3 have been applied but this one has issues:

Applying: hog: Create a FIFO for dummy suspend
WARNING:LONG_LINE: line over 80 characters
#64: FILE: profiles/input/suspend-dummy.c:84:
+	 * or output operation. When the remote closes the channel, hup signal is

Please fix and resend.

Johan

^ permalink raw reply

* Re: [PATCH 1/2] proximity: Split profile driver into Monitor and Reporter
From: Johan Hedberg @ 2012-09-28  9:47 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth
In-Reply-To: <1348064839-5602-1-git-send-email-andrzej.kaczmarek@tieto.com>

Hi Andrzej,

On Wed, Sep 19, 2012, Andrzej Kaczmarek wrote:
> Proximity Monitor interface is registered for each GATT device since profile
> driver matches by GATT_UUID which is intended only for Reporter role.
> 
> This patches splits Proximity profile driver into two separate drivers for
> Monitor and Reporter role to register interfaces properly.
> ---
>  profiles/proximity/manager.c  | 29 +++++++++++++++++++++--------
>  profiles/proximity/reporter.c |  2 +-
>  profiles/proximity/reporter.h |  2 +-
>  3 files changed, 23 insertions(+), 10 deletions(-)

Both of these patches have been applied after some rework to match the
newer profile API.

Johan

^ permalink raw reply

* Re: [PATCH] adapter: Remove unused struct btd_adapter members
From: Johan Hedberg @ 2012-09-28  9:39 UTC (permalink / raw)
  To: Szymon Janc; +Cc: linux-bluetooth
In-Reply-To: <1348824929-9884-1-git-send-email-szymon.janc@tieto.com>

Hi Szymon,

On Fri, Sep 28, 2012, Szymon Janc wrote:
> stop_discov_id and state are not used anymore.
> 
> ---
>  src/adapter.c |    3 ---
>  1 file changed, 3 deletions(-)

Applied. Thanks.

Johan

^ permalink raw reply

* Re: [RFC] Convert storage to use per-remote device directories
From: Johan Hedberg @ 2012-09-28  9:37 UTC (permalink / raw)
  To: Frederic Danis; +Cc: linux-bluetooth@vger.kernel.org
In-Reply-To: <50642389.40107@intel.com>

Hi Frederic,

On Thu, Sep 27, 2012, Frederic Danis wrote:
> Here is my proposal for new storage directory structure using
> ini-file format.
> 
> Each adapter directory (/var/lib/bluetooth/<adapter address>/) will
> contain a config file for the local adapter and one directory per
> remote device.
> The adapter config file just need to be converted to ini-file format
> with only 1 group called [adapter].
> 
> Each of remote device directories' name will be based on remote
> device address and address type (address#type).
> This directory will contain a config file with remote device infos
> and a linkkey file.
> Remote device config file will include a [device] group with general
> device infos (name, alias, profiles or services list, ...), and
> groups named by profile uuid (or service uuid) with related infos.
> 
> So the directory structure should be:
>    /var/lib/bluetooth/<adapter address>/
>        ./config
>        ./<remote device address#type>/
>            ./config
>            ./linkkey
>        ./<remote device address#type>/
>            ./config
>            ./linkkey
>        ...

So far this all looks good, though maybe we should follow the convention
of having .conf suffixes like with our other INI files. Or maybe that's
not needed?

> I attached sample of adapter and device config files.

> [adapter]
> name=desktop-0
> class=0x780011
> pairable=yes
> onmode=discoverable
> mode=discoverable

I hope you've looked at our existing INI files like audio.conf and
main.conf. You should have noticed that we use CamelCase for the section
and variable names, so at least that needs fixing.

> [device]
> name=MyPhone
> alias=Fred's phone
> class=0x180204
> device_id=FFFF 0000 0000 0000
> eir=040D040218
> manufacturer=15
> lmp_version=2
> lmp_subversion=777
> features=FFFE0D0008080000
> lastseen=2012-09-26 11:19:40 GMT
> lastused=2012-09-26 11:43:42 GMT
> trusted=yes
> profiles=00001101-0000-1000-8000-00805f9b34fb;00001103-0000-1000-8000-00805f9b34fb
> 
> [00001101-0000-1000-8000-00805f9b34fb]
> handle=10001
> record=35470900000A000100010900013503191101090004350C350319010035051900030802090005350319100209000935083506191101090100090100250C53657269616C20506F727400
> 
> [00001103-0000-1000-8000-00805f9b34fb]
> handle=10002
> record=35530900000A000100020900013503191103090004350C35031901003505190003080309000535031910020900093508350619110309010009010025134469616C2D7570204E6574776F726B696E67000903052800

This looks ok too, except for the lack of CamelCase naming. One of the
most interesting files I was waiting to see is the linkkey one since in
the existing storage we've crammed lots of separate variables into the
same entry. Could you send a proposal for this too in your next
revision? Since we also need storage for LTK's maybe it'd make sense to
have a single "keys" file with [LinkKey] and [LongTermKey] sections? Or
do you think those should be separate files?

Also, it seems you've left out all LE-specific information from this
initial proposal, like the conversion of the existing primaries file.

Johan

^ permalink raw reply

* [PATCH] adapter: Remove unused struct btd_adapter members
From: Szymon Janc @ 2012-09-28  9:35 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Szymon Janc

stop_discov_id and state are not used anymore.

---
 src/adapter.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index b1ea3ea..48d6294 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -112,7 +112,6 @@ struct btd_adapter {
 	uint32_t dev_class;		/* Class of Device */
 	char *name;			/* adapter name */
 	gboolean allow_name_changes;	/* whether the adapter name can be changed */
-	guint stop_discov_id;		/* stop inquiry/scanning id */
 	uint32_t discov_timeout;	/* discoverable time(sec) */
 	guint pairable_timeout_id;	/* pairable timeout id */
 	uint32_t pairable_timeout;	/* pairable time(sec) */
@@ -122,8 +121,6 @@ struct btd_adapter {
 					 * limited */
 	uint8_t global_mode;		/* last valid global mode */
 	struct session_req *pending_mode;
-	int state;			/* standard inq, periodic inq, name
-					 * resolving, suspended discovery */
 	GSList *found_devices;
 	GSList *oor_devices;		/* out of range device list */
 	struct agent *agent;		/* For the new API */
-- 
1.7.9.5


^ permalink raw reply related

* Re: [PATCH 0/5] mSBC tests
From: Dalleau, Frederic @ 2012-09-28  8:21 UTC (permalink / raw)
  To: Siarhei Siamashka; +Cc: Frédéric Dalleau, linux-bluetooth
In-Reply-To: <20120927233420.3441f14e@i7>

Hi Siarhei,

Thanks for reply,

On Thu, Sep 27, 2012 at 10:34 PM, Siarhei Siamashka
<siarhei.siamashka@gmail.com> wrote:
> Have you tried to tweak the SIMD code to make it process one
> block at a time instead of batches of 4?

I looked at sbc_analyze_4b_8s_simd but I missed the logic behind
reordering. That's
why I reused generic code in order to make sure that it would work.
If there is some interest I can look into it deeper. And if you have
any pointers
they are welcome.

> If we just could make SIMD code work, that would be the best option in
> my opinion.

Agreed, for sure.

>> Work done from the spec, not tested with any reference encoder.
>
> Does a reference mSBC encoder exist somewhere? Or any ideas how to test
> the new code with something other than bluez just to be sure that it's
> correct?

IIRC there used to be one, it was a windows program sbcencoder.exe but
I couldn't find
it anymore, and I'm not sure that it would support mSBC.

Regards,
Frédéric

^ permalink raw reply

* [bluetooth:master 231/247] net/bluetooth/amp.c:150:55: sparse: bad constant expression
From: Fengguang Wu @ 2012-09-28  1:24 UTC (permalink / raw)
  To: Dmitry Kasatkin
  Cc: kernel-janitors, Gustavo Padovan, Andrei Emeltchenko,
	linux-bluetooth

Hi Dmitry,

FYI, there are new sparse warnings show up in

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
head:   29d8a5909bba10accb82eb805c884a2943d7710f
commit: ba221bbabadd5fc2c80677b52178138fd694cc26 [231/247] Bluetooth: Add function to derive AMP key using hmac

+ net/bluetooth/amp.c:150:55: sparse: bad constant expression
  net/bluetooth/amp.c:130:12: warning: 'hmac_sha256' defined but not used [-Wunused-function]

vim +150 net/bluetooth/amp.c

ba221bba (Dmitry Kasatkin 2012-09-27  144) 	ret = crypto_shash_setkey(tfm, key, ksize);
ba221bba (Dmitry Kasatkin 2012-09-27  145) 	if (ret) {
ba221bba (Dmitry Kasatkin 2012-09-27  146) 		BT_DBG("crypto_ahash_setkey failed: err %d", ret);
ba221bba (Dmitry Kasatkin 2012-09-27  147) 	} else {
ba221bba (Dmitry Kasatkin 2012-09-27  148) 		struct {
ba221bba (Dmitry Kasatkin 2012-09-27  149) 			struct shash_desc shash;
ba221bba (Dmitry Kasatkin 2012-09-27 @150) 			char ctx[crypto_shash_descsize(tfm)];
ba221bba (Dmitry Kasatkin 2012-09-27  151) 		} desc;
ba221bba (Dmitry Kasatkin 2012-09-27  152) 
ba221bba (Dmitry Kasatkin 2012-09-27  153) 		desc.shash.tfm = tfm;
ba221bba (Dmitry Kasatkin 2012-09-27  154) 		desc.shash.flags = CRYPTO_TFM_REQ_MAY_SLEEP;
ba221bba (Dmitry Kasatkin 2012-09-27  155) 
ba221bba (Dmitry Kasatkin 2012-09-27  156) 		ret = crypto_shash_digest(&desc.shash, plaintext, psize,
ba221bba (Dmitry Kasatkin 2012-09-27  157) 					  output);
ba221bba (Dmitry Kasatkin 2012-09-27  158) 	}

---
0-DAY kernel build testing backend         Open Source Technology Centre
Fengguang Wu, Yuanhan Liu                              Intel Corporation

^ permalink raw reply

* [bluetooth:master 230/240] net/bluetooth/a2mp.c:358:2: warning: format '%u' expects argument of type 'unsigned int', but argument 5 has type 'long unsigned int'
From: Fengguang Wu @ 2012-09-28  1:17 UTC (permalink / raw)
  To: Andrei Emeltchenko
  Cc: Yuanhan Liu, kernel-janitors, Gustavo Padovan, linux-bluetooth

Hi Andrei,

FYI, there are new compile warnings show up in

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
head:   392f44d3e7894f6fe314b85b4a1611b7b1d98226
commit: 9a5e94dbb4aa306742a47cbbcb0a44d4fc77a9e4 [230/240] Bluetooth: A2MP: Process A2MP Get AMP Assoc Rsp
config: x86_64-allmodconfig

All warnings:

net/bluetooth/a2mp.c: In function 'a2mp_getampassoc_rsp':
net/bluetooth/a2mp.c:358:2: warning: format '%u' expects argument of type 'unsigned int', but argument 5 has type 'long unsigned int' [-Wformat]

vim +358 net/bluetooth/a2mp.c

9a5e94db (Andrei Emeltchenko 2012-09-27  346) static int a2mp_getampassoc_rsp(struct amp_mgr *mgr, struct sk_buff *skb,
9a5e94db (Andrei Emeltchenko 2012-09-27  347) 				struct a2mp_cmd *hdr)
9a5e94db (Andrei Emeltchenko 2012-09-27  348) {
9a5e94db (Andrei Emeltchenko 2012-09-27  349) 	struct a2mp_amp_assoc_rsp *rsp = (void *) skb->data;
9a5e94db (Andrei Emeltchenko 2012-09-27  350) 	u16 len = le16_to_cpu(hdr->len);
9a5e94db (Andrei Emeltchenko 2012-09-27  351) 	struct hci_dev *hdev;
9a5e94db (Andrei Emeltchenko 2012-09-27  352) 	struct amp_ctrl *ctrl;
9a5e94db (Andrei Emeltchenko 2012-09-27  353) 	struct hci_conn *hcon;
9a5e94db (Andrei Emeltchenko 2012-09-27  354) 
9a5e94db (Andrei Emeltchenko 2012-09-27  355) 	if (len < sizeof(*rsp))
9a5e94db (Andrei Emeltchenko 2012-09-27  356) 		return -EINVAL;
9a5e94db (Andrei Emeltchenko 2012-09-27  357) 
9a5e94db (Andrei Emeltchenko 2012-09-27 @358) 	BT_DBG("id %d status 0x%2.2x assoc len %u", rsp->id, rsp->status,
9a5e94db (Andrei Emeltchenko 2012-09-27  359) 	       len - sizeof(*rsp));
9a5e94db (Andrei Emeltchenko 2012-09-27  360) 
9a5e94db (Andrei Emeltchenko 2012-09-27  361) 	if (rsp->status)
9a5e94db (Andrei Emeltchenko 2012-09-27  362) 		return -EINVAL;

---
0-DAY kernel build testing backend         Open Source Technology Centre
Fengguang Wu, Yuanhan Liu                              Intel Corporation

^ permalink raw reply

* [bluetooth:master 224/240] net/bluetooth/hci_event.c:886:3: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t'
From: Fengguang Wu @ 2012-09-28  1:16 UTC (permalink / raw)
  To: Andrei Emeltchenko
  Cc: Yuanhan Liu, kernel-janitors, Gustavo Padovan, linux-bluetooth

Hi Andrei,

FYI, there are new compile warnings show up in

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
head:   392f44d3e7894f6fe314b85b4a1611b7b1d98226
commit: 903e45411099ae8292f5ce637ad0c72f6fef61db [224/240] Bluetooth: AMP: Use HCI cmd to Read Loc AMP Assoc
config: x86_64-allmodconfig

All warnings:

net/bluetooth/hci_event.c: In function 'hci_cc_read_local_amp_assoc':
net/bluetooth/hci_event.c:886:3: warning: format '%d' expects argument of type 'int', but argument 3 has type 'size_t' [-Wformat]
net/bluetooth/hci_event.c:886:3: warning: format '%d' expects argument of type 'int', but argument 4 has type 'size_t' [-Wformat]

vim +886 net/bluetooth/hci_event.c

903e4541 (Andrei Emeltchenko 2012-09-27  870) static void hci_cc_read_local_amp_assoc(struct hci_dev *hdev,
903e4541 (Andrei Emeltchenko 2012-09-27  871) 					struct sk_buff *skb)
903e4541 (Andrei Emeltchenko 2012-09-27  872) {
903e4541 (Andrei Emeltchenko 2012-09-27  873) 	struct hci_rp_read_local_amp_assoc *rp = (void *) skb->data;
903e4541 (Andrei Emeltchenko 2012-09-27  874) 	struct amp_assoc *assoc = &hdev->loc_assoc;
903e4541 (Andrei Emeltchenko 2012-09-27  875) 	size_t rem_len, frag_len;
903e4541 (Andrei Emeltchenko 2012-09-27  876) 
903e4541 (Andrei Emeltchenko 2012-09-27  877) 	BT_DBG("%s status 0x%2.2x", hdev->name, rp->status);
903e4541 (Andrei Emeltchenko 2012-09-27  878) 
903e4541 (Andrei Emeltchenko 2012-09-27  879) 	if (rp->status)
903e4541 (Andrei Emeltchenko 2012-09-27  880) 		goto a2mp_rsp;
903e4541 (Andrei Emeltchenko 2012-09-27  881) 
903e4541 (Andrei Emeltchenko 2012-09-27  882) 	frag_len = skb->len - sizeof(*rp);
903e4541 (Andrei Emeltchenko 2012-09-27  883) 	rem_len = __le16_to_cpu(rp->rem_len);
903e4541 (Andrei Emeltchenko 2012-09-27  884) 
903e4541 (Andrei Emeltchenko 2012-09-27  885) 	if (rem_len > frag_len) {
903e4541 (Andrei Emeltchenko 2012-09-27 @886) 		BT_DBG("frag_len %d rem_len %d", frag_len, rem_len);
903e4541 (Andrei Emeltchenko 2012-09-27  887) 
903e4541 (Andrei Emeltchenko 2012-09-27  888) 		memcpy(assoc->data + assoc->offset, rp->frag, frag_len);
903e4541 (Andrei Emeltchenko 2012-09-27  889) 		assoc->offset += frag_len;
903e4541 (Andrei Emeltchenko 2012-09-27  890) 
903e4541 (Andrei Emeltchenko 2012-09-27  891) 		/* Read other fragments */
903e4541 (Andrei Emeltchenko 2012-09-27  892) 		amp_read_loc_assoc_frag(hdev, rp->phy_handle);
903e4541 (Andrei Emeltchenko 2012-09-27  893) 
903e4541 (Andrei Emeltchenko 2012-09-27  894) 		return;

---
0-DAY kernel build testing backend         Open Source Technology Centre
Fengguang Wu, Yuanhan Liu                              Intel Corporation

^ permalink raw reply

* Re: [PATCH] Re: A2DP quality (bluetooth-alsa)
From: Siarhei Siamashka @ 2012-09-27 22:25 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <20120928012005.1c6c6d87@i7>

On Fri, Sep 28, 2012 at 1:20 AM, Siarhei Siamashka
<siarhei.siamashka@gmail.com> wrote:
> On Tue, 31 Jul 2012 13:09:19 +0300
> Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>
>> Hi Siarhei,

By the way, thanks for moving libsbc into its own library. This
definitely looks like the right thing.

-- 
Best regards,
Siarhei Siamashka

^ permalink raw reply

* Re: [PATCH] Re: A2DP quality (bluetooth-alsa)
From: Siarhei Siamashka @ 2012-09-27 22:20 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <CABBYNZKzJCqnkManeGxtWuDsKF_hHK+Ucbb1SXzUuce5cLdmqw@mail.gmail.com>

On Tue, 31 Jul 2012 13:09:19 +0300
Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:

> Hi Siarhei,
> 
> On Wed, Apr 4, 2012 at 11:12 AM, Siarhei Siamashka
> <siarhei.siamashka@gmail.com> wrote:
> > On Tue, Apr 3, 2012 at 3:56 AM, qduaty <qduaty@gmail.com> wrote:
> >> 2012/4/2 Siarhei Siamashka <siarhei.siamashka@gmail.com>:
> >>> Could you provide a short summary for this whole discussion? The
> >>> discussion has been changing from bitpool negotiation to SBC codec
> >>> improvements back and forth. And now it's becoming hard to follow
> >>> and see which problems are still relevant and need to be solved.
> >>> Also do you want to work on some code yourself or mostly trying
> >>> to escalate the problems?
> >>>
> >>> A2DP is in the list of ideas for BlueZ GSoC, so maybe that's a
> >>> good chance to get some improvements implemented:
> >>> http://www.bluez.org/development/gsoc/gsoc-ideas-list-2012/
> >>
> >> Ok. This is the summary.
> >> 1. It was found that Bluez can occasionally limit available bitpool
> >> because some devices report a narrow bitpool range that does not
> >> reflect their real capabilities. A solution was proposed by means
> >> of encoding at a higher bitpool than negotiated, and it was
> >> confirmed to both work and not break compatibility, unless it is
> >> misused. 2. It was found that Bluez audio sink has quality
> >> problems even on high bitpools. An SBC encoder fix was proposed,
> >> which (re)enables floating point processing. Tests are needed to
> >> confirm whether it improves quality in all cases.
> >> 3. It was (previously) found and now confirmed that reducing
> >> volume in SBC encoder by a factor of 0.7-0.8 improves quality by
> >> eliminating audible effects of clipping in the decoder.
> >
> > OK, thanks.
> >
> >> For myself, I'm likely to be done with it. I've already solved the
> >> problems that prevented me from using A2DP on Linux. Further work
> >> would require more time and I cannot benefit from it (but students
> >> obviously can).
> >
> > Fair enough.
> >
> >> My proposals:
> >> 1. Introduce a less restrictive input format for the audio sink,
> >> such as float32. Currently it supports only S16LE, which is not
> >> suitable for audio processing (ALSA softvol is a known example).
> >> 2. Find out what quality problems the SBC decoder has and fix them.
> >> Possibly introduce floating point output to eliminate clipping.
> >> 3. Implement proper (adaptive?) clipping prevention in the SBC
> >> encoder.

Sorry for a late reply, I have missed this one.
 
> Is there anyone working on this proposals?

These were all qduaty's ideas. If (s)he is not working on this stuff,
then I guess nobody does.

> Specially the decoder seems to need some optimization as it consumes
> quite a bit of CPU (6-8% on my i7) 

Yes, there are lots of low hanging fruits. For example, getting rid of
integer division and better bitstream reading should provide a
significant performance boost for the decoder. I might do that
eventually.

> and some people seems to notice some quality issues.

Are there some links to more information? Or to some audio quality
tests, which can be confirmed/reproduced?

The precision of fixed point calculations in the decoder is not
very good, so it definitely can be improved. But it should not be
too bad either.

The audio quality should be quite good for the encoder. Except for the
clamping problem, which may be happening on the decoder side with some
heavily compressed audio data and a poor/buggy A2DP headset. This
can be workarounded by reducing audio volume in the encoder by 20-30%.
The SBC encoder may get these extra volume adjustment settings, but the
big unresolved question is who and when is going to enable it? Would it
be some configuration file (pulseaudio/alsa/anything else)? Or would it
be some automatic heuristics kicking in for known bad headsets?

-- 
Best regards,
Siarhei Siamashka

^ permalink raw reply

* Re: [PATCH 2/2] Bluetooth: Factor out Create Configuration Response
From: Gustavo Padovan @ 2012-09-27 21:14 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth, johan.hedberg
In-Reply-To: <1348219805-19406-2-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

* Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-09-21 12:30:05 +0300]:

> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Use function to factor out similar code. For BR/EDR send EFS
> Configuration Response immediately, for HS response will be sent
> after receiving HCI Logical Link Complete event in the following
> patches.
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  net/bluetooth/l2cap_core.c |   32 +++++++++++++++++++-------------
>  1 file changed, 19 insertions(+), 13 deletions(-)

Both patch have been applied to bluetooth-next. Thanks.

	Gustavo

^ permalink raw reply

* Re: [PATCHv2 0/4] Remove unsafe batostr
From: Gustavo Padovan @ 2012-09-27 21:11 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1348566586-28572-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

* Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-09-25 12:49:42 +0300]:

> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Since the patch "vsprintf: add %pMR for Bluetooth MAC address" in in bluetooth-next resend the patch series.
> 
> Changes:
> 	* v2: Rebase against recent bluetooth-next.
> 
> Andrei Emeltchenko (4):
>   Bluetooth: Use %pMR in debug instead of batostr
>   Bluetooth: Use %pMR in sprintf/seq_printf  instead of batostr
>   Bluetooth: Use %pMR instead of baswap in seq_show
>   bluetooth: Remove unneeded batostr function
> 
>  include/net/bluetooth/bluetooth.h |    1 -
>  net/bluetooth/af_bluetooth.c      |   10 ++++------
>  net/bluetooth/bnep/core.c         |    3 +--
>  net/bluetooth/cmtp/core.c         |    2 +-
>  net/bluetooth/hci_conn.c          |    6 +++---
>  net/bluetooth/hci_core.c          |   22 +++++++++++-----------
>  net/bluetooth/hci_event.c         |   15 +++++++--------
>  net/bluetooth/hci_sysfs.c         |   10 +++++-----
>  net/bluetooth/hidp/core.c         |    8 ++++++--
>  net/bluetooth/l2cap_core.c        |   17 ++++++++---------
>  net/bluetooth/lib.c               |   14 --------------
>  net/bluetooth/rfcomm/core.c       |   15 +++++++--------
>  net/bluetooth/rfcomm/sock.c       |    9 ++++-----
>  net/bluetooth/rfcomm/tty.c        |    6 +++---
>  net/bluetooth/sco.c               |   12 ++++++------
>  15 files changed, 66 insertions(+), 84 deletions(-)

all 4 patches are now applied to bluetooth-next. Thanks.

	Gustavo

^ permalink raw reply

* Re: [PATCH] Bluetooth: Force the process of unpair command if disconnect failed
From: Gustavo Padovan @ 2012-09-27 21:09 UTC (permalink / raw)
  To: Jefferson Delfes; +Cc: linux-bluetooth
In-Reply-To: <1347989814-4958-1-git-send-email-jefferson.delfes@openbossa.org>

Hi Jefferson,

* Jefferson Delfes <jefferson.delfes@openbossa.org> [2012-09-18 13:36:54 -0400]:

> The unpair process tries to disconnect any connection pending with
> remote. If there are some connection in connecting state, disconnect
> command will fail and unpair mgmt command will stay pending.
> 
> That pending mgmt command can cause strange behavior like automatic
> unpair after a lost connection.
> ---
>  net/bluetooth/mgmt.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Patch has been applied to bluetooth-next. Thanks.

	Gustavo

^ permalink raw reply

* Re: [PATCHv1 3/3] Bluetooth: btmrvl: Fix skb buffer overflow
From: Gustavo Padovan @ 2012-09-27 20:51 UTC (permalink / raw)
  To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <1348732308-29348-3-git-send-email-Andrei.Emeltchenko.news@gmail.com>

Hi Andrei,

* Andrei Emeltchenko <Andrei.Emeltchenko.news@gmail.com> [2012-09-27 10:51:48 +0300]:

> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> Add extra check to avoid skb buffer overflow. Fixes crash below:
> 
>  [  101.030427] ------------[ cut here ]------------
>  [  101.030459] kernel BUG at net/core/skbuff.c:127!
>  [  101.030486] invalid opcode: 0000 [#1] SMP
> ...
>  [  101.030806] Pid: 2010, comm: btmrvl_main_ser Not tainted 3.5.0+ #80 Laptop
>  [  101.030859] EIP: 0060:[<c14f2ba9>] EFLAGS: 00010282 CPU: 0
>  [  101.030894] EIP is at skb_put+0x99/0xa0
>  [  101.030919] EAX: 00000080 EBX: f129380b ECX: ef923540 EDX: 00000001
>  [  101.030956] ESI: f00a4000 EDI: 00001003 EBP: ed4a5efc ESP: ed4a5ecc
>  [  101.030992]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
>  [  101.031024] CR0: 8005003b CR2: 08fca014 CR3: 30960000 CR4: 000407f0
>  [  101.031062] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
>  [  101.031100] DR6: ffff0ff0 DR7: 00000400
>  [  101.031125] Process btmrvl_main_ser (pid: 2010, ti=ed4a4000 task=ef923540 task.ti=ed4a4000)
>  [  101.031174] Stack:
>  [  101.031188]  c18126f8 c1651938 f853f8d2 00001003 00001003 f1292800 f1292808 f129380b
>  [  101.031250]  f1292940 f00a4000 eddb1280 efc0f9c0 ed4a5f44 f853f8d2 00000040 00000000
>  [  101.031312]  ef923540 c15ee096 ef923540 eddb12d4 00000004 f00a4000 00000040 00000000
>  [  101.031376] Call Trace:
>  [  101.031396]  [<f853f8d2>] ? btmrvl_sdio_process_int_status+0x272/0x3d0 [btmrvl_sdio]
>  [  101.031444]  [<f853f8d2>] btmrvl_sdio_process_int_status+0x272/0x3d0 [btmrvl_sdio]
>  [  101.031488]  [<c15ee096>] ? _raw_spin_unlock_irqrestore+0x36/0x70
>  [  101.031526]  [<f85a46e4>] btmrvl_service_main_thread+0x244/0x300 [btmrvl]
>  [  101.031568]  [<f853fb50>] ? btmrvl_sdio_poll_card_status.isra.6.constprop.7+0x90/0x90 [btmrvl_sdio]
>  [  101.031619]  [<c107eda0>] ? try_to_wake_up+0x270/0x270
>  [  101.031648]  [<f85a44a0>] ? btmrvl_process_event+0x3b0/0x3b0 [btmrvl]
>  [  101.031686]  [<c106d19d>] kthread+0x7d/0x90
>  [  101.031713]  [<c106d120>] ? flush_kthread_work+0x150/0x150
>  [  101.031745]  [<c15f5a82>] kernel_thread_helper+0x6/0x10
> ...
>  [  101.032008] EIP: [<c14f2ba9>] skb_put+0x99/0xa0 SS:ESP 0068:ed4a5ecc
>  [  101.056125] ---[ end trace a0bd01d1a9a796c8 ]---
> 
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> ---
>  drivers/bluetooth/btmrvl_sdio.c |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
> index 1457f25..b3b3680 100644
> --- a/drivers/bluetooth/btmrvl_sdio.c
> +++ b/drivers/bluetooth/btmrvl_sdio.c
> @@ -554,7 +554,16 @@ static int btmrvl_sdio_card_to_host(struct btmrvl_private *priv)
>  	 */
>  
>  	buf_len = payload[0];
> -	buf_len |= (u16) payload[1] << 8;
> +	buf_len |= payload[1] << 8;
> +	buf_len |= payload[2] << 16;
> +
> +	if (buf_len > blksz * num_blocks) {
> +		BT_ERR("Skip incorrect packet: hdrlen %d buffer %d",
> +		       buf_len, blksz * num_blocks);
> +		ret = -EIO;
> +		goto exit;
> +	}
> +
>  	type = payload[3];
>  
>  	BT_DBG("%s len %zu type %d", hdev->name, buf_len, type);

All three patches doesn't apply. Please check them.

	Gustavo

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox