[PATCH] atmodem/gprs: added LTE and NR indicators

[PATCH] atmodem/gprs: added LTE and NR indicators

[Giacinto Cifelli]

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

According to the 27.007 CGREG (both as URC and command) is no longer
sufficient to determine the registration state.
New indicators: CEREG for LTE, C5GREG for NR/5G are introduced.

In practice, legacy modems have CGREG, LTE-only modems have CEREG, and
several others have both.
With NR the picture will become even larger.

The handling of these indicators is not straighforward, because each
will report the status in its own access technology. So we might have
CGREG=4 but CEREG=1. Therefore they must be checked together.
Particularly troublesome is the case of the URC. In case of hand-over,
for example for a CSFB call, a common case is to have '+CEREG: 4'
followed by '+CGREG: 1'. The first URC must be discarded, and this is
done checking the technology reported by 'AT+COPS?' after the URC is
received, in case the status is different from 1 or 5 (to be done:
add the check for other roaming states for NR once the first modems are
available). Note that according to the 27.007 CREG does not apply to PS
networks, so it cannot be reliably used.
If COPS does not report a technology or reports the same technology,
we take the indicator.

Similar handling for AT+CGREG?, AT+CEREG?, AT+C5GREG? : we take the
best result.

An initial verification for the presence of any of these indicators is
required. Also how many of them are available is needed information.
---
 drivers/atmodem/gprs.c | 420 +++++++++++++++++++++++++++++++++++------
 1 file changed, 364 insertions(+), 56 deletions(-)

diff --git a/drivers/atmodem/gprs.c b/drivers/atmodem/gprs.c
index df37d05f..6e01738a 100644
--- a/drivers/atmodem/gprs.c
+++ b/drivers/atmodem/gprs.c
@@ -4,6 +4,7 @@
  *
  *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
  *  Copyright (C) 2010  ST-Ericsson AB.
+ *  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
@@ -35,6 +36,7 @@
 #include <ofono/log.h>
 #include <ofono/modem.h>
 #include <ofono/gprs.h>
+#include <common.h>
 
 #include "gatchat.h"
 #include "gatresult.h"
@@ -43,6 +45,8 @@
 #include "vendor.h"
 
 static const char *cgreg_prefix[] = { "+CGREG:", NULL };
+static const char *cereg_prefix[] = { "+CEREG:", NULL };
+static const char *c5greg_prefix[] = { "+C5GREG:", NULL };
 static const char *cgdcont_prefix[] = { "+CGDCONT:", NULL };
 static const char *none_prefix[] = { NULL };
 
@@ -51,7 +55,22 @@ struct gprs_data {
 	unsigned int vendor;
 	unsigned int last_auto_context_id;
 	gboolean telit_try_reattach;
+	gboolean has_cgreg;
+	gboolean has_cereg;
+	gboolean has_c5greg;
+	gboolean nb_inds;
 	int attached;
+	int cgreg_status;
+	int cereg_status;
+	int c5greg_status;
+};
+
+struct netreg_info {
+	struct ofono_gprs *gprs;
+	struct gprs_data *gd;
+	const char *ind;
+	int status;
+	int bearer;
 };
 
 static void at_cgatt_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -92,21 +111,101 @@ static void at_cgreg_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	struct ofono_error error;
 	int status;
 	struct gprs_data *gd = cbd->user;
+	gboolean last = !(gd->has_cereg || gd->has_c5greg);
 
 	decode_at_error(&error, g_at_result_final_response(result));
 
 	if (!ok) {
-		cb(&error, -1, cbd->data);
-		return;
+		status = -1;
+		goto end;
 	}
 
 	if (at_util_parse_reg(result, "+CGREG:", NULL, &status,
 				NULL, NULL, NULL, gd->vendor) == FALSE) {
-		CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
-		return;
+		error.type = OFONO_ERROR_TYPE_FAILURE;
+		error.error = 0;
+		status = -1;
+		goto end;
+	}
+
+end:
+	gd->cgreg_status = status;
+
+	if (last)
+		cb(&error, status, cbd->data);
+}
+
+static void at_cereg_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_gprs_status_cb_t cb = cbd->cb;
+	struct ofono_error error;
+	int status;
+	struct gprs_data *gd = cbd->user;
+	gboolean last = !gd->has_c5greg;
+
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok) {
+		status = -1;
+		goto end;
+	}
+
+	if (at_util_parse_reg(result, "+CEREG:", NULL, &status,
+				NULL, NULL, NULL, gd->vendor) == FALSE) {
+		error.type = OFONO_ERROR_TYPE_FAILURE;
+		error.error = 0;
+		status = -1;
+		goto end;
+	}
+
+end:
+	gd->cereg_status = status;
+
+	if (last) {
+
+		if (gd->cgreg_status == NETWORK_REGISTRATION_STATUS_DENIED ||
+		     gd->cgreg_status == NETWORK_REGISTRATION_STATUS_REGISTERED)
+			cb(&error, gd->cgreg_status, cbd->data);
+		else
+			cb(&error, status, cbd->data);
 	}
