[PATCH 01/15] Skipping AT+CUAD sending for QUECTEL_EC2X vendor

[PATCH 01/15] Skipping AT+CUAD sending for QUECTEL_EC2X vendor

[MaxLyubimov]

From: Sergei Golubtsov <sg@aqsi.ru>

This workarounds bug in EC200A firmware which leads to modem
reset in case of usage of some SIM cards when sending the next
command
---
 drivers/atmodem/sim.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 67fe21e3..3cb8c7bd 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -1640,7 +1640,14 @@ static void at_discover_apps(struct ofono_sim *sim,
 				void *data)
 {
 	struct sim_data *sd = ofono_sim_get_data(sim);
-	struct cb_data *cbd = cb_data_new(cb, data);
+	struct cb_data *cbd;
+
+	/* QUECTEL EC2X reboots when executing the AT+CUAD command with SIM cards
+	of some operators */
+	if (sd->vendor == OFONO_VENDOR_QUECTEL_EC2X)
+		goto error;
+
+	cbd = cb_data_new(cb, data);
 
 	if (g_at_chat_send(sd->chat, "AT+CUAD", cuad_prefix,
 			at_discover_apps_cb, cbd, g_free) > 0)
@@ -1648,6 +1655,7 @@ static void at_discover_apps(struct ofono_sim *sim,
 
 	g_free(cbd);
 
+error:
 	CALLBACK_WITH_FAILURE(cb, NULL, 0, data);
 }
 
-- 
2.25.1

[PATCH 02/15] drivers: adding support for the SIMCom A7605E-H

[MaxLyubimov]

The features of the SIMCom A7605E-H modem are taken into account in the atmodem driver
---
 drivers/atmodem/gprs.c               |  59 ++++++-
 drivers/atmodem/sim.c                |   7 +-
 drivers/simcommodem/radio-settings.c | 236 +++++++++++++++++++++++++++
 3 files changed, 298 insertions(+), 4 deletions(-)
 create mode 100644 drivers/simcommodem/radio-settings.c

diff --git a/drivers/atmodem/gprs.c b/drivers/atmodem/gprs.c
index 8c352ba6..530d86a5 100644
--- a/drivers/atmodem/gprs.c
+++ b/drivers/atmodem/gprs.c
@@ -41,6 +41,7 @@
 #include "gatchat.h"
 #include "gatresult.h"
 
+#include "common.h"
 #include "vendor.h"
 
 #define MAX_CONTEXTS 255
@@ -544,6 +545,54 @@ static void telit_mode_notify(GAtResult *result, gpointer user_data)
 	ofono_gprs_bearer_notify(gprs, bearer);
 }
 
