[PATCH] Init telephony before adding HFP SDP records

[PATCH] Init telephony before adding HFP SDP records

[Daniel Örstadius]

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

Please review.

Regards,
Daniel

[-- Attachment #2: 0001-Init-telephony-before-adding-HFP-SDP-records.patch --]
[-- Type: text/x-patch, Size: 2887 bytes --]

From bbc64db8c307b060b55b53c7006f57d63a86effe Mon Sep 17 00:00:00 2001
From: Daniel Orstadius <daniel.orstadius@nokia.com>
Date: Thu, 26 May 2011 15:35:19 +0300
Subject: [PATCH] Init telephony before adding HFP SDP records

By patch b9d85c00108977b24ae91ea8ecedb9ceffe6f648 the initialization
of the telephony subsystem is delayed in the bluetoothd startup
procedure. As a result the SupportedFeatures bitmap has not been set
when creating the HFP SDP record. This patch calls the first
initialization from the previous place so that the record
gets the correct value.

Although the corresponding bit in +BRSF is correctly set, the
missing bit for the "Three-way calling" feature in the SDP record
causes some headsets not to send AT+CHLD=? in certain situations.
This results in failed connections since BlueZ does not enter the
"connected" state on the headset interface before that command is
received, if the feature is supported by both sides.
---
 audio/manager.c |   39 ++++++++++++++++++++++++---------------
 1 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/audio/manager.c b/audio/manager.c
index 3518137..ce20676 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -854,25 +854,12 @@ static struct audio_adapter *audio_adapter_get(struct btd_adapter *adapter)
 	return adp;
 }
 