+}
 
-	cb(&error, status, cbd->data);
+static void at_c5greg_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_gprs_status_cb_t cb = cbd->cb;
+	struct ofono_error error;
+	int status;
+	struct gprs_data *gd = cbd->user;
+
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok) {
+		status = -1;
+		goto end;
+	}
+
+	if (at_util_parse_reg(result, "+C5GREG:", NULL, &status,
+				NULL, NULL, NULL, gd->vendor) == FALSE) {
+		error.type = OFONO_ERROR_TYPE_FAILURE;
+		error.error = 0;
+		status = -1;
+		goto end;
+	}
+
+end:
+	gd->c5greg_status = status;
+
+	if (gd->cgreg_status == NETWORK_REGISTRATION_STATUS_DENIED ||
+		     gd->cgreg_status == NETWORK_REGISTRATION_STATUS_REGISTERED)
+		cb(&error, gd->cgreg_status, cbd->data);
+	else if (gd->cereg_status == NETWORK_REGISTRATION_STATUS_DENIED ||
+		gd->cereg_status == NETWORK_REGISTRATION_STATUS_REGISTERED)
+		cb(&error, gd->cereg_status, cbd->data);
+	else
+		cb(&error, status, cbd->data);
 }
 
 static void at_gprs_registration_status(struct ofono_gprs *gprs,
@@ -114,9 +213,6 @@ static void at_gprs_registration_status(struct ofono_gprs *gprs,
 					void *data)
 {
 	struct gprs_data *gd = ofono_gprs_get_data(gprs);
-	struct cb_data *cbd = cb_data_new(cb, data);
-
-	cbd->user = gd;
 
 	switch (gd->vendor) {
 	case OFONO_VENDOR_GOBI:
@@ -137,13 +233,51 @@ static void at_gprs_registration_status(struct ofono_gprs *gprs,
 		break;
 	}
 
-	if (g_at_chat_send(gd->chat, "AT+CGREG?", cgreg_prefix,
-				at_cgreg_cb, cbd, g_free) > 0)
-		return;
+	/*
+	 * this is long: send all indicators, compare at the end if one reports
+	 * attached and use it, otherwise report status for last indicator
+	 * tested (higher technology).
+	 * Note: AT+CGATT? is not good because doesn't tell us if we are roaming
+	 */
+	if (gd->has_cgreg) {
+		struct cb_data *cbd = cb_data_new(cb, data);
+		cbd->user = gd;
+		gd->cgreg_status = -1; /* preset in case of fail of the at send */
+
+		/* g_at_chat_send fails only if g_new_try fails, so we stop */
+		if (g_at_chat_send(gd->chat, "AT+CGREG?", cgreg_prefix,
+					at_cgreg_cb, cbd, g_free) == 0) {
+			g_free(cbd);
+			CALLBACK_WITH_FAILURE(cb, -1, data);
+			return;
+		}
+	}
 
-	g_free(cbd);
+	if (gd->has_cereg) {
+		struct cb_data *cbd = cb_data_new(cb, data);
+		cbd->user = gd;
+		gd->cereg_status = -1;
 
-	CALLBACK_WITH_FAILURE(cb, -1, data);
+		if (g_at_chat_send(gd->chat, "AT+CEREG?", cereg_prefix,
+					at_cereg_cb, cbd, g_free) == 0) {
+			g_free(cbd);
+			CALLBACK_WITH_FAILURE(cb, -1, data);
+			return;
+		}
+	}
+
+	if (gd->has_c5greg) {
+		struct cb_data *cbd = cb_data_new(cb, data);
+		cbd->user = gd;
+		gd->c5greg_status = -1;
+
+		if (g_at_chat_send(gd->chat, "AT+C5GREG?", c5greg_prefix,
+					at_c5greg_cb, cbd, g_free) == 0) {
+			g_free(cbd);
+			CALLBACK_WITH_FAILURE(cb, -1, data);
+			return;
+		}
+	}
 }
 
 static void at_cgdcont_read_cb(gboolean ok, GAtResult *result,
@@ -188,14 +322,100 @@ static void at_cgdcont_read_cb(gboolean ok, GAtResult *result,
 				activated_cid);
 }
 
+static int cops_cb(gboolean ok, GAtResult *result)
+{
+	GAtResultIter iter;
+	int format, tech = -1;
+
+	if (!ok)
+		goto error;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "+COPS:"))
+		goto error;
+
+	g_at_result_iter_skip_next(&iter); /* mode: automatic, manual, ... */
+
+	if (!g_at_result_iter_next_number(&iter, &format))
+		goto error;
+
+	g_at_result_iter_skip_next(&iter); /* operator name or code */
+
+	if (!g_at_result_iter_next_number(&iter, &tech))
+		tech = -1; /* make sure it has not been set to something */
+error:
+	return tech;
+}
+
+
+static void netreg_notify_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct netreg_info *nri = user_data;
+	int cops_tech = cops_cb(ok, result);
+
+	if (cops_tech == -1) { /* take the indicator status */
+		ofono_gprs_status_notify(nri->gprs, nri->status);
+		return;
+	}
+
+	/*
+	 * values taken from the 3GPP 27.007 rel.15
+	 * matching enum access_technology in common.h up to 7.
+	 */
+	if (g_str_equal(nri->ind,"CGREG") && (cops_tech < 7 || cops_tech == 8))
+		ofono_gprs_status_notify(nri->gprs, nri->status);
+	else if (g_str_equal(nri->ind,"CEREG") && (cops_tech == 7 ||
+					cops_tech == 9 || cops_tech == 12))
+		ofono_gprs_status_notify(nri->gprs, nri->status);
+	else if (g_str_equal(nri->ind,"C5GREG") && (cops_tech == 10 ||
+					cops_tech == 11 || cops_tech == 13))
+		ofono_gprs_status_notify(nri->gprs, nri->status);
+	/* all other cases ignored: indicator not for current AcT */
+}
+
+static void netreg_notify(struct ofono_gprs *gprs, const char* ind, int status,
+								int bearer)
+{
+	struct gprs_data *gd = ofono_gprs_get_data(gprs);
+	struct netreg_info *nri;
+
+	if (status == NETWORK_REGISTRATION_STATUS_DENIED ||
+			status == NETWORK_REGISTRATION_STATUS_REGISTERED ||
+			status == NETWORK_REGISTRATION_STATUS_ROAMING ||
+			gd->nb_inds == 1) {
+		/* accept this status and process */
+		ofono_gprs_status_notify(gprs, status);
+
+		if (bearer != -1)
+			ofono_gprs_bearer_notify(gprs, bearer);
+
+		return;
+	}
+
+	/*
+	 * in this case nb_inds>1 && status not listed above
+	 * we check AT+COPS? for a second opinion.
+	 */
+
+	nri = g_new0(struct netreg_info, 1);
+	nri->gprs = gprs;
+	nri->gd = gd;
+	nri->ind = ind;
+	nri->status = status;
+	nri->bearer = bearer;
+	g_at_chat_send(gd->chat, "AT+COPS?", none_prefix, netreg_notify_cb,
+		nri, g_free);
+}
+
 static void cgreg_notify(GAtResult *result, gpointer user_data)
 {
 	struct ofono_gprs *gprs = user_data;
-	int status;
 	struct gprs_data *gd = ofono_gprs_get_data(gprs);
+	int status, bearer;
 
-	if (at_util_parse_reg_unsolicited(result, "+CGREG:", &status,
-				NULL, NULL, NULL, gd->vendor) == FALSE)
+	if (!at_util_parse_reg_unsolicited(result, "+CGREG:", &status,
+				NULL, NULL, &bearer, gd->vendor) == FALSE)
 		return;
 
 	/*
@@ -220,7 +440,33 @@ static void cgreg_notify(GAtResult *result, gpointer user_data)
 		gd->telit_try_reattach = FALSE;
 	}
 
-	ofono_gprs_status_notify(gprs, status);
+	netreg_notify(gprs, "CGREG", status, bearer);
+}
+
+static void cereg_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_gprs *gprs = user_data;
+	struct gprs_data *gd = ofono_gprs_get_data(gprs);
+	int status, bearer;
+
+	if (!at_util_parse_reg_unsolicited(result, "+CEREG:", &status,
+				NULL, NULL, &bearer, gd->vendor) == FALSE)
+		return;
+
+	netreg_notify(gprs, "CEREG", status, bearer);
+}
+
+static void c5greg_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_gprs *gprs = user_data;
+	struct gprs_data *gd = ofono_gprs_get_data(gprs);
+	int status, bearer;
+
+	if (!at_util_parse_reg_unsolicited(result, "+C5GREG:", &status,
+				NULL, NULL, &bearer, gd->vendor) == FALSE)
+		return;
+
+	netreg_notify(gprs, "C5GREG", status, bearer);
 }
 
 static void cgev_notify(GAtResult *result, gpointer user_data)
@@ -439,14 +685,37 @@ static void cpsb_notify(GAtResult *result, gpointer user_data)
 	ofono_gprs_bearer_notify(gprs, bearer);
 }
 
-static void gprs_initialized(gboolean ok, GAtResult *result, gpointer user_data)
+static void gprs_initialized(struct ofono_gprs *gprs)
 {
-	struct ofono_gprs *gprs = user_data;
 	struct gprs_data *gd = ofono_gprs_get_data(gprs);
 
+	g_at_chat_send(gd->chat, "AT+CGAUTO=0", none_prefix, NULL, NULL,
+								NULL);
+
+	switch (gd->vendor) {
+	case OFONO_VENDOR_MBM:
+		/* Ericsson MBM and ST-E modems don't support AT+CGEREP=2,1 */
+		g_at_chat_send(gd->chat, "AT+CGEREP=1,0", none_prefix,
+			NULL, NULL, NULL);
+		break;
+	case OFONO_VENDOR_NOKIA:
+		/* Nokia data cards don't support AT+CGEREP=1,0 either */
+		g_at_chat_send(gd->chat, "AT+CGEREP=1", none_prefix,
+			NULL, NULL, NULL);
+		break;
+	default:
+		g_at_chat_send(gd->chat, "AT+CGEREP=2,1", none_prefix,
+			NULL, NULL, NULL);
+		break;
+	}
+
 	g_at_chat_register(gd->chat, "+CGEV:", cgev_notify, FALSE, gprs, NULL);
