Re: [PATCH v2 6/6] cdmamodem: Add serving system identifier support

Re: [PATCH v2 6/6] cdmamodem: Add serving system identifier support

[Denis Kenzior]

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

Hi Philippe,

On 11/17/2011 10:21 AM, Philippe Nunes wrote:
> ---
>  drivers/cdmamodem/network-registration.c |   78 ++++++++++++++++++++++++++++++
>  include/cdma-netreg.h                    |    7 +++
>  2 files changed, 85 insertions(+), 0 deletions(-)
> 

So the obvious comment is that this patch should be split into two, one
for adding new driver API and one for the driver changes.

I would also like to see how you're planning to use this information in
the core, before reviewing this patch in detail.  Do note that after
taking a quick peek at Huawei CDMA manuals and ModemManager, I very much
doubt that +CSS is what you want to use in the first place.

Regards,
-Denis

Re: [PATCH v2 1/6] mbpi: Add mbpi_lookup_cdma_provider_name API

[Denis Kenzior]

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

Hi Philippe,

On 11/17/2011 10:20 AM, Philippe Nunes wrote:
> ---
>  plugins/mbpi.c |  146 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  plugins/mbpi.h |    2 +
>  2 files changed, 148 insertions(+), 0 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH v2 2/6] tools: Add utility for looking CDMA network name from database

[Denis Kenzior]

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

Hi Philippe,

On 11/17/2011 10:21 AM, Philippe Nunes wrote:
> ---
>  Makefile.am                  |    7 +++-
>  tools/lookup-provider-name.c |  100 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 106 insertions(+), 1 deletions(-)
>  create mode 100644 tools/lookup-provider-name.c
> 

Patch has been applied, thanks.

Regards,
-Denis

[PATCH v2 0/6] Add a parser to retrieve the CDMA network name

[Philippe Nunes]

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

- Fix according Denis review (remove unnecessary checking, fix coding style issues).
- Add the serving system query based on the standard CDMA AT command AT+CSS?.
- Merge the huaweicdmamodem driver with cdmamodem driver.



Philippe Nunes (6):
  mbpi: Add mbpi_lookup_cdma_provider_name API
  tools: Add utility for looking CDMA network name from database
  Huaweicdmamodem: remove this specific driver
  huaweicdmamodem: Merge this driver with cdmamodem driver
  cdmamodem: Add CDMA network-registration support
  cdmamodem: Add serving system identifier support

 Makefile.am                                    |   19 +-
 drivers/cdmamodem/cdmamodem.c                  |    2 +
 drivers/cdmamodem/cdmamodem.h                  |    2 +
 drivers/cdmamodem/network-registration.c       |  279 ++++++++++++++++++++++++
 drivers/huaweicdmamodem/huaweicdmamodem.c      |   45 ----
 drivers/huaweicdmamodem/huaweicdmamodem.h      |   23 --
 drivers/huaweicdmamodem/network-registration.c |  199 -----------------
 include/cdma-netreg.h                          |    7 +
 plugins/huaweicdma.c                           |    5 +-
 plugins/mbpi.c                                 |  146 ++++++++++++
 plugins/mbpi.h                                 |    2 +
 tools/lookup-provider-name.c                   |  100 +++++++++
 12 files changed, 552 insertions(+), 277 deletions(-)
 create mode 100644 drivers/cdmamodem/network-registration.c
 delete mode 100644 drivers/huaweicdmamodem/huaweicdmamodem.c
 delete mode 100644 drivers/huaweicdmamodem/huaweicdmamodem.h
 delete mode 100644 drivers/huaweicdmamodem/network-registration.c
 create mode 100644 tools/lookup-provider-name.c

[PATCH v2 1/6] mbpi: Add mbpi_lookup_cdma_provider_name API

[Philippe Nunes]

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

---
 plugins/mbpi.c |  146 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 plugins/mbpi.h |    2 +
 2 files changed, 148 insertions(+), 0 deletions(-)

diff --git a/plugins/mbpi.c b/plugins/mbpi.c
index 1e41ecb..415633f 100644
--- a/plugins/mbpi.c
+++ b/plugins/mbpi.c
@@ -58,6 +58,12 @@ struct gsm_data {
 	gboolean allow_duplicates;
 };
 
