Battery information

Battery information

[Bastien Nocera]

[-- Attachment #1: Type: text/plain, Size: 1035 bytes --]

Hello,

A colleague has a Samsung WEP470 headset which triggers an error from
bluetoothd.

Attached is a minimal patch to avoid the error. Would be nice if we
could export that property through bluetoothd.

Cheers

Nov  6 10:27:57 luminos bluetoothd[4848]: link_key_notify
(sba=00:1C:26:F5:21:BB
, dba=00:0D:E6:A6:99:7B, type=0)
Nov  6 10:27:57 luminos bluetoothd[4848]: Stopping discovery
Nov  6 10:27:58 luminos bluetoothd[4848]: Stopping discovery
Nov  6 10:27:58 luminos rtkit-daemon[1606]: Sucessfully made thread 4893 of proc
ess 3010 (/usr/bin/pulseaudio) owned by '500' RT at priority 5.
Nov  6 10:27:58 luminos bluetoothd[4848]: Badly formated or unrecognized command
: AT+CBC=99
Nov  6 10:29:15 luminos bluetoothd[4848]: Software caused connection abort (103)
Nov  6 10:29:15 luminos pulseaudio[3010]: module-bluetooth-device.c: Failed to w
rite data to SCO socket: Transport endpoint is not connected
Nov  6 10:29:59 luminos bluetoothd[4848]: Disconnected from 00:0D:E6:A6:99:7B, /
org/bluez/4847/hci0/dev_00_0D_E6_A6_99_7B

[-- Attachment #2: 0001-Avoid-errors-when-audio-headset-tells-us-its-battery.patch --]
[-- Type: text/x-patch, Size: 984 bytes --]

>From 6e9f62f7251086f09d476d3fce9090e3d22acf63 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Fri, 6 Nov 2009 18:49:53 +0000
Subject: [PATCH] Avoid errors when audio headset tells us its battery charge

Like the Samsung WEP470 does.
---
 audio/headset.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/audio/headset.c b/audio/headset.c
index cf1ae61..36a84c2 100644
--- a/audio/headset.c
+++ b/audio/headset.c
@@ -1132,6 +1132,11 @@ static int nr_and_ec(struct audio_device *device, const char *buf)
 	return 0;
 }
 
+static int battery_event(struct audio_device *device, const char *buf)
+{
+	return 0;
+}
+
 static struct event event_callbacks[] = {
 	{ "ATA", answer_call },
 	{ "ATD", dial_number },
@@ -1152,6 +1157,7 @@ static struct event event_callbacks[] = {
 	{ "AT+CCWA", call_waiting_notify },
 	{ "AT+COPS", operator_selection },
 	{ "AT+NREC", nr_and_ec },
+	{ "AT+CBC", battery_event },
 	{ 0 }
 };
 
-- 
1.6.5.2

Re: Battery information

[Johan Hedberg]

Hi Bastien,

On Fri, Nov 06, 2009, Bastien Nocera wrote:
> A colleague has a Samsung WEP470 headset which triggers an error from
> bluetoothd.
> 
> Attached is a minimal patch to avoid the error. Would be nice if we
> could export that property through bluetoothd.

I don't really see the point of this patch. What's the benefit of
returning "OK" instead of "ERROR" in this case, especially since the
added command handler doesn't do anything? As far as I see returning
"ERROR" is the right behavior for us when we encounter a command which
we don't understand (there are also no other side effects on our side
when we send an error, i.e. the connection is allowed proceed normally).

As far as exporting the battery charge info goes, yes we could consider
that. However, I'm a bit hesitant about this since AT+CBC isn't part of
the HFP spec. It is in the 3GPP AT-command spec (TS 07.07), but even
there it's only specified to be used for querying the ME's (BlueZ in
this case) battery level. I.e. the way this headset uses it doesn't seem
to conform to any standard (or at least any that I'm aware of).

Johan

Re: Battery information

[Bastien Nocera]

On Fri, 2009-11-06 at 21:58 +0200, Johan Hedberg wrote:
> Hi Bastien,
> 
> On Fri, Nov 06, 2009, Bastien Nocera wrote:
> > A colleague has a Samsung WEP470 headset which triggers an error from
> > bluetoothd.
> > 
> > Attached is a minimal patch to avoid the error. Would be nice if we
> > could export that property through bluetoothd.
> 
> I don't really see the point of this patch. What's the benefit of
> returning "OK" instead of "ERROR" in this case, especially since the
> added command handler doesn't do anything? As far as I see returning
> "ERROR" is the right behavior for us when we encounter a command which
> we don't understand (there are also no other side effects on our side
> when we send an error, i.e. the connection is allowed proceed normally).

Nobody likes errors showing up in their /var/log/messages. In any case,
that was a minimal patch to avoid that.

> As far as exporting the battery charge info goes, yes we could consider
> that. However, I'm a bit hesitant about this since AT+CBC isn't part of
> the HFP spec. It is in the 3GPP AT-command spec (TS 07.07), but even
> there it's only specified to be used for querying the ME's (BlueZ in
> this case) battery level. I.e. the way this headset uses it doesn't seem
> to conform to any standard (or at least any that I'm aware of).

I'm pretty sure it's a vendor extension from Samsung. And I guess that
it would actually show the battery information when paired with a
Samsung phone. So it might be something useful to handle in bluetoothd.

Re: Battery information

[Johan Hedberg]

On Fri, Nov 06, 2009, Bastien Nocera wrote:
> > I don't really see the point of this patch. What's the benefit of
> > returning "OK" instead of "ERROR" in this case, especially since the
> > added command handler doesn't do anything? As far as I see returning
> > "ERROR" is the right behavior for us when we encounter a command which
> > we don't understand (there are also no other side effects on our side
> > when we send an error, i.e. the connection is allowed proceed normally).
> 
> Nobody likes errors showing up in their /var/log/messages. In any case,
> that was a minimal patch to avoid that.

If that's the main issue here I'd rather just change the error() into a
debug() in the code. You'll get much more than just one message like
that when connecting e.g. to a CSR based headset since they use lots of
vendor-specific AT commands.

Johan