Re: [PATCH] add support for HFP plugin for oFono(ofono.org)

[Zhao Forrest <forrest.zhao@gmail.com>]

>
> On Wed, May 13, 2009, Forrest Zhao wrote:
>> +static int send_method_call(const char *dest, const char *path,
>> +                                const char *interface, const char *method,
>> +                                DBusPendingCallNotifyFunction cb,
>> +                                void *user_data, int type, ...);
>
> I don't think there should be any need for this forward declaration. Just
> move the function higher up in the file.
OK.

>> +static void answer_reply(DBusPendingCall *call, void *user_data)
>> +{
>> +     DBusMessage *reply;
>> +     DBusError err;
>> +
>> +     reply = dbus_pending_call_steal_reply(call);
>> +     dbus_error_init(&err);
>> +     if (dbus_set_error_from_message(&err, reply)) {
>> +             error("oFono answer incoming call failed: %s, %s",
>> +                     err.name, err.message);
>> +             dbus_error_free(&err);
>> +             goto done;
>> +     }
>> +
>> +done:
>> +     dbus_message_unref(reply);
>> +}
>> +
>> +void telephony_answer_call_req(void *telephony_device)
>> +{
>> +     struct voice_call *vc = find_vc_with_status(CALL_STATUS_INCOMING);
>> +     int ret;
>> +
>> +     if (!vc) {
>> +             telephony_answer_call_rsp(telephony_device,
>> +                                     CME_ERROR_NOT_ALLOWED);
>> +             return;
>> +     }
>> +
>> +     ret = send_method_call(OFONO_BUS_NAME, vc->obj_path,
>> +                     OFONO_VC_INTERFACE,
>> +                     "Answer", answer_reply,
>> +                     NULL, DBUS_TYPE_INVALID);
>> +
>> +     if (ret < 0) {
>> +             telephony_answer_call_rsp(telephony_device,
>> +                                     CME_ERROR_AG_FAILURE);
>> +             return;
>> +     }
>> +
>> +     telephony_answer_call_rsp(telephony_device, CME_ERROR_NONE);
>> +}
>
> If you're actually going to handle the D-Bus reply to the Answer method
> call (which the maemo driver doesn't do -- it relies on the state signals
> instead), doesn't it make sense to call telephony_answer_call_rsp in the
> reply handler (answer_reply) instead of here?
After reviewing the patch I think I should remove answer_reply() because
it also depends on the state signals to know if "answer" actually succeed.

Thanks,
Forrest