+static void simcom_mode_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_gprs *gprs = user_data;
+	GAtResultIter iter;
+	gint stat, bearer;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CNSMOD:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter,&stat))
+		return;
+
+	switch (stat) {
+	case 0:
+		bearer = PACKET_BEARER_NONE;
+		break;
+	case 1:
+	case 2:
+		bearer = PACKET_BEARER_GPRS;
+		break;
+	case 3:
+		bearer = PACKET_BEARER_EGPRS;
+		break;
+	case 4:
+		bearer = PACKET_BEARER_UMTS;
+		break;
+	case 5:
+		bearer = PACKET_BEARER_HSDPA;
+		break;
+	case 6:
+		bearer = PACKET_BEARER_HSUPA;
+		break;
+	case 7:
+		bearer = PACKET_BEARER_HSUPA_HSDPA;
+		break;
+	case 8:
+		bearer = PACKET_BEARER_EPS;
+		break;
+	default:
+		bearer = PACKET_BEARER_NONE;
+		break;
+	}
+
+	ofono_gprs_bearer_notify(gprs, bearer);
+}
+
 static void ublox_ureg_notify(GAtResult *result, gpointer user_data)
 {
 	struct ofono_gprs *gprs = user_data;
@@ -626,6 +675,12 @@ static void gprs_initialized(gboolean ok, GAtResult *result, gpointer user_data)
 		g_at_chat_send(gd->chat, "AT#PSNT=1", none_prefix,
 						NULL, NULL, NULL);
 		break;
+	case OFONO_VENDOR_SIMCOM:
+		g_at_chat_register(gd->chat, "+CNSMOD:", simcom_mode_notify,
+						FALSE, gprs, NULL);
+		g_at_chat_send(gd->chat, "AT+CNSMOD=1", none_prefix,
+						NULL, NULL, NULL);
+		break;
 	case OFONO_VENDOR_QUECTEL_EC2X:
 	case OFONO_VENDOR_QUECTEL_SERIAL:
 		break;
@@ -749,7 +804,9 @@ retry:
 		goto error;
 
 	g_at_chat_send(gd->chat, cmd, none_prefix, NULL, NULL, NULL);
-	g_at_chat_send(gd->chat, "AT+CGAUTO=0", none_prefix, NULL, NULL, NULL);
+	if (gd->vendor != OFONO_VENDOR_SIMCOM) {
+		g_at_chat_send(gd->chat, "AT+CGAUTO=0", none_prefix, NULL, NULL, NULL);
+	}
 
 	switch (gd->vendor) {
 	case OFONO_VENDOR_MBM:
diff --git a/drivers/atmodem/sim.c b/drivers/atmodem/sim.c
index 3cb8c7bd..8849a129 100644
--- a/drivers/atmodem/sim.c
+++ b/drivers/atmodem/sim.c
@@ -1642,9 +1642,10 @@ static void at_discover_apps(struct ofono_sim *sim,
 	struct sim_data *sd = ofono_sim_get_data(sim);
 	struct cb_data *cbd;
 
-	/* QUECTEL EC2X reboots when executing the AT+CUAD command with SIM cards
-	of some operators */
-	if (sd->vendor == OFONO_VENDOR_QUECTEL_EC2X)
+	/* QUECTEL EC2X and SIMCom A7605E-H reboots when executing the AT+CUAD
+	command with SIM cards of some operators */
+	if (sd->vendor == OFONO_VENDOR_QUECTEL_EC2X
+			|| sd->vendor == OFONO_VENDOR_SIMCOM)
 		goto error;
 
 	cbd = cb_data_new(cb, data);
diff --git a/drivers/simcommodem/radio-settings.c b/drivers/simcommodem/radio-settings.c
new file mode 100644
index 00000000..2a856daf
--- /dev/null
+++ b/drivers/simcommodem/radio-settings.c
@@ -0,0 +1,236 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2017 Intel Corporation. All rights reserved.
+ *  Copyright (C) 2021 Sergey Matyukevich. All rights reserved.
+ *
+ *  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
+
+#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/radio-settings.h>
+
+#include <drivers/atmodem/atutil.h>
+
+#include "gatchat.h"
+#include "gatresult.h"
+
+static const char *none_prefix[] = { NULL };
+static const char *cnmp_prefix[] = { "+CNMP:", NULL };
+
+struct radio_settings_data {
+	GAtChat *chat;
+};
+
+static void cnmp_query_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_radio_settings_rat_mode_query_cb_t cb = cbd->cb;
+	unsigned int mode;
+	struct ofono_error error;
+	int r_mode;
+	GAtResultIter iter;
+
+	DBG("ok %d", ok);
+
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok) {
+		cb(&error, -1, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+CNMP:"))
+		goto error;
+
+	if (!g_at_result_iter_next_number(&iter, &r_mode))
+		goto error;
+
+	DBG("r_mode %d", r_mode);
+
+	switch (r_mode) {
+	case 2:
+		mode = OFONO_RADIO_ACCESS_MODE_ANY;
+		break;
+	case 13:
+		mode = OFONO_RADIO_ACCESS_MODE_GSM;
+		break;
+	case 14:
+		mode = OFONO_RADIO_ACCESS_MODE_UMTS;
+		break;
+	case 38:
+		mode = OFONO_RADIO_ACCESS_MODE_LTE;
+		break;
+	default:
+		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+		return;
+	}
+
+	cb(&error, mode, cbd->data);
+
+	return;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+}
+
+static void simcom_query_rat_mode(struct ofono_radio_settings *rs,
+				ofono_radio_settings_rat_mode_query_cb_t cb,
+				void *data)
+{
+	struct radio_settings_data *rsd = ofono_radio_settings_get_data(rs);
+	struct cb_data *cbd = cb_data_new(cb, data);
+
+	DBG("");
+
+	if (g_at_chat_send(rsd->chat, "AT+CNMP?", cnmp_prefix,
+				cnmp_query_cb, cbd, g_free) == 0) {
+		CALLBACK_WITH_FAILURE(cb, -1, data);
+		g_free(cbd);
+	}
+}
+
+static void cnmp_modify_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_radio_settings_rat_mode_set_cb_t cb = cbd->cb;
+	struct ofono_error error;
+
+	DBG("ok %d", ok);
+
+	decode_at_error(&error, g_at_result_final_response(result));
+	cb(&error, cbd->data);
+}
+
+static void simcom_set_rat_mode(struct ofono_radio_settings *rs,
+				unsigned int m,
+				ofono_radio_settings_rat_mode_set_cb_t cb,
+				void *data)
+{
+	struct radio_settings_data *rsd = ofono_radio_settings_get_data(rs);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	int w_mode = 2;
+	char buf[20];
+
+	DBG("mode %d", m);
+
+	switch (m) {
+	case OFONO_RADIO_ACCESS_MODE_ANY:
+		w_mode = 2;
+		break;
+	case OFONO_RADIO_ACCESS_MODE_GSM:
+		w_mode = 13;
+		break;
+	case OFONO_RADIO_ACCESS_MODE_UMTS:
+	case OFONO_RADIO_ACCESS_MODE_UMTS|OFONO_RADIO_ACCESS_MODE_GSM:
+		w_mode = 14;
+		break;
+	case OFONO_RADIO_ACCESS_MODE_LTE:
+	case OFONO_RADIO_ACCESS_MODE_LTE|OFONO_RADIO_ACCESS_MODE_GSM:
+	case OFONO_RADIO_ACCESS_MODE_LTE|OFONO_RADIO_ACCESS_MODE_UMTS:
+		w_mode = 38;
+		break;
+	}
+
+	snprintf(buf, sizeof(buf), "AT+CNMP=%u", w_mode);
+
+	if (g_at_chat_send(rsd->chat, buf, none_prefix,
+				cnmp_modify_cb, cbd, g_free) > 0)
+		return;
+
+	CALLBACK_WITH_FAILURE(cb, data);
+	g_free(cbd);
+}
+
+static void simcom_query_available_rats(struct ofono_radio_settings *rs,
+			ofono_radio_settings_available_rats_query_cb_t cb,
+			void *data)
+{
+	unsigned int available_rats = OFONO_RADIO_ACCESS_MODE_GSM
+				| OFONO_RADIO_ACCESS_MODE_UMTS
+				| OFONO_RADIO_ACCESS_MODE_LTE;
+
+	CALLBACK_WITH_SUCCESS(cb, available_rats, data);
+}
+
+static void cnmp_support_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_radio_settings *rs = user_data;
+
+	DBG("ok %d", ok);
+
+	if (!ok) {
+		ofono_radio_settings_remove(rs);
+		return;
+	}
+
+	ofono_radio_settings_register(rs);
+}
+
+static int simcom_radio_settings_probe(struct ofono_radio_settings *rs,
+					unsigned int vendor, void *data)
+{
+	GAtChat *chat = data;
+	struct radio_settings_data *rsd;
+
+	DBG("");
+
+	rsd = g_new0(struct radio_settings_data, 1);
+
+	rsd->chat = g_at_chat_clone(chat);
+
+	ofono_radio_settings_set_data(rs, rsd);
+
+	g_at_chat_send(rsd->chat, "AT+CNMP=?", cnmp_prefix,
+			cnmp_support_cb, rs, NULL);
+
+	return 0;
+}
+
+static void simcom_radio_settings_remove(struct ofono_radio_settings *rs)
+{
+	struct radio_settings_data *rsd = ofono_radio_settings_get_data(rs);
+
+	DBG("");
+
+	ofono_radio_settings_set_data(rs, NULL);
+	g_at_chat_unref(rsd->chat);
+	g_free(rsd);
+}
+
+static const struct ofono_radio_settings_driver driver = {
+	.probe			= simcom_radio_settings_probe,
+	.remove			= simcom_radio_settings_remove,
+	.query_rat_mode		= simcom_query_rat_mode,
+	.set_rat_mode		= simcom_set_rat_mode,
+	.query_available_rats	= simcom_query_available_rats
+};
+
+OFONO_ATOM_DRIVER_BUILTIN(radio_settings, simcommodem, &driver)
-- 
2.25.1

Re: [PATCH 02/15] drivers: adding support for the SIMCom A7605E-H

[Denis Kenzior]

Hi Maxim,

On 10/17/23 05:48, MaxLyubimov wrote:
> The features of the SIMCom A7605E-H modem are taken into account in the atmodem driver
> ---
>   drivers/atmodem/gprs.c               |  59 ++++++-
>   drivers/atmodem/sim.c                |   7 +-
>   drivers/simcommodem/radio-settings.c | 236 +++++++++++++++++++++++++++
>   3 files changed, 298 insertions(+), 4 deletions(-)
>   create mode 100644 drivers/simcommodem/radio-settings.c
> 

This patch fails because you're trying to add a file that already exists 
upstream (radio-settings.c)

Regards,
-Denis

Re: [PATCH 02/15] drivers: adding support for the SIMCom A7605E-H

[Любимов Максим]

Good afternoon, Denis

On 18/10/2023 в 20:30 -0500, Denis Kenzior пишет:
> On 10/17/23 05:48, MaxLyubimov wrote:
> > The features of the SIMCom A7605E-H modem are taken into account in
> > the atmodem driver
> > ---
> >   drivers/atmodem/gprs.c               |  59 ++++++-
> >   drivers/atmodem/sim.c                |   7 +-
> >   drivers/simcommodem/radio-settings.c | 236
> > +++++++++++++++++++++++++++
> >   3 files changed, 298 insertions(+), 4 deletions(-)
> >   create mode 100644 drivers/simcommodem/radio-settings.c
> > 
> 
> This patch fails because you're trying to add a file that already
> exists 
> upstream (radio-settings.c)
> 

I checked, there is no drivers/simcommodem/radio-settings.c file in the
https://git.kernel.org/pub/scm/network/ofono/ofono.git/tree/drivers
repository, how can I fix this problem?

Best regards,
Lyubimov Maxim

Re: [PATCH 02/15] drivers: adding support for the SIMCom A7605E-H

[Denis Kenzior]

Hi Максим,

> 
> I checked, there is no drivers/simcommodem/radio-settings.c file in the
> https://git.kernel.org/pub/scm/network/ofono/ofono.git/tree/drivers
> repository, how can I fix this problem?
> 

Okay, this was a problem on my side.  I went ahead and applied this patch now 
with some style modifications.  Please double check that I didn't screw anything up.

Regards,
-Denis

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

[MaxLyubimov]

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 +++++++
 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
+ *
+ *  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>
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdlib.h>
+#include <errno.h>
+
+#include <glib.h>
+#include <gatchat.h>
+#include <gattty.h>
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include <ofono/plugin.h>
+#include <ofono/log.h>
+#include <ofono/modem.h>
+#include <ofono/call-barring.h>
+#include <ofono/call-forwarding.h>
+#include <ofono/call-meter.h>
+#include <ofono/call-settings.h>
+#include <ofono/devinfo.h>
+#include <ofono/message-waiting.h>
+#include <ofono/netreg.h>
+#include <ofono/phonebook.h>
+#include <ofono/sim.h>
+#include <ofono/sms.h>
+#include <ofono/ussd.h>
+#include <ofono/voicecall.h>
+#include <ofono/gprs.h>
+#include <ofono/gprs-context.h>
+#include <ofono/lte.h>
+#include <ofono/radio-settings.h>
+
+#include <drivers/atmodem/vendor.h>
+#include <drivers/atmodem/atutil.h>
+
+struct simcom_a76xx_data {
+	GAtChat *at;
+	GAtChat *ppp;
+};
+
+static const char *cfun_prefix[] = { "+CFUN:", NULL };
+
+static void simcom_a76xx_debug(const char *str, void *user_data)
+{
+	ofono_info("%s%s", (const char *)user_data, str);
+}
+
+/* Detect hardware, and initialize if found */
+static int simcom_a76xx_probe(struct ofono_modem *modem)
+{
+	struct simcom_a76xx_data *data;
+
+	DBG("");
+
+	data = g_try_new0(struct simcom_a76xx_data, 1);
+	if (data == NULL)
+		return -ENOMEM;
+
+	ofono_modem_set_data(modem, data);
+
+	return 0;
+}
+
+static void simcom_a76xx_remove(struct ofono_modem *modem)
+{
+	struct simcom_a76xx_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	if (!data)
+		return;
+
+	if (data->at)
+		g_at_chat_unref(data->at);
+
+	if (data->ppp)
+		g_at_chat_unref(data->ppp);
+
+	ofono_modem_set_data(modem, NULL);
+	g_free (data);
+}
+
+static void cfun_set_on_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct simcom_a76xx_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	if (ok)
+		ofono_modem_set_powered(modem, TRUE);
+}
+
+static int open_device(struct ofono_modem *modem, const char *devkey,
+			GAtChat **chatp)
+{
+	GIOChannel *channel;
+	GAtSyntax *syntax;
+	GAtChat *chat;
+	const char *device;
+
+	DBG("devkey=%s", devkey);
+
+	device = ofono_modem_get_string(modem, devkey);
+	if (device == NULL)
+		return -EINVAL;
+
+	channel = g_at_tty_open(device, NULL);
+	if (channel == NULL)
+		return -EIO;
+
+	syntax = g_at_syntax_new_gsm_permissive();
+	chat = g_at_chat_new(channel, syntax);
+	g_at_syntax_unref(syntax);
+	g_io_channel_unref(channel);
+
+	if (chat == NULL)
+		return -EIO;
+
+	if (getenv("OFONO_AT_DEBUG"))
+		g_at_chat_set_debug(chat, simcom_a76xx_debug, (void*) devkey);
+
+	*chatp = chat;
+	return 0;
+}
+
+static int simcom_a76xx_enable(struct ofono_modem *modem)
+{
+	struct simcom_a76xx_data *data = ofono_modem_get_data(modem);
+	int err;
+
+	DBG("");
+
+	err = open_device(modem, "AT", &data->at);
+	if (err < 0)
+		return err;
+
+	err = open_device(modem, "PPP", &data->ppp);
+	if (err < 0)
+		return err;
+
+	g_at_chat_set_slave(data->ppp, data->at);
+
+	/* ensure modem is in a known state; verbose on, echo/quiet off */
+	g_at_chat_send(data->at, "ATE0Q0V1", NULL, NULL, NULL, NULL);
+
+	g_at_chat_send(data->at, "AT+CATR=3", NULL, NULL, NULL, NULL);
+
+	/* power up modem */
+	g_at_chat_send(data->at, "AT+CFUN=4", NULL, cfun_set_on_cb,
+								modem, NULL);
+
+	return 0;
+}
+
+static void cfun_set_off_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct simcom_a76xx_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	g_at_chat_unref(data->ppp);
+	g_at_chat_unref(data->at);
+	data->at = data->ppp = NULL;
+
+	if (ok)
+		ofono_modem_set_powered(modem, FALSE);
+}
+
+static int simcom_a76xx_disable(struct ofono_modem *modem)
+{
+	struct simcom_a76xx_data *data = ofono_modem_get_data(modem);
+
+	DBG("");
+
+	/* power down modem */
+	g_at_chat_cancel_all(data->ppp);
+	g_at_chat_cancel_all(data->at);
+	g_at_chat_unregister_all(data->ppp);
+	g_at_chat_unregister_all(data->at);
+	g_at_chat_send(data->at, "AT+CFUN=0", NULL, cfun_set_off_cb,
+								modem, NULL);
+
+	return -EINPROGRESS;
+}
+
+static void set_online_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_modem_online_cb_t cb = cbd->cb;
+	struct ofono_error error;
+
+	DBG("%p", user_data);
+
+	decode_at_error(&error, g_at_result_final_response(result));
+	cb(&error, cbd->data);
+}
+
+static void simcom_a76xx_set_online(struct ofono_modem *modem, ofono_bool_t online,
+				ofono_modem_online_cb_t cb, void *user_data)
+{
+	struct simcom_a76xx_data *data = ofono_modem_get_data(modem);
+	struct cb_data *cbd = cb_data_new(cb, user_data);
+	char const *command = online ? "AT+CFUN=1" : "AT+CFUN=4";
+
+	DBG("modem %p %s", modem, online ? "online" : "offline");
+
+	if (g_at_chat_send(data->at, command, cfun_prefix, set_online_cb, cbd,
+				g_free) > 0)
+		return;
+
+	CALLBACK_WITH_FAILURE(cb, cbd->data);
+
+	g_free(cbd);
+}
+
+static void simcom_a76xx_pre_sim(struct ofono_modem *modem)
+{
+	struct simcom_a76xx_data *data = ofono_modem_get_data(modem);
+	struct ofono_sim *sim;
+
+	DBG("");
+
+	ofono_devinfo_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", data->at);
+	sim = ofono_sim_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", data->at);
+	ofono_voicecall_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", data->at);
+
+	if (sim) {
+		ofono_sim_inserted_notify(sim, TRUE);
+	}
+}
+
+static void simcom_a76xx_post_sim(struct ofono_modem *modem)
+{
+	struct simcom_a76xx_data *data = ofono_modem_get_data(modem);
+	struct ofono_message_waiting *mw;
+	struct ofono_gprs *gprs = NULL;
+	struct ofono_gprs_context *gc = NULL;
+
+	DBG("");
+
+	ofono_ussd_create(modem, 0, "atmodem", data->at);
+	ofono_call_forwarding_create(modem, 0, "atmodem", data->at);
+	ofono_call_settings_create(modem, 0, "atmodem", data->at);
+	ofono_call_meter_create(modem, 0, "atmodem", data->at);
+	ofono_call_barring_create(modem, 0, "atmodem", data->at);
+	ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", data->at);
+	ofono_phonebook_create(modem, 0, "atmodem", data->at);
+	ofono_radio_settings_create(modem, 0, "simcommodem", data->at);
+
+	gprs = ofono_gprs_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", data->at);
+	gc = ofono_gprs_context_create(modem, 0, "atmodem", data->ppp);
+
+	if (gprs && gc)
+		ofono_gprs_add_context(gprs, gc);
+
+	mw = ofono_message_waiting_create(modem);
+	if (mw)
+		ofono_message_waiting_register(mw);
+}
+
+static void simcom_a76xx_post_online(struct ofono_modem *modem)
+{
+	struct simcom_a76xx_data *data = ofono_modem_get_data(modem);
+
+	DBG("%p", modem);
+
+	ofono_netreg_create(modem, OFONO_VENDOR_SIMCOM, "atmodem", data->at);
+}
+
+static struct ofono_modem_driver simcom_a76xx_driver = {
+	.name			= "simcom_a76xx",
+	.probe			= simcom_a76xx_probe,
+	.remove			= simcom_a76xx_remove,
+	.enable			= simcom_a76xx_enable,
+	.disable		= simcom_a76xx_disable,
+	.set_online		= simcom_a76xx_set_online,
+	.pre_sim		= simcom_a76xx_pre_sim,
+	.post_sim		= simcom_a76xx_post_sim,
+	.post_online	= simcom_a76xx_post_online,
+};
+
+static int simcom_a76xx_init(void)
+{
+	return ofono_modem_driver_register(&simcom_a76xx_driver);
+}
+
+static void simcom_a76xx_exit(void)
+{
+	ofono_modem_driver_unregister(&simcom_a76xx_driver);
+}
+
+OFONO_PLUGIN_DEFINE(simcom_a76xx, "SIMCom A76xx modem driver", VERSION,
+		OFONO_PLUGIN_PRIORITY_DEFAULT, simcom_a76xx_init, simcom_a76xx_exit)
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;
+			}
+		}
+	}
+
+	if (mdm == NULL)
+		return FALSE;
+
+	DBG("at=%s ppp=%s gps=%s diag=%s",
+						mdm, ppp, gps, diag);
+
+	ofono_modem_set_driver(modem->modem, "simcom_a76xx");
+
+	ofono_modem_set_string(modem->modem, "Diag", diag);
+	ofono_modem_set_string(modem->modem, "GPS", gps);
+	ofono_modem_set_string(modem->modem, "AT", mdm);
+	ofono_modem_set_string(modem->modem, "PPP", ppp);
+	return TRUE;
+}
+
 static struct {
 	const char *name;
 	gboolean (*setup)(struct modem_info *modem);
@@ -1574,6 +1627,7 @@ static struct {
 	{ "telitqmi",	setup_telitqmi	},
 	{ "simcom",	setup_simcom	},
 	{ "sim7x00",	setup_sim7x00	},
+	{ "simcom_a76xx",	setup_simcom_a76xx	},
 	{ "zte",	setup_zte	},
 	{ "icera",	setup_icera	},
 	{ "samsung",	setup_samsung	},
@@ -1951,6 +2005,7 @@ static struct {
 	{ "simcom",	"option",	"05c6", "9000"	},
 	{ "sim7x00",	"option",	"1e0e", "9001"	},
 	{ "sim7x00",	"qmi_wwan",	"1e0e",	"9001"	},
+	{ "simcom_a76xx",	"option",	"1e0e", "9011"	},
 	{ "telit",	"usbserial",	"1bc7"		},
 	{ "telit",	"option",	"1bc7"		},
 	{ "telit",	"cdc_acm",	"1bc7", "0021"	},
-- 
2.25.1

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

[Denis Kenzior]

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

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

[Любимов Максим]

On 18/10/2023 в 20:42 -0500, Denis Kenzior wrote:
> 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.

I deliberately divided the patch into several parts so that they
affected only one directory, does this patch only affect the plugins
directory or did I understand something wrong?

> >   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?

I slightly modified the file plugins/sim7100.c, I left the copyright
unchanged

> > + *
> > + *  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?

Simcom modems of the SIM7x00 and A76xx series are built on different
chips, Qualcomm and ASR, respectively, so their functionality is very
different. I may try to combine these plugins, so the commands are
generally compatible. But I don’t have a SIM7x00 series modem, so I
won’t be able to check the correctness of my modification. There is
also a question about the name of the plugin, SIM7100 is the name of a
series of modems, as well as A76xx, how to combine them in the name of
one plugin?

> Also, you still have lines > 80 chars here.

I will fix it

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

I divided the code to make it more readable, since setup_sim7x00
configures qmi, which is not supported in simcom a76xx, and the
setup_simcom function, as far as I understand, is not used because
there is no modem driver named simcom. Which of these setup_sim7x00 or
setup_simcom functions should I use to add a simcom a76xx?

Best regards,
Lyubimov Maxim

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

[Denis Kenzior]

Hi Максим,

On 10/27/23 04:01, Любимов Максим wrote:
> 
> 
> On 18/10/2023 в 20:42 -0500, Denis Kenzior wrote:
>> 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.
> 
> I deliberately divided the patch into several parts so that they
> affected only one directory, does this patch only affect the plugins
> directory or did I understand something wrong?

After having another look, please ignore my comment.  You're fine.

<snip>

>>> + *  Copyright (C) 2008-2011  Intel Corporation. All rights
>>> reserved.
>>> + *  Copyright (C) 2009  Collabora Ltd. All rights reserved.
>>> + *  Copyright 2018 Purism SPC
>>
>> Copyrights seem suspect?
> 
> I slightly modified the file plugins/sim7100.c, I left the copyright
> unchanged
> 

Ok, fair enough.

>>
>> In fact this looks 95% the same to plugins/simcom7100.c.  Can we
>> unify the two?
> 
> Simcom modems of the SIM7x00 and A76xx series are built on different
> chips, Qualcomm and ASR, respectively, so their functionality is very
> different. I may try to combine these plugins, so the commands are
> generally compatible. But I don’t have a SIM7x00 series modem, so I
> won’t be able to check the correctness of my modification. There is
> also a question about the name of the plugin, SIM7100 is the name of a
> series of modems, as well as A76xx, how to combine them in the name of
> one plugin?
> 

I would rename this driver to simcom.c and try your best to combine the two. 
Even if you somehow break the 7100, then someone with a 7100 can come in and 
complain.  We'll then fix it.  In fact, I think we even had a simcom.c before.

>>
>> Again, this block seems rather similar to what is in
>> setup_simcom.  Just the
>> interface numbers are different.  Please unify the two.
> 
> I divided the code to make it more readable, since setup_sim7x00
> configures qmi, which is not supported in simcom a76xx, and the
> setup_simcom function, as far as I understand, is not used because
> there is no modem driver named simcom. Which of these setup_sim7x00 or
> setup_simcom functions should I use to add a simcom a76xx?

Long term I think I want to get rid of QMI setup in all vendor setup functions. 
Almost always these are the same across all modems in QMI mode, just the 
interface numbers might be slightly different.  Probably just need to create a 
lookup table instead.

So for now, I would just modify the setup_simcom function with the A76xx details.

Regards,
-Denis

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

[Любимов Максим]

Hi Denis

> So for now, I would just modify the setup_simcom function with the
> A76xx details.

Ok

Best regards,
Lyubimov Maxim

[PATCH 04/15] build: adding support for the SIMCom A7605E-H

[MaxLyubimov]

Modified the Makefile.am file to build the simcom_a76xx plugin
---
 Makefile.am | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index aac88f54..83c36d34 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -505,6 +505,9 @@ builtin_sources += drivers/atmodem/atutil.h \
 			drivers/xmm7modem/ims.c \
 			drivers/xmm7modem/netmon.c
 
+builtin_sources += drivers/atmodem/atutil.h \
+			drivers/simcommodem/radio-settings.c
+
 if PHONESIM
 builtin_modules += phonesim
 builtin_sources += plugins/phonesim.c
@@ -586,6 +589,9 @@ builtin_sources += plugins/sim900.c
 builtin_modules += sim7100
 builtin_sources += plugins/sim7100.c
 
+builtin_modules += simcom_a76xx
+builtin_sources += plugins/simcom_a76xx.c
+
 builtin_modules += connman
 builtin_sources += plugins/connman.c
 
-- 
2.25.1

[PATCH 05/15] drivers: quectel: Add radio settings

[MaxLyubimov]

---
 drivers/quectelmodem/radio-settings.c | 260 ++++++++++++++++++++++++++
 1 file changed, 260 insertions(+)
 create mode 100644 drivers/quectelmodem/radio-settings.c

diff --git a/drivers/quectelmodem/radio-settings.c b/drivers/quectelmodem/radio-settings.c
new file mode 100644
index 00000000..2bc42ef7
--- /dev/null
+++ b/drivers/quectelmodem/radio-settings.c
@@ -0,0 +1,260 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2017 Intel Corporation. All rights reserved.
+ *  Copyright (C) 2021 Sergey Matyukevich. All rights reserved.
+ *
+ *  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
+
+#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/radio-settings.h>
+
+#include <drivers/atmodem/atutil.h>
+
+#include "gatchat.h"
+#include "gatresult.h"
+
+static const char *none_prefix[] = { NULL };
+static const char *qcfg_prefix[] = { "+QCFG:", NULL };
+
+struct radio_settings_data {
+	GAtChat *chat;
+};
+
+static void qcfg_query_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_radio_settings_rat_mode_query_cb_t cb = cbd->cb;
+	unsigned int mode;
+	struct ofono_error error;
+	int nwscanseq;
+	GAtResultIter iter;
+
+	DBG("ok %d", ok);
+
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok) {
+		cb(&error, -1, cbd->data);
+		return;
+	}
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+QCFG: \"nwscanseq\","))
+		goto error;
+
+	if (!g_at_result_iter_next_number(&iter, &nwscanseq))
+		goto error;
+
+	DBG("nwscanseq %d", nwscanseq);
+
+	switch (nwscanseq) {
+	case 0:
+	case 4:
+	case 5:
+	case 12:
+		mode = OFONO_RADIO_ACCESS_MODE_ANY;
+		break;
+	case 1:
+		mode = OFONO_RADIO_ACCESS_MODE_GSM;
+		break;
+	case 2:
+		mode = OFONO_RADIO_ACCESS_MODE_UMTS;
+		break;
+	case 3:
+		mode = OFONO_RADIO_ACCESS_MODE_LTE;
+		break;
+	case 6:
+	case 8:
+		mode = OFONO_RADIO_ACCESS_MODE_UMTS |
+			OFONO_RADIO_ACCESS_MODE_LTE;
+		break;
+	case 7:
+	case 10:
+		mode = OFONO_RADIO_ACCESS_MODE_GSM |
+			OFONO_RADIO_ACCESS_MODE_LTE;
+		break;
+	case 9:
+	case 11:
+		mode = OFONO_RADIO_ACCESS_MODE_GSM |
+			OFONO_RADIO_ACCESS_MODE_UMTS;
+		break;
+	default:
+		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+		return;
+	}
+
+	cb(&error, mode, cbd->data);
+
+	return;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+}
+
+static void quectel_query_rat_mode(struct ofono_radio_settings *rs,
+				ofono_radio_settings_rat_mode_query_cb_t cb,
+				void *data)
+{
+	struct radio_settings_data *rsd = ofono_radio_settings_get_data(rs);
+	struct cb_data *cbd = cb_data_new(cb, data);
+
+	DBG("");
+
+	if (g_at_chat_send(rsd->chat, "AT+QCFG=\"nwscanseq\"", qcfg_prefix,
+				qcfg_query_cb, cbd, g_free) == 0) {
+		CALLBACK_WITH_FAILURE(cb, -1, data);
+		g_free(cbd);
+	}
+}
+
+static void qcfg_modify_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_radio_settings_rat_mode_set_cb_t cb = cbd->cb;
+	struct ofono_error error;
+
+	DBG("ok %d", ok);
+
+	decode_at_error(&error, g_at_result_final_response(result));
+	cb(&error, cbd->data);
+}
+
+static void quectel_set_rat_mode(struct ofono_radio_settings *rs,
+				unsigned int m,
+				ofono_radio_settings_rat_mode_set_cb_t cb,
+				void *data)
+{
+	struct radio_settings_data *rsd = ofono_radio_settings_get_data(rs);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	int nwscanseq;
+	char buf[30];
+
+	DBG("mode %d", m);
+
+	switch (m) {
+	case OFONO_RADIO_ACCESS_MODE_ANY:
+		nwscanseq = 0;
+		break;
+	case OFONO_RADIO_ACCESS_MODE_GSM:
+		nwscanseq = 1;
+		break;
+	case OFONO_RADIO_ACCESS_MODE_UMTS:
+		nwscanseq = 2;
+		break;
+	case OFONO_RADIO_ACCESS_MODE_LTE:
+		nwscanseq = 3;
+		break;
+	case OFONO_RADIO_ACCESS_MODE_UMTS|OFONO_RADIO_ACCESS_MODE_GSM:
+		nwscanseq = 9;
+		break;
+	case OFONO_RADIO_ACCESS_MODE_LTE|OFONO_RADIO_ACCESS_MODE_UMTS:
+		nwscanseq = 6;
+		break;
+	case OFONO_RADIO_ACCESS_MODE_LTE|OFONO_RADIO_ACCESS_MODE_GSM:
+		nwscanseq = 7;
+		break;
+	}
+
+	snprintf(buf, sizeof(buf), "AT+QCFG=\"nwscanseq\",%u", nwscanseq);
+
+	if (g_at_chat_send(rsd->chat, buf, none_prefix,
+				qcfg_modify_cb, cbd, g_free) > 0)
+		return;
+
+	CALLBACK_WITH_FAILURE(cb, data);
+	g_free(cbd);
+}
+
+static void quectel_query_available_rats(struct ofono_radio_settings *rs,
+			ofono_radio_settings_available_rats_query_cb_t cb,
+			void *data)
+{
+	unsigned int available_rats = OFONO_RADIO_ACCESS_MODE_GSM
+				| OFONO_RADIO_ACCESS_MODE_UMTS
+				| OFONO_RADIO_ACCESS_MODE_LTE;
+
+	CALLBACK_WITH_SUCCESS(cb, available_rats, data);
+}
+
+static void qcfg_support_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_radio_settings *rs = user_data;
+
+	DBG("ok %d", ok);
+
+	if (!ok) {
+		ofono_radio_settings_remove(rs);
+		return;
+	}
+
+	ofono_radio_settings_register(rs);
+}
+
+static int quectel_radio_settings_probe(struct ofono_radio_settings *rs,
+					unsigned int vendor, void *data)
+{
+	GAtChat *chat = data;
+	struct radio_settings_data *rsd;
+
+	DBG("");
+
+	rsd = g_new0(struct radio_settings_data, 1);
+
+	rsd->chat = g_at_chat_clone(chat);
+
+	ofono_radio_settings_set_data(rs, rsd);
+
+	g_at_chat_send(rsd->chat, "AT+QCFG=\"nwscanseq\"", qcfg_prefix,
+			qcfg_support_cb, rs, NULL);
+
+	return 0;
+}
+
+static void quectel_radio_settings_remove(struct ofono_radio_settings *rs)
+{
+	struct radio_settings_data *rsd = ofono_radio_settings_get_data(rs);
+
+	DBG("");
+
+	ofono_radio_settings_set_data(rs, NULL);
+	g_at_chat_unref(rsd->chat);
+	g_free(rsd);
+}
+
+static const struct ofono_radio_settings_driver driver = {
+	.probe			= quectel_radio_settings_probe,
+	.remove			= quectel_radio_settings_remove,
+	.query_rat_mode		= quectel_query_rat_mode,
+	.set_rat_mode		= quectel_set_rat_mode,
+	.query_available_rats	= quectel_query_available_rats
+};
+
+OFONO_ATOM_DRIVER_BUILTIN(radio_settings, quectelmodem, &driver)
-- 
2.25.1