+struct cdma_data {
+	const char *match_sid;
+	char *provider_name;
+	gboolean match_found;
+};
+
 const char *mbpi_ap_type(enum ofono_gprs_context_type type)
 {
 	switch (type) {
@@ -281,6 +287,32 @@ static void apn_handler(GMarkupParseContext *context, struct gsm_data *gsm,
 	g_markup_parse_context_push(context, &apn_parser, ap);
 }
 
+static void sid_handler(GMarkupParseContext *context,
+				struct cdma_data *cdma,
+				const gchar **attribute_names,
+				const gchar **attribute_values,
+				GError **error)
+{
+	const char *sid = NULL;
+	int i;
+
+	for (i = 0; attribute_names[i]; i++)
+		if (g_str_equal(attribute_names[i], "value") == TRUE) {
+			sid = attribute_values[i];
+			break;
+		}
+
+	if (sid == NULL) {
+		mbpi_g_set_error(context, error, G_MARKUP_ERROR,
+					G_MARKUP_ERROR_MISSING_ATTRIBUTE,
+					"Missing attribute: sid");
+		return;
+	}
+
+	if (g_str_equal(sid, cdma->match_sid))
+		cdma->match_found = TRUE;
+}
+
 static void gsm_start(GMarkupParseContext *context, const gchar *element_name,
 			const gchar **attribute_names,
 			const gchar **attribute_values,
@@ -347,6 +379,71 @@ static const GMarkupParser gsm_parser = {
 	NULL,
 };
 
+static void cdma_start(GMarkupParseContext *context, const gchar *element_name,
+			const gchar **attribute_names,
+			const gchar **attribute_values,
+			gpointer userdata, GError **error)
+{
+	if (g_str_equal(element_name, "sid")) {
+		struct cdma_data *cdma = userdata;
+		/*
+		 * For entries with multiple sid elements, don't bother
+		 * searching if we already have a match
+		 */
+		if (cdma->match_found == TRUE)
+			return;
+
+		sid_handler(context, cdma, attribute_names, attribute_values,
+				error);
+	}
+}
+
+static const GMarkupParser cdma_parser = {
+	cdma_start,
+	NULL,
+	NULL,
+	NULL,
+	NULL,
+};
+
+static void provider_start(GMarkupParseContext *context,
+				const gchar *element_name,
+				const gchar **attribute_names,
+				const gchar **attribute_values,
+				gpointer userdata, GError **error)
+{
+	if (g_str_equal(element_name, "name")) {
+		struct cdma_data *cdma = userdata;
+
+		g_free(cdma->provider_name);
+		cdma->provider_name = NULL;
+		g_markup_parse_context_push(context, &text_parser,
+						&cdma->provider_name);
+	} else if (g_str_equal(element_name, "gsm"))
+		g_markup_parse_context_push(context, &skip_parser, NULL);
+	else if (g_str_equal(element_name, "cdma"))
+		g_markup_parse_context_push(context, &cdma_parser, userdata);
+}
+
+static void provider_end(GMarkupParseContext *context,
+					const gchar *element_name,
+					gpointer userdata, GError **error)
+{
+	if (g_str_equal(element_name, "name") ||
+				g_str_equal(element_name, "gsm") ||
+				g_str_equal(element_name, "cdma"))
+		g_markup_parse_context_pop(context);
+
+}
+
+static const GMarkupParser provider_parser = {
+	provider_start,
+	provider_end,
+	NULL,
+	NULL,
+	NULL,
+};
+
 static void toplevel_gsm_start(GMarkupParseContext *context,
 					const gchar *element_name,
 					const gchar **atribute_names,
@@ -379,6 +476,40 @@ static const GMarkupParser toplevel_gsm_parser = {
 	NULL,
 };
 
+static void toplevel_cdma_start(GMarkupParseContext *context,
+					const gchar *element_name,
+					const gchar **atribute_names,
+					const gchar **attribute_values,
+					gpointer userdata, GError **error)
+{
+	struct cdma_data *cdma = userdata;
+
+	if (g_str_equal(element_name, "provider")) {
+		if (cdma->match_found == TRUE)
+			g_markup_parse_context_push(context, &skip_parser,
+								NULL);
+		else
+			g_markup_parse_context_push(context, &provider_parser,
+								userdata);
+	}
+}
+
+static void toplevel_cdma_end(GMarkupParseContext *context,
+					const gchar *element_name,
+					gpointer userdata, GError **error)
+{
+	if (g_str_equal(element_name, "provider"))
+		g_markup_parse_context_pop(context);
+}
+
+static const GMarkupParser toplevel_cdma_parser = {
+	toplevel_cdma_start,
+	toplevel_cdma_end,
+	NULL,
+	NULL,
+	NULL,
+};
+
 static gboolean mbpi_parse(const GMarkupParser *parser, gpointer userdata,
 				GError **error)
 {
@@ -453,3 +584,18 @@ GSList *mbpi_lookup_apn(const char *mcc, const char *mnc,
 
 	return gsm.apns;
 }
+
+char *mbpi_lookup_cdma_provider_name(const char *sid, GError **error)
+{
+	struct cdma_data cdma;
+
+	memset(&cdma, 0, sizeof(cdma));
+	cdma.match_sid = sid;
+
+	if (mbpi_parse(&toplevel_cdma_parser, &cdma, error) == FALSE) {
+		g_free(cdma.provider_name);
+		cdma.provider_name = NULL;
+	}
+
+	return cdma.provider_name;
+}
diff --git a/plugins/mbpi.h b/plugins/mbpi.h
index 6d34358..64b7ea5 100644
--- a/plugins/mbpi.h
+++ b/plugins/mbpi.h
@@ -25,3 +25,5 @@ void mbpi_ap_free(struct ofono_gprs_provision_data *data);
 
 GSList *mbpi_lookup_apn(const char *mcc, const char *mnc,
 			gboolean allow_duplicates, GError **error);
+
+char *mbpi_lookup_cdma_provider_name(const char *sid, GError **error);
-- 
1.7.1

[PATCH v2 2/6] tools: Add utility for looking CDMA network name from database

[Philippe Nunes]

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

---
 Makefile.am                  |    7 +++-
 tools/lookup-provider-name.c |  100 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+), 1 deletions(-)
 create mode 100644 tools/lookup-provider-name.c

diff --git a/Makefile.am b/Makefile.am
index a28f790..337aeb7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -627,7 +627,8 @@ TESTS = $(unit_tests)
 
 if TOOLS
 noinst_PROGRAMS += tools/huawei-audio tools/auto-enable \
-			tools/get-location tools/lookup-apn
+			tools/get-location tools/lookup-apn \
+			tools/lookup-provider-name
 
 tools_huawei_audio_SOURCES = $(gdbus_sources) tools/huawei-audio.c
 tools_huawei_audio_LDADD = @GLIB_LIBS@ @DBUS_LIBS@
@@ -640,6 +641,10 @@ tools_get_location_LDADD = @GLIB_LIBS@ @DBUS_LIBS@
 
 tools_lookup_apn_SOURCES = plugins/mbpi.c plugins/mbpi.h tools/lookup-apn.c
 tools_lookup_apn_LDADD = @GLIB_LIBS@
+
+tools_lookup_provider_name_SOURCES = plugins/mbpi.c plugins/mbpi.h \
+				tools/lookup-provider-name.c
+tools_lookup_provider_name_LDADD = @GLIB_LIBS@
 endif
 
 noinst_PROGRAMS += gatchat/gsmdial gatchat/test-server gatchat/test-qcdm
diff --git a/tools/lookup-provider-name.c b/tools/lookup-provider-name.c
new file mode 100644
index 0000000..596b6c1
--- /dev/null
+++ b/tools/lookup-provider-name.c
@@ -0,0 +1,100 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2011  Intel Corporation. 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 <stdlib.h>
+
+#include <glib.h>
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include <ofono/modem.h>
+#include <ofono/gprs-provision.h>
+
+#include "plugins/mbpi.h"
+
+static void lookup_cdma_provider_name(const char *match_sid)
+{
+	GError *error = NULL;
+	char *name;
+
+	g_print("Searching for serving network name with SID: %s\n", match_sid);
+
+	name = mbpi_lookup_cdma_provider_name(match_sid, &error);
+
+	if (name == NULL) {
+		if (error != NULL) {
+			g_printerr("Lookup failed: %s\n", error->message);
+			g_error_free(error);
+		} else
+			g_printerr("Not found\n");
+
+		return;
+	}
+
+	g_print("CDMA provider name: %s\n", name);
+
+	g_free(name);
+}
+
+static gboolean option_version = FALSE;
+
+static GOptionEntry options[] = {
+	{ "version", 'v', 0, G_OPTION_ARG_NONE, &option_version,
+				"Show version information and exit" },
+	{ NULL },
+};
+
+int main(int argc, char **argv)
+{
+	GOptionContext *context;
+	GError *error = NULL;
+
+	context = g_option_context_new(NULL);
+	g_option_context_add_main_entries(context, options, NULL);
+
+	if (g_option_context_parse(context, &argc, &argv, &error) == FALSE) {
+		if (error != NULL) {
+			g_printerr("%s\n", error->message);
+			g_error_free(error);
+		} else
+			g_printerr("An unknown error occurred\n");
+		exit(1);
+	}
+
+	g_option_context_free(context);
+
+	if (option_version == TRUE) {
+		g_print("%s\n", VERSION);
+		exit(0);
+	}
+
+	if (argc < 1) {
+		g_printerr("Missing parameters\n");
+		exit(1);
+	}
+
+	lookup_cdma_provider_name(argv[1]);
+
+	return 0;
+}
-- 
1.7.1

[PATCH v2 3/6] Huaweicdmamodem: remove this specific driver

[Philippe Nunes]

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

---
 Makefile.am |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 337aeb7..4c3ca84 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -274,12 +274,6 @@ builtin_sources += drivers/cdmamodem/cdmamodem.h \
 			drivers/cdmamodem/voicecall.c \
 			drivers/cdmamodem/devinfo.c \
 			drivers/cdmamodem/connman.c
-
-builtin_modules += huaweicdmamodem
-builtin_sources += drivers/huaweicdmamodem/huaweicdmamodem.h \
-			drivers/huaweicdmamodem/huaweicdmamodem.c \
-			drivers/huaweicdmamodem/network-registration.c
-
 endif
 
 builtin_modules += g1
-- 
1.7.1

[PATCH v2 4/6] huaweicdmamodem: Merge this driver with cdmamodem driver

[Philippe Nunes]

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

---
 drivers/huaweicdmamodem/huaweicdmamodem.c      |   45 ------
 drivers/huaweicdmamodem/huaweicdmamodem.h      |   23 ---
 drivers/huaweicdmamodem/network-registration.c |  199 ------------------------
 3 files changed, 0 insertions(+), 267 deletions(-)
 delete mode 100644 drivers/huaweicdmamodem/huaweicdmamodem.c
 delete mode 100644 drivers/huaweicdmamodem/huaweicdmamodem.h
 delete mode 100644 drivers/huaweicdmamodem/network-registration.c

diff --git a/drivers/huaweicdmamodem/huaweicdmamodem.c b/drivers/huaweicdmamodem/huaweicdmamodem.c
deleted file mode 100644
index 4e9c400..0000000
--- a/drivers/huaweicdmamodem/huaweicdmamodem.c
+++ /dev/null
@@ -1,45 +0,0 @@
-/*
- *
- *  oFono - Open Source Telephony
- *
- *  Copyright (C) 2008-2011  Intel Corporation. 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
-
-#define OFONO_API_SUBJECT_TO_CHANGE
-#include <ofono/plugin.h>
-
-#include "huaweicdmamodem.h"
-
-static int huaweicdmamodem_init(void)
-{
-	huaweicdma_netreg_init();
-
-	return 0;
-}
-
-static void huaweicdmamodem_exit(void)
-{
-	huaweicdma_netreg_exit();
-}
-
-OFONO_PLUGIN_DEFINE(huaweicdmamodem, "Huawei CDMA modem driver", VERSION,
-			OFONO_PLUGIN_PRIORITY_DEFAULT,
-			huaweicdmamodem_init, huaweicdmamodem_exit)
diff --git a/drivers/huaweicdmamodem/huaweicdmamodem.h b/drivers/huaweicdmamodem/huaweicdmamodem.h
deleted file mode 100644
index 799d3a8..0000000
--- a/drivers/huaweicdmamodem/huaweicdmamodem.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/*
- *
- *  oFono - Open Source Telephony
- *
- *  Copyright (C) 2008-2011  Intel Corporation. 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
- *
- */
-
-extern void huaweicdma_netreg_init(void);
-extern void huaweicdma_netreg_exit(void);
diff --git a/drivers/huaweicdmamodem/network-registration.c b/drivers/huaweicdmamodem/network-registration.c
deleted file mode 100644
index 25c6291..0000000
--- a/drivers/huaweicdmamodem/network-registration.c
+++ /dev/null
@@ -1,199 +0,0 @@
-/*
- *
- *  oFono - Open Source Telephony
- *
- *  Copyright (C) 2008-2011  Intel Corporation. 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
-
-#define _GNU_SOURCE
-#include <glib.h>
-#include <errno.h>
-
-#include <ofono/log.h>
-#include <ofono/modem.h>
-#include <ofono/cdma-netreg.h>
-
-#include "gatchat.h"
-
-#include "huaweicdmamodem.h"
-
-static const char *sysinfo_prefix[] = { "^SYSINFO:", NULL };
-
-static gboolean parse_sysinfo(GAtResult *result, gint *status)
-{
-	GAtResultIter iter;
-	gint srv_status;
-	gint srv_domain;
-	gint roaming_status;
-
-	g_at_result_iter_init(&iter, result);
-
-	if (!g_at_result_iter_next(&iter, "^SYSINFO:"))
-		return FALSE;
-
-	if (!g_at_result_iter_next_number(&iter, &srv_status))
-		return FALSE;
-
-	if (!g_at_result_iter_next_number(&iter, &srv_domain))
-		return FALSE;
-
-	if (!g_at_result_iter_next_number(&iter, &roaming_status))
-		return FALSE;
-
-	DBG("%d, %d, %d", srv_status, srv_domain, roaming_status);
-
-	switch (srv_status) {
-	case 1:	/* Restricted service */
-	case 2:	/* Service valid */
-	case 3:	/* Restricted region service */
-		if (roaming_status)
-			*status = CDMA_NETWORK_REGISTRATION_STATUS_ROAMING;
-		else
-			*status = CDMA_NETWORK_REGISTRATION_STATUS_REGISTERED;
-		break;
-	case 0:	/* No service */
-	case 4:	/* Not registered */
-	default:
-		*status = CDMA_NETWORK_REGISTRATION_STATUS_NOT_REGISTERED;
-		break;
-	}
-
-	switch (srv_domain) {
-	case 0: /* No service */
-	case 255: /* CDMA not supported */
-		*status = CDMA_NETWORK_REGISTRATION_STATUS_NOT_REGISTERED;
-		break;
-	case 1: /* Only CS */
-	case 2: /* Only PS */
-	case 3: /* CS  PS */
-	case 4: /* CS registered, PS in searching state */
-		break;
-	}
-
-	return TRUE;
-}
-
-static void sysinfo_cb(gboolean ok, GAtResult *result, gpointer user_data)
-{
-	struct ofono_cdma_netreg *netreg = user_data;
-	int status;
-
-	if (!ok)
-		return;
-
-	if (parse_sysinfo(result, &status) == FALSE) {
-		ofono_error("Invalid SYSINFO values");
-		return;
-	}
-
-	ofono_cdma_netreg_status_notify(netreg, status);
-}
-
-static void mode_notify(GAtResult *result, gpointer user_data)
-{
-	struct ofono_cdma_netreg *netreg = user_data;
-	GAtChat *chat = ofono_cdma_netreg_get_data(netreg);
-
-	g_at_chat_send(chat, "AT^SYSINFO", sysinfo_prefix,
-				sysinfo_cb, netreg, NULL);
-}
-
-static void rssilvl_notify(GAtResult *result, gpointer user_data)
-{
-	struct ofono_cdma_netreg *netreg = user_data;
-	int strength;
-	GAtResultIter iter;
-
-	g_at_result_iter_init(&iter, result);
-
-	if (!g_at_result_iter_next(&iter, "^RSSILVL:"))
-		goto error;
-
-	if (!g_at_result_iter_next_number(&iter, &strength))
-		goto error;
-
-	if (strength == 99)
-		strength = 100;
-
-	ofono_cdma_netreg_strength_notify(netreg, strength);
-
-	return;
-
-error:
-	ofono_error("Invalid RSSILVL value");
-}
-
-static void probe_cb(gboolean ok, GAtResult *result, gpointer user_data)
-{
-	struct ofono_cdma_netreg *netreg = user_data;
-	GAtChat *chat = ofono_cdma_netreg_get_data(netreg);
-
-	if (!ok) {
-		ofono_cdma_netreg_remove(netreg);
-		return;
-	}
-
-	g_at_chat_register(chat, "^MODE:",
-				mode_notify, FALSE, netreg, NULL);
-
-	g_at_chat_register(chat, "^RSSILVL:",
-				rssilvl_notify, FALSE, netreg, NULL);
-
-	ofono_cdma_netreg_register(netreg);
-}
-
-static int huaweicdma_netreg_probe(struct ofono_cdma_netreg *netreg,
-				unsigned int vendor, void *data)
-{
-	GAtChat *chat = g_at_chat_clone(data);
-
-	ofono_cdma_netreg_set_data(netreg, chat);
-
-	g_at_chat_send(chat, "AT^SYSINFO", sysinfo_prefix,
-				probe_cb, netreg, NULL);
-
-	return 0;
-}
-
-static void huaweicdma_netreg_remove(struct ofono_cdma_netreg *netreg)
-{
-	GAtChat *chat = ofono_cdma_netreg_get_data(netreg);
-
-	ofono_cdma_netreg_set_data(netreg, NULL);
-
-	g_at_chat_unref(chat);
-}
-
-static struct ofono_cdma_netreg_driver driver = {
-	.name	= "huaweicdmamodem",
-	.probe	= huaweicdma_netreg_probe,
-	.remove	= huaweicdma_netreg_remove,
-};
-
-void huaweicdma_netreg_init(void)
-{
-	ofono_cdma_netreg_driver_register(&driver);
-}
-
-void huaweicdma_netreg_exit(void)
-{
-	ofono_cdma_netreg_driver_unregister(&driver);
-}
-- 
1.7.1

[PATCH v2 5/6] cdmamodem: Add CDMA network-registration support

[Philippe Nunes]

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

---
 Makefile.am                              |    6 +-
 drivers/cdmamodem/cdmamodem.c            |    2 +
 drivers/cdmamodem/cdmamodem.h            |    2 +
 drivers/cdmamodem/network-registration.c |  201 ++++++++++++++++++++++++++++++
 plugins/huaweicdma.c                     |    5 +-
 5 files changed, 213 insertions(+), 3 deletions(-)
 create mode 100644 drivers/cdmamodem/network-registration.c

diff --git a/Makefile.am b/Makefile.am
index 4c3ca84..6002eb0 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -269,11 +269,13 @@ endif
 
 if CDMAMODEM
 builtin_modules += cdmamodem
-builtin_sources += drivers/cdmamodem/cdmamodem.h \
+builtin_sources += drivers/atmodem/atutil.h \
+			drivers/cdmamodem/cdmamodem.h \
 			drivers/cdmamodem/cdmamodem.c \
 			drivers/cdmamodem/voicecall.c \
 			drivers/cdmamodem/devinfo.c \
-			drivers/cdmamodem/connman.c
+			drivers/cdmamodem/connman.c \
+			drivers/cdmamodem/network-registration.c
 endif
 
 builtin_modules += g1
diff --git a/drivers/cdmamodem/cdmamodem.c b/drivers/cdmamodem/cdmamodem.c
index 50908e3..1548102 100644
--- a/drivers/cdmamodem/cdmamodem.c
+++ b/drivers/cdmamodem/cdmamodem.c
@@ -37,6 +37,7 @@ static int cdmamodem_init(void)
 	cdma_voicecall_init();
 	cdma_devinfo_init();
 	cdma_connman_init();
+	cdma_netreg_init();
 
 	return 0;
 }
@@ -46,6 +47,7 @@ static void cdmamodem_exit(void)
 	cdma_voicecall_exit();
 	cdma_devinfo_exit();
 	cdma_connman_exit();
+	cdma_netreg_exit();
 }
 
 OFONO_PLUGIN_DEFINE(cdmamodem, "CDMA AT modem driver", VERSION,
diff --git a/drivers/cdmamodem/cdmamodem.h b/drivers/cdmamodem/cdmamodem.h
index 3554705..d496214 100644
--- a/drivers/cdmamodem/cdmamodem.h
+++ b/drivers/cdmamodem/cdmamodem.h
@@ -27,3 +27,5 @@ extern void cdma_devinfo_init(void);
 extern void cdma_devinfo_exit(void);
 extern void cdma_connman_init(void);
 extern void cdma_connman_exit(void);
+extern void cdma_netreg_init(void);
+extern void cdma_netreg_exit(void);
diff --git a/drivers/cdmamodem/network-registration.c b/drivers/cdmamodem/network-registration.c
new file mode 100644
index 0000000..a34db91
--- /dev/null
+++ b/drivers/cdmamodem/network-registration.c
@@ -0,0 +1,201 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2011  Intel Corporation. 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
+
+#define _GNU_SOURCE
+#include <glib.h>
+#include <errno.h>
+
+#include <ofono/log.h>
+#include <ofono/modem.h>
+#include <ofono/cdma-netreg.h>
+
+#include "gatchat.h"
+
+#include "cdmamodem.h"
+#include <drivers/atmodem/vendor.h>
+
+static const char *sysinfo_prefix[] = { "^SYSINFO:", NULL };
+
+static gboolean parse_sysinfo(GAtResult *result, gint *status)
+{
+	GAtResultIter iter;
+	gint srv_status;
+	gint srv_domain;
+	gint roaming_status;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "^SYSINFO:"))
+		return FALSE;
+
+	if (!g_at_result_iter_next_number(&iter, &srv_status))
+		return FALSE;
+
+	if (!g_at_result_iter_next_number(&iter, &srv_domain))
+		return FALSE;
+
+	if (!g_at_result_iter_next_number(&iter, &roaming_status))
+		return FALSE;
+
+	DBG("%d, %d, %d", srv_status, srv_domain, roaming_status);
+
+	switch (srv_status) {
+	case 1:	/* Restricted service */
+	case 2:	/* Service valid */
+	case 3:	/* Restricted region service */
+		if (roaming_status)
+			*status = CDMA_NETWORK_REGISTRATION_STATUS_ROAMING;
+		else
+			*status = CDMA_NETWORK_REGISTRATION_STATUS_REGISTERED;
+		break;
+	case 0:	/* No service */
+	case 4:	/* Not registered */
+	default:
+		*status = CDMA_NETWORK_REGISTRATION_STATUS_NOT_REGISTERED;
+		break;
+	}
+
+	switch (srv_domain) {
+	case 0: /* No service */
+	case 255: /* CDMA not supported */
+		*status = CDMA_NETWORK_REGISTRATION_STATUS_NOT_REGISTERED;
+		break;
+	case 1: /* Only CS */
+	case 2: /* Only PS */
+	case 3: /* CS  PS */
+	case 4: /* CS registered, PS in searching state */
+		break;
+	}
+
+	return TRUE;
+}
+
+static void sysinfo_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_cdma_netreg *netreg = user_data;
+	int status;
+
+	if (!ok)
+		return;
+
+	if (parse_sysinfo(result, &status) == FALSE) {
+		ofono_error("Invalid SYSINFO values");
+		return;
+	}
+
+	ofono_cdma_netreg_status_notify(netreg, status);
+}
+
+static void mode_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_cdma_netreg *netreg = user_data;
+	GAtChat *chat = ofono_cdma_netreg_get_data(netreg);
+
+	g_at_chat_send(chat, "AT^SYSINFO", sysinfo_prefix,
+				sysinfo_cb, netreg, NULL);
+}
+
+static void rssilvl_notify(GAtResult *result, gpointer user_data)
+{
+	struct ofono_cdma_netreg *netreg = user_data;
+	int strength;
+	GAtResultIter iter;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "^RSSILVL:"))
+		goto error;
+
+	if (!g_at_result_iter_next_number(&iter, &strength))
+		goto error;
+
+	if (strength == 99)
+		strength = 100;
+
+	ofono_cdma_netreg_strength_notify(netreg, strength);
+
+	return;
+
+error:
+	ofono_error("Invalid RSSILVL value");
+}
+
+static void probe_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_cdma_netreg *netreg = user_data;
+	GAtChat *chat = ofono_cdma_netreg_get_data(netreg);
+
+	if (!ok) {
+		ofono_cdma_netreg_remove(netreg);
+		return;
+	}
+
+	g_at_chat_register(chat, "^MODE:",
+				mode_notify, FALSE, netreg, NULL);
+
+	g_at_chat_register(chat, "^RSSILVL:",
+				rssilvl_notify, FALSE, netreg, NULL);
+
+	ofono_cdma_netreg_register(netreg);
+}
+
+static int cdma_netreg_probe(struct ofono_cdma_netreg *netreg,
+				unsigned int vendor, void *data)
+{
+	GAtChat *chat = g_at_chat_clone(data);
+
+	ofono_cdma_netreg_set_data(netreg, chat);
+
+	if (vendor == OFONO_VENDOR_HUAWEI)
+		g_at_chat_send(chat, "AT^SYSINFO", sysinfo_prefix,
+				probe_cb, netreg, NULL);
+
+	return 0;
+}
+
+static void cdma_netreg_remove(struct ofono_cdma_netreg *netreg)
+{
+	GAtChat *chat = ofono_cdma_netreg_get_data(netreg);
+
+	ofono_cdma_netreg_set_data(netreg, NULL);
+
+	g_at_chat_unref(chat);
+}
+
+static struct ofono_cdma_netreg_driver driver = {
+	.name	= "cdmamodem",
+	.probe	= cdma_netreg_probe,
+	.remove	= cdma_netreg_remove,
+};
+
+void cdma_netreg_init(void)
+{
+	ofono_cdma_netreg_driver_register(&driver);
+}
+
+void cdma_netreg_exit(void)
+{
+	ofono_cdma_netreg_driver_unregister(&driver);
+}
diff --git a/plugins/huaweicdma.c b/plugins/huaweicdma.c
index 4c83114..5618417 100644
--- a/plugins/huaweicdma.c
+++ b/plugins/huaweicdma.c
@@ -38,6 +38,8 @@
 #include <ofono/cdma-connman.h>
 #include <ofono/log.h>
 
