[Merge] lp:~phablet-team/ofono/midori-support into lp:~phablet-team/ofono/ubuntu

[Konrad Zapałowicz]

Review: Needs Information code

LGTM however I have a few uncertainties - check the questions in comments.

Diff comments:

> 
> === added file 'drivers/mtk2modem/voicecall.c'
> --- drivers/mtk2modem/voicecall.c	1970-01-01 00:00:00 +0000
> +++ drivers/mtk2modem/voicecall.c	2016-06-28 15:38:35 +0000
> @@ -0,0 +1,149 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2016  Canonical Ltd.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#define _GNU_SOURCE
> +#include <string.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <errno.h>
> +
> +#include <glib.h>
> +
> +#include <ofono/log.h>
> +#include <ofono/modem.h>
> +#include <ofono/voicecall.h>
> +
> +#include "mtk2_constants.h"
> +#include "drivers/mtkmodem/mtkunsol.h"
> +#include "drivers/mtkmodem/mtkrequest.h"
> +
> +#include "common.h"
> +#include "mtk2modem.h"
> +#include "drivers/rilmodem/rilutil.h"
> +#include "drivers/rilmodem/voicecall.h"
> +
> +/*
> + * This is the voicecall atom implementation for mtk2modem. Most of the
> + * implementation can be found in the rilmodem atom. The main reason for
> + * creating a new atom is the need to handle specific MTK requests and
> + * unsolicited events.
> + */
> +
> +static void mtk2_set_indication_cb(struct ril_msg *message, gpointer user_data)
> +{
> +	struct ofono_voicecall *vc = user_data;
> +	struct ril_voicecall_data *vd = ofono_voicecall_get_data(vc);
> +
> +	if (message->error == RIL_E_SUCCESS)
> +		g_ril_print_response_no_args(vd->ril, message);

Are we sure that the vd will always be valid at this point (and following as this pattern repeats)?

> +	else
> +		ofono_error("%s: RIL error %s", __func__,
> +				ril_error_to_string(message->error));
> +}
> +
> +static void mtk2_incoming_notify(struct ril_msg *message, gpointer user_data)
> +{
> +	struct ofono_voicecall *vc = user_data;
> +	struct ril_voicecall_data *vd = ofono_voicecall_get_data(vc);
> +	struct parcel rilp;
> +	struct unsol_call_indication *call_ind;
> +
> +	call_ind = g_mtk_unsol_parse_incoming_call_indication(vd->ril, message);
> +	if (call_ind == NULL) {
> +		ofono_error("%s: error parsing event", __func__);
> +		return;
> +	}
> +
> +	g_mtk_request_set_call_indication(vd->ril, MTK_CALL_INDIC_MODE_AVAIL,
> +						call_ind->call_id,
> +						call_ind->seq_number, &rilp);
> +
> +	if (g_ril_send(vd->ril, MTK2_RIL_REQUEST_SET_CALL_INDICATION,
> +			&rilp, mtk2_set_indication_cb, vc, NULL) == 0)
> +		ofono_error("%s: cannot send indication", __func__);
> +
> +	g_mtk_unsol_free_call_indication(call_ind);
> +}
> +
> +static gboolean mtk2_delayed_register(gpointer user_data)
> +{
> +	struct ofono_voicecall *vc = user_data;
> +	struct ril_voicecall_data *vd = ofono_voicecall_get_data(vc);
> +
> +	/* Indicates incoming call, before telling the network our state */
> +	g_ril_register(vd->ril, MTK2_RIL_UNSOL_INCOMING_CALL_INDICATION,
> +			mtk2_incoming_notify, vc);
> +
> +	/* This makes the timeout a single-shot */
> +	return FALSE;
> +}
> +
> +static int mtk2_voicecall_probe(struct ofono_voicecall *vc, unsigned int vendor,
> +				void *data)
> +{
> +	struct ril_voicecall_driver_data *driver_data = data;
> +	struct ril_voicecall_data *vd;
> +
> +	vd = g_try_new0(struct ril_voicecall_data, 1);
> +	if (vd == NULL)
> +		return -ENOMEM;
> +
> +	ril_voicecall_start(driver_data, vc, vendor, vd);
> +
> +	/*
> +	 * Register events after ofono_voicecall_register() is called from
> +	 * ril_delayed_register().
> +	 */
> +	g_idle_add(mtk2_delayed_register, vc);
> +
> +	return 0;
> +}
> +
> +static struct ofono_voicecall_driver driver = {
> +	.name			= MTK2MODEM,
> +	.probe			= mtk2_voicecall_probe,
> +	.remove			= ril_voicecall_remove,
> +	.dial			= ril_dial,
> +	.answer			= ril_answer,
> +	.hangup_all		= ril_hangup_all,
> +	.release_specific	= ril_hangup_specific,
> +	.send_tones		= ril_send_dtmf,
> +	.create_multiparty	= ril_create_multiparty,
> +	.private_chat		= ril_private_chat,
> +	.swap_without_accept	= ril_swap_without_accept,
> +	.hold_all_active	= ril_hold_all_active,
> +	.release_all_held	= ril_release_all_held,
> +	.set_udub		= ril_set_udub,
> +	.release_all_active	= ril_release_all_active,
> +};
> +
> +void mtk2_voicecall_init(void)
> +{
> +	ofono_voicecall_driver_register(&driver);
> +}
> +
> +void mtk2_voicecall_exit(void)
> +{
> +	ofono_voicecall_driver_unregister(&driver);
> +}
> 
> === modified file 'drivers/rilmodem/ussd.c'
> --- drivers/rilmodem/ussd.c	2014-05-12 14:21:13 +0000
> +++ drivers/rilmodem/ussd.c	2016-06-28 15:38:35 +0000
> @@ -178,8 +180,10 @@
>  	}
>  
>  	/* To fix bug in MTK: USSD-Notify arrive with type 2 instead of 0 */
> -	if (g_ril_vendor(ud->ril) == OFONO_RIL_VENDOR_MTK &&
> -			unsol->message != NULL && unsol->type == 2)
> +	vendor = g_ril_vendor(ud->ril);
> +	if ((vendor == OFONO_RIL_VENDOR_MTK ||
> +				vendor == OFONO_RIL_VENDOR_MTK2)

Looks complex, would it be better to replace with a func or macro such as "is vendor mtk"?

> +			&& unsol->message != NULL && unsol->type == 2)
>  		unsol->type = 0;
>  
>  	/*
> 
> === modified file 'gril/grilreply.h'
> --- gril/grilreply.h	2015-03-18 14:05:58 +0000
> +++ gril/grilreply.h	2016-06-28 15:38:35 +0000
> @@ -97,6 +97,15 @@
>  	void *data;
>  };
>  
> +struct reply_radio_capability {
> +	int version;
> +	int session;
> +	int phase;
> +	int rat;
> +	char modem_uuid[RIL_MAX_UUID_LENGTH];
> +	int status;
> +};

Are we sure that all of the integers can be negative or some of them can be unsigned?

> +
>  void g_ril_reply_free_avail_ops(struct reply_avail_ops *reply);
>  
>  struct reply_avail_ops *g_ril_reply_parse_avail_ops(GRil *gril,


-- 
https://code.launchpad.net/~phablet-team/ofono/midori-support/+merge/298542
Your team Ubuntu Phablet Team is subscribed to branch lp:~phablet-team/ofono/ubuntu.