Re: [PATCH 05/15] drivers: quectel: Add radio settings

[Denis Kenzior]

Hi Maxim,

On 10/17/23 05:48, MaxLyubimov wrote:
> ---
>   drivers/quectelmodem/radio-settings.c | 260 ++++++++++++++++++++++++++
>   1 file changed, 260 insertions(+)
>   create mode 100644 drivers/quectelmodem/radio-settings.c
> 

I combined this patch with patch 06, updated the author info and applied, thanks.

Regards,
-Denis

[PATCH 06/15] build: Add quectel radio settings rules

[MaxLyubimov]

---
 Makefile.am | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 83c36d34..c5ffd102 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -508,6 +508,9 @@ builtin_sources += drivers/atmodem/atutil.h \
 builtin_sources += drivers/atmodem/atutil.h \
 			drivers/simcommodem/radio-settings.c
 
+builtin_sources += drivers/atmodem/atutil.h \
+			drivers/quectelmodem/radio-settings.c
+
 if PHONESIM
 builtin_modules += phonesim
 builtin_sources += plugins/phonesim.c
-- 
2.25.1

[PATCH 07/15] plugins: quectel: Add radio settings

[MaxLyubimov]

---
 plugins/quectel.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/plugins/quectel.c b/plugins/quectel.c
