Re: [PATCH 03/15] plugins: adding support for the SIMCom A7605E-H

[Denis Kenzior <denkenz@gmail.com>]

Hi Maxim,

On 10/17/23 05:48, MaxLyubimov wrote:
> The udevng plugin has been modified to detect the SIMCom A7605E-H modem and the simcom_a76xx plugin has been added to work with it
> ---
>   plugins/simcom_a76xx.c | 323 +++++++++++++++++++++++++++++++++++++++++
>   plugins/udevng.c       |  55 +++++++

Please see HACKING, 'Submitting patches' section, item 3.

>   2 files changed, 378 insertions(+)
>   create mode 100644 plugins/simcom_a76xx.c
> 
> diff --git a/plugins/simcom_a76xx.c b/plugins/simcom_a76xx.c
> new file mode 100644
> index 00000000..059773b5
> --- /dev/null
> +++ b/plugins/simcom_a76xx.c
> @@ -0,0 +1,323 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
> + *  Copyright (C) 2009  Collabora Ltd. All rights reserved.
> + *  Copyright 2018 Purism SPC

Copyrights seem suspect?

> + *
> + *  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
> + *
> + */
> +
> +/*
> + * This file was originally copied from g1.c and
> + * modified by Bob Ham <bob.ham@puri.sm>
> + */
> +

In fact this looks 95% the same to plugins/simcom7100.c.  Can we unify the two?

Also, you still have lines > 80 chars here.

<snip>

> +#ifdef HAVE_CONFIG_H> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index f6580aba..7fccf670 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -1554,6 +1554,59 @@ static gboolean setup_sim7x00(struct modem_info *modem)
>   	return TRUE;
>   }
>   
> +static gboolean setup_simcom_a76xx(struct modem_info *modem)
> +{
> +	const char *diag = NULL, *gps = NULL;
> +	const char *mdm = NULL, *ppp = NULL;
> +	GSList *list;
> +
> +	DBG("%s", modem->syspath);
> +
> +	for (list = modem->devices; list; list = list->next) {
> +		struct device_info *info = list->data;
> +		const char *subsystem =
> +				udev_device_get_subsystem(info->udev_device);
> +
> +		DBG("%s %s %s %s %s %s", info->devnode, info->interface,
> +						info->number, info->label,
> +						info->sysattr, subsystem);
> +
> +		/*
> +		 * SIMCom a76xx serial port layout:
> +		 * 2: DIAG
> +		 * 3: NMEA
> +		 * 4: AT
> +		 * 5: AT/PPP
> +		 */
> +		if (g_strcmp0(subsystem, "tty") == 0) {
> +			if (g_strcmp0(info->interface, "255/0/0") == 0) {
> +				if (g_strcmp0(info->number, "02") == 0)
> +					diag = info->devnode;
> +				else if (g_strcmp0(info->number, "03") == 0)
> +					gps = info->devnode;
> +				else if (g_strcmp0(info->number, "04") == 0)
> +					mdm = info->devnode;
> +				else if (g_strcmp0(info->number, "05") == 0)
> +					ppp = info->devnode;

Again, this block seems rather similar to what is in setup_simcom.  Just the 
interface numbers are different.  Please unify the two.

Regards,
-Denis