-static void state_changed(struct btd_adapter *adapter, gboolean powered)
+static void setup_telephony(gboolean powered)
 {
 	struct audio_adapter *adp;
 	static gboolean telephony = FALSE;
 	GSList *l;
 
-	DBG("%s powered %s", adapter_get_path(adapter),
-						powered ? "on" : "off");
-
-	/* ignore powered change, adapter is powering down */
-	if (powered && adapter_powering_down(adapter))
-		return;
-
-	adp = find_adapter(adapters, adapter);
-	if (!adp)
-		return;
-
-	adp->powered = powered;
-
 	if (powered) {
 		/* telephony driver already initialized*/
 		if (telephony == TRUE)
@@ -897,6 +884,26 @@ static void state_changed(struct btd_adapter *adapter, gboolean powered)
 	telephony = FALSE;
 }
 
+static void state_changed(struct btd_adapter *adapter, gboolean powered)
+{
+	struct audio_adapter *adp;
+
+	error("%s powered %s", adapter_get_path(adapter),
+						powered ? "on" : "off");
+
+	/* ignore powered change, adapter is powering down */
+	if (powered && adapter_powering_down(adapter))
+		return;
+
+	adp = find_adapter(adapters, adapter);
+	if (!adp)
+		return;
+
+	adp->powered = powered;
+
+	setup_telephony(powered);
+}
+
 static int headset_server_probe(struct btd_adapter *adapter)
 {
 	struct audio_adapter *adp;
@@ -1223,8 +1230,10 @@ proceed:
 	if (enabled.media)
 		btd_register_adapter_driver(&media_server_driver);
 
-	if (enabled.headset)
+	if (enabled.headset) {
+		setup_telephony(TRUE);
 		btd_register_adapter_driver(&headset_server_driver);
+	}
 
 	if (enabled.gateway)
 		btd_register_adapter_driver(&gateway_server_driver);
-- 
1.6.0.4

Re: [PATCH] Init telephony before adding HFP SDP records

[Luiz Augusto von Dentz]

Hi Daniel,

On Thu, May 26, 2011 at 4:58 PM, Daniel Örstadius
<daniel.orstadius@gmail.com> wrote:
> Please review.

For telephony-maemo6 it should be enough to just call
headset_server_init after state_changed/telephony_init.

Now this won't work with telephony-ofono since it waits some network
to be available before it signals it is ready, but I guess we can move
to its call of telepfony_ready_ind to telephony_init since the
features bits does not seems to depend in network availability.

Regards,

-- 
Luiz Augusto von Dentz
Computer Engineer

Re: [PATCH] Init telephony before adding HFP SDP records

[Daniel Örstadius]

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

Hi Luiz,

On Thu, May 26, 2011 at 6:44 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Daniel,
>
> On Thu, May 26, 2011 at 4:58 PM, Daniel Örstadius
> <daniel.orstadius@gmail.com> wrote:
>> Please review.
>
> For telephony-maemo6 it should be enough to just call
> headset_server_init after state_changed/telephony_init.
>

Thanks for the feedback, that seems like a much leaner solution.
Attaching new patch.

Regards,
Daniel

[-- Attachment #2: 0001-Init-telephony-before-adding-the-HFP-SDP-record.patch --]
[-- Type: text/x-patch, Size: 1658 bytes --]

From 6ef2596d1b42abd6db51d22a5cf2239a0669ca12 Mon Sep 17 00:00:00 2001
From: Daniel Orstadius <daniel.orstadius@nokia.com>
Date: Fri, 27 May 2011 11:26:18 +0300
Subject: [PATCH] Init telephony before adding the HFP SDP record

By patch b9d85c00108977b24ae91ea8ecedb9ceffe6f648 the initialization
of the telephony subsystem is delayed in the bluetoothd startup
procedure. As a result the SupportedFeatures bitmap has not been set
when creating the HFP SDP record. This patch changes the order of
the telephony initialization (via the state_changed function) and
the registration of the record, so that it gets the right value.

Although the corresponding bit in +BRSF is correctly set, the
missing bit for the "Three-way calling" feature in the SDP record
causes some headsets not to send AT+CHLD=? in certain situations.
This results in failed connections since BlueZ does not enter the
"connected" state on the headset interface before that command is
received, if the feature is supported by both sides.
---
 audio/manager.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/audio/manager.c b/audio/manager.c
index 3518137..911af45 100644
--- a/audio/manager.c
+++ b/audio/manager.c
@@ -909,15 +909,15 @@ static int headset_server_probe(struct btd_adapter *adapter)
 	if (!adp)
 		return -EINVAL;
 
+	btd_adapter_register_powered_callback(adapter, state_changed);
+	state_changed(adapter, TRUE);
+
 	err = headset_server_init(adp);
 	if (err < 0) {
 		audio_adapter_unref(adp);
 		return err;
 	}
 
-	btd_adapter_register_powered_callback(adapter, state_changed);
-	state_changed(adapter, TRUE);
-
 	return 0;
 }
 
-- 
1.6.0.4

Re: [PATCH] Init telephony before adding HFP SDP records

[Luiz Augusto von Dentz]

Hi Daniel,

On Fri, May 27, 2011 at 11:40 AM, Daniel Örstadius
<daniel.orstadius@gmail.com> wrote:
> Hi Luiz,
>
> On Thu, May 26, 2011 at 6:44 PM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Daniel,
>>
>> On Thu, May 26, 2011 at 4:58 PM, Daniel Örstadius
>> <daniel.orstadius@gmail.com> wrote:
>>> Please review.
>>
>> For telephony-maemo6 it should be enough to just call
>> headset_server_init after state_changed/telephony_init.
>>
>
> Thanks for the feedback, that seems like a much leaner solution.
> Attaching new patch.

Looks much better, shall I take care of fixing telephony-ofono or you
already have a patch for it?

-- 
Luiz Augusto von Dentz
Computer Engineer

Re: [PATCH] Init telephony before adding HFP SDP records

[Daniel Örstadius]

Hi Luiz,

On Fri, May 27, 2011 at 1:26 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Daniel,
>
> On Fri, May 27, 2011 at 11:40 AM, Daniel Örstadius
> <daniel.orstadius@gmail.com> wrote:
>> Hi Luiz,
>>
>> On Thu, May 26, 2011 at 6:44 PM, Luiz Augusto von Dentz
>> <luiz.dentz@gmail.com> wrote:
>>> Hi Daniel,
>>>
>>> On Thu, May 26, 2011 at 4:58 PM, Daniel Örstadius
>>> <daniel.orstadius@gmail.com> wrote:
>>>> Please review.
>>>
>>> For telephony-maemo6 it should be enough to just call
>>> headset_server_init after state_changed/telephony_init.
>>>
>>
>> Thanks for the feedback, that seems like a much leaner solution.
>> Attaching new patch.
>
> Looks much better, shall I take care of fixing telephony-ofono or you
> already have a patch for it?
>
> --
> Luiz Augusto von Dentz
> Computer Engineer
>

No, since I'm not familiar with ofono I didn't make a patch for that issue.

Regards,
Daniel

Re: [PATCH] Init telephony before adding HFP SDP records

[Daniel Örstadius]

Hi Luiz,

On Fri, May 27, 2011 at 1:45 PM, Daniel Örstadius
<daniel.orstadius@gmail.com> wrote:
> Hi Luiz,
>
> On Fri, May 27, 2011 at 1:26 PM, Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Daniel,
>>
>> On Fri, May 27, 2011 at 11:40 AM, Daniel Örstadius
>> <daniel.orstadius@gmail.com> wrote:
>>> Hi Luiz,
>>>
>>> On Thu, May 26, 2011 at 6:44 PM, Luiz Augusto von Dentz
>>> <luiz.dentz@gmail.com> wrote:
>>>> Hi Daniel,
>>>>
>>>> On Thu, May 26, 2011 at 4:58 PM, Daniel Örstadius
>>>> <daniel.orstadius@gmail.com> wrote:
>>>>> Please review.
>>>>
>>>> For telephony-maemo6 it should be enough to just call
>>>> headset_server_init after state_changed/telephony_init.
>>>>
>>>
>>> Thanks for the feedback, that seems like a much leaner solution.
>>> Attaching new patch.
>>
>> Looks much better, shall I take care of fixing telephony-ofono or you
>> already have a patch for it?
>>
>> --
>> Luiz Augusto von Dentz
>> Computer Engineer
>>
>
> No, since I'm not familiar with ofono I didn't make a patch for that issue.
>
> Regards,
> Daniel
>

Since the patch was not applied I'm wondering how to proceed with it?

(Maybe I misunderstood, but isn't the ofono issue a separate bug that
exists regardless of the patch?)

Regards,
Daniel

Re: [PATCH] Init telephony before adding HFP SDP records

[Luiz Augusto von Dentz]

Hi Daniel,

On Wed, Jun 1, 2011 at 10:58 AM, Daniel Örstadius
<daniel.orstadius@gmail.com> wrote:
> Hi Luiz,
>
> On Fri, May 27, 2011 at 1:45 PM, Daniel Örstadius
> <daniel.orstadius@gmail.com> wrote:
>> Hi Luiz,
>>
>> On Fri, May 27, 2011 at 1:26 PM, Luiz Augusto von Dentz
>> <luiz.dentz@gmail.com> wrote:
>>> Hi Daniel,
>>>
>>> On Fri, May 27, 2011 at 11:40 AM, Daniel Örstadius
>>> <daniel.orstadius@gmail.com> wrote:
>>>> Hi Luiz,
>>>>
>>>> On Thu, May 26, 2011 at 6:44 PM, Luiz Augusto von Dentz
>>>> <luiz.dentz@gmail.com> wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> On Thu, May 26, 2011 at 4:58 PM, Daniel Örstadius
>>>>> <daniel.orstadius@gmail.com> wrote:
>>>>>> Please review.
>>>>>
>>>>> For telephony-maemo6 it should be enough to just call
>>>>> headset_server_init after state_changed/telephony_init.
>>>>>
>>>>
>>>> Thanks for the feedback, that seems like a much leaner solution.
>>>> Attaching new patch.
>>>
>>> Looks much better, shall I take care of fixing telephony-ofono or you
>>> already have a patch for it?
>>>
>>> --
>>> Luiz Augusto von Dentz
>>> Computer Engineer
>>>
>>
>> No, since I'm not familiar with ofono I didn't make a patch for that issue.
>>
>> Regards,
>> Daniel
>>
>
> Since the patch was not applied I'm wondering how to proceed with it?
>
> (Maybe I misunderstood, but isn't the ofono issue a separate bug that
> exists regardless of the patch?)

Yep, it is unrelated, Im fine with it.

Regards,

-- 
Luiz Augusto von Dentz
Computer Engineer

Re: [PATCH] Init telephony before adding HFP SDP records

[Johan Hedberg]

Hi Daniel,

On Fri, May 27, 2011, Daniel Örstadius wrote:
> Thanks for the feedback, that seems like a much leaner solution.
> Attaching new patch.
> 
> Regards,
> Daniel

> From 6ef2596d1b42abd6db51d22a5cf2239a0669ca12 Mon Sep 17 00:00:00 2001
> From: Daniel Orstadius <daniel.orstadius@nokia.com>
> Date: Fri, 27 May 2011 11:26:18 +0300
> Subject: [PATCH] Init telephony before adding the HFP SDP record
> 
> By patch b9d85c00108977b24ae91ea8ecedb9ceffe6f648 the initialization
> of the telephony subsystem is delayed in the bluetoothd startup
> procedure. As a result the SupportedFeatures bitmap has not been set
> when creating the HFP SDP record. This patch changes the order of
> the telephony initialization (via the state_changed function) and
> the registration of the record, so that it gets the right value.
> 
> Although the corresponding bit in +BRSF is correctly set, the
> missing bit for the "Three-way calling" feature in the SDP record
> causes some headsets not to send AT+CHLD=? in certain situations.
> This results in failed connections since BlueZ does not enter the
> "connected" state on the headset interface before that command is
> received, if the feature is supported by both sides.
> ---
>  audio/manager.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)

Applied. Thanks.

Johan