index 464845ff..1dd67e80 100644
--- a/plugins/quectel.c
+++ b/plugins/quectel.c
@@ -53,6 +53,7 @@
 #include <ofono/gprs-context.h>
 #include <ofono/log.h>
 #include <ofono/dbus.h>
+#include <ofono/radio-settings.h>
 
 #include <gdbus/gdbus.h>
 
@@ -1366,6 +1367,7 @@ static void quectel_post_sim(struct ofono_modem *modem)
 	ofono_sms_create(modem, data->vendor, "atmodem", data->aux);
 	ofono_phonebook_create(modem, data->vendor, "atmodem", data->aux);
 	ofono_call_volume_create(modem, data->vendor, "atmodem", data->aux);
+	ofono_radio_settings_create(modem, 0, "quectelmodem", data->aux);
 
 	if (data->model == QUECTEL_EC21 || data->model == QUECTEL_EC200)
 		ofono_ussd_create(modem, data->vendor, "atmodem", data->aux);
-- 
2.25.1

Re: [PATCH 07/15] plugins: quectel: Add radio settings

[Denis Kenzior]

Hi Maxim,

On 10/17/23 05:48, MaxLyubimov wrote:
> ---
>   plugins/quectel.c | 2 ++
>   1 file changed, 2 insertions(+)
> 

Patches 07-11 applied with some style fixes.  Please double check that I didn't 
screw anything up.

Regards,
-Denis

[PATCH 08/15] drivers: gemalto: Add models list

[MaxLyubimov]

---
 drivers/gemaltomodem/models.h | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 drivers/gemaltomodem/models.h

diff --git a/drivers/gemaltomodem/models.h b/drivers/gemaltomodem/models.h
new file mode 100644
index 00000000..6d0d6490
--- /dev/null
+++ b/drivers/gemaltomodem/models.h
@@ -0,0 +1,32 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2017 Vincent Cesson. All rights reserved.
+ *  Copyright (C) 2018 Gemalto M2M
+ *
+ *  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
+ *
+ */
+
+/* Supported gemalto's modem */
+/* PHS8-P modem */
+#define GEMALTO_MODEL_PHS8P	"0053"
+/* ALS3, PLS8-E, and PLS8-X family */
+#define GEMALTO_MODEL_ALS3_PLS8x	"0061"
+/* ELS81 modem */
+#define GEMALTO_MODEL_ELS81x	"005b"
+/* EHS5-E modem */
+#define GEMALTO_MODEL_EHS5_E	"0058"
+
-- 
2.25.1