+#include <drivers/atmodem/vendor.h>
+
 struct huaweicdma_data {
 	GAtChat *modem;
 	GAtChat *pcui;
@@ -211,7 +213,8 @@ static void huaweicdma_post_online(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	ofono_cdma_netreg_create(modem, 0, "huaweicdmamodem", data->modem);
+	ofono_cdma_netreg_create(modem, OFONO_VENDOR_HUAWEI, "cdmamodem",
+					data->modem);
 
 	ofono_cdma_connman_create(modem, 0, "cdmamodem", data->modem);
 }
-- 
1.7.1

[PATCH v2 6/6] cdmamodem: Add serving system identifier support

[Philippe Nunes]

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

---
 drivers/cdmamodem/network-registration.c |   78 ++++++++++++++++++++++++++++++
 include/cdma-netreg.h                    |    7 +++
 2 files changed, 85 insertions(+), 0 deletions(-)

diff --git a/drivers/cdmamodem/network-registration.c b/drivers/cdmamodem/network-registration.c
index a34db91..fe8b4f2 100644
--- a/drivers/cdmamodem/network-registration.c
+++ b/drivers/cdmamodem/network-registration.c
@@ -24,6 +24,7 @@
 #endif
 
 #define _GNU_SOURCE
+#include <string.h>
 #include <glib.h>
 #include <errno.h>
 
@@ -108,6 +109,67 @@ static void sysinfo_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_cdma_netreg_status_notify(netreg, status);
 }
 
