[PATCH] add quirk for cid and lac handling for Huawei

[PATCH] add quirk for cid and lac handling for Huawei

[Jussi Kukkonen]

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

Hello,

My Eee-integrated Huawei E620 reports "local area code" and "cell id" as 
unquoted strings. Denis informed me on IRC that this is non-compliant 
and that I could add a quirk for it.

Attached is my attempt at that. The actual quirk is in 
drivers/atmodem/atutil.c, which means I needed to change the 
at_util_parse_reg*() definitions. Let me know if my approach seems wrong 
-- this was the first time I've even looked at ofono code, and I can't 
claim to understand it fully.

The huawei plugin seems to use OFONO_VENDOR_QUALCOMM_MSM for sms... 
Should that be updated so OFONO_VENDOR_HUAWEI (which I added) hits the 
same quirk(s) or do Huawei and Qualcomm really have something in common?

  - Jussi


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-quirk-for-cid-and-lac-handling-for-Huawei.patch --]
[-- Type: text/x-patch, Size: 8556 bytes --]

>From 216b1370e604115deeb0fa71f72ea25d5888c78d Mon Sep 17 00:00:00 2001
From: Jussi Kukkonen <jku@linux.intel.com>
Date: Mon, 18 Jan 2010 16:30:13 +0200
Subject: [PATCH] Add quirk for cid and lac handling for Huawei

---
 drivers/atmodem/atutil.c               |   69 +++++++++++++++++++++++--------
 drivers/atmodem/atutil.h               |    6 ++-
 drivers/atmodem/gprs.c                 |   10 ++++-
 drivers/atmodem/network-registration.c |    8 +++-
 drivers/atmodem/vendor.h               |    1 +
 plugins/huawei.c                       |    3 +-
 6 files changed, 72 insertions(+), 25 deletions(-)

diff --git a/drivers/atmodem/atutil.c b/drivers/atmodem/atutil.c
index b72e919..c2dfd21 100644
--- a/drivers/atmodem/atutil.c
+++ b/drivers/atmodem/atutil.c
@@ -33,6 +33,7 @@
 #include <ofono/types.h>
 
 #include "atutil.h"