[PATCH 09/15] plugins: gemalto: Include models list

[MaxLyubimov]

Include models list from drivers/gemaltomodem/models.h
---
 plugins/gemalto.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/plugins/gemalto.c b/plugins/gemalto.c
index d1962ab8..fb6b79e7 100644
--- a/plugins/gemalto.c
+++ b/plugins/gemalto.c
@@ -53,16 +53,10 @@
 
 #include <drivers/atmodem/atutil.h>
 #include <drivers/atmodem/vendor.h>
+#include <drivers/gemaltomodem/models.h>
 
 #define HARDWARE_MONITOR_INTERFACE OFONO_SERVICE ".cinterion.HardwareMonitor"
 
-/* Supported gemalto's modem */
-#define GEMALTO_MODEL_PHS8P	"0053"
-/* ALS3, PLS8-E, and PLS8-X family */
-#define GEMALTO_MODEL_ALS3_PLS8x	"0061"
-/* ELS81 modem */
-#define GEMALTO_MODEL_ELS81x   "005b"
-
 static const char *none_prefix[] = { NULL };
 static const char *sctm_prefix[] = { "^SCTM:", NULL };
 static const char *sbv_prefix[] = { "^SBV:", NULL };
-- 
2.25.1

[PATCH 10/15] plugins: udevng: Add support gemalto EHS5-E

[MaxLyubimov]

---
 plugins/udevng.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/plugins/udevng.c b/plugins/udevng.c
index 7fccf670..4adb626b 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1356,7 +1356,7 @@ static gboolean setup_gemalto(struct modem_info *modem)
 				qmi = info->devnode;
 		}
 
