Skip to contentSkip to conversation

add quirk for cid and lac handling for Huawei

PATCH In review Jussi Kukkonen 2 participants 2 messages Jan 18, 2010 – Jan 20, 2010

lore
Share & tools
lore

Jussi Kukkonen

author
#0 lore raw

PATCH 1/1 add quirk for cid and lac handling for Huawei

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

Quoted 1 line

From 216b1370e604115deeb0fa71f72ea25d5888c78d Mon Sep 17 00:00:00 2001

From: Jussi Kukkonen <[email protected]>
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(-)
drivers/atmodem/atutil.c +51 -18
index b72e919..c2dfd21 100644
@@ -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);
drivers/atmodem/atutil.h +4 -2
index 4b60cd1..0ab207d 100644
@@ -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;
drivers/atmodem/gprs.c +8 -2
index 76085d9..6034c42 100644
@@ -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);
drivers/atmodem/network-registration.c +6 -2
index 202256a..c80acc5 100644
@@ -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);
drivers/atmodem/vendor.h +1 -0
index 8d9ed47..d0df3ab 100644
@@ -24,4 +24,5 @@ enum ofono_vendor {
 	OFONO_VENDOR_CALYPSO,
 	OFONO_VENDOR_QUALCOMM_MSM,
 	OFONO_VENDOR_OPTION_HSO,
+	OFONO_VENDOR_HUAWEI,
 };
plugins/huawei.c +2 -1
index 56119cc..1492446 100644
@@ -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);
 }
Signature · 1 line
1.6.5
#1 lore raw

Hi Jussi,

Quoted 5 lines

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.

Quoted 4 lines

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