RE: [PATCH 02/11] Changes done as per the changed driver API

2010-09-02 Thread Jeevaka.Badrappan
 
Hi Pekka,

> I thought the modem already unpacks the ussd message to 8-bit gsm and
the dcs parameter is rather meaningless in that case. Perhaps the
atmodem ussd should re-pack the string in order to achieve the 
> desired symmetry on the API?

Agree that the modem already unpacks the string received from the
network to the "character set" set by TE(UCS2 or UTF-8 or ...). Atom
driver will do the conversion to UTF-8 based on active character set
supported by modem, so it should have information of the active
character set. At present, we read and set the "character set" in the
phonebook atom driver. USSD atom driver could to do the same or the
reading/setting of character set can be done during the modem
initialisation (eg: atgen.c). I would prefer to have the reading and
setting of character set during the modem initialisation. I hope the
character set is not set by some other component apart from ofono :-)

Regards,
jeevaka




Please note: This e-mail may contain confidential information
intended solely for the addressee. If you have received this
e-mail in error, please do not disclose it to anyone, notify
the sender promptly, and delete the message from your system.
Thank you.




Please note: This e-mail may contain confidential information
intended solely for the addressee. If you have received this
e-mail in error, please do not disclose it to anyone, notify
the sender promptly, and delete the message from your system.
Thank you.

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 02/11] Changes done as per the changed driver API

2010-09-01 Thread Andrzej Zaborowski
Hi,

On 1 September 2010 13:00, Jeevaka Badrappan
 wrote:
> ---
>  drivers/atmodem/ussd.c |   87 +--
>  1 files changed, 54 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/atmodem/ussd.c b/drivers/atmodem/ussd.c
> index 1e1fc25..c386fcd 100644
> --- a/drivers/atmodem/ussd.c
> +++ b/drivers/atmodem/ussd.c
> @@ -56,11 +56,12 @@ static void cusd_parse(GAtResult *result, struct 
> ofono_ussd *ussd)
>        int status;
>        int dcs;
>        const char *content;
> -       char *converted = NULL;
>        gboolean udhi;
>        enum sms_charset charset;
>        gboolean compressed;
>        gboolean iso639;
> +       unsigned char *msg = NULL;
> +       int msg_len = 0;
>
>        g_at_result_iter_init(&iter, result);
>
> @@ -83,24 +84,15 @@ static void cusd_parse(GAtResult *result, struct 
> ofono_ussd *ussd)
>        } else
>                charset = SMS_CHARSET_7BIT;
>
> -       if (charset == SMS_CHARSET_7BIT)
> -               converted = convert_gsm_to_utf8((const guint8 *) content,
> -                                               strlen(content), NULL, NULL, 
> 0);
> -
> -       else if (charset == SMS_CHARSET_8BIT) {
> -               /* TODO: Figure out what to do with 8 bit data */
> -               ofono_error("8-bit coded USSD response received");
> -               status = 4; /* Not supported */
> -       } else {
> -               /* No other encoding is mentioned in TS27007 7.15 */
> -               ofono_error("Unsupported USSD data coding scheme (%02x)", 
> dcs);
> -               status = 4; /* Not supported */
> + out:
> +       if (content){
> +               msg = g_memdup(content, strlen(content));
> +               msg_len = strlen(content);
>        }
>
> -out:
> -       ofono_ussd_notify(ussd, status, converted);
> +       ofono_ussd_notify(ussd, status, dcs, msg, msg_len);
>
> -       g_free(converted);
> +       g_free(msg);
>  }

I agree with Pekka that maybe this should repack the 7-bit messages
into packed 7-bit, as this is what STK will be expecting in response
(I think).  Also, why are we g_memdup'ing the content instead of using
directly?

