Hi Vinicius,

On 04/03/2013 06:24 PM, Vinicius Costa Gomes wrote:
---
  plugins/hfp_hf_bluez5.c | 41 ++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/plugins/hfp_hf_bluez5.c b/plugins/hfp_hf_bluez5.c
index a6cc156..4c8f412 100644
--- a/plugins/hfp_hf_bluez5.c
+++ b/plugins/hfp_hf_bluez5.c
@@ -67,6 +67,8 @@ struct hfp {
        struct hfp_slc_info info;
        DBusMessage *msg;
        struct ofono_handsfree_card *card;
+       ofono_handsfree_card_connect_cb_t cb;
+       void *user_data;

Please use the cb_data_new mechanism instead of storing this here.

  };

  static GDBusClient *bluez = NULL;
@@ -324,11 +326,48 @@ static void hfp16_card_remove(struct ofono_handsfree_card 
*card)

  }

+static void bcc_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+       struct hfp *hfp = user_data;
+       struct hfp_slc_info *info =&hfp->info;
+
+       if (ok)
+               CALLBACK_WITH_SUCCESS(hfp->cb, hfp->user_data);
+       else
+               CALLBACK_WITH_FAILURE(hfp->cb, hfp->user_data);

Please use decode_at_error instead.

e.g. decode_at_error(&error, g_at_result_final_response(result));
callback(&error, ...);

+
+       g_at_chat_unref(info->chat);

No need to reference count the g_at_chat.  This is just needless overhead.

+}
+
  static void hfp16_card_connect(struct ofono_handsfree_card *card,
                                        ofono_handsfree_card_connect_cb_t cb,
                                        void *data)
  {
-       CALLBACK_WITH_FAILURE(cb, data);
+       struct hfp *hfp = ofono_handsfree_card_get_data(card);
+       struct hfp_slc_info *info =&hfp->info;
+
+       if (info->chat == NULL) {
+               CALLBACK_WITH_FAILURE(cb, data);
+               return;
+       }

Can this case even happen?

+
+       if (!(info->hf_features&  HFP_HF_FEATURE_CODEC_NEGOTIATION&&
+                       info->ag_features&  HFP_AG_FEATURE_CODEC_NEGOTIATION)) {
+               /*
+                * If both sides don't support codec negotiation,
+                * fallback to direct SCO connection. Calling
+                * connect_sco() hands the connection responsability
+                * to the core, so no need to call the callback
+                */
+               ofono_handsfree_card_connect_sco(card);
+               return;
+       }

Can we reverse this logic to make it more readable?

e.g. if (HF has CodecNegotiation and AG has CodecNegotiation) {
        send BCC;
        return;
}

+
+       info->chat = g_at_chat_ref(info->chat);

No need to take a reference.  Remove this part

+       hfp->cb = cb;
+       hfp->user_data = data;
+

Use the cb_data_new mechanism here

+       g_at_chat_send(info->chat, "AT+BCC", NULL, bcc_cb, hfp, NULL);

And make sure to set the destroy callback. There are examples of this throughout the drivers/atmodem/* and drivers/hfpmodem/*.

  }

  static struct ofono_handsfree_card_driver hfp16_hf_driver = {

Regards,
-Denis
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono

Reply via email to