-		/* Cinterion ALS3, PLS8-E, PLS8-X */
+		/* Cinterion ALS3, PLS8-E, PLS8-X, EHS5-E */
 		if (g_strcmp0(info->interface, "2/2/1") == 0) {
 			if (g_strcmp0(info->number, "00") == 0)
 				mdm = info->devnode;
@@ -2035,6 +2035,7 @@ static struct {
 	{ "gemalto",	"option",	"1e2d",	"0053"	},
 	{ "gemalto",	"cdc_wdm",	"1e2d",	"0053"	},
 	{ "gemalto",	"qmi_wwan",	"1e2d",	"0053"	},
+	{ "gemalto",	"cdc_acm",	"1e2d",	"0058"	},
 	{ "gemalto",	"cdc_acm",	"1e2d",	"0061"	},
 	{ "gemalto",	"cdc_ether",	"1e2d",	"0061"	},
 	{ "gemalto",	"cdc_acm",	"1e2d",	"005b"	},
-- 
2.25.1

[PATCH 11/15] gemalto: radio-settings: Add support EHS5-E

[MaxLyubimov]

---
 drivers/gemaltomodem/radio-settings.c | 232 ++++++++++++++++++--------
 1 file changed, 161 insertions(+), 71 deletions(-)

diff --git a/drivers/gemaltomodem/radio-settings.c b/drivers/gemaltomodem/radio-settings.c
index d711658d..6bbf16f6 100644
--- a/drivers/gemaltomodem/radio-settings.c
+++ b/drivers/gemaltomodem/radio-settings.c
@@ -40,6 +40,8 @@
 #include "gatchat.h"
 #include "gatresult.h"
 
+#include "models.h"
+
 static const char *none_prefix[] = { NULL };
 static const char *sxrat_prefix[] = { "^SXRAT:", NULL };
 
@@ -47,13 +49,32 @@ struct radio_settings_data {
 	GAtChat *chat;
 };
 
+static void gemalto_query_available_rats(struct ofono_radio_settings *rs,
+			ofono_radio_settings_available_rats_query_cb_t cb,
+			void *data) {
+	unsigned int available_rats;
+	struct ofono_modem *modem = ofono_radio_settings_get_modem(rs);
+	const char *model = ofono_modem_get_string(modem, "Model");
+
+	available_rats = OFONO_RADIO_ACCESS_MODE_GSM
+				| OFONO_RADIO_ACCESS_MODE_UMTS;
+
+	if (!g_strcmp0(model, GEMALTO_MODEL_ALS3_PLS8x) ||
+		!g_strcmp0(model, GEMALTO_MODEL_ELS81x))
+		available_rats |= OFONO_RADIO_ACCESS_MODE_LTE;
+
+	CALLBACK_WITH_SUCCESS(cb, available_rats, data);
+}
+
 static void sxrat_query_cb(gboolean ok, GAtResult *result, gpointer user_data)
 {
 	struct cb_data *cbd = user_data;
 	ofono_radio_settings_rat_mode_query_cb_t cb = cbd->cb;
+	struct ofono_radio_settings *rs = cbd->data;
+	struct ofono_modem *modem = ofono_radio_settings_get_modem(rs);
+	const char *model = ofono_modem_get_string(modem, "Model");
 	unsigned int mode;
 	struct ofono_error error;
-	int value, pref1, pref2;
 	GAtResultIter iter;
 
 	DBG("ok %d", ok);
@@ -70,42 +91,76 @@ static void sxrat_query_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	if (!g_at_result_iter_next(&iter, "^SXRAT:"))
 		goto error;
 
-	if (!g_at_result_iter_next_number(&iter, &value))
+	if (!g_strcmp0(model, GEMALTO_MODEL_ALS3_PLS8x) ||
+		!g_strcmp0(model, GEMALTO_MODEL_ELS81x)) {
+		int value, pref1, pref2;
+		
+		if (!g_at_result_iter_next_number(&iter, &value))
+			goto error;
+		
+		g_at_result_iter_next_number_default(&iter, -1, &pref1);
+		g_at_result_iter_next_number_default(&iter, -1, &pref2);
+
+		DBG("mode %d pref1 %d pref2 %d", value, pref1, pref2);
+
+		switch (value) {
+		case 0:
+			mode = OFONO_RADIO_ACCESS_MODE_GSM;
+			break;
+		case 1:
+			mode = OFONO_RADIO_ACCESS_MODE_GSM |
+				OFONO_RADIO_ACCESS_MODE_UMTS;
+			break;
+		case 2:
+			mode = OFONO_RADIO_ACCESS_MODE_UMTS;
+			break;
+		case 3:
+			mode = OFONO_RADIO_ACCESS_MODE_LTE;
+			break;
+		case 4:
+			mode = OFONO_RADIO_ACCESS_MODE_UMTS |
+				OFONO_RADIO_ACCESS_MODE_LTE;
+			break;
+		case 5:
+			mode = OFONO_RADIO_ACCESS_MODE_GSM |
+				OFONO_RADIO_ACCESS_MODE_LTE;
+			break;
+		case 6:
+			mode = OFONO_RADIO_ACCESS_MODE_ANY;
+			break;
+		default:
+			CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+			return;
+		}
+	}
+	else if (!g_strcmp0(model, GEMALTO_MODEL_EHS5_E)) {
+		int act, act_pref;
+		
+		if (!g_at_result_iter_next_number(&iter, &act))
+			goto error;
+		
+		g_at_result_iter_next_number_default(&iter, -1, &act_pref);
+		
+		DBG("act %d act_pref %d", act, act_pref);
+		
+		switch (act) {
+		case 0:
+			mode = OFONO_RADIO_ACCESS_MODE_GSM;
+			break;
+		case 1:
+			mode = OFONO_RADIO_ACCESS_MODE_GSM |
+				OFONO_RADIO_ACCESS_MODE_UMTS;
+			break;
+		case 2:
+			mode = OFONO_RADIO_ACCESS_MODE_UMTS;
+			break;
+		default:
+			CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+			return;
+		}
+	}
+	else {
 		goto error;
-
-	g_at_result_iter_next_number_default(&iter, -1, &pref1);
-	g_at_result_iter_next_number_default(&iter, -1, &pref2);
-
-	DBG("mode %d pref1 %d pref2 %d", value, pref1, pref2);
-
-	switch (value) {
-	case 0:
-		mode = OFONO_RADIO_ACCESS_MODE_GSM;
-		break;
-	case 1:
-		mode = OFONO_RADIO_ACCESS_MODE_GSM |
-			OFONO_RADIO_ACCESS_MODE_UMTS;
-		break;
-	case 2:
-		mode = OFONO_RADIO_ACCESS_MODE_UMTS;
-		break;
-	case 3:
-		mode = OFONO_RADIO_ACCESS_MODE_LTE;
-		break;
-	case 4:
-		mode = OFONO_RADIO_ACCESS_MODE_UMTS |
-			OFONO_RADIO_ACCESS_MODE_LTE;
-		break;
-	case 5:
-		mode = OFONO_RADIO_ACCESS_MODE_GSM |
-			OFONO_RADIO_ACCESS_MODE_LTE;
-		break;
-	case 6:
-		mode = OFONO_RADIO_ACCESS_MODE_ANY;
-		break;
-	default:
-		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
-		return;
 	}
 
 	cb(&error, mode, cbd->data);
@@ -151,51 +206,85 @@ static void gemalto_set_rat_mode(struct ofono_radio_settings *rs,
 {
 	struct radio_settings_data *rsd = ofono_radio_settings_get_data(rs);
 	struct cb_data *cbd = cb_data_new(cb, data);
-	int val= 6, p1 = 3, p2 = 2;
+	struct ofono_modem *modem = ofono_radio_settings_get_modem(rs);
+	const char *model = ofono_modem_get_string(modem, "Model");
 	char buf[20];
 
 	DBG("mode %d", m);
 
-	switch (m) {
-	case OFONO_RADIO_ACCESS_MODE_ANY:
-		val = 6;
-		p1 = 3;
-		p2 = 2;
-		break;
-	case OFONO_RADIO_ACCESS_MODE_GSM:
-		val = 0;
-		break;
-	case OFONO_RADIO_ACCESS_MODE_UMTS:
-		val = 2;
-		break;
-	case OFONO_RADIO_ACCESS_MODE_LTE:
-		val = 3;
-		break;
-	case OFONO_RADIO_ACCESS_MODE_UMTS|OFONO_RADIO_ACCESS_MODE_GSM:
-		val = 1;
-		p1 = 2;
-		break;
-	case OFONO_RADIO_ACCESS_MODE_LTE|OFONO_RADIO_ACCESS_MODE_UMTS:
-		val = 4;
-		p1 = 3;
-		break;
-	case OFONO_RADIO_ACCESS_MODE_LTE|OFONO_RADIO_ACCESS_MODE_GSM:
-		val = 5;
-		p1 = 3;
-		break;
+	if (!g_strcmp0(model, GEMALTO_MODEL_ALS3_PLS8x) ||
+		!g_strcmp0(model, GEMALTO_MODEL_ELS81x)) {
+		int val= 6, p1 = 3, p2 = 2;
+		switch (m) {
+		case OFONO_RADIO_ACCESS_MODE_ANY:
+			val = 6;
+			p1 = 3;
+			p2 = 2;
+			break;
+		case OFONO_RADIO_ACCESS_MODE_GSM:
+			val = 0;
+			break;
+		case OFONO_RADIO_ACCESS_MODE_UMTS:
+			val = 2;
+			break;
+		case OFONO_RADIO_ACCESS_MODE_LTE:
+			val = 3;
+			break;
+		case OFONO_RADIO_ACCESS_MODE_UMTS|OFONO_RADIO_ACCESS_MODE_GSM:
+			val = 1;
+			p1 = 2;
+			break;
+		case OFONO_RADIO_ACCESS_MODE_LTE|OFONO_RADIO_ACCESS_MODE_UMTS:
+			val = 4;
+			p1 = 3;
+			break;
+		case OFONO_RADIO_ACCESS_MODE_LTE|OFONO_RADIO_ACCESS_MODE_GSM:
+			val = 5;
+			p1 = 3;
+			break;
+		}
+
+		if (val == 6)
+			snprintf(buf, sizeof(buf), "AT^SXRAT=%u,%u,%u", val, p1, p2);
+		else if (val == 1 || val == 4 || val == 5)
+			snprintf(buf, sizeof(buf), "AT^SXRAT=%u,%u", val, p1);
+		else
+			snprintf(buf, sizeof(buf), "AT^SXRAT=%u", val);
+	}
+	else if (!g_strcmp0(model, GEMALTO_MODEL_EHS5_E)) {
+		int act = 1, act_pref = 2;
+		switch (m) {
+		case OFONO_RADIO_ACCESS_MODE_ANY:
+		case OFONO_RADIO_ACCESS_MODE_UMTS|OFONO_RADIO_ACCESS_MODE_GSM:
+			act = 1;
+			act_pref = 2;
+			break;
+		case OFONO_RADIO_ACCESS_MODE_GSM:
+			act = 0;
+			break;
+		case OFONO_RADIO_ACCESS_MODE_UMTS:
+			act = 2;
+			break;
+		case OFONO_RADIO_ACCESS_MODE_LTE:
+		case OFONO_RADIO_ACCESS_MODE_LTE|OFONO_RADIO_ACCESS_MODE_UMTS:
+		case OFONO_RADIO_ACCESS_MODE_LTE|OFONO_RADIO_ACCESS_MODE_GSM:
+			goto error;
+		}
+
+		if (act == 1)
+			snprintf(buf, sizeof(buf), "AT^SXRAT=%u,%u", act, act_pref);
+		else
+			snprintf(buf, sizeof(buf), "AT^SXRAT=%u", act);
+	}
+	else {
+		goto error;
 	}
-
-	if (val == 6)
-		snprintf(buf, sizeof(buf), "AT^SXRAT=%u,%u,%u", val, p1, p2);
-	else if (val == 1 || val == 4 || val == 5)
-		snprintf(buf, sizeof(buf), "AT^SXRAT=%u,%u", val, p1);
-	else
-		snprintf(buf, sizeof(buf), "AT^SXRAT=%u", val);
 
 	if (g_at_chat_send(rsd->chat, buf, none_prefix,
 				sxrat_modify_cb, cbd, g_free) > 0)
 		return;
 
+error:
 	CALLBACK_WITH_FAILURE(cb, data);
 	g_free(cbd);
 }
@@ -248,6 +337,7 @@ static void gemalto_radio_settings_remove(struct ofono_radio_settings *rs)
 static const struct ofono_radio_settings_driver driver = {
 	.probe			= gemalto_radio_settings_probe,
 	.remove			= gemalto_radio_settings_remove,
+	.query_available_rats = gemalto_query_available_rats,
 	.query_rat_mode		= gemalto_query_rat_mode,
 	.set_rat_mode		= gemalto_set_rat_mode
 };
-- 
2.25.1

[PATCH 12/15] Fix PPP LCP Configure-Reject

[MaxLyubimov]

According to RFC 1661 5.4

If some Configuration Options received in a Configure-Request are
not recognizable or are not acceptable for negotiation (as
configured by a network administrator), then the implementation
MUST transmit a Configure-Reject.  The Options field is filled
with only the unacceptable Configuration Options from the
Configure-Request.  All recognizable and negotiable Configuration
Options are filtered out of the Configure-Reject, but otherwise
the Configuration Options MUST NOT be reordered or modified in any
way.

Therefore, if the authorization type is NONE, then the authorization
type must be added to the list of not acceptable options.
---
 gatchat/ppp_lcp.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/gatchat/ppp_lcp.c b/gatchat/ppp_lcp.c
index 7c45a27f..26ee7456 100644
--- a/gatchat/ppp_lcp.c
+++ b/gatchat/ppp_lcp.c
@@ -236,6 +236,7 @@ static enum rcr_result lcp_rcr(struct pppcp_data *pppcp,
 		{
 			const guint8 *option_data =
 				ppp_option_iter_get_data(&iter);
+			guint8 option_length = ppp_option_iter_get_length(&iter);
 			guint16 proto = get_host_short(option_data);
 			guint8 method = option_data[2];
 			guint8 *option;
@@ -283,6 +284,15 @@ static enum rcr_result lcp_rcr(struct pppcp_data *pppcp,
 				return RCR_NAK;
 
 			case G_AT_PPP_AUTH_METHOD_NONE:
+				option = g_try_malloc0(option_length + 2);
+				if (option == NULL)
+					return RCR_REJECT;
+
+				option[0] = AUTH_PROTO;
+				option[1] = option_length + 2;
+				memcpy(option + 2, option_data, option_length);
+				*new_options = option;
+				*new_len = option_length + 2;
 				return RCR_REJECT;
 			}
 			break;
-- 
2.25.1

Re: [PATCH 12/15] Fix PPP LCP Configure-Reject

[Denis Kenzior]

Hi Maxim,

On 10/17/23 05:48, MaxLyubimov wrote:
> According to RFC 1661 5.4
> 
> If some Configuration Options received in a Configure-Request are
> not recognizable or are not acceptable for negotiation (as
> configured by a network administrator), then the implementation
> MUST transmit a Configure-Reject.  The Options field is filled
> with only the unacceptable Configuration Options from the
> Configure-Request.  All recognizable and negotiable Configuration
> Options are filtered out of the Configure-Reject, but otherwise
> the Configuration Options MUST NOT be reordered or modified in any
> way.
> 
> Therefore, if the authorization type is NONE, then the authorization
> type must be added to the list of not acceptable options.
> ---
>   gatchat/ppp_lcp.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 

Applied after a minor style fix and rewording of the git commit heading.

Regards,
-Denis

[PATCH 13/15] atmodem: added context deactivation event handling

[MaxLyubimov]

---
 drivers/atmodem/gprs-context.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/atmodem/gprs-context.c b/drivers/atmodem/gprs-context.c
index 1954c6c6..d92b6392 100644
--- a/drivers/atmodem/gprs-context.c
+++ b/drivers/atmodem/gprs-context.c
@@ -359,9 +359,12 @@ static void cgev_notify(GAtResult *result, gpointer user_data)
 	struct ofono_gprs_context *gc = user_data;
 	struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc);
 	const char *event;
-	int cid;
+	unsigned int cid;
+	int r;
 	GAtResultIter iter;
 
+	DBG("");
+
 	g_at_result_iter_init(&iter, result);
 
 	if (!g_at_result_iter_next(&iter, "+CGEV:"))
@@ -370,18 +373,23 @@ static void cgev_notify(GAtResult *result, gpointer user_data)
 	if (!g_at_result_iter_next_unquoted_string(&iter, &event))
 		return;
 
-	if (g_str_has_prefix(event, "NW DEACT") == FALSE)
-		return;
-
-	if (!g_at_result_iter_skip_next(&iter))
+	if (g_str_has_prefix(event, "NW PDN DEACT"))
+		r = sscanf(event, "%*s %*s %*s %u", &cid);
+	else if (g_str_has_prefix(event, "ME PDN DEACT"))
+		r = sscanf(event, "%*s %*s %*s %u", &cid);
+	else if (g_str_has_prefix(event, "NW DEACT"))
+		r = sscanf(event, "%*s %*s %u", &cid);
+	else if (g_str_has_prefix(event, "ME DEACT"))
+		r = sscanf(event, "%*s %*s %u", &cid);
+	else
 		return;
 
-	if (!g_at_result_iter_next_number(&iter, &cid))
+	if (r != 1)
 		return;
 
-	DBG("cid %d", cid);
+	DBG("cid %u", cid);
 
-	if ((unsigned int) cid != gcd->active_context)
+	if (cid != gcd->active_context)
 		return;
 
 	if (gcd->state != STATE_IDLE && gcd->ppp)
-- 
2.25.1

Re: [PATCH 13/15] atmodem: added context deactivation event handling

[Denis Kenzior]

Hi Maxim,

On 10/17/23 05:49, MaxLyubimov wrote:
> ---
>   drivers/atmodem/gprs-context.c | 24 ++++++++++++++++--------
>   1 file changed, 16 insertions(+), 8 deletions(-)
> 

Applied, thanks.

Regards,
-Denis

[PATCH 14/15] gatchat: added command completion by timeout

[MaxLyubimov]

---
 gatchat/gatchat.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/gatchat/gatchat.c b/gatchat/gatchat.c
index 9e777107..f4249406 100644
--- a/gatchat/gatchat.c
+++ b/gatchat/gatchat.c
@@ -38,11 +38,16 @@
 
 /* #define WRITE_SCHEDULER_DEBUG 1 */
 
+#define COMMAND_RESPONSE_TIMEOUT_SEC 20
+
 #define COMMAND_FLAG_EXPECT_PDU			0x1
 #define COMMAND_FLAG_EXPECT_SHORT_PROMPT	0x2
 
 struct at_chat;
 static void chat_wakeup_writer(struct at_chat *chat);
+static void set_response_timeout(struct at_chat *chat);
+static void delete_response_timeout(struct at_chat *chat);
+static void refresh_response_timeout(struct at_chat *chat);
 
 static const char *none_prefix[] = { NULL };
 
@@ -92,6 +97,8 @@ struct at_chat {
 	gpointer debug_data;			/* Data to pass to debug func */
 	char *pdu_notify;			/* Unsolicited Resp w/ PDU */
 	GSList *response_lines;			/* char * lines of the response */
+	guint response_timeout_source;
+	guint response_timeout_counter;
 	char *wakeup;				/* command sent to wakeup modem */
 	gint timeout_source;
 	gdouble inactivity_time;		/* Period of inactivity */
@@ -360,6 +367,9 @@ static void chat_cleanup(struct at_chat *chat)
 		g_slist_free_full(chat->terminator_list, free_terminator);
 		chat->terminator_list = NULL;
 	}
+
+	delete_response_timeout(chat);
+	chat->response_timeout_counter = 0;
 }
 
 static void io_disconnect(gpointer user_data)
@@ -431,11 +441,14 @@ static gboolean at_chat_match_notify(struct at_chat *chat, char *line)
 	return ret;
 }
 
+
 static void at_chat_finish_command(struct at_chat *p, gboolean ok, char *final)
 {
 	struct at_command *cmd = g_queue_pop_head(p->command_queue);
 	GSList *response_lines;
 
+	refresh_response_timeout(p);
+
 	/* Cannot happen, but lets be paranoid */
 	if (cmd == NULL)
 		return;
@@ -938,6 +951,8 @@ static void at_chat_suspend(struct at_chat *chat)
 	g_at_io_set_write_handler(chat->io, NULL, NULL);
 	g_at_io_set_read_handler(chat->io, NULL, NULL);
 	g_at_io_set_debug(chat->io, NULL, NULL);
+
+	delete_response_timeout(chat);
 }
 
 static void at_chat_resume(struct at_chat *chat)
@@ -956,6 +971,8 @@ static void at_chat_resume(struct at_chat *chat)
 
 	if (g_queue_get_length(chat->command_queue) > 0)
 		chat_wakeup_writer(chat);
+
+	refresh_response_timeout(chat);
 }
 
 static void at_chat_unref(struct at_chat *chat)
@@ -1044,6 +1061,8 @@ static guint at_chat_send_common(struct at_chat *chat, guint gid,
 	if (g_queue_get_length(chat->command_queue) == 1)
 		chat_wakeup_writer(chat);
 
+	set_response_timeout(chat);
+
 	return c->id;
 }
 
@@ -1125,6 +1144,8 @@ static gboolean at_chat_cancel(struct at_chat *chat, guint group, guint id)
 		g_queue_remove(chat->command_queue, c);
 	}
 
+	refresh_response_timeout(chat);
+
 	return TRUE;
 }
 
@@ -1152,6 +1173,8 @@ static gboolean at_chat_cancel_group(struct at_chat *chat, guint group)
 		g_queue_remove(chat->command_queue, c);
 	}
 
+	refresh_response_timeout(chat);
+
 	return TRUE;
 }
 