+static gboolean parse_css(GAtResult *result, const char **sid)
+{
+	GAtResultIter iter;
+	/*
+	* According TIA/EIA/IS-707, CSS query returns <AB>,<SID> but
+	* according TIA/EIA/IS-707-A , it returns <Band_Class>,<Band>,<SID>
+	* PREV field which has been added afterward is ignored
+	*/
+
+	g_at_result_iter_init(&iter, result);
+
+	g_at_result_iter_next(&iter, NULL);
+
+	/* Skip first field since we are not interested in this */
+	if (!g_at_result_iter_skip_next(&iter))
+		return FALSE;
+
+	if (!g_at_result_iter_next_unquoted_string(&iter, sid))
+		return FALSE;
+	/*
+	* As CSS answer may differ according which revision of TIA/EIA/IS-707
+	* the modem is compliant, we need to check if this field is Band or SID
+	*/
+	if (*sid[0] >= 'A' && *sid[0] <= 'F') {
+		/* This is the band field, the next field is the SID*/
+		if (!g_at_result_iter_next_unquoted_string(&iter, sid))
+			return FALSE;
+	}
+
+	if (!strcmp(*sid, "99999"))
+		/* The mobile station is not registered.*/
+		return FALSE;
+
+	return TRUE;
+}
+
+static void serving_system_cb(gboolean ok, GAtResult *result,
+				gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_cdma_netreg_serving_system_cb_t cb = cbd->cb;
+	struct ofono_error error;
+	const char* sid;
+
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok) {
+		cb(&error, NULL, cbd->data);
+		return;
+	}
+
+	if (parse_css(result, &sid) == FALSE) {
+		CALLBACK_WITH_FAILURE(cb, NULL, cbd->data);
+		return;
+	}
+
+	DBG("serving system: SID %s", sid);
+
+	cb(&error, sid, cbd->data);
+}
+
 static void mode_notify(GAtResult *result, gpointer user_data)
 {
 	struct ofono_cdma_netreg *netreg = user_data;
@@ -184,10 +246,26 @@ static void cdma_netreg_remove(struct ofono_cdma_netreg *netreg)
 	g_at_chat_unref(chat);
 }
 
