Re: [PATCH 2/2] ussd: Fix speedup ussd encoding

Re: [PATCH 2/2] ussd: Fix speedup ussd encoding

[Denis Kenzior]

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

Hi Nicolas,

On 04/10/2012 10:35 AM, Nicolas Bertrand wrote:
> Speedup dongles encode USSD strings in GSM7
> packed, so we don't have to pack it ourselves.
> ---
>  drivers/atmodem/ussd.c |   13 ++++++++++---
>  plugins/speedup.c      |    2 +-
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 

Patch looks good, but please split it into two in accordance with our
patch submission guidelines.  See HACKING document, 'Submitting patches'
section.

Regards,
-Denis

Re: [PATCH 1/2] ussd: rise up buffer lenght for ussd strings

[Denis Kenzior]

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

Hi Nicolas,

On 04/10/2012 10:35 AM, Nicolas Bertrand wrote:
> Some ZTE modems used to convert any ussd strings in UCS2,
> and so the buffer length is double. Considering the limit of
> 182 characters, 364 will avoid crash.
> ---
>  drivers/atmodem/ussd.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/atmodem/ussd.c b/drivers/atmodem/ussd.c
> index 443251a..f5b38e2 100644
> --- a/drivers/atmodem/ussd.c
> +++ b/drivers/atmodem/ussd.c
> @@ -106,7 +106,7 @@ static void cusd_parse(GAtResult *result, struct ofono_ussd *ussd)
>  	const char *content;
>  	int dcs;
>  	enum sms_charset charset;
> -	unsigned char msg[160];
> +	unsigned char msg[364];

Actually I'm not happy with this since the core is still expecting raw
160 byte PDUs.  If ZTE reports all USSDs as unpacked UCS2 data, then we
need to pack that back up into packed GSM 7 bit.  This probably requires
a custom driver for ZTE modems.

Alternatively you can see whether ZTE provides some way to modify the
USSD reporting behavior, similar to what Huawei did with ^USSDMODE.

Regards,
-Denis

[PATCH 0/2] ussd fixes

[Nicolas Bertrand]

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

Fix some issues with ussd:
        - Crash with speedup dongles
        - Crash with ZTE dongles

Nicolas Bertrand (2):
  ussd: rise up buffer lenght for ussd strings
  ussd: Fix speedup ussd encoding

 drivers/atmodem/ussd.c |   15 +++++++++++----
 plugins/speedup.c      |    2 +-
 2 files changed, 12 insertions(+), 5 deletions(-)

-- 
1.7.5.4

[PATCH 1/2] ussd: rise up buffer lenght for ussd strings

[Nicolas Bertrand]

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

Some ZTE modems used to convert any ussd strings in UCS2,
and so the buffer length is double. Considering the limit of
182 characters, 364 will avoid crash.
---
 drivers/atmodem/ussd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/atmodem/ussd.c b/drivers/atmodem/ussd.c
index 443251a..f5b38e2 100644
--- a/drivers/atmodem/ussd.c
+++ b/drivers/atmodem/ussd.c
@@ -106,7 +106,7 @@ static void cusd_parse(GAtResult *result, struct ofono_ussd *ussd)
 	const char *content;
 	int dcs;
 	enum sms_charset charset;
-	unsigned char msg[160];
+	unsigned char msg[364];
 	const unsigned char *msg_ptr = NULL;
 	long msg_len;
 
-- 
1.7.5.4

[PATCH 2/2] ussd: Fix speedup ussd encoding

[Nicolas Bertrand]

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

Speedup dongles encode USSD strings in GSM7
packed, so we don't have to pack it ourselves.
---
 drivers/atmodem/ussd.c |   13 ++++++++++---
 plugins/speedup.c      |    2 +-
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/atmodem/ussd.c b/drivers/atmodem/ussd.c
index f5b38e2..e90db11 100644
--- a/drivers/atmodem/ussd.c
+++ b/drivers/atmodem/ussd.c
@@ -136,9 +136,15 @@ static void cusd_parse(GAtResult *result, struct ofono_ussd *ussd)
 	case SMS_CHARSET_7BIT:
 		switch (data->charset) {
 		case AT_UTIL_CHARSET_GSM:
-			msg_ptr = pack_7bit_own_buf((const guint8 *) content,
-							-1, 0, TRUE, &msg_len,
-							0, msg);
+			if (data->vendor == OFONO_VENDOR_SPEEDUP)
+				msg_ptr = decode_hex_own_buf(content, -1,
+								&msg_len,
+								0, msg);
+			else
+				msg_ptr = pack_7bit_own_buf((const guint8 *)
+								content, -1, 0,
+								TRUE, &msg_len,
+								0, msg);
 			break;
 
 		case AT_UTIL_CHARSET_UTF8:
@@ -242,6 +248,7 @@ static void cusd_cancel_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	switch (data->vendor) {
 	case OFONO_VENDOR_GOBI:
 	case OFONO_VENDOR_QUALCOMM_MSM:
+	case OFONO_VENDOR_SPEEDUP:
 		/* All errors and notifications arrive unexpected and
 		 * thus just reset the state here. This is safer than
 		 * getting stuck in a dead-lock. */
diff --git a/plugins/speedup.c b/plugins/speedup.c
index ca6ed13..5cc380f 100644
--- a/plugins/speedup.c
+++ b/plugins/speedup.c
@@ -282,7 +282,7 @@ static void speedup_post_online(struct ofono_modem *modem)
 
 	ofono_cbs_create(modem, OFONO_VENDOR_QUALCOMM_MSM,
 						"atmodem", data->aux);
-	ofono_ussd_create(modem, OFONO_VENDOR_QUALCOMM_MSM,
+	ofono_ussd_create(modem, OFONO_VENDOR_SPEEDUP,
 						"atmodem", data->aux);
 }
 
-- 
1.7.5.4

Re: [PATCH 2/2] ussd: Fix speedup ussd encoding

[Marcel Holtmann]

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

Hi Denis,

> > Speedup dongles encode USSD strings in GSM7
> > packed, so we don't have to pack it ourselves.
> > ---
> >  drivers/atmodem/ussd.c |   13 ++++++++++---
> >  plugins/speedup.c      |    2 +-
> >  2 files changed, 11 insertions(+), 4 deletions(-)
> > 
> 
> Patch looks good, but please split it into two in accordance with our
> patch submission guidelines.  See HACKING document, 'Submitting patches'
> section.

if the modem uses a fixed encoding all the time, would it not be simpler
to just add a specific modem driver for it? Like we did for Huawei and
its binary mode.

Regards

Marcel