@@ -1314,6 +1337,8 @@ static struct at_chat *create_chat(GIOChannel *channel, GIOFlags flags,
 	g_at_io_set_read_handler(chat->io, new_bytes, chat);
 
 	chat->syntax = g_at_syntax_ref(syntax);
+	chat->response_timeout_source = 0;
+	chat->response_timeout_counter = 0;
 
 	return chat;
 
@@ -1351,6 +1376,63 @@ static GAtChat *g_at_chat_new_common(GIOChannel *channel, GIOFlags flags,
 	return chat;
 }
 
+static gboolean response_timeout_cb(gpointer user_data)
+{
+	struct at_chat *chat = user_data;
+	char* error_str;
+	char* line;
+
+	if (chat->suspended) {
+		chat->response_timeout_source = 0;
+		return FALSE;
+	}
+
+	if (chat->response_timeout_counter < COMMAND_RESPONSE_TIMEOUT_SEC) {
+		chat->response_timeout_counter++;
+		return TRUE;
+	}
+	chat->response_timeout_counter = 0;
+
+	if (g_queue_peek_head(chat->command_queue)) {
+		if (chat->debugf)
+			chat->debugf(": Command response timeout\n", chat->debug_data);
+		error_str = "+CME ERROR: 100";
+		line = g_try_new(char, strlen(error_str) + 1);
+		if (line) {
+			strcpy(line, error_str);
+		}
+		at_chat_finish_command(chat, FALSE, line);
+	}
+	else {
+		chat->response_timeout_source = 0;
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
+static void set_response_timeout(struct at_chat *chat) {
+	if (!chat->suspended && !chat->response_timeout_source) {
+		chat->response_timeout_source = g_timeout_add(1000, response_timeout_cb, chat);
+		chat->response_timeout_counter = 0;
+	}
+}
+
+static void delete_response_timeout(struct at_chat *chat) {
+	if (chat->response_timeout_source) {
+		g_source_remove(chat->response_timeout_source);
+		chat->response_timeout_source = 0;
+		chat->response_timeout_counter = 0;
+	}
+}
+
+static void refresh_response_timeout(struct at_chat *chat) {
+	chat->response_timeout_counter = 0;
+	if (g_queue_peek_head(chat->command_queue) && !chat->suspended && !chat->response_timeout_source) {
+		chat->response_timeout_source = g_timeout_add(1000, response_timeout_cb, chat);
+	}
+}
+
 GAtChat *g_at_chat_new(GIOChannel *channel, GAtSyntax *syntax)
 {
 	return g_at_chat_new_common(channel, G_IO_FLAG_NONBLOCK, syntax);
-- 
2.25.1

Re: [PATCH 14/15] gatchat: added command completion by timeout

[Denis Kenzior]

Hi Maxim,

On 10/17/23 05:49, MaxLyubimov wrote:
> ---
>   gatchat/gatchat.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 82 insertions(+)
> 

So I think you're going to need to give a bit more context for how this is 
expected to work.  There are commands (on older modems) that can take minutes to 
complete.  This is why multiplexers were invented.  Now this is probably quite 
rare, but I imagine it can still happen.

Also, what happens if the command times out?  You generate a synthetic CME 100 
error, but what then?

Also, this change is full of coding style violations, please fix those before I 
look further.

Regards,
-Denis

Re: [PATCH 14/15] gatchat: added command completion by timeout

[Любимов Максим]

On 18/10/2023 в 21:21 -0500, Denis Kenzior wrote:
> So I think you're going to need to give a bit more context for how
> this is 
> expected to work.  There are commands (on older modems) that can take
> minutes to 
> complete.  This is why multiplexers were invented.  Now this is
> probably quite 
> rare, but I imagine it can still happen.

This functionality was added because some modems sometimes do not
respond to the command. Therefore, there is an endless wait for a
response and blocking of other functions with a message that the action
is still in progress. It was experimentally verified that if the
command is repeated, the modem can immediately respond to it and there
will be no blocking. What do you suggest to do if the modem does not
respond to the command either after a minute, or after an hour, never?

> Also, what happens if the command times out?  You generate a
> synthetic CME 100 
> error, but what then?

The function that sent the command must take action based on its
operating logic, for example, repeat the command, or fail.

> Also, this change is full of coding style violations, please fix
> those before I 
> look further.

Ok

Best regards,
Lyubimov Maxim

Re: [PATCH 14/15] gatchat: added command completion by timeout

[Denis Kenzior]

Hi Максим,

On 10/27/23 04:45, Любимов Максим wrote:
> On 18/10/2023 в 21:21 -0500, Denis Kenzior wrote:
>> So I think you're going to need to give a bit more context for how
>> this is
>> expected to work.  There are commands (on older modems) that can take
>> minutes to
>> complete.  This is why multiplexers were invented.  Now this is
>> probably quite
>> rare, but I imagine it can still happen.
> 
> This functionality was added because some modems sometimes do not
> respond to the command. Therefore, there is an endless wait for a
> response and blocking of other functions with a message that the action

Ok, but which modems?  Do you have an OFONO_AT_DEBUG log trace you can share?

Anyhow, at the very least you need to make this an opt-in feature.  As I pointed 
out, opting in *all* modem drivers into this functionality will break some of them.

> is still in progress. It was experimentally verified that if the
> command is repeated, the modem can immediately respond to it and there
> will be no blocking. What do you suggest to do if the modem does not
> respond to the command either after a minute, or after an hour, never?

Call the vendor and have them fix their firmware? :)  But seriously, how many 
commands are we talking about?  If it is one, then handle it specifically for 
that command.  If it is a dozen, create a utility function.

I'm not convinced adding this functionality directly to GAtChat is the right 
approach.  There is no such thing as 'AT command timeout' in V.250 specification 
or any other standard.  AT command cancellation as a concept exists, but it only 
works sometimes and only for some commands, and isn't what you're doing here 
from what I can tell.

What you're describing is vendor specific behavior and has to be treated as such.

> 
>> Also, what happens if the command times out?  You generate a
>> synthetic CME 100
>> error, but what then?
> 
> The function that sent the command must take action based on its
> operating logic, for example, repeat the command, or fail.
> 

Okay, so where is this code?

Regards,
-Denis

Re: [PATCH 14/15] gatchat: added command completion by timeout

[Любимов Максим]

Hi, Denis

> > > So I think you're going to need to give a bit more context for
> > > how
> > > this is
> > > expected to work.  There are commands (on older modems) that can
> > > take
> > > minutes to
> > > complete.  This is why multiplexers were invented.  Now this is
> > > probably quite
> > > rare, but I imagine it can still happen.
> > 
> > This functionality was added because some modems sometimes do not
> > respond to the command. Therefore, there is an endless wait for a
> > response and blocking of other functions with a message that the
> > action
> 
> Ok, but which modems?  Do you have an OFONO_AT_DEBUG log trace you
> can share?

Similar behavior is observed on simcom modems. There are no logs yet,
but it clearly shows that there is no response to the command.

> Anyhow, at the very least you need to make this an opt-in
> feature.  As I pointed 
> out, opting in *all* modem drivers into this functionality will break
> some of them.

I can suggest adding a property to the modem driver data that will
enable this function for it.

> > is still in progress. It was experimentally verified that if the
> > command is repeated, the modem can immediately respond to it and
> > there
> > will be no blocking. What do you suggest to do if the modem does
> > not
> > respond to the command either after a minute, or after an hour,
> > never?
> 
> Call the vendor and have them fix their firmware? :)  But seriously,
> how many 
> commands are we talking about?  If it is one, then handle it
> specifically for 
> that command.  If it is a dozen, create a utility function.

Multiple commands that are sent over an interface shared with a PPP
connection. I have a suspicion that when switching from data
transmission to command processing, the modem does not process AT
commands through this interface. But the time frame for this switch is
not clear.

> I'm not convinced adding this functionality directly to GAtChat is
> the right 
> approach.  There is no such thing as 'AT command timeout' in V.250
> specification 
> or any other standard.  AT command cancellation as a concept exists,
> but it only 
> works sometimes and only for some commands, and isn't what you're
> doing here 
> from what I can tell.
> 
> What you're describing is vendor specific behavior and has to be
> treated as such.

How to properly handle a lack of response to a command? Should I add a
property to the modem driver to enable this feature in GAtChat? Or
should we separately process the lack of response for those commands
for which such modem behavior was observed?

Best regards,
Lyubimov Maxim

Re: [PATCH 14/15] gatchat: added command completion by timeout

[Denis Kenzior]

Hi Максим,

>>
>> Ok, but which modems?  Do you have an OFONO_AT_DEBUG log trace you
>> can share?
> 
> Similar behavior is observed on simcom modems. There are no logs yet,
> but it clearly shows that there is no response to the command.

A log would be useful to help me understand what is happening.

> 
>> Anyhow, at the very least you need to make this an opt-in
>> feature.  As I pointed
>> out, opting in *all* modem drivers into this functionality will break
>> some of them.
> 
> I can suggest adding a property to the modem driver data that will
> enable this function for it.

Sure, that's fine.

> 
> Multiple commands that are sent over an interface shared with a PPP
> connection. I have a suspicion that when switching from data
> transmission to command processing, the modem does not process AT
> commands through this interface. But the time frame for this switch is
> not clear.

Most devices come setup with 2-3 pre-multiplexed AT command ports.  Most modem 
drivers without a high-speed network interface follow the following pattern:
- One AT command port for most AT commands, always active (Modem)
- One AT command port for PPP.  This port is used to send CGDCONT/CGDATA 
commands and is suspended for the duration of the PPP session (Data)

Are you saying that when the PPP session is ended (NO CARRIER sent?) then 
subsequent commands sent to the Data port time out?  Or what is happening?

>> What you're describing is vendor specific behavior and has to be
>> treated as such.
> 
> How to properly handle a lack of response to a command? Should I add a
> property to the modem driver to enable this feature in GAtChat? Or
> should we separately process the lack of response for those commands
> for which such modem behavior was observed?

I'd like to better understand the problem before offering solutions.  What you 
outline above are all possible strategies.

Regards,
-Denis

[PATCH 15/15] ppp: message tracing

[MaxLyubimov]

Added OFONO_PPP_RECORDING_PATH environment variable handling to enable PPP message tracing
---
 gatchat/gatppp.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
index 259e6d5c..a655b35c 100644
--- a/gatchat/gatppp.c
+++ b/gatchat/gatppp.c
@@ -561,10 +561,16 @@ gboolean g_at_ppp_listen(GAtPPP *ppp, GAtIO *io)
 /* Administrative Open */
 gboolean g_at_ppp_open(GAtPPP *ppp, GAtIO *io)
 {
+	char* recording_path;
+
 	ppp->hdlc = g_at_hdlc_new_from_io(io);
 	if (ppp->hdlc == NULL)
 		return FALSE;
 
+	recording_path = getenv("OFONO_PPP_RECORDING_PATH");
+	if (recording_path)
+		g_at_ppp_set_recording(ppp, recording_path);
+
 	ppp->suspended = FALSE;
 	g_at_hdlc_set_receive(ppp->hdlc, ppp_receive, ppp);
 	g_at_hdlc_set_suspend_function(ppp->hdlc,
-- 
2.25.1

Re: [PATCH 15/15] ppp: message tracing

[Denis Kenzior]

Hi Maxim,

On 10/17/23 05:49, MaxLyubimov wrote:
> Added OFONO_PPP_RECORDING_PATH environment variable handling to enable PPP message tracing
> ---
>   gatchat/gatppp.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
> index 259e6d5c..a655b35c 100644
> --- a/gatchat/gatppp.c
> +++ b/gatchat/gatppp.c
> @@ -561,10 +561,16 @@ gboolean g_at_ppp_listen(GAtPPP *ppp, GAtIO *io)
>   /* Administrative Open */
>   gboolean g_at_ppp_open(GAtPPP *ppp, GAtIO *io)
>   {
> +	char* recording_path;
> +
>   	ppp->hdlc = g_at_hdlc_new_from_io(io);
>   	if (ppp->hdlc == NULL)
>   		return FALSE;
>   
> +	recording_path = getenv("OFONO_PPP_RECORDING_PATH");
> +	if (recording_path)
> +		g_at_ppp_set_recording(ppp, recording_path);
> +

Hmm, this is better off in drivers/atmodem/gprs-context.c since you have an 
OFONO_ prefix for the environment variable.

>   	ppp->suspended = FALSE;
>   	g_at_hdlc_set_receive(ppp->hdlc, ppp_receive, ppp);
>   	g_at_hdlc_set_suspend_function(ppp->hdlc,

Regards,
-Denis

Re: [PATCH 15/15] ppp: message tracing

[Любимов Максим]

Good afternoon, Denis

On 18/10/2023 в 21:22 -0500, Denis Kenzior wrote:
> On 10/17/23 05:49, MaxLyubimov wrote:
> > Added OFONO_PPP_RECORDING_PATH environment variable handling to
> > enable PPP message tracing
> > ---
> >   gatchat/gatppp.c | 6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/gatchat/gatppp.c b/gatchat/gatppp.c
> > index 259e6d5c..a655b35c 100644
> > --- a/gatchat/gatppp.c
> > +++ b/gatchat/gatppp.c
> > @@ -561,10 +561,16 @@ gboolean g_at_ppp_listen(GAtPPP *ppp, GAtIO
> > *io)
> >   /* Administrative Open */
> >   gboolean g_at_ppp_open(GAtPPP *ppp, GAtIO *io)
> >   {
> > +	char* recording_path;
> > +
> >   	ppp->hdlc = g_at_hdlc_new_from_io(io);
> >   	if (ppp->hdlc == NULL)
> >   		return FALSE;
> >   
> > +	recording_path = getenv("OFONO_PPP_RECORDING_PATH");
> > +	if (recording_path)
> > +		g_at_ppp_set_recording(ppp, recording_path);
> > +
> 
> Hmm, this is better off in drivers/atmodem/gprs-context.c since you
> have an 
> OFONO_ prefix for the environment variable.

I had to call the g_at_ppp_set_recording function here, because if you
call it before g_at_ppp_open, then the ppp->hdlc variable is not
defined and the function does nothing, and if you call it after
g_at_ppp_open, then there is a possibility of losing events in the log,
because g_at_ppp_open calls functions

pppcp_signal_up(ppp->lcp);
pppcp_signal_open(ppp->lcp);
ppp_enter_phase(ppp, PPP_PHASE_ESTABLISHMENT);

What prefix should I choose for this environment variable?

Best regards,
Lyubimov Maxim

Re: [PATCH 15/15] ppp: message tracing

[Denis Kenzior]

Hi Максим,

>>
>> Hmm, this is better off in drivers/atmodem/gprs-context.c since you
>> have an
>> OFONO_ prefix for the environment variable.
> 
> I had to call the g_at_ppp_set_recording function here, because if you
> call it before g_at_ppp_open, then the ppp->hdlc variable is not
> defined and the function does nothing, and if you call it after
> g_at_ppp_open, then there is a possibility of losing events in the log,
> because g_at_ppp_open calls functions

Okay, so that looks like an implementation problem in GAtPPP.  One possible fix 
is to store the recording filename until g_at_hdlc_new* is called, and call 
g_at_hdlc_set_recording at the right moment.

> 
> pppcp_signal_up(ppp->lcp);
> pppcp_signal_open(ppp->lcp);
> ppp_enter_phase(ppp, PPP_PHASE_ESTABLISHMENT);
> 
> What prefix should I choose for this environment variable?

The original comment is still applicable.  Our pattern is not to invoke getenv() 
inside the libraries themselves.  The clients of the libraries are responsible 
for that as well as setting up the library accordingly.

Regards,
-Denis

Re: [PATCH 15/15] ppp: message tracing

[Любимов Максим]

Hi, Denis

> > > Hmm, this is better off in drivers/atmodem/gprs-context.c since
> > > you
> > > have an
> > > OFONO_ prefix for the environment variable.
> > 
> > I had to call the g_at_ppp_set_recording function here, because if
> > you
> > call it before g_at_ppp_open, then the ppp->hdlc variable is not
> > defined and the function does nothing, and if you call it after
> > g_at_ppp_open, then there is a possibility of losing events in the
> > log,
> > because g_at_ppp_open calls functions
> 
> Okay, so that looks like an implementation problem in GAtPPP.  One
> possible fix 
> is to store the recording filename until g_at_hdlc_new* is called,
> and call 
> g_at_hdlc_set_recording at the right moment.

Can I fix this and redo this patch?

Best regards,
Lyubimov Maxim

Re: [PATCH 15/15] ppp: message tracing

[Denis Kenzior]

Hi Максим,

>>
>> Okay, so that looks like an implementation problem in GAtPPP.  One
>> possible fix
>> is to store the recording filename until g_at_hdlc_new* is called,
>> and call
>> g_at_hdlc_set_recording at the right moment.
> 
> Can I fix this and redo this patch?

Yes, please.

Regards,
-Denis

Re: [PATCH 01/15] Skipping AT+CUAD sending for QUECTEL_EC2X vendor

[Denis Kenzior]

Hi Max,

On 10/17/23 05:48, MaxLyubimov wrote:
> From: Sergei Golubtsov <sg@aqsi.ru>
> 
> This workarounds bug in EC200A firmware which leads to modem
> reset in case of usage of some SIM cards when sending the next
> command
> ---
>   drivers/atmodem/sim.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 

<snip>

> +
> +	/* QUECTEL EC2X reboots when executing the AT+CUAD command with SIM cards
> +	of some operators */

Note doc/coding-style.txt item M2.  Also note that this line is > 80 chars.  I 
fixed this for you.

> +	if (sd->vendor == OFONO_VENDOR_QUECTEL_EC2X)
> +		goto error;
> +
> +	cbd = cb_data_new(cb, data);
>   
>   	if (g_at_chat_send(sd->chat, "AT+CUAD", cuad_prefix,
>   			at_discover_apps_cb, cbd, g_free) > 0)

Applied, thanks.

Regards,
-Denis