+static void cdma_netreg_serving_system(struct ofono_cdma_netreg *netreg,
+		ofono_cdma_netreg_serving_system_cb_t cb, void *data)
+{
+	GAtChat *chat = ofono_cdma_netreg_get_data(netreg);
+	struct cb_data *cbd = cb_data_new(cb, data);
+
+	if (g_at_chat_send(chat, "AT+CSS=?", NULL, serving_system_cb, cbd,
+				g_free) > 0)
+			return;
+
+	g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, NULL, data);
+}
+
 static struct ofono_cdma_netreg_driver driver = {
 	.name	= "cdmamodem",
 	.probe	= cdma_netreg_probe,
 	.remove	= cdma_netreg_remove,
+	.serving_system = cdma_netreg_serving_system,
 };
 
 void cdma_netreg_init(void)
diff --git a/include/cdma-netreg.h b/include/cdma-netreg.h
index 31ed289..529dab5 100644
--- a/include/cdma-netreg.h
+++ b/include/cdma-netreg.h
@@ -36,12 +36,19 @@ enum cdma_netreg_status {
 
 struct ofono_cdma_netreg;
 
+typedef void (*ofono_cdma_netreg_serving_system_cb_t)(
+				const struct ofono_error *error,
+				const char *sid,
+				void *data);
+
 struct ofono_cdma_netreg_driver {
 	const char *name;
 	int (*probe)(struct ofono_cdma_netreg *cdma_netreg,
 				unsigned int vendor,
 				void *data);
 	void (*remove)(struct ofono_cdma_netreg *cdma_netreg);
+	void (*serving_system)(struct ofono_cdma_netreg *cdma_netreg,
+			ofono_cdma_netreg_serving_system_cb_t cb, void *data);
 };
 
 void ofono_cdma_netreg_status_notify(struct ofono_cdma_netreg *netreg,
-- 
1.7.1

Re: [PATCH v2 6/6] cdmamodem: Add serving system identifier support

[Philippe Nunes]

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

Hi Denis,

On 11/16/2011 11:26 AM, Denis Kenzior wrote:
> Hi Philippe,
>
> On 11/17/2011 10:21 AM, Philippe Nunes wrote:
>> ---
>>   drivers/cdmamodem/network-registration.c |   78 ++++++++++++++++++++++++++++++
>>   include/cdma-netreg.h                    |    7 +++
>>   2 files changed, 85 insertions(+), 0 deletions(-)
>>
>
> So the obvious comment is that this patch should be split into two, one
> for adding new driver API and one for the driver changes.

OK, sorry to bother again with such issue.

> I would also like to see how you're planning to use this information in
> the core, before reviewing this patch in detail.  Do note that after
> taking a quick peek at Huawei CDMA manuals and ModemManager, I very much
> doubt that +CSS is what you want to use in the first place.

I'm really not familiar with Huawei CDMA AT commands but looking to the 
command interface specification from Huawei and to TIA/EIA/IS-707, I 
can't see other solution than the serving system query to retrieve the 
SID. Also, I don't see any unsolicited result codes which could bring 
such information.

Note that this AT command seems supported by Huawei cdma modem according 
the first revison of TIA/EIA/IS-707 (here, the band class is not 
supported, neither the PREV field).

Now, it's true that we would need to retrieve also the NID (as stated in 
TODO), but so far, I don't see how...

Regards,

Philippe.


>
> Regards,
> -Denis
>

Re: [PATCH v2 6/6] cdmamodem: Add serving system identifier support

[Denis Kenzior]

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

Hi Philippe,

>> I would also like to see how you're planning to use this information in
>> the core, before reviewing this patch in detail.  Do note that after
>> taking a quick peek at Huawei CDMA manuals and ModemManager, I very much
>> doubt that +CSS is what you want to use in the first place.
> 
> I'm really not familiar with Huawei CDMA AT commands but looking to the
> command interface specification from Huawei and to TIA/EIA/IS-707, I
> can't see other solution than the serving system query to retrieve the
> SID. Also, I don't see any unsolicited result codes which could bring
> such information.
> 
> Note that this AT command seems supported by Huawei cdma modem according
> the first revison of TIA/EIA/IS-707 (here, the band class is not
> supported, neither the PREV field).
> 
> Now, it's true that we would need to retrieve also the NID (as stated in
> TODO), but so far, I don't see how...