>
>  static void cusd_request_cb(gboolean ok, GAtResult *result, gpointer 
> user_data)
> @@ -117,47 +109,76 @@ static void cusd_request_cb(gboolean ok, GAtResult 
> *result, gpointer user_data)
>        cusd_parse(result, ussd);
>  }
>
> -static void at_ussd_request(struct ofono_ussd *ussd, const char *str,
> -                               ofono_ussd_cb_t cb, void *user_data)
> +static void at_ussd_request(struct ofono_ussd *ussd, int dcs,
> +                            const unsigned char *str, int str_len,
> +                            ofono_ussd_cb_t cb, void *user_data)
>  {
>        struct ussd_data *data = ofono_ussd_get_data(ussd);
>        struct cb_data *cbd = cb_data_new(cb, user_data);
> +       int dcs_value = 0x0f;  // GSM 7 bit default alphabet
>        unsigned char *converted = NULL;
> -       int dcs;
> -       int max_len;
> +       unsigned char *coded_str = NULL;
> +       long coded_str_len;
> +       long num_packed;
>        long written;
>        char buf[256];
> +       char *encoded_data = NULL;
>
>        if (!cbd)
>                goto error;
>
>        cbd->user = ussd;
>
> -       converted = convert_utf8_to_gsm(str, strlen(str), NULL, &written, 0);
> +       if (dcs == -1){
> +               converted = convert_utf8_to_gsm((const char *) str, str_len, 
> NULL, &written, 0);
> +
> +               if (!converted)
> +                       goto error;
> +
> +               /* As per 3GPP TS 23.038, When this character set is used,
> +                * the characters of the message are packed in octets as 
> shown in section 6.1.2.1.1,
> +                * and the message can consist of up to 160 characters.
> +                */
> +
> +               coded_str = pack_7bit(converted, written, 0, TRUE,
> +                                     &num_packed, 0);

You could use pack_7bit_own_buf here too.

Please check the style doc about spaces after/before {}.

Best regards
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 02/11] Changes done as per the changed driver API

2010-09-01 Thread Pekka Pessi
Hi Jeevaka,

2010/9/1 Jeevaka Badrappan :
> ---
>  drivers/atmodem/ussd.c |   87 +--
>  1 files changed, 54 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/atmodem/ussd.c b/drivers/atmodem/ussd.c
> index 1e1fc25..c386fcd 100644
> --- a/drivers/atmodem/ussd.c
> +++ b/drivers/atmodem/ussd.c
> @@ -56,11 +56,12 @@ static void cusd_parse(GAtResult *result, struct 
> ofono_ussd *ussd)
>        int status;
>        int dcs;
>        const char *content;
> -       char *converted = NULL;
>        gboolean udhi;
>        enum sms_charset charset;
>        gboolean compressed;
>        gboolean iso639;
> +       unsigned char *msg = NULL;
> +       int msg_len = 0;
>
>        g_at_result_iter_init(&iter, result);
>
> @@ -83,24 +84,15 @@ static void cusd_parse(GAtResult *result, struct 
> ofono_ussd *ussd)
>        } else
>                charset = SMS_CHARSET_7BIT;
>
> -       if (charset == SMS_CHARSET_7BIT)
> -               converted = convert_gsm_to_utf8((const guint8 *) content,
> -                                               strlen(content), NULL, NULL, 
> 0);
> -
> -       else if (charset == SMS_CHARSET_8BIT) {
> -               /* TODO: Figure out what to do with 8 bit data */
> -               ofono_error("8-bit coded USSD response received");
> -               status = 4; /* Not supported */
> -       } else {
> -               /* No other encoding is mentioned in TS27007 7.15 */
> -               ofono_error("Unsupported USSD data coding scheme (%02x)", 
> dcs);
> -               status = 4; /* Not supported */
> + out:
> +       if (content){
> +               msg = g_memdup(content, strlen(content));
> +               msg_len = strlen(content);
>        }
>
> -out:
> -       ofono_ussd_notify(ussd, status, converted);
> +       ofono_ussd_notify(ussd, status, dcs, msg, msg_len);

I thought the modem already unpacks the ussd message to 8-bit gsm and
the dcs parameter is rather meaningless in that case. Perhaps the
atmodem ussd should re-pack the string in order to achieve the desired
symmetry on the API?

-- 
Pekka.Pessi mail at nokia.com
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono