Re: [PATCH 02/19] plugins/udevng: support different interface strings to detect TOBY series

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Dongsu,

On 03/09/2016 09:44 AM, Dragos Tatulea wrote:
> From: Dongsu Park <dongsu@endocode.com>
>
> Each modem expresses their interfaces with its own interface string,
> which is composed of 3 different USB attributes:
> "bInterfaceClass/bInterfaceSubClass/bInterfaceProtocol".
> While the old models like LISA support only "2/2/1" for modem
> interfaces, TOBY-L2 also supports an unique string for NetworkInterface
> for each profile.
>
> * low-medium throughput profile : 2/6/0
> * fairly backward-compatible profile : 10/0/0
> * high throughput profile : 224/1/3
>
> Besides the condition for checking NULL for mdm/aux/net should be relaxed
> a little bit.
> ---
>   plugins/udevng.c | 22 +++++++++++++++++++---
>   1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index 52b667e..2170c90 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -838,7 +838,7 @@ static gboolean setup_quectel(struct modem_info *modem)
>
>   static gboolean setup_ublox(struct modem_info *modem)
>   {
> -	const char *aux = NULL, *mdm = NULL;
> +	const char *aux = NULL, *mdm = NULL, *net = NULL;
>   	GSList *list;
>
>   	DBG("%s", modem->syspath);
> @@ -857,21 +857,37 @@ static gboolean setup_ublox(struct modem_info *modem)
>   			mdm = info->devnode;
>   			if (aux != NULL)
>   				break;
> +		/*
> +		 * "2/2/1"
> +		 *  - a common modem interface both for older models like LISA,
> +		 *    and for newer models like TOBY.
> +		 * For TOBY-L2, NetworkInterface can be detected for each profile:
> +		 *  - low-medium throughput profile : 2/6/0
> +		 *  - fairly backward-compatible profile : 10/0/0
> +		 *  - high throughput profile : 224/1/3
> +		 * */
>   		} else if (g_strcmp0(info->interface, "2/2/1") == 0) {
>   			if (g_strcmp0(info->number, "02") == 0)
>   				aux = info->devnode;
>   			else if (g_strcmp0(info->number, "00") == 0)
>   				mdm = info->devnode;
> +		} else if (g_strcmp0(info->interface, "2/6/0") == 0 ||
> +			   g_strcmp0(info->interface, "10/0/0") == 0 ||
> +			   g_strcmp0(info->interface, "224/1/3") == 0) {
> +			net = info->devnode;
>   		}
>   	}
>
> -	if (aux == NULL || mdm == NULL)
> +	/* Abort only if both interfaces are NULL, as it's highly possible that
> +	 * only one of 2 interfaces is available for U-blox modem. */

Not our comment style, please look at doc/coding-style.txt item M2

> +	if (aux == NULL && mdm == NULL)
>   		return FALSE;
>
> -	DBG("aux=%s modem=%s", aux, mdm);
> +	DBG("aux=%s modem=%s net=%s", aux, mdm, net);
>
>   	ofono_modem_set_string(modem->modem, "Aux", aux);
>   	ofono_modem_set_string(modem->modem, "Modem", mdm);
> +	ofono_modem_set_string(modem->modem, "NetworkInterface", net);
>
>   	return TRUE;
>   }
>

Regards,
-Denis