I don't really have the answer, but take a peek at ModemManager.  +CSS
is not how it retrieves SID in the most common case.  In fact it doesn't
even do it via AT commands.  If that's the case, then you might want to
rethink this implementation completely.

Regards,
-Denis

Re: [PATCH v2 6/6] cdmamodem: Add serving system identifier support

[Denis Kenzior]

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

Hi Philippe,

On 11/24/2011 11:35 AM, Philippe Nunes wrote:
> Hi Denis,
> 
> On 11/18/2011 04:18 PM, Denis Kenzior wrote:
>> Hi Philippe,
>>
>>>> I would also like to see how you're planning to use this information in
>>>> the core, before reviewing this patch in detail.  Do note that after
>>>> taking a quick peek at Huawei CDMA manuals and ModemManager, I very
>>>> much
>>>> doubt that +CSS is what you want to use in the first place.
>>>
>>> I'm really not familiar with Huawei CDMA AT commands but looking to the
>>> command interface specification from Huawei and to TIA/EIA/IS-707, I
>>> can't see other solution than the serving system query to retrieve the
>>> SID. Also, I don't see any unsolicited result codes which could bring
>>> such information.
>>>
>>> Note that this AT command seems supported by Huawei cdma modem according
>>> the first revison of TIA/EIA/IS-707 (here, the band class is not
>>> supported, neither the PREV field).
>>>
>>> Now, it's true that we would need to retrieve also the NID (as stated in
>>> TODO), but so far, I don't see how...
>>
>> I don't really have the answer, but take a peek at ModemManager.  +CSS
>> is not how it retrieves SID in the most common case.  In fact it doesn't
>> even do it via AT commands.  If that's the case, then you might want to
>> rethink this implementation completely.
>>
>> Regards,
>> -Denis
>>
> 
> Looking to the modem manager, the priority is indeed to retrieve the SID
> using the qcdm port com (thanks to the result of a radio status query).
> In case of error, there is however a fallback to use the AT command
> +CSS. Also, if no qcdm port is available, the SID is retrieved  by using
> this AT command (please refer to 'legacy_get_serving_system').
> 
> Note that in modem manager, there is also a special treatment for Huawei
> modem which use the IS-707-A format (no band and class).

