Hi Forrest,
In general the patch looks ok'ish (which it should since it seems to be
largely based on the maemo driver), though I do have a couple of comments:
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.
> +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?
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