-	g_at_chat_register(gd->chat, "+CGREG:", cgreg_notify,
-						FALSE, gprs, NULL);
+	g_at_chat_register(gd->chat, "+CGREG:", cgreg_notify, FALSE, gprs,
+									NULL);
+	g_at_chat_register(gd->chat, "+CEREG:", cereg_notify, FALSE, gprs,
+									NULL);
+	g_at_chat_register(gd->chat, "+C5GREG:", c5greg_notify, FALSE, gprs,
+									NULL);
 
 	switch (gd->vendor) {
 	case OFONO_VENDOR_HUAWEI:
@@ -489,16 +758,33 @@ static void gprs_initialized(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_gprs_register(gprs);
 }
 
-static void at_cgreg_test_cb(gboolean ok, GAtResult *result,
+static void set_indreg(struct gprs_data *gd, const char *ind, gboolean present)
+{
+	if (g_str_equal(ind,"CGREG"))
+		gd->has_cgreg = present;
+
+	if (g_str_equal(ind,"CEREG"))
+		gd->has_cereg = present;
+
+	if (g_str_equal(ind,"C5GREG"))
+		gd->has_c5greg = present;
+
+}
+
+static void at_indreg_test_cb(gboolean ok, GAtResult *result,
 				gpointer user_data)
 {
-	struct ofono_gprs *gprs = user_data;
+	struct cb_data *cbd = user_data;
+	struct ofono_gprs *gprs = cbd->cb;
+	const char *ind=cbd->data;
+	const char *last=cbd->user;
+
 	struct gprs_data *gd = ofono_gprs_get_data(gprs);
 	gint range[2];
 	GAtResultIter iter;
 	int cgreg1 = 0;
 	int cgreg2 = 0;
-	const char *cmd;
+	char buf[32];
 
 	if (!ok)
 		goto error;
@@ -506,7 +792,8 @@ static void at_cgreg_test_cb(gboolean ok, GAtResult *result,
 	g_at_result_iter_init(&iter, result);
 
 retry:
-	if (!g_at_result_iter_next(&iter, "+CGREG:"))
+	sprintf(buf,"+%s:",ind);
+	if (!g_at_result_iter_next(&iter, buf))
 		goto error;
 
 	if (!g_at_result_iter_open_list(&iter))
@@ -521,45 +808,69 @@ retry:
 
 	g_at_result_iter_close_list(&iter);
 
-	if (cgreg2)
-		cmd = "AT+CGREG=2";
-	else if (cgreg1)
-		cmd = "AT+CGREG=1";
-	else
+	if (cgreg1) {
+		sprintf(buf,"AT+%s=1", ind);
+	} else if (cgreg2) {
+		sprintf(buf,"AT+%s=2", ind);
+	} else
 		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);
-
-	switch (gd->vendor) {
-	case OFONO_VENDOR_MBM:
-		/* Ericsson MBM and ST-E modems don't support AT+CGEREP=2,1 */
-		g_at_chat_send(gd->chat, "AT+CGEREP=1,0", none_prefix,
-			gprs_initialized, gprs, NULL);
-		break;
-	case OFONO_VENDOR_NOKIA:
-		/* Nokia data cards don't support AT+CGEREP=1,0 either */
-		g_at_chat_send(gd->chat, "AT+CGEREP=1", none_prefix,
-			gprs_initialized, gprs, NULL);
-		break;
-	default:
-		g_at_chat_send(gd->chat, "AT+CGEREP=2,1", none_prefix,
-			gprs_initialized, gprs, NULL);
-		break;
-	}
+	set_indreg(gd, ind,TRUE);
+	g_at_chat_send(gd->chat, buf, none_prefix, NULL, NULL, NULL);
 
+	if (last)
+		goto endcheck;
 	return;
 
 error:
-	ofono_info("GPRS not supported on this device");
-	ofono_gprs_remove(gprs);
+	set_indreg(gd, ind,FALSE);
+	if (!last)
+		return;
+
+endcheck:
+	if (gd->has_cgreg)
+		gd->nb_inds++;
+	if (gd->has_cereg)
+		gd->nb_inds++;
+	if (gd->has_c5greg)
+		gd->nb_inds++;
+
+	if (gd->nb_inds == 0) {
+		ofono_info("GPRS not supported on this device");
+		ofono_gprs_remove(gprs);
+		return;
+	}
+
+	gprs_initialized(gprs);
+}
+
+static void test_and_set_regstatus(struct ofono_gprs *gprs) {
+	struct gprs_data *gd = ofono_gprs_get_data(gprs);
+	struct cb_data *cbd_cg  = cb_data_new(gprs, "CGREG");
+	struct cb_data *cbd_ce  = cb_data_new(gprs, "CEREG");
+	struct cb_data *cbd_c5g = cb_data_new(gprs, "C5GREG");
+
+	cbd_c5g->user="last";
+
+	/*
+	 * modules can support one to all of the network registration indicators
+	 *
+	 * ofono will execute the next commands and related callbacks in order
+	 * therefore it is possible to verify all result on the last one.
+	 */
+
+	g_at_chat_send(gd->chat, "AT+CGREG=?", cgreg_prefix,
+					at_indreg_test_cb, cbd_cg, g_free);
+	g_at_chat_send(gd->chat, "AT+CEREG=?", cereg_prefix,
+					at_indreg_test_cb, cbd_ce, g_free);
+	g_at_chat_send(gd->chat, "AT+C5GREG=?", c5greg_prefix,
+					at_indreg_test_cb, cbd_c5g, g_free);
 }
 
 static void at_cgdcont_test_cb(gboolean ok, GAtResult *result,
 				gpointer user_data)
 {
 	struct ofono_gprs *gprs = user_data;
-	struct gprs_data *gd = ofono_gprs_get_data(gprs);
 	GAtResultIter iter;
 	int min, max;
 	const char *pdp_type;
@@ -600,10 +911,7 @@ static void at_cgdcont_test_cb(gboolean ok, GAtResult *result,
 		goto error;
 
 	ofono_gprs_set_cid_range(gprs, min, max);
-
-	g_at_chat_send(gd->chat, "AT+CGREG=?", cgreg_prefix,
-			at_cgreg_test_cb, gprs, NULL);
-
+	test_and_set_regstatus(gprs);
 	return;
 
 error:
-- 
2.17.1

Re: [PATCH] atmodem/gprs: added LTE and NR indicators

[Denis Kenzior]

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

Hi Giacinto,

On 09/23/2018 12:15 AM, Giacinto Cifelli wrote:
> According to the 27.007 CGREG (both as URC and command) is no longer
> sufficient to determine the registration state.

What does CGREG have to do with the overall registration state?  CGREG 
is only used for packet domain registrations in 2G/3G.  +CREG is still 
the definitive source of information, and in fact the registration state 
is generally ignored by src/gprs.c if technology is set to E-UTRAN.

> New indicators: CEREG for LTE, C5GREG for NR/5G are introduced.
> 
> In practice, legacy modems have CGREG, LTE-only modems have CEREG, and
> several others have both.
> With NR the picture will become even larger.
> 
> The handling of these indicators is not straighforward, because each
> will report the status in its own access technology. So we might have
> CGREG=4 but CEREG=1. Therefore they must be checked together.
> Particularly troublesome is the case of the URC. In case of hand-over,
> for example for a CSFB call, a common case is to have '+CEREG: 4'
> followed by '+CGREG: 1'. The first URC must be discarded, and this is
> done checking the technology reported by 'AT+COPS?' after the URC is

Err, why?  We should get a proper status from +CREG.  And the 
network-registration state overrides whatever status the gprs atom 
thinks it is in.

Also, the driver is the wrong place for all this logic anyway.  CEREG 
and C5REG status reporting is standardized, so we can easily add this to 
the oFono core (gprs) API.  E.g. ofono_gprs_status_notify was for 2G/3G 
PS domain registration state reporting (CGREG).  We should now introduce 
ofono_gprs_eutran_status_notify for reporting CEREG status and 
ofono_gprs_5g_status_notify for C5REG.  The core then can collate this 
information and act accordingly.  Having each driver doing all this 
logic is wasteful and error prone.

> received, in case the status is different from 1 or 5 (to be done:
> add the check for other roaming states for NR once the first modems are
> available). Note that according to the 27.007 CREG does not apply to PS
> networks, so it cannot be reliably used.

It does not apply to Packet Switched 2G/3G networks.  The registration 
to packet domain is separate in that case.  It still applies to EUTRAN:

"an unsolicited result code +CREG: <stat> when <n>=1 and there is a 
change in the MT’s circuit mode network registration status in 
GERAN/UTRAN/E-UTRAN, or unsolicited result code 
+CREG: <stat>[,[<lac>],[<ci>],[<AcT>]] when <n>=2 and there is a change 
of the network cell in GERAN/UTRAN/E-UTRAN."

> If COPS does not report a technology or reports the same technology,
> we take the indicator.

This sounds unnecessary to me...

> 
> Similar handling for AT+CGREG?, AT+CEREG?, AT+C5GREG? : we take the
> best result.
> 
> An initial verification for the presence of any of these indicators is
> required. Also how many of them are available is needed information.

So the real question is, why do you think you need all this anyway?  The 
GPRS registration status is only important for one case, and that is 
control of packet domain registration while roaming on 2G/3G networks. 
On EUTRAN networks that is not possible anyway, so we're essentially 
always Powered & Attached regardless of the RoamingAllowed setting.

> ---
>   drivers/atmodem/gprs.c | 420 +++++++++++++++++++++++++++++++++++------
>   1 file changed, 364 insertions(+), 56 deletions(-)
> 

Regards,
-Denis

Re: [PATCH] atmodem/gprs: added LTE and NR indicators

[Giacinto Cifelli]

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

Hi Denis,

On Thu, Sep 27, 2018 at 5:46 PM Denis Kenzior <denkenz@gmail.com> wrote:
>
> Hi Giacinto,
>
> On 09/23/2018 12:15 AM, Giacinto Cifelli wrote:
> > According to the 27.007 CGREG (both as URC and command) is no longer
> > sufficient to determine the registration state.
>
> What does CGREG have to do with the overall registration state?  CGREG
> is only used for packet domain registrations in 2G/3G.  +CREG is still
> the definitive source of information, and in fact the registration state
> is generally ignored by src/gprs.c if technology is set to E-UTRAN.

Apologies, wrong terminology, I meant attach.

>
> > New indicators: CEREG for LTE, C5GREG for NR/5G are introduced.
> >
> > In practice, legacy modems have CGREG, LTE-only modems have CEREG, and
> > several others have both.
> > With NR the picture will become even larger.
> >
> > The handling of these indicators is not straighforward, because each
> > will report the status in its own access technology. So we might have
> > CGREG=4 but CEREG=1. Therefore they must be checked together.
> > Particularly troublesome is the case of the URC. In case of hand-over,
> > for example for a CSFB call, a common case is to have '+CEREG: 4'
> > followed by '+CGREG: 1'. The first URC must be discarded, and this is
> > done checking the technology reported by 'AT+COPS?' after the URC is
>
> Err, why?  We should get a proper status from +CREG.  And the
> network-registration state overrides whatever status the gprs atom
> thinks it is in.

yes and no. In case of handover things get messy today.
I will try to collect a log to show this.

>
> Also, the driver is the wrong place for all this logic anyway.  CEREG
> and C5REG status reporting is standardized, so we can easily add this to
> the oFono core (gprs) API.  E.g. ofono_gprs_status_notify was for 2G/3G
> PS domain registration state reporting (CGREG).  We should now introduce
> ofono_gprs_eutran_status_notify for reporting CEREG status and
> ofono_gprs_5g_status_notify for C5REG.  The core then can collate this
> information and act accordingly.  Having each driver doing all this
> logic is wasteful and error prone.

this is specific to AT-based modem. qmi and mbim do not suffer from this.
I don't know about dun, ril, isi drivers (the other ones that
implement a gprs atom)
So, in my opinion is related to the 27.007, and stays in the driver.

>
> > received, in case the status is different from 1 or 5 (to be done:
> > add the check for other roaming states for NR once the first modems are
> > available). Note that according to the 27.007 CREG does not apply to PS
> > networks, so it cannot be reliably used.
>
> It does not apply to Packet Switched 2G/3G networks.  The registration
> to packet domain is separate in that case.  It still applies to EUTRAN:

here I am wrong.
I have re-checked the 27.007 and CREG applies for all technologies.

>
> "an unsolicited result code +CREG: <stat> when <n>=1 and there is a
> change in the MT’s circuit mode network registration status in
> GERAN/UTRAN/E-UTRAN, or unsolicited result code
> +CREG: <stat>[,[<lac>],[<ci>],[<AcT>]] when <n>=2 and there is a change
> of the network cell in GERAN/UTRAN/E-UTRAN."
>
> > If COPS does not report a technology or reports the same technology,
> > we take the indicator.
>
> This sounds unnecessary to me...

the problem here is that when there are different indicators for 2G/3G and LTE,
in case of change of technology one of them reports a deregistration
(from the previous technology), and promptly ofono sends AT+CGATT=0.

>
> >
> > Similar handling for AT+CGREG?, AT+CEREG?, AT+C5GREG? : we take the
> > best result.
> >
> > An initial verification for the presence of any of these indicators is
> > required. Also how many of them are available is needed information.
>
> So the real question is, why do you think you need all this anyway?  The
> GPRS registration status is only important for one case, and that is
> control of packet domain registration while roaming on 2G/3G networks.
> On EUTRAN networks that is not possible anyway, so we're essentially
> always Powered & Attached regardless of the RoamingAllowed setting.
>

maybe this is the issue: roaming should be handled at context-activation stage,
not with the attach.
All hand-over cases don't work well with ofono.
Even a simple registration in 3G first then upgraded to LTE by the
network will not work because
the module would send +CGREG=4 and ofono AT+CGATT=0, and detach from LTE,
back to 3G, perform another attach and repeat forever...

LTE modules are supposed to react to AT+CGATT=x, just like any pre-LTE ones.
I haven't seen anywhere in the 27.007 that CGATT doesn't apply there.

I understand from your comments that the aim of ofono is to handle
first the registration with CREG,
then the attach.
I see if it is possible to come with some clean scenarios for this in
case of multiple technologies,
but I fear not without this kind of code.
Note that it will be the same for LTE->NR as it is now for 2G/3G->LTE.

> > ---
> >   drivers/atmodem/gprs.c | 420 +++++++++++++++++++++++++++++++++++------
> >   1 file changed, 364 insertions(+), 56 deletions(-)
> >
>
> Regards,
> -Denis

Regards,
Giacinto

Re: [PATCH] atmodem/gprs: added LTE and NR indicators

[Denis Kenzior]

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

Hi Giacinto,

>> Err, why?  We should get a proper status from +CREG.  And the
>> network-registration state overrides whatever status the gprs atom
>> thinks it is in.
> 
> yes and no. In case of handover things get messy today.
> I will try to collect a log to show this.
> 

Great, that would be useful.

>>
>> Also, the driver is the wrong place for all this logic anyway.  CEREG
>> and C5REG status reporting is standardized, so we can easily add this to
>> the oFono core (gprs) API.  E.g. ofono_gprs_status_notify was for 2G/3G
>> PS domain registration state reporting (CGREG).  We should now introduce
>> ofono_gprs_eutran_status_notify for reporting CEREG status and
>> ofono_gprs_5g_status_notify for C5REG.  The core then can collate this
>> information and act accordingly.  Having each driver doing all this
>> logic is wasteful and error prone.
> 
> this is specific to AT-based modem. qmi and mbim do not suffer from this.
> I don't know about dun, ril, isi drivers (the other ones that
> implement a gprs atom)
> So, in my opinion is related to the 27.007, and stays in the driver.
> 

So just as a bit of background, oFono follows 27.007 to the letter when 
possible.  If you look at the way atom driver APIs are defined, they map 
1:1 to 27.007.  We only introduce enums and operations that are not 
present in 27.007 for very specific circumstances.

In the end QMI was also very similar to 27.007.  I don't know if they 
have kept up with some of the latest features introduced by 3GPP, but 
all these status reporting operations can remain optional.  If QMI/MBIM 
can emulate them, that's great.  If not, things should still work.

Also note that gprs driver might not be shared across all AT command 
based modems in the future.  E.g. you may want to have your own version 
for gemalto if the vendor specific logic gets too invasive.  In which 
case you'd need to duplicate all this logic...  No, it really does 
belong in the core.

>>> If COPS does not report a technology or reports the same technology,
>>> we take the indicator.
>>
>> This sounds unnecessary to me...
> 
> the problem here is that when there are different indicators for 2G/3G and LTE,
> in case of change of technology one of them reports a deregistration
> (from the previous technology), and promptly ofono sends AT+CGATT=0.
> 

Unsurprising.  This is a common issue with AT commands.  The issue is 
that AT command order matters.  So if the modem sends them in a sequence 
that isn't useful to the upper layers, things become hard.  Each command 
is processed individually without actual context as to what is going on 
elsewhere.  This is why I'm actually against using C5REG/EREG except for 
informational purposes only.  The definitive information needs to come 
from CREG.

>>
>>>
>>> Similar handling for AT+CGREG?, AT+CEREG?, AT+C5GREG? : we take the
>>> best result.
>>>
>>> An initial verification for the presence of any of these indicators is
>>> required. Also how many of them are available is needed information.
>>
>> So the real question is, why do you think you need all this anyway?  The
>> GPRS registration status is only important for one case, and that is
>> control of packet domain registration while roaming on 2G/3G networks.
>> On EUTRAN networks that is not possible anyway, so we're essentially
>> always Powered & Attached regardless of the RoamingAllowed setting.
>>
> 
> maybe this is the issue: roaming should be handled at context-activation stage,
> not with the attach.
> All hand-over cases don't work well with ofono.
> Even a simple registration in 3G first then upgraded to LTE by the
> network will not work because
> the module would send +CGREG=4 and ofono AT+CGATT=0, and detach from LTE,
> back to 3G, perform another attach and repeat forever...

Yep, I agree, the logic is overly aggressive.  The reason is that 
there's no 27.007 standard for disabling automatic PS attach while 
roaming.  So in order to implement RoamingAllowed we had to issue the 
big CGATT=0 hammer to stop the modem from attaching until we told it to 
do so.

Do we have any vendor specific commands that we can use to send a hint 
to the modem not to auto-attach while roaming?  Then gprs can stop being 
so paranoid.  Another option would be to only detach once we're roaming 
and attached...

> 
> LTE modules are supposed to react to AT+CGATT=x, just like any pre-LTE ones.
> I haven't seen anywhere in the 27.007 that CGATT doesn't apply there.
> 

Agreed, that is my interpretation of what 27.007 says as well.  However, 
I think 3GPP messed this up...

> I understand from your comments that the aim of ofono is to handle
> first the registration with CREG,
> then the attach.
> I see if it is possible to come with some clean scenarios for this in
> case of multiple technologies,
> but I fear not without this kind of code.
> Note that it will be the same for LTE->NR as it is now for 2G/3G->LTE.

Lets fix the LTE cases first and then we worry about NR :)

Regards,
-Denis