I also noticed various comments stating that +CSS does not report any
information when registered to EV-DO and it seems that the Huawei driver
doesn't even bother using fallback mechanisms for EVDO capable hardware
in the first place.  But maybe I'm not reading the code right...

> 
> So far, the AT command "Serving system" seems relevant. Now, do you
> expect us to communicate also through the qcdm port when using cdma modem?

Have you tested that your theories work?  Right now I'm seeing you
making lots of assumptions, and moving code from the huawei specific
driver to some generic driver when in fact no such generality exists.
CDMA does not have an AT command standard ;)

And yes, if the AT command does not do what we need, then using QCDM
would be required.

Regards,
-Denis

Re: [PATCH v2 6/6] cdmamodem: Add serving system identifier support

[Philippe Nunes]

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

Hi Denis,

On 11/18/2011 04:18 PM, Denis Kenzior wrote:
> Hi Philippe,
>
>>> I would also like to see how you're planning to use this information in
>>> the core, before reviewing this patch in detail.  Do note that after
>>> taking a quick peek at Huawei CDMA manuals and ModemManager, I very much
>>> doubt that +CSS is what you want to use in the first place.
>>
>> I'm really not familiar with Huawei CDMA AT commands but looking to the
>> command interface specification from Huawei and to TIA/EIA/IS-707, I
>> can't see other solution than the serving system query to retrieve the
>> SID. Also, I don't see any unsolicited result codes which could bring
>> such information.
>>
>> Note that this AT command seems supported by Huawei cdma modem according
>> the first revison of TIA/EIA/IS-707 (here, the band class is not
>> supported, neither the PREV field).
>>
>> Now, it's true that we would need to retrieve also the NID (as stated in
>> TODO), but so far, I don't see how...
>
> I don't really have the answer, but take a peek at ModemManager.  +CSS
> is not how it retrieves SID in the most common case.  In fact it doesn't
> even do it via AT commands.  If that's the case, then you might want to
> rethink this implementation completely.
>
> Regards,
> -Denis
>

