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

Alfonso Sanchez-Beato alfonso.sanchez-beato at canonical.com
Thu Jun 30 08:17:58 UTC 2016


@Konrad, thanks for the comments, I have answered them below. Note however that for historical reasons in ofono we do usually the reviews in our github repo, and import the code to bzr when merged there. For this branch, we had PR https://github.com/rilmodem/ofono/pull/240 . Usually here we just make sure that changelog and other contents in debian/ (which are not in github) are good.

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);

Yes, it is safe (gets initiated in ril_voicecall_start()).

> +	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)

The issue is that some times quirks apply to both, other times to only one. So I prefer to expose this in all cases.

> +			&& 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;
> +};

ril uses the format of bind parcels, using only signed integers as payload.

> +
>  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.



More information about the Ubuntu-reviews mailing list