Re: [PATCH] EMI UCP Numerical message for OT 01

I'm -0 for this patch

As it's presented as a new feature, your patch looks incomplete.
Please refer to EMI/UCP 4.6 Specs. (MT=4 is missing for example)
Then you are using redundant  code when you initiate emimsg->fields[E01_AMSG]

Vincent.

--
Telemaque - 06200 NICE - (FR)
Service Technique/Reseau - NOC 
Developpement SMS/MMS/Kiosques
http://www.telemaque.fr/
[email protected]
Tel : +33 4 93 97 71 64 (fax 68)

----- Original Message ----- 
From: "Colin Pitrat" <[email protected]>
To: "Kannel Devel" <[email protected]>
Sent: Friday, August 18, 2006 2:57 PM
Subject: [PATCH] EMI UCP Numerical message for OT 01


Hi,
this patch add support for numerical message for operation call input 
operation (Call input operation is OT=01, Numerical message is MT=2).
As we're dealing with numerical data, there should be no need of charset 
conversion.

Regards,
-- 
Colin Pitrat (Bull Services Telco)
Bull,  Architect of an Open World (TM)
Tél : +33 (0)  1 30 80 72 93
www.bull.com



--------------------------------------------------------------------------------


> --- gateway/gw/smsc/smsc_emi.c 2006-08-18 10:38:14.000000000 +0200
> +++ gateway-new/gw/smsc/smsc_emi.c 2006-08-18 14:37:59.000000000 +0200
> @@ -585,12 +585,6 @@
>      */
>     case 01:
>  msg = msg_create(sms);
> - if (emimsg->fields[E01_AMSG] == NULL)
> -     emimsg->fields[E01_AMSG] = octstr_create("");
> - else if (octstr_hex_to_binary(emimsg->fields[E01_AMSG]) == -1)
> -     warning(0, "EMI2[%s]: Couldn't decode message text",
> -     octstr_get_cstr(privdata->name));
> -
>  if (emimsg->fields[E01_MT] == NULL) {
>      warning(0, "EMI2[%s]: required field MT missing",
>      octstr_get_cstr(privdata->name));
> @@ -599,21 +593,38 @@
>      emimsg->fields[E01_MT] = octstr_create("3");
>  }
> 
> - if (octstr_get_char(emimsg->fields[E01_MT], 0) == '3') {
> -     msg->sms.msgdata = emimsg->fields[E01_AMSG];
> -     emimsg->fields[E01_AMSG] = NULL; /* So it's not freed */
> -
> -        /* obey the NRC (national replacement codes) */
> -        if (privdata->alt_charset == EMI_NRC_ISO_21)
> -            charset_nrc_iso_21_german_to_gsm(msg->sms.msgdata);
> + switch(octstr_get_char(emimsg->fields[E01_MT], 0))
> + {
> +     case '2':
> + if (emimsg->fields[E01_AMSG] == NULL)
> +     emimsg->fields[E01_AMSG] = octstr_create("");
> + msg->sms.msgdata = emimsg->fields[E01_AMSG];
> + emimsg->fields[E01_AMSG] = NULL; /* So it's not freed */
> + break;
> 
> -     charset_gsm_to_latin1(msg->sms.msgdata);
> - }
> - else {
> -     error(0, "EMI2[%s]: MT == %s isn't supported for operation type 01",
> -   octstr_get_cstr(privdata->name),
> -   octstr_get_cstr(emimsg->fields[E01_MT]));
> -     msg->sms.msgdata = octstr_create("");
> +     case '3':
> + if (emimsg->fields[E01_AMSG] == NULL)
> +     emimsg->fields[E01_AMSG] = octstr_create("");
> + else if (octstr_hex_to_binary(emimsg->fields[E01_AMSG]) == -1)
> +     warning(0, "EMI2[%s]: Couldn't decode message text",
> +     octstr_get_cstr(privdata->name));
> +
> + msg->sms.msgdata = emimsg->fields[E01_AMSG];
> + emimsg->fields[E01_AMSG] = NULL; /* So it's not freed */
> +
> + /* obey the NRC (national replacement codes) */
> + if (privdata->alt_charset == EMI_NRC_ISO_21)
> +     charset_nrc_iso_21_german_to_gsm(msg->sms.msgdata);
> +
> + charset_gsm_to_latin1(msg->sms.msgdata);
> + break; 
> +
> +     default:
> + error(0, "EMI2[%s]: MT == %s isn't supported for operation type 01",
> + octstr_get_cstr(privdata->name),
> + octstr_get_cstr(emimsg->fields[E01_MT]));
> + msg->sms.msgdata = octstr_create("");
> + break;
>  }
> 
>  msg->sms.sender = octstr_duplicate(emimsg->fields[E01_OADC]);
>