Looking to the modem manager, the priority is indeed to retrieve the SID 
using the qcdm port com (thanks to the result of a radio status query).
In case of error, there is however a fallback to use the AT command 
+CSS. Also, if no qcdm port is available, the SID is retrieved  by using 
this AT command (please refer to 'legacy_get_serving_system').

Note that in modem manager, there is also a special treatment for Huawei 
modem which use the IS-707-A format (no band and class).

So far, the AT command "Serving system" seems relevant. Now, do you 
expect us to communicate also through the qcdm port when using cdma modem?

Regards,

Philippe.

Re: [PATCH v2 6/6] cdmamodem: Add serving system identifier support

[Denis Kenzior]

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

Hi Philippe,

>>
>>>
>>> So far, the AT command "Serving system" seems relevant. Now, do you
>>> expect us to communicate also through the qcdm port when using cdma
>>> modem?
>>
>> Have you tested that your theories work?
> Yes, I checked locally with huawei cdma dongles and the speedup SU 7300
> that the command "AT+CSS?" was effectively supported.
> I tested also remotely (through SSH with a machine in China where a
> huawei cdma dongle is plugged) that the SID was correctly returned when
> registered.
>

Fair enough.

> In fact, I applied the same approach as for the atmodem driver.
> We only have one network-registration driver which is taking care of all
> manufacturer specific commands (as for Huawei).
> As the AT command '+CSS' is supported by both Speedup cdma And Huawei
> cdma, I took the initiative indeed to move the specific huawei part into
> a generic driver.

Well not really, the generic driver for GSM uses mostly standard
commands and only supports vendor extensions.  The split is probably 70%
standards, 30% extensions, and even then mostly because just about every
vendor decided to invent one for timezone / network technology / signal
strength indications.

What I see in your proposal is 70% huawei specific and the rest taken up
by CSS which is still of questionable utility.  And if you find that you
do indeed need to use QCDM for finding the SID, then you won't be able
to use the new generic driver anyway, so lets hold off on this for now.

And by the way, there's nothing wrong with using huawei atom drivers /
quirks from non-huawei modem drivers (e.g. speedup).  See ste/mbm
drivers for examples.

Regards,
-Denis

Re: [PATCH v2 6/6] cdmamodem: Add serving system identifier support

[Philippe Nunes]

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

Hi Denis,

>> Looking to the modem manager, the priority is indeed to retrieve the SID
>> using the qcdm port com (thanks to the result of a radio status query).
>> In case of error, there is however a fallback to use the AT command
>> +CSS. Also, if no qcdm port is available, the SID is retrieved  by using
>> this AT command (please refer to 'legacy_get_serving_system').
>>
>> Note that in modem manager, there is also a special treatment for Huawei
>> modem which use the IS-707-A format (no band and class).
>
> I also noticed various comments stating that +CSS does not report any
> information when registered to EV-DO and it seems that the Huawei driver
> doesn't even bother using fallback mechanisms for EVDO capable hardware
> in the first place.  But maybe I'm not reading the code right...

You're right. I missed this point as this consideration seems rather 
linked with the registration state query.
In fact, I started to see the code from the network manager applet to 
see how the SID was retrieved.
That's why, I looked only for the 'GetServingSystem' method in modem 
manager. This method is implemented in the generic part and the flag 
'reg_try_css' is not checked here.

In fact, this problematic is not explicitely considered when getting the 
serving system in modem manager as the query is done first on the qcdm 
port. So, I presume that this port should always be available with EVDO 
capabale hardware.
So, the qcdm part seems mandatory.


>
>>
>> So far, the AT command "Serving system" seems relevant. Now, do you
>> expect us to communicate also through the qcdm port when using cdma modem?
>
> Have you tested that your theories work?
Yes, I checked locally with huawei cdma dongles and the speedup SU 7300 
that the command "AT+CSS?" was effectively supported.
I tested also remotely (through SSH with a machine in China where a 
huawei cdma dongle is plugged) that the SID was correctly returned when 
registered.

As you can see below:

ofonod[30357]: Modem: < \r\n^MODE: 
2\r\r\n\r\r\n^RSSILVL:60\r\r\n\r\r\n^HRSSILVL:0\r\n
ofonod[30357]: PCUI: < \r\n^MODE: 
2\r\r\n\r\r\n^RSSILVL:60\r\r\n\r\r\n^HRSSILVL:0\r\n
ofonod[30357]: Modem: > AT^SYSINFO\r
ofonod[30357]: Modem: < \r\n^SYSINFO:2,255,0,2,1\r\n\r\nOK\r\n
ofonod[30357]: drivers/cdmamodem/network-registration.c:parse_sysinfo() 
2, 255, 0
ofonod[30357]: Modem: > AT+CSS=?\r
ofonod[30357]: Modem: < \r\n?, 13840\r\n\r\nOK\r\n
ofonod[30357]: 
drivers/cdmamodem/network-registration.c:serving_system_cb() serving 
system: SID 13840
ofonod[30357]: src/cdma-netreg.c:serving_system_callback() Serving 
system Identifier: 13840

Here the provider name is not found. Maybe the provider info file placed 
in the remote desktop is not sufficiently populated.


Right now I'm seeing you
> making lots of assumptions, and moving code from the huawei specific
> driver to some generic driver when in fact no such generality exists.
> CDMA does not have an AT command standard ;)

In fact, I applied the same approach as for the atmodem driver.
We only have one network-registration driver which is taking care of all 
manufacturer specific commands (as for Huawei).
As the AT command '+CSS' is supported by both Speedup cdma And Huawei 
cdma, I took the initiative indeed to move the specific huawei part into 
a generic driver.
Also, the AT command AT+CSS is implemented in the generic part in modem 
manager.

>
> And yes, if the AT command does not do what we need, then using QCDM
> would be required.

As stated above, I will start to implement the QCDM part as this should 
be required when using EVDO capable hardware.

Thanks.

Regards,

Philippe.