+#include "vendor.h"
 
 void dump_response(const char *func, gboolean ok, GAtResult *result)
 {
@@ -149,7 +150,8 @@ GSList *at_util_parse_clcc(GAtResult *result)
 
 gboolean at_util_parse_reg_unsolicited(GAtResult *result, const char *prefix,
 					int *status,
-					int *lac, int *ci, int *tech)
+					int *lac, int *ci, int *tech,
+					unsigned int vendor)
 {
 	GAtResultIter iter;
 	int s;
@@ -164,15 +166,30 @@ gboolean at_util_parse_reg_unsolicited(GAtResult *result, const char *prefix,
 	if (g_at_result_iter_next_number(&iter, &s) == FALSE)
 		return FALSE;
 
-	if (g_at_result_iter_next_string(&iter, &str) == TRUE)
-		l = strtol(str, NULL, 16);
-	else
-		goto out;
+	switch (vendor) {
+	case OFONO_VENDOR_HUAWEI:
+		if (g_at_result_iter_next_unquoted_string(&iter, &str) == TRUE)
+			l = strtol(str, NULL, 16);
+		else
+			goto out;
+
+		if (g_at_result_iter_next_unquoted_string(&iter, &str) == TRUE)
+			c = strtol(str, NULL, 16);
+		else
+			goto out;
 
-	if (g_at_result_iter_next_string(&iter, &str) == TRUE)
-		c = strtol(str, NULL, 16);
-	else
-		goto out;
+		break;
+	default:
+		if (g_at_result_iter_next_string(&iter, &str) == TRUE)
+			l = strtol(str, NULL, 16);
+		else
+			goto out;
+
+		if (g_at_result_iter_next_string(&iter, &str) == TRUE)
+			c = strtol(str, NULL, 16);
+		else
+			goto out;
+	}
 
 	g_at_result_iter_next_number(&iter, &t);
 
@@ -194,7 +211,8 @@ out:
 
 gboolean at_util_parse_reg(GAtResult *result, const char *prefix,
 				int *mode, int *status,
-				int *lac, int *ci, int *tech)
+				int *lac, int *ci, int *tech,
+				unsigned int vendor)
 {
 	GAtResultIter iter;
 	int m, s;
@@ -210,15 +228,30 @@ gboolean at_util_parse_reg(GAtResult *result, const char *prefix,
 		if (g_at_result_iter_next_number(&iter, &s) == FALSE)
 			continue;
 
-		if (g_at_result_iter_next_string(&iter, &str) == TRUE)
-			l = strtol(str, NULL, 16);
-		else
-			goto out;
+		switch (vendor) {
+		case OFONO_VENDOR_HUAWEI:
+			if (g_at_result_iter_next_unquoted_string(&iter, &str) == TRUE)
+				l = strtol(str, NULL, 16);
+			else
+				goto out;
 
-		if (g_at_result_iter_next_string(&iter, &str) == TRUE)
-			c = strtol(str, NULL, 16);
-		else
-			goto out;
+			if (g_at_result_iter_next_unquoted_string(&iter, &str) == TRUE)
+				c = strtol(str, NULL, 16);
+			else
+				goto out;
+
+			break;
+		default:
+			if (g_at_result_iter_next_string(&iter, &str) == TRUE)
+				l = strtol(str, NULL, 16);
+			else
+				goto out;
+
+			if (g_at_result_iter_next_string(&iter, &str) == TRUE)
+				c = strtol(str, NULL, 16);
+			else
+				goto out;
+		}
 
 		g_at_result_iter_next_number(&iter, &t);
 
diff --git a/drivers/atmodem/atutil.h b/drivers/atmodem/atutil.h
index 4b60cd1..0ab207d 100644
--- a/drivers/atmodem/atutil.h
+++ b/drivers/atmodem/atutil.h
@@ -27,10 +27,12 @@ gint at_util_call_compare(gconstpointer a, gconstpointer b);
 GSList *at_util_parse_clcc(GAtResult *result);
 gboolean at_util_parse_reg(GAtResult *result, const char *prefix,
 				int *mode, int *status,
-				int *lac, int *ci, int *tech);
+				int *lac, int *ci, int *tech,
+				unsigned int vendor);
 gboolean at_util_parse_reg_unsolicited(GAtResult *result, const char *prefix,
 					int *status, int *lac,
-					int *ci, int *tech);
+					int *ci, int *tech,
+					unsigned int vendor);
 
 struct cb_data {
 	void *cb;
diff --git a/drivers/atmodem/gprs.c b/drivers/atmodem/gprs.c
index 76085d9..6034c42 100644
--- a/drivers/atmodem/gprs.c
+++ b/drivers/atmodem/gprs.c
@@ -45,6 +45,7 @@ static const char *none_prefix[] = { NULL };
 
 struct gprs_data {
 	GAtChat *chat;
+	unsigned int vendor;
 };
 
 static void at_cgatt_cb(gboolean ok, GAtResult *result, gpointer user_data)
@@ -88,6 +89,7 @@ static void at_cgreg_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_gprs_status_cb_t cb = cbd->cb;
 	struct ofono_error error;
 	int status, lac, ci, tech;
+	struct gprs_data *gd = cbd->user;
 
 	dump_response("at_cgreg_cb", ok, result);
 	decode_at_error(&error, g_at_result_final_response(result));
@@ -98,7 +100,7 @@ static void at_cgreg_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	}
 
 	if (at_util_parse_reg(result, "+CGREG:", NULL, &status,
-				&lac, &ci, &tech) == FALSE) {
+				&lac, &ci, &tech, gd->vendor) == FALSE) {
 		CALLBACK_WITH_FAILURE(cb, -1, -1, -1, -1, cbd->data);
 		return;
 	}
@@ -116,6 +118,8 @@ static void at_gprs_registration_status(struct ofono_gprs *gprs,
 	if (!cbd)
 		goto error;
 
+	cbd->user = gd;
+
 	if (g_at_chat_send(gd->chat, "AT+CGREG?", cgreg_prefix,
 				at_cgreg_cb, cbd, g_free) > 0)
 		return;
@@ -131,11 +135,12 @@ static void cgreg_notify(GAtResult *result, gpointer user_data)
 {
 	struct ofono_gprs *gprs = user_data;
 	int status, lac, ci, tech;
+	struct gprs_data *gd = ofono_gprs_get_data(gprs);
 
 	dump_response("cgreg_notify", TRUE, result);
 
 	if (at_util_parse_reg_unsolicited(result, "+CGREG:", &status,
-				&lac, &ci, &tech) == FALSE)
+				&lac, &ci, &tech, gd->vendor) == FALSE)
 		return;
 
 	ofono_gprs_status_notify(gprs, status, lac, ci, tech);
@@ -289,6 +294,7 @@ static int at_gprs_probe(struct ofono_gprs *gprs,
 
 	gd = g_new0(struct gprs_data, 1);
 	gd->chat = chat;
+	gd->vendor = vendor;
 
 	ofono_gprs_set_data(gprs, gd);
 
diff --git a/drivers/atmodem/network-registration.c b/drivers/atmodem/network-registration.c
index 202256a..c80acc5 100644
--- a/drivers/atmodem/network-registration.c
+++ b/drivers/atmodem/network-registration.c
@@ -69,6 +69,7 @@ static void at_creg_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_netreg_status_cb_t cb = cbd->cb;
 	int status, lac, ci, tech;
 	struct ofono_error error;
+	struct netreg_data *nd = cbd->user;
 
 	dump_response("at_creg_cb", ok, result);
 	decode_at_error(&error, g_at_result_final_response(result));
@@ -79,7 +80,7 @@ static void at_creg_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	}
 
 	if (at_util_parse_reg(result, "+CREG:", NULL, &status,
-				&lac, &ci, &tech) == FALSE) {
+				&lac, &ci, &tech, nd->vendor) == FALSE) {
 		CALLBACK_WITH_FAILURE(cb, -1, -1, -1, -1, cbd->data);
 		return;
 	}
@@ -97,6 +98,8 @@ static void at_registration_status(struct ofono_netreg *netreg,
 	if (!cbd)
 		goto error;
 
+	cbd->user = nd;
+
 	if (g_at_chat_send(nd->chat, "AT+CREG?", creg_prefix,
 				at_creg_cb, cbd, g_free) > 0)
 		return;
@@ -626,11 +629,12 @@ static void creg_notify(GAtResult *result, gpointer user_data)
 {
 	struct ofono_netreg *netreg = user_data;
 	int status, lac, ci, tech;
+	struct netreg_data *nd = ofono_netreg_get_data(netreg);
 
 	dump_response("creg_notify", TRUE, result);
 
 	if (at_util_parse_reg_unsolicited(result, "+CREG:", &status,
-				&lac, &ci, &tech) == FALSE)
+				&lac, &ci, &tech, nd->vendor) == FALSE)
 		return;
 
 	ofono_netreg_status_notify(netreg, status, lac, ci, tech);
diff --git a/drivers/atmodem/vendor.h b/drivers/atmodem/vendor.h
index 8d9ed47..d0df3ab 100644
--- a/drivers/atmodem/vendor.h
+++ b/drivers/atmodem/vendor.h
@@ -24,4 +24,5 @@ enum ofono_vendor {
 	OFONO_VENDOR_CALYPSO,
 	OFONO_VENDOR_QUALCOMM_MSM,
 	OFONO_VENDOR_OPTION_HSO,
+	OFONO_VENDOR_HUAWEI,
 };
diff --git a/plugins/huawei.c b/plugins/huawei.c
index 56119cc..1492446 100644
--- a/plugins/huawei.c
+++ b/plugins/huawei.c
@@ -38,6 +38,7 @@
 #include <ofono/netreg.h>
 #include <ofono/sim.h>
 #include <ofono/sms.h>
+#include <ofono/gprs.h>
 #include <ofono/voicecall.h>
 #include <ofono/log.h>
 
@@ -176,7 +177,7 @@ static void huawei_post_sim(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
-	ofono_netreg_create(modem, 0, "atmodem", data->chat);
+	ofono_netreg_create(modem, OFONO_VENDOR_HUAWEI, "atmodem", data->chat);
 	ofono_sms_create(modem, OFONO_VENDOR_QUALCOMM_MSM, "atmodem", data->chat);
 }
 
-- 
1.6.5

Re: [PATCH] add quirk for cid and lac handling for Huawei

[Denis Kenzior]

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

Hi Jussi,

> Attached is my attempt at that. The actual quirk is in
> drivers/atmodem/atutil.c, which means I needed to change the
> at_util_parse_reg*() definitions. Let me know if my approach seems wrong
> -- this was the first time I've even looked at ofono code, and I can't
> claim to understand it fully.

I'm actually fine with this approach.  Patch has been applied.  I did a trivial 
style fix afterward to keep the line length less than 80 characters.

> 
> The huawei plugin seems to use OFONO_VENDOR_QUALCOMM_MSM for sms...
> Should that be updated so OFONO_VENDOR_HUAWEI (which I added) hits the
> same quirk(s) or do Huawei and Qualcomm really have something in common?

Huawei uses Qualcomm chips in their devices and thus share the same 'quirks'.  
I expect the vendor.h quirks to get refined over time anyway, so for now it is 
fine to leave it the way it is.

Thanks,
-Denis