Re: [PATCH 05/19] plugins/ublox: allow enabling of TOBY L2 modems

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2991 bytes --]

Hi Dragos,

On 03/09/2016 09:44 AM, Dragos Tatulea wrote:
> For this we need to:
> * Set the vendor family based on model id.
> * Not use modem interface for the TOBY L2 family.
> ---
>   plugins/ublox.c | 53 ++++++++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 44 insertions(+), 9 deletions(-)
>
> diff --git a/plugins/ublox.c b/plugins/ublox.c
> index 89ca709..171d59d 100644
> --- a/plugins/ublox.c
> +++ b/plugins/ublox.c
> @@ -47,6 +47,7 @@ static const char *none_prefix[] = { NULL };
>   struct ublox_data {
>   	GAtChat *modem;
>   	GAtChat *aux;
> +	enum ofono_vendor vendor_family;
>   };
>
>   static void ublox_debug(const char *str, void *user_data)
> @@ -138,24 +139,58 @@ static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
>   static int ublox_enable(struct ofono_modem *modem)
>   {
>   	struct ublox_data *data = ofono_modem_get_data(modem);
> +	const char *model_str = NULL;
> +	int model_id;
>
>   	DBG("%p", modem);
>
> -	data->modem = open_device(modem, "Modem", "Modem: ");
> -	if (data->modem == NULL)
> +	model_str = ofono_modem_get_string(modem, "Model");
> +	if (model_str == NULL)
>   		return -EINVAL;
>
> +	/*
> +	 * Toby L2 devices are more complex and special than previously
> +	 * supported U-Blox devices. So they need a vendor of their own.
> +	 */
> +	model_id = atoi(model_str);
> +	switch (model_id) {
> +		case 1102:
> +			data->vendor_family = OFONO_VENDOR_UBLOX;
> +			break;
> +		case 1141:
> +		case 1146:
> +			data->vendor_family = OFONO_VENDOR_UBLOX_TOBY_L2;
> +			break;
> +		case 1143:
> +			DBG("low/medium throughtput profile unsupported");
> +		default:
> +			DBG("unknown ublox model id %d", model_id);
> +			return -EINVAL;
> +	}
> +
>   	data->aux = open_device(modem, "Aux", "Aux: ");
> -	if (data->aux == NULL) {
> -		g_at_chat_unref(data->modem);
> -		data->modem = NULL;
> -		return -EIO;
> +	if (data->aux == NULL)
> +		return -EINVAL;
> +
> +	if (data->vendor_family == OFONO_VENDOR_UBLOX) {
> +

No empty line here please

> +		data->modem = open_device(modem, "Modem", "Modem: ");
> +		if (data->modem == NULL) {
> +			g_at_chat_unref(data->aux);
> +			data->aux = NULL;
> +			return -EIO;
> +		}
> +
> +		g_at_chat_set_slave(data->modem, data->aux);
> +
> +		g_at_chat_send(data->modem, "ATE0 +CMEE=1", none_prefix,
> +						NULL, NULL, NULL);
> +

No unneeded whitespace here please

>   	}
>
> -	g_at_chat_set_slave(data->modem, data->aux);
> +	/* The modem can take a while to wake up if just powered on. */
> +	g_at_chat_set_wakeup_command(data->aux, "AT\r", 1000, 11000);

This is a pretty nasty hack specific for TI Calypso.  I wouldn't use it 
unless you really need it.

>
> -	g_at_chat_send(data->modem, "ATE0 +CMEE=1", none_prefix,
> -					NULL, NULL, NULL);
>   	g_at_chat_send(data->aux, "ATE0 +CMEE=1", none_prefix,
>   					NULL, NULL, NULL);
>
>

Regards,
-Denis