Skip to contentSkip to conversation

Fix: simplify isimodem call-barring driver.

PATCH In review [email protected] 3 participants 4 messages Mar 23, 2010 – Mar 29, 2010

lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/1 Fix: simplify isimodem call-barring driver.

From: Pekka Pessi <[email protected]>

Add support for lock codes AG and AC (MMI codes 333 and 353. respectively).

---
 drivers/isimodem/call-barring.c |  153 ++++++++++++---------------------------
 drivers/isimodem/ss.h           |   28 ++++----
 2 files changed, 63 insertions(+), 118 deletions(-)
drivers/isimodem/call-barring.c +48 -105
index 6487ae8..b0246fa 100644
@@ -48,6 +48,28 @@ struct barr_data {
 	GIsiClient *client;
 };
 
+static int lock_code_to_mmi(char const *lock)
+{
+	if (strcmp(lock, "AO") == 0)
+		return SS_GSM_BARR_ALL_OUT;
+	else if (strcmp(lock, "OI") == 0)
+		return SS_GSM_BARR_OUT_INTER;
+	else if (strcmp(lock, "OX") == 0)
+		return SS_GSM_BARR_OUT_INTER_EXC_HOME;
+	else if (strcmp(lock, "AI") == 0)
+		return SS_GSM_BARR_ALL_IN;
+	else if (strcmp(lock, "IR") == 0)
+		return SS_GSM_BARR_ALL_IN_ROAM;
+	else if (strcmp(lock, "AB") == 0)
+		return SS_GSM_ALL_BARRINGS;
+	else if (strcmp(lock, "AG") == 0)
+		return SS_GSM_OUTGOING_BARR_SERV;
+	else if (strcmp(lock, "AC") == 0)
+		return SS_GSM_INCOMING_BARR_SERV;
+	else
+		return 0;
+}
+
 static bool set_resp_cb(GIsiClient *client, const void *restrict data,
 				size_t len, uint16_t object, void *opaque)
 {

@@ -84,19 +106,19 @@ static void isi_set(struct ofono_call_barring *barr, const char *lock,
 {
 	struct barr_data *bd = ofono_call_barring_get_data(barr);
 	struct isi_cb_data *cbd = isi_cb_data_new(barr, cb, data);
-	int ss_code;
-	char *ucs2 = NULL;
+	int ss_code = lock_code_to_mmi(lock);
 
 	unsigned char msg[] = {
 		SS_SERVICE_REQ,
 		enable ? SS_ACTIVATION : SS_DEACTIVATION,
 		SS_ALL_TELE_AND_BEARER,
-		0, 0,				/* Supplementary services code */
-		SS_SEND_ADDITIONAL_INFO,
+		ss_code >> 8, ss_code & 0xFF,	/* Supplementary services code */
+		SS_SEND_ADDITIONAL_INFO,	/* ? */
 		1,				/* Subblock count */
 		SS_GSM_PASSWORD,
 		28,				/* Subblock length */
-		0, 0, 0, 0, 0, 0, 0, 0,		/* Password */
+		0, passwd[0], 0, passwd[1],	/* Password */
+		0, passwd[2], 0, passwd[3],
 		0, 0, 0, 0, 0, 0, 0, 0,		/* Filler */
 		0, 0, 0, 0, 0, 0, 0, 0,		/* Filler */
 		0, 0				/* Filler */

@@ -105,43 +127,10 @@ static void isi_set(struct ofono_call_barring *barr, const char *lock,
 	DBG("lock code %s enable %d class %d password %s\n",
 		lock, enable, cls, passwd);
 
-	if (!cbd || !passwd || strlen(passwd) > 4 || cls != 7)
-		goto error;
-
-	if (strcmp(lock, "AO") == 0)
-		ss_code = SS_GSM_BARR_ALL_OUT;
-	else if (strcmp(lock, "OI") == 0)
-		ss_code = SS_GSM_BARR_OUT_INTER;
-	else if (strcmp(lock, "OX") == 0)
-		ss_code = SS_GSM_BARR_OUT_INTER_EXC_HOME;
-	else if (strcmp(lock, "AI") == 0)
-		ss_code = SS_GSM_BARR_ALL_IN;
-	else if (strcmp(lock, "IR") == 0)
-		ss_code = SS_GSM_BARR_ALL_IN_ROAM;
-	else if (strcmp(lock, "AB") == 0)
-		ss_code = SS_GSM_ALL_BARRINGS;
-	else if (strcmp(lock, "AG") == 0)
-		ss_code = SS_GSM_BARR_ALL_OUT;
-	else if (strcmp(lock, "AC") == 0)
-		ss_code = SS_GSM_BARR_ALL_IN;
-	else
-		goto error;
-
-	msg[3] = ss_code >> 8;
-	msg[4] = ss_code & 0xFF;
-
-	ucs2 = g_convert(passwd, 4, "UCS-2BE", "UTF-8//TRANSLIT",
-				NULL, NULL, NULL);
-	if (ucs2 == NULL)
-		goto error;
-
-	memcpy((char *)msg + 9, ucs2, 8);
-	g_free(ucs2);
-
-	if (g_isi_request_make(bd->client, msg, sizeof(msg), SS_TIMEOUT,
-				set_resp_cb, cbd))
+	if (cbd && g_isi_request_make(bd->client, msg, sizeof(msg), SS_TIMEOUT,
+				      set_resp_cb, cbd))
 		return;
-error:
+
 	CALLBACK_WITH_FAILURE(cb, data);
 	g_free(cbd);
 }

@@ -267,43 +256,23 @@ static void isi_query(struct ofono_call_barring *barr, const char *lock, int cls
 {
 	struct barr_data *bd = ofono_call_barring_get_data(barr);
 	struct isi_cb_data *cbd = isi_cb_data_new(barr, cb, data);
-	int ss_code;
+	int ss_code = lock_code_to_mmi(lock);
 
 	unsigned char msg[] = {
 		SS_SERVICE_REQ,
 		SS_INTERROGATION,
 		SS_ALL_TELE_AND_BEARER,
-		0, 0,				/* Supplementary services code */
-		SS_SEND_ADDITIONAL_INFO,
+		ss_code >> 8, ss_code & 0xFF,	/* Supplementary services code */
+		SS_SEND_ADDITIONAL_INFO,	/* Get BER-encoded result, too */
 		0				/* Subblock count */
 	};
 
-	DBG("barring query lock code %s class %d\n", lock, cls);
-
-	if (!cbd || cls != 7)
-		goto error;
-
-	if (strcmp(lock, "AO") == 0)
-		ss_code = SS_GSM_BARR_ALL_OUT;
-	else if (strcmp(lock, "OI") == 0)
-		ss_code = SS_GSM_BARR_OUT_INTER;
-	else if (strcmp(lock, "OX") == 0)
-		ss_code = SS_GSM_BARR_OUT_INTER_EXC_HOME;
-	else if (strcmp(lock, "AI") == 0)
-		ss_code = SS_GSM_BARR_ALL_IN;
-	else if (strcmp(lock, "IR") == 0)
-		ss_code = SS_GSM_BARR_ALL_IN_ROAM;
-	else
-		goto error;
-
-	msg[3] = ss_code >> 8;
-	msg[4] = ss_code & 0xFF;
+	DBG("barring query lock code %s\n", lock);
 
-	if (g_isi_request_make(bd->client, msg, sizeof(msg), SS_TIMEOUT,
-				query_resp_cb, cbd))
+	if (cbd && g_isi_request_make(bd->client, msg, sizeof(msg), SS_TIMEOUT,
+				      query_resp_cb, cbd))
 		return;
 
-error:
 	CALLBACK_WITH_FAILURE(cb, 0, data);
 	g_free(cbd);
 }

@@ -343,60 +312,34 @@ static void isi_set_passwd(struct ofono_call_barring *barr, const char *lock,
 {
 	struct barr_data *bd = ofono_call_barring_get_data(barr);
 	struct isi_cb_data *cbd = isi_cb_data_new(barr, cb, data);
-	int ss_code;
-	char *ucs2 = NULL;
+	int ss_code = lock_code_to_mmi(lock);
 
 	unsigned char msg[] = {
 		SS_SERVICE_REQ,
 		SS_GSM_PASSWORD_REGISTRATION,
 		SS_ALL_TELE_AND_BEARER,
-		0, 0,				/* Supplementary services code */
+		ss_code >> 8, ss_code & 0xFF,	/* Supplementary services code */
 		SS_SEND_ADDITIONAL_INFO,
 		1,				/* Subblock count */
 		SS_GSM_PASSWORD,
 		28,				/* Subblock length */
-		0, 0, 0, 0, 0, 0, 0, 0,		/* Old password */
-		0, 0, 0, 0, 0, 0, 0, 0,		/* New password */
-		0, 0, 0, 0, 0, 0, 0, 0,		/* New password */
+		0, old_passwd[0], 0, old_passwd[1],
+		0, old_passwd[2], 0, old_passwd[3],
+		0, new_passwd[0], 0, new_passwd[1],
+		0, new_passwd[2], 0, new_passwd[3],
+		0, new_passwd[0], 0, new_passwd[1],
+		0, new_passwd[2], 0, new_passwd[3],
 		0, 0				/* Filler */
 	};
 
-	if (!cbd || strlen(old_passwd) > 4 || strlen(new_passwd) > 4)
-		goto error;
-
-	DBG("lock code %s old password %s new password %s\n",
-		lock, old_passwd, new_passwd);
-
-	if (strcmp(lock, "AB") == 0)
-		ss_code = SS_GSM_ALL_BARRINGS;
-	else
-		goto error;
-
-	msg[3] = ss_code >> 8;
-	msg[4] = ss_code & 0xFF;
+	DBG("lock code %s (%u) old password %s new password %s\n",
+	    lock, ss_code, old_passwd, new_passwd);
 
-	ucs2 = g_convert(old_passwd, 4, "UCS-2BE", "UTF-8//TRANSLIT",
-				NULL, NULL, NULL);
-	if (ucs2 == NULL)
-		goto error;
-
-	memcpy((char *)msg + 9, ucs2, 8);
-	g_free(ucs2);
-
-	ucs2 = g_convert(new_passwd, 4, "UCS-2BE", "UTF-8//TRANSLIT",
-				NULL, NULL, NULL);
-	if (ucs2 == NULL)
-		goto error;
-
-	memcpy((char *)msg + 17, ucs2, 8);
-	memcpy((char *)msg + 25, ucs2, 8);
-	g_free(ucs2);
-
-	if (g_isi_request_make(bd->client, msg, sizeof(msg), SS_TIMEOUT,
+	if (cbd &&
+	    g_isi_request_make(bd->client, msg, sizeof(msg), SS_TIMEOUT,
 				set_passwd_resp_cb, cbd))
 		return;
 
-error:
 	CALLBACK_WITH_FAILURE(cb, data);
 	g_free(cbd);
 }
drivers/isimodem/ss.h +15 -13
index 7c20bcc..2313ae6 100644
@@ -81,19 +81,21 @@ enum ss_basic_service_codes {
 };
 
 enum ss_codes {
-	SS_GSM_ALL_FORWARDINGS = 0x02,
-	SS_GSM_ALL_COND_FORWARDINGS = 0x04,
-	SS_GSM_FORW_UNCONDITIONAL = 0x15,
-	SS_GSM_BARR_ALL_OUT = 0x21,
-	SS_GSM_BARR_ALL_IN = 0x23,
-	SS_GSM_CALL_WAITING = 0x2B,
-	SS_GSM_FORW_NO_REPLY = 0x3D,
-	SS_GSM_FORW_NO_REACH = 0x3E,
-	SS_GSM_FORW_BUSY = 0x43,
-	SS_GSM_ALL_BARRINGS = 0x014A,
-	SS_GSM_BARR_OUT_INTER = 0x014B,
-	SS_GSM_BARR_OUT_INTER_EXC_HOME = 0x014C,
-	SS_GSM_BARR_ALL_IN_ROAM = 0x015F
+	SS_GSM_ALL_FORWARDINGS = 002,
+	SS_GSM_ALL_COND_FORWARDINGS = 004,
+	SS_GSM_FORW_UNCONDITIONAL = 21,
+	SS_GSM_BARR_ALL_OUT = 33,
+	SS_GSM_OUTGOING_BARR_SERV = 333,
+	SS_GSM_INCOMING_BARR_SERV = 353,
+	SS_GSM_BARR_ALL_IN = 35,
+	SS_GSM_CALL_WAITING = 43,
+	SS_GSM_FORW_NO_REPLY = 61,
+	SS_GSM_FORW_NO_REACH = 62,
+	SS_GSM_FORW_BUSY = 67,
+	SS_GSM_ALL_BARRINGS = 330,
+	SS_GSM_BARR_OUT_INTER = 331,
+	SS_GSM_BARR_OUT_INTER_EXC_HOME = 332,
+	SS_GSM_BARR_ALL_IN_ROAM = 351,
 };
 
 enum ss_response_data {
Signature · 1 line
1.6.3.3
#1 lore raw

Hi Pekka,

Quoted 3 lines

From: Pekka Pessi <[email protected]>

Add support for lock codes AG and AC (MMI codes 333 and 353. respectively).

First of all, please keep your commit messages less than 72 characters long and headings less than 50 characters long.

Secondly, please keep your patches whitespace clean, there are several cases of spaces being used instead of tabs for indentation.

Quoted 11 lines
---
 drivers/isimodem/call-barring.c |  153
 ++++++++++++--------------------------- drivers/isimodem/ss.h           | 
  28 ++++----
 2 files changed, 63 insertions(+), 118 deletions(-)
(quoted diff) +0 -0
 b/drivers/isimodem/call-barring.c index 6487ae8..b0246fa 100644

drivers/isimodem/call-barring.c struct barr_data { · line 53

Quoted 6 lines
drivers/isimodem/call-barring.c +2 -0
@@ -48,6 +48,28 @@ struct barr_data {
 	GIsiClient *client;
 };
 
 
+	DBG("lock code %s (%u) old password %s new password %s\n",
+	    lock, ss_code, old_passwd, new_passwd);

Here is an instance of whitespace error.

drivers/isimodem/call-barring.c static void isi_set_passwd(struct ofono_call_barring *barr, const char *lock, · line 341

Quoted 5 lines
drivers/isimodem/call-barring.c +2 -1
-	if (g_isi_request_make(bd->client, msg, sizeof(msg), SS_TIMEOUT,
+	if (cbd &&
+	    g_isi_request_make(bd->client, msg, sizeof(msg), SS_TIMEOUT,
 				set_passwd_resp_cb, cbd))
 		return;

Here's another one..

Quoted 6 lines

enum ss_codes {

drivers/isimodem/call-barring.c +0 -5
-	SS_GSM_ALL_FORWARDINGS = 0x02,
-	SS_GSM_ALL_COND_FORWARDINGS = 0x04,
-	SS_GSM_FORW_UNCONDITIONAL = 0x15,
-	SS_GSM_BARR_ALL_OUT = 0x21,
-	SS_GSM_BARR_ALL_IN = 0x23,

drivers/isimodem/call-barring.c

Quoted 10 lines
drivers/isimodem/call-barring.c +2 -8
-	SS_GSM_CALL_WAITING = 0x2B,
-	SS_GSM_FORW_NO_REPLY = 0x3D,
-	SS_GSM_FORW_NO_REACH = 0x3E,
-	SS_GSM_FORW_BUSY = 0x43,
-	SS_GSM_ALL_BARRINGS = 0x014A,
-	SS_GSM_BARR_OUT_INTER = 0x014B,
-	SS_GSM_BARR_OUT_INTER_EXC_HOME = 0x014C,
-	SS_GSM_BARR_ALL_IN_ROAM = 0x015F
+	SS_GSM_ALL_FORWARDINGS = 002,
+	SS_GSM_ALL_COND_FORWARDINGS = 004,

Are you sure this isn't octal syntax? It still works, but you might simply use 2 and 4 here.

Quoted 4 lines
drivers/isimodem/call-barring.c +4 -0
+	SS_GSM_FORW_UNCONDITIONAL = 21,
+	SS_GSM_BARR_ALL_OUT = 33,
+	SS_GSM_OUTGOING_BARR_SERV = 333,
+	SS_GSM_INCOMING_BARR_SERV = 353,

drivers/isimodem/call-barring.c

Quoted 10 lines
drivers/isimodem/call-barring.c +9 -0
+	SS_GSM_BARR_ALL_IN = 35,
+	SS_GSM_CALL_WAITING = 43,
+	SS_GSM_FORW_NO_REPLY = 61,
+	SS_GSM_FORW_NO_REACH = 62,
+	SS_GSM_FORW_BUSY = 67,
+	SS_GSM_ALL_BARRINGS = 330,
+	SS_GSM_BARR_OUT_INTER = 331,
+	SS_GSM_BARR_OUT_INTER_EXC_HOME = 332,
+	SS_GSM_BARR_ALL_IN_ROAM = 351,
 };

From now on we'll require all enum values to be tab aligned so they're a bit easier to read. See src/stkutil.h for reference.

Regards,
-Denis

#2 lore raw

ti, 2010-03-23 kello 22:53 +0100, ext Denis Kenzior kirjoitti:

Quoted 2 lines

Secondly, please keep your patches whitespace clean, there are several cases of spaces being used instead of tabs for indentation.

Pekka is an emacs guy, and emacs likes syntactic indentation a lot. I'm using this custom mode with fairly good results:

(defun linux-c-mode ()
"C mode with adjusted defaults for use with the Linux kernel." (interactive)
(c-mode)
(c-set-style "K&R")
(setq c-syntactic-indentation nil)
(setq c-basic-offset 8))

Cheers,
Aki

author
#3 lore raw
In reply to Denis Kenzior

2010/3/23 Denis Kenzior <[email protected]>:

Quoted 4 lines
Quoted 1 line

Add support for lock codes AG and AC (MMI codes 333 and 353. respectively).

First of all, please keep your commit messages less than 72 characters long and headings less than 50 characters long.

Is there any git setting or nice filter which could complain automatically if the limits are reached?

Quoted 5 lines
Quoted 2 lines

+     SS_GSM_ALL_FORWARDINGS = 002,
+     SS_GSM_ALL_COND_FORWARDINGS = 004,

Are you sure this isn't octal syntax?  It still works, but you might simply use 2 and 4  here.

Those values equal to MMI codes (002 and 004); I just think it is a good idea to show it.

Quoted 2 lines

From now on we'll require all enum values to be tab aligned so they're a bit easier to read.  See src/stkutil.h for reference.

I've reformatted <ss.h>.

Signature · 1 line
Pekka.Pessi mail at nokia.com