Hi Denis,
On 18:07 Fri 12 Apr, Denis Kenzior wrote:
> Hi Vinicius,
>
> On 04/10/2013 02:37 PM, Vinicius Costa Gomes wrote:
> >This patch adds a function to monitor when the AG sends a new codec
> >before establishing the SCO connection.
> >---
> > plugins/hfp_hf_bluez5.c | 51
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 51 insertions(+)
> >
> >diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
> >index c63b1a2..efbb209 100644
> >--- a/plugins/hfp_hf_bluez5.c
> >+++ b/plugins/hfp_hf_bluez5.c
> >@@ -314,9 +314,60 @@ static struct ofono_modem_driver hfp_driver = {
> > .post_sim = hfp_post_sim,
> > };
> >
> >+static void bcs_notify(GAtResult *result, gpointer user_data)
> >+{
> >+ struct hfp *hfp = user_data;
> >+ struct hfp_slc_info *info =&hfp->info;
> >+ GAtResultIter iter;
> >+ char str[32];
> >+ int value;
> >+
> >+ g_at_result_iter_init(&iter, result);
> >+
> >+ if (!g_at_result_iter_next(&iter, "+BCS:"))
> >+ return;
> >+
> >+ if (!g_at_result_iter_next_number(&iter,&value))
> >+ return;
> >+
> >+ memset(str, 0, sizeof(str));
>
> Why do you need this exactly?
I don't. Will fix.
>
> >+
> >+ switch (value) {
> >+ case HFP_CODEC_MSBC:
> >+ if (!ofono_handsfree_audio_has_wideband()) {
> >+ /* No wideband speech support, so we only have CVSD */
> >+ sprintf(str, "AT+BAC=%d", HFP_CODEC_CVSD);
> >+ break;
> >+ }
> >+
> >+ /* fall through */
> >+
>
> This fall through and the default statement make this code extremely
> hard to read. Why don't we move the codec selection logic into the
> core instead?
>
> Then this becomes something like:
>
> if (ofono_handsfree_audio_card_set_codec(card, value) == FALSE) {
> if (ofono_handsfree_audio_has_wideband())
> ...
> else
> ...
>
> goto done;
> }
>
> sprintf(str, "AT+BCS=%d", value);
>
> done:
> ....
Sounds good. Let's see how it ends up looking.
>
>
> >+ case HFP_CODEC_CVSD:
> >+ /* Confirm the codec */
> >+ sprintf(str, "AT+BCS=%d", value);
> >+ break;
> >+
> >+ default:
> >+ /* Unsupported codec, re-send our codecs */
> >+ if (ofono_handsfree_audio_has_wideband())
> >+ sprintf(str, "AT+BAC=%d,%d", HFP_CODEC_CVSD,
> >+ HFP_CODEC_MSBC);
> >+ else
> >+ sprintf(str, "AT+BAC=%d", HFP_CODEC_CVSD);
> >+ }
> >+
> >+ g_at_chat_send(info->chat, str, NULL, NULL, NULL, NULL);
> >+}
> >+
> > static int hfp16_card_probe(struct ofono_handsfree_card *card,
> > unsigned int vendor, void *data)
> > {
> >+ struct hfp *hfp = data;
> >+ struct hfp_slc_info *info =&hfp->info;
> >+
> >+ g_at_chat_register(info->chat, "+BCS:", bcs_notify, FALSE,
> >+ hfp, NULL);
> >+
>
> You read my mind, I was going to suggest doing this exactly :)
>
> > return 0;
> > }
> >
>
> Regards,
> -Denis
Cheers,
--
Vinicius
_______________________________________________
ofono mailing list
[email protected]
https://lists.ofono.org/mailman/listinfo/ofono
