* [PATCH 1/2] android/handsfree: Add helper to check codec negotiation support
@ 2014-10-29 23:07 Lukasz Rymanowski
2014-10-29 23:07 ` [PATCH 2/2] android/handsfree: Be more strict on SLC creation Lukasz Rymanowski
2014-10-30 10:45 ` [PATCH 1/2] android/handsfree: Add helper to check codec negotiation support Szymon Janc
0 siblings, 2 replies; 5+ messages in thread
From: Lukasz Rymanowski @ 2014-10-29 23:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lukasz Rymanowski
This patch adds helper to test if codec negotiation is supported.
This patch fixes also logic behind that. So far we assumed that codec
negotiation is there if HFP HF does support it, even HFP AG does not
support it. It cause a problem with e.g. SLC creation in scenario where
HFP AG is not supporting codec negoctiation. In such a case we
incorrectly reply with ERROR on AT+CIND? because we incorretly expected
AT+BAC before that command. Issue found on UPF.
---
android/handsfree.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/android/handsfree.c b/android/handsfree.c
index 356dbe0..595919a 100644
--- a/android/handsfree.c
+++ b/android/handsfree.c
@@ -938,6 +938,12 @@ done:
hfp_gw_send_info(dev->gw, "+BCS: %u", type);
}
+static bool codec_negotiation_supported(struct hf_device *dev)
+{
+ return (dev->features & HFP_HF_FEAT_CODEC) &&
+ (hfp_ag_features & HFP_AG_FEAT_CODEC);
+}
+
static void connect_sco_cb(GIOChannel *chan, GError *err, gpointer user_data)
{
struct hf_device *dev = user_data;
@@ -947,7 +953,7 @@ static void connect_sco_cb(GIOChannel *chan, GError *err, gpointer user_data)
set_audio_state(dev, HAL_EV_HANDSFREE_AUDIO_STATE_DISCONNECTED);
- if (!(dev->features & HFP_HF_FEAT_CODEC))
+ if (!codec_negotiation_supported(dev))
return;
/* If other failed, try connecting with CVSD */
@@ -977,7 +983,7 @@ static bool connect_sco(struct hf_device *dev)
if (dev->sco)
return false;
- if (!(dev->features & HFP_HF_FEAT_CODEC))
+ if (!codec_negotiation_supported(dev))
voice_settings = 0;
else if (dev->negotiated_codec != CODEC_ID_CVSD)
voice_settings = BT_VOICE_TRANSPARENT;
@@ -1012,7 +1018,7 @@ static void at_cmd_bcc(struct hfp_gw_result *result, enum hfp_gw_cmd_type type,
switch (type) {
case HFP_GW_CMD_TYPE_COMMAND:
- if (!(dev->features & HFP_HF_FEAT_CODEC))
+ if (!codec_negotiation_supported(dev))
break;
if (hfp_gw_result_has_next(result))
@@ -1204,7 +1210,7 @@ static void at_cmd_cind(struct hfp_gw_result *result, enum hfp_gw_cmd_type type,
* If device supports Codec Negotiation, AT+BAC should be
* received first
*/
- if ((dev->features & HFP_HF_FEAT_CODEC) &&
+ if (codec_negotiation_supported(dev) &&
!dev->codecs[CVSD_OFFSET].remote_supported)
break;
@@ -1336,7 +1342,7 @@ static void at_cmd_bac(struct hfp_gw_result *result, enum hfp_gw_cmd_type type,
switch (type) {
case HFP_GW_CMD_TYPE_SET:
- if (!(dev->features & HFP_HF_FEAT_CODEC))
+ if (!codec_negotiation_supported(dev))
goto failed;
/* set codecs to defaults */
@@ -1765,7 +1771,7 @@ static bool connect_audio(struct hf_device *dev)
return false;
/* we haven't negotiated codec, start selection */
- if ((dev->features & HFP_HF_FEAT_CODEC) && !dev->negotiated_codec) {
+ if (codec_negotiation_supported(dev) && !dev->negotiated_codec) {
select_codec(dev, 0);
return true;
}
--
1.8.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] android/handsfree: Be more strict on SLC creation
2014-10-29 23:07 [PATCH 1/2] android/handsfree: Add helper to check codec negotiation support Lukasz Rymanowski
@ 2014-10-29 23:07 ` Lukasz Rymanowski
2014-10-30 10:23 ` Szymon Janc
2014-10-30 10:45 ` [PATCH 1/2] android/handsfree: Add helper to check codec negotiation support Szymon Janc
1 sibling, 1 reply; 5+ messages in thread
From: Lukasz Rymanowski @ 2014-10-29 23:07 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Lukasz Rymanowski
In case of any issue during SLC creation lets just drop the link
---
android/handsfree.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/android/handsfree.c b/android/handsfree.c
index 595919a..c8d3c04 100644
--- a/android/handsfree.c
+++ b/android/handsfree.c
@@ -1251,6 +1251,9 @@ static void at_cmd_cind(struct hfp_gw_result *result, enum hfp_gw_cmd_type type,
}
hfp_gw_send_result(dev->gw, HFP_RESULT_ERROR);
+
+ if (dev->state != HAL_EV_HANDSFREE_CONN_STATE_SLC_CONNECTED)
+ hfp_gw_disconnect(dev->gw);
}
static void at_cmd_brsf(struct hfp_gw_result *result, enum hfp_gw_cmd_type type,
@@ -1280,6 +1283,9 @@ static void at_cmd_brsf(struct hfp_gw_result *result, enum hfp_gw_cmd_type type,
}
hfp_gw_send_result(dev->gw, HFP_RESULT_ERROR);
+
+ if (dev->state != HAL_EV_HANDSFREE_CONN_STATE_SLC_CONNECTED)
+ hfp_gw_disconnect(dev->gw);
}
static void at_cmd_chld(struct hfp_gw_result *result, enum hfp_gw_cmd_type type,
@@ -1319,6 +1325,9 @@ static void at_cmd_chld(struct hfp_gw_result *result, enum hfp_gw_cmd_type type,
}
hfp_gw_send_result(dev->gw, HFP_RESULT_ERROR);
+
+ if (dev->state != HAL_EV_HANDSFREE_CONN_STATE_SLC_CONNECTED)
+ hfp_gw_disconnect(dev->gw);
}
static struct hfp_codec *find_codec_by_type(struct hf_device *dev, uint8_t type)
@@ -1392,6 +1401,9 @@ static void at_cmd_bac(struct hfp_gw_result *result, enum hfp_gw_cmd_type type,
failed:
hfp_gw_send_result(dev->gw, HFP_RESULT_ERROR);
+
+ if (dev->state != HAL_EV_HANDSFREE_CONN_STATE_SLC_CONNECTED)
+ hfp_gw_disconnect(dev->gw);
}
static void register_slc_at(struct hf_device *dev)
--
1.8.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] android/handsfree: Be more strict on SLC creation
2014-10-29 23:07 ` [PATCH 2/2] android/handsfree: Be more strict on SLC creation Lukasz Rymanowski
@ 2014-10-30 10:23 ` Szymon Janc
2014-10-30 15:39 ` Lukasz Rymanowski
0 siblings, 1 reply; 5+ messages in thread
From: Szymon Janc @ 2014-10-30 10:23 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
Hi Łukasz,
On Thursday 30 of October 2014 00:07:33 Lukasz Rymanowski wrote:
> In case of any issue during SLC creation lets just drop the link
> ---
> android/handsfree.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/android/handsfree.c b/android/handsfree.c
> index 595919a..c8d3c04 100644
> --- a/android/handsfree.c
> +++ b/android/handsfree.c
> @@ -1251,6 +1251,9 @@ static void at_cmd_cind(struct hfp_gw_result *result,
> enum hfp_gw_cmd_type type, }
>
> hfp_gw_send_result(dev->gw, HFP_RESULT_ERROR);
> +
> + if (dev->state != HAL_EV_HANDSFREE_CONN_STATE_SLC_CONNECTED)
> + hfp_gw_disconnect(dev->gw);
> }
>
> static void at_cmd_brsf(struct hfp_gw_result *result, enum hfp_gw_cmd_type
> type, @@ -1280,6 +1283,9 @@ static void at_cmd_brsf(struct hfp_gw_result
> *result, enum hfp_gw_cmd_type type, }
>
> hfp_gw_send_result(dev->gw, HFP_RESULT_ERROR);
> +
> + if (dev->state != HAL_EV_HANDSFREE_CONN_STATE_SLC_CONNECTED)
> + hfp_gw_disconnect(dev->gw);
> }
>
> static void at_cmd_chld(struct hfp_gw_result *result, enum hfp_gw_cmd_type
> type, @@ -1319,6 +1325,9 @@ static void at_cmd_chld(struct hfp_gw_result
> *result, enum hfp_gw_cmd_type type, }
>
> hfp_gw_send_result(dev->gw, HFP_RESULT_ERROR);
> +
> + if (dev->state != HAL_EV_HANDSFREE_CONN_STATE_SLC_CONNECTED)
> + hfp_gw_disconnect(dev->gw);
> }
>
> static struct hfp_codec *find_codec_by_type(struct hf_device *dev, uint8_t
> type) @@ -1392,6 +1401,9 @@ static void at_cmd_bac(struct hfp_gw_result
> *result, enum hfp_gw_cmd_type type,
>
> failed:
> hfp_gw_send_result(dev->gw, HFP_RESULT_ERROR);
> +
> + if (dev->state != HAL_EV_HANDSFREE_CONN_STATE_SLC_CONNECTED)
> + hfp_gw_disconnect(dev->gw);
> }
>
> static void register_slc_at(struct hf_device *dev)
In general I agree with dropping connection if there are any issues with SLC
creation. But it looks like you miss AT+CMER command.
--
BR
Szymon Janc
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] android/handsfree: Be more strict on SLC creation
2014-10-30 10:23 ` Szymon Janc
@ 2014-10-30 15:39 ` Lukasz Rymanowski
0 siblings, 0 replies; 5+ messages in thread
From: Lukasz Rymanowski @ 2014-10-30 15:39 UTC (permalink / raw)
To: Szymon Janc; +Cc: linux-bluetooth@vger.kernel.org
Hi Szymon,
On 30 October 2014 11:23, Szymon Janc <szymon.janc@tieto.com> wrote:
> Hi Łukasz,
>
> On Thursday 30 of October 2014 00:07:33 Lukasz Rymanowski wrote:
>> In case of any issue during SLC creation lets just drop the link
>> ---
>> android/handsfree.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/android/handsfree.c b/android/handsfree.c
>> index 595919a..c8d3c04 100644
>> --- a/android/handsfree.c
>> +++ b/android/handsfree.c
>> @@ -1251,6 +1251,9 @@ static void at_cmd_cind(struct hfp_gw_result *result,
>> enum hfp_gw_cmd_type type, }
>>
>> hfp_gw_send_result(dev->gw, HFP_RESULT_ERROR);
>> +
>> + if (dev->state != HAL_EV_HANDSFREE_CONN_STATE_SLC_CONNECTED)
>> + hfp_gw_disconnect(dev->gw);
>> }
>>
>> static void at_cmd_brsf(struct hfp_gw_result *result, enum hfp_gw_cmd_type
>> type, @@ -1280,6 +1283,9 @@ static void at_cmd_brsf(struct hfp_gw_result
>> *result, enum hfp_gw_cmd_type type, }
>>
>> hfp_gw_send_result(dev->gw, HFP_RESULT_ERROR);
>> +
>> + if (dev->state != HAL_EV_HANDSFREE_CONN_STATE_SLC_CONNECTED)
>> + hfp_gw_disconnect(dev->gw);
>> }
>>
>> static void at_cmd_chld(struct hfp_gw_result *result, enum hfp_gw_cmd_type
>> type, @@ -1319,6 +1325,9 @@ static void at_cmd_chld(struct hfp_gw_result
>> *result, enum hfp_gw_cmd_type type, }
>>
>> hfp_gw_send_result(dev->gw, HFP_RESULT_ERROR);
>> +
>> + if (dev->state != HAL_EV_HANDSFREE_CONN_STATE_SLC_CONNECTED)
>> + hfp_gw_disconnect(dev->gw);
>> }
>>
>> static struct hfp_codec *find_codec_by_type(struct hf_device *dev, uint8_t
>> type) @@ -1392,6 +1401,9 @@ static void at_cmd_bac(struct hfp_gw_result
>> *result, enum hfp_gw_cmd_type type,
>>
>> failed:
>> hfp_gw_send_result(dev->gw, HFP_RESULT_ERROR);
>> +
>> + if (dev->state != HAL_EV_HANDSFREE_CONN_STATE_SLC_CONNECTED)
>> + hfp_gw_disconnect(dev->gw);
>> }
>>
>> static void register_slc_at(struct hf_device *dev)
>
> In general I agree with dropping connection if there are any issues with SLC
> creation. But it looks like you miss AT+CMER command.
True, will fix that.
>
> --
> BR
> Szymon Janc
\Łukasz
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] android/handsfree: Add helper to check codec negotiation support
2014-10-29 23:07 [PATCH 1/2] android/handsfree: Add helper to check codec negotiation support Lukasz Rymanowski
2014-10-29 23:07 ` [PATCH 2/2] android/handsfree: Be more strict on SLC creation Lukasz Rymanowski
@ 2014-10-30 10:45 ` Szymon Janc
1 sibling, 0 replies; 5+ messages in thread
From: Szymon Janc @ 2014-10-30 10:45 UTC (permalink / raw)
To: Lukasz Rymanowski; +Cc: linux-bluetooth
Hi Łukasz,
On Thursday 30 of October 2014 00:07:32 Lukasz Rymanowski wrote:
> This patch adds helper to test if codec negotiation is supported.
>
> This patch fixes also logic behind that. So far we assumed that codec
> negotiation is there if HFP HF does support it, even HFP AG does not
> support it. It cause a problem with e.g. SLC creation in scenario where
> HFP AG is not supporting codec negoctiation. In such a case we
> incorrectly reply with ERROR on AT+CIND? because we incorretly expected
> AT+BAC before that command. Issue found on UPF.
> ---
> android/handsfree.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/android/handsfree.c b/android/handsfree.c
> index 356dbe0..595919a 100644
> --- a/android/handsfree.c
> +++ b/android/handsfree.c
> @@ -938,6 +938,12 @@ done:
> hfp_gw_send_info(dev->gw, "+BCS: %u", type);
> }
>
> +static bool codec_negotiation_supported(struct hf_device *dev)
> +{
> + return (dev->features & HFP_HF_FEAT_CODEC) &&
> + (hfp_ag_features & HFP_AG_FEAT_CODEC);
> +}
> +
> static void connect_sco_cb(GIOChannel *chan, GError *err, gpointer
> user_data) {
> struct hf_device *dev = user_data;
> @@ -947,7 +953,7 @@ static void connect_sco_cb(GIOChannel *chan, GError
> *err, gpointer user_data)
>
> set_audio_state(dev, HAL_EV_HANDSFREE_AUDIO_STATE_DISCONNECTED);
>
> - if (!(dev->features & HFP_HF_FEAT_CODEC))
> + if (!codec_negotiation_supported(dev))
> return;
>
> /* If other failed, try connecting with CVSD */
> @@ -977,7 +983,7 @@ static bool connect_sco(struct hf_device *dev)
> if (dev->sco)
> return false;
>
> - if (!(dev->features & HFP_HF_FEAT_CODEC))
> + if (!codec_negotiation_supported(dev))
> voice_settings = 0;
> else if (dev->negotiated_codec != CODEC_ID_CVSD)
> voice_settings = BT_VOICE_TRANSPARENT;
> @@ -1012,7 +1018,7 @@ static void at_cmd_bcc(struct hfp_gw_result *result,
> enum hfp_gw_cmd_type type,
>
> switch (type) {
> case HFP_GW_CMD_TYPE_COMMAND:
> - if (!(dev->features & HFP_HF_FEAT_CODEC))
> + if (!codec_negotiation_supported(dev))
> break;
>
> if (hfp_gw_result_has_next(result))
> @@ -1204,7 +1210,7 @@ static void at_cmd_cind(struct hfp_gw_result *result,
> enum hfp_gw_cmd_type type, * If device supports Codec Negotiation, AT+BAC
> should be
> * received first
> */
> - if ((dev->features & HFP_HF_FEAT_CODEC) &&
> + if (codec_negotiation_supported(dev) &&
> !dev->codecs[CVSD_OFFSET].remote_supported)
> break;
>
> @@ -1336,7 +1342,7 @@ static void at_cmd_bac(struct hfp_gw_result *result,
> enum hfp_gw_cmd_type type,
>
> switch (type) {
> case HFP_GW_CMD_TYPE_SET:
> - if (!(dev->features & HFP_HF_FEAT_CODEC))
> + if (!codec_negotiation_supported(dev))
> goto failed;
>
> /* set codecs to defaults */
> @@ -1765,7 +1771,7 @@ static bool connect_audio(struct hf_device *dev)
> return false;
>
> /* we haven't negotiated codec, start selection */
> - if ((dev->features & HFP_HF_FEAT_CODEC) && !dev->negotiated_codec) {
> + if (codec_negotiation_supported(dev) && !dev->negotiated_codec) {
> select_codec(dev, 0);
> return true;
> }
This patch is now applied, thanks.
--
BR
Szymon Janc
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-10-30 15:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-29 23:07 [PATCH 1/2] android/handsfree: Add helper to check codec negotiation support Lukasz Rymanowski
2014-10-29 23:07 ` [PATCH 2/2] android/handsfree: Be more strict on SLC creation Lukasz Rymanowski
2014-10-30 10:23 ` Szymon Janc
2014-10-30 15:39 ` Lukasz Rymanowski
2014-10-30 10:45 ` [PATCH 1/2] android/handsfree: Add helper to check codec negotiation support Szymon Janc
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).