Re: user guide

Hi,

thanks for patches!

Some notes:
- please check addr length before any charset converts. Maybe I'm wrong 
there, opinions from others?
- in a patch alpha_gsm patch your use SMPP_CHARSET that is not defined 
in this patch. Please rename it to something like SMPP_DEFAULT_CHARSET.
- don't you think that this is a ugly hack to replace '@'? I don't think 
it belongs into smpp module. Opinions from others?



Thanks,
Alex

Peter Christensen schrieb:
> Hi again,
> 
> I've attached revised versions of the cp1252 patch and the alpha_gsm patch.
> 
> Changes to cp1252 patch:
>  * Change default character set from ISO-8859-1 to WINDOWS-1252 in user 
> guide
> 
> Changed to alpha_gsm patch:
>  * Feature is controlled with alt-addr-charset. Possible values are 
> "gsm" or any iconv charset. Default is windows-1252.
>  * convert_addr_from_pdu() now converts FROM alt-addr-charset to 
> windows-1252 and not the other way around (are used for incoming messages)
>  * Added documentation of alt-addr-charset to user guide
> 
> 
> Just a side note: Kannel rejects messages with alphanumeric addresses 
> longer than 11 characters, which have its origin in the GSM charset. 
> Basically, when default assumed charset is windows-1252 (or previously 
> iso-8859-1), the string "test [test]" is actually still invalid since [ 
> and ] both takes up two septets in the GSM charset. Anyway, since kannel 
> can now be configured to assume utf-8 or whatever, an alphanumeric 
> address could actually be 44 characters long and still be valid (11 
> greek letters).
> 
> 
> Med venlig hilsen / Best regards
> 
> Peter Christensen
> 
> Developer
> ------------------
> Cool Systems ApS
> 
> Tel: +45 2888 1600
>  @ : [email protected]
> www: www.coolsystems.dk
> 
> 
> Hillel wrote:
>> Hi,
>>
>> I apologise for not waiting for your response.  If you had not replied I
>> would have assumed you were not going to submit the patch.  Unfortunately
>> from previous experience a developer was emailing the list and me about a
>> patch, which got people excited, he then never completed it. This 
>> patch is
>> fortunately exactly at the time I need it and for that I thank you. I was
>> hoping you would follow this to completion, but if not then I needed to
>> know.
>>
>> A suggested format:
>>
>> sender-charset = <charset>  (The charset for the sender address)
>>
>> msg-charset = <charset> (The charset for the message)
>>
>> With the default current charsets as per Alex's email (sender: latin1, 
>> msg:
>> gsm)
>>
>> Maybe others on the Kannel Devel list have better ideas.
>>
>> Rgds
>>
>>> -----Original Message-----
>>> From: Peter Christensen [mailto:[email protected]]
>>> Sent: 18 January 2006 15:38
>>> To: [email protected]
>>> Cc: Kannel Devel
>>> Subject: Re: user guide
>>>
>>>
>>> Hi,
>>>
>>> Yes, I know that. No need to tell me that three times. But please be
>>> patient, although I WOULD like to have the work done quickly, I don't
>>> have an infinite amount of spare time, and some things are more urgent.
>>> I hope that I will have revised patches (including user guide
>>> documentation) done within this week, but I cannot promise anything.
>>>
>>> Btw., any suggestions to what I should call the alpha_gsm option, and
>>> exactly how it should work? One way would be
>>>
>>> addr-charset = <charset>
>>>
>>> In this way the user will be able to use practically any charset, but
>>> the consequence is that GSM charset becomes the default, which might not
>>> be desirable. Alternatively the option could be boolean:
>>>
>>> addr-gsm = <boolean>
>>>
>>>
>>> Med venlig hilsen / Best regards
>>>
>>> Peter Christensen
>>>
>>> Developer
>>> ------------------
>>> Cool Systems ApS
>>>
>>> Tel: +45 2888 1600
>>>   @ : [email protected]
>>> www: www.coolsystems.dk
>>>
>>>
>>> Hillel wrote:
>>>> Hi,
>>>>
>>>> As per email from Alexander Malysh <[email protected]>
>>>> Date: Fri, 13 Jan 2006 17:24:01 +0100
>>>> From: Alexander Malysh <[email protected]>
>>>> Subject: Re: please reconsider the +0 you gave the patches
>>>>
>>>> Both your patches kannel.smpp.cp1252.diff and
>>>> kannel.smpp.alpha_gsm.diff.have been accepted with added configurable
>>>> parameters for the body & source_addr independently.
>>>>
>>>> Please when you submit your patch, which I'd like to start
>>> using, will you
>>>> send through some details to the user guide on which char set
>>> corresponds to
>>>> which configurable option for the body and source independently and 
>>>> what
>>>> options to set to send the @, pound,Euro and ¡ symbols.
>>>>
>>>> Thanks for adding these patches.
>>>>
>>>> Rgds
>>>>
>>>>
>>>
>>
>>
>>
> 
> ------------------------------------------------------------------------
> 
> diff -Nru gateway.cp1252/doc/userguide/userguide.xml gateway.alpha_gsm/doc/userguide/userguide.xml
> --- gateway.cp1252/doc/userguide/userguide.xml	2006-01-18 17:39:36.000000000 +0100
> +++ gateway.alpha_gsm/doc/userguide/userguide.xml	2006-01-18 17:39:50.000000000 +0100
> @@ -3177,6 +3177,17 @@
>  		  should be presented for this directive.
>       </entry></row>
>  
> +     <row><entry><literal>alt-addr-charset</literal></entry>
> +        <entry><literal>string</literal></entry>
> +        <entry valign="bottom">
> +            Defines which character encoding is used for alphanumeric addresses.
> +            When set to <literal>GSM</literal>, addresses are converted into the
> +            GSM 03.38 charset (Since @ is translated into 0x00 which will break
> +			the SMPP PDU, @ are stripped from addresses). If set to another value,
> +            <literal>iconv()</literal> is used.
> +            (Defaults to latin1)
> +        </entry></row>
> +
>      <row><entry><literal>connection-timeout</literal></entry>
>        <entry><literal>number (seconds)</literal></entry>
>        <entry valign="bottom">
> diff -Nru gateway.cp1252/gw/smsc/smsc_smpp.c gateway.alpha_gsm/gw/smsc/smsc_smpp.c
> --- gateway.cp1252/gw/smsc/smsc_smpp.c	2006-01-18 17:39:36.000000000 +0100
> +++ gateway.alpha_gsm/gw/smsc/smsc_smpp.c	2006-01-18 17:59:47.000000000 +0100
> @@ -158,6 +158,7 @@
>      int smpp_msg_id_type;  /* msg id in C string, hex or decimal */
>      int autodetect_addr;
>      Octstr *alt_charset;
> +    Octstr *alt_addr_charset;
>      long connection_timeout;
>      long wait_ack;
>      int wait_ack_action;
> @@ -211,7 +212,7 @@
>                           int enquire_link_interval, int max_pending_submits, 
>                           int version, int priority, int validity,
>                           Octstr *my_number, int smpp_msg_id_type, 
> -                         int autodetect_addr, Octstr *alt_charset, 
> +                         int autodetect_addr, Octstr *alt_charset, Octstr *alt_addr_charset,
>                           Octstr *service_type, long connection_timeout,
>                           long wait_ack, int wait_ack_action) 
>  { 
> @@ -250,6 +251,7 @@
>      smpp->smpp_msg_id_type = smpp_msg_id_type;    
>      smpp->autodetect_addr = autodetect_addr;
>      smpp->alt_charset = octstr_duplicate(alt_charset);
> +    smpp->alt_addr_charset = octstr_duplicate(alt_addr_charset);
>      smpp->connection_timeout = connection_timeout;
>      smpp->wait_ack = wait_ack;
>      smpp->wait_ack_action = wait_ack_action;
> @@ -273,6 +275,7 @@
>          octstr_destroy(smpp->address_range); 
>          octstr_destroy(smpp->my_number);
>          octstr_destroy(smpp->alt_charset);
> +        octstr_destroy(smpp->alt_addr_charset);
>          gw_free(smpp); 
>      } 
>  } 
> @@ -327,7 +330,7 @@
>  }
>  
>  
> -static long convert_addr_from_pdu(Octstr *id, Octstr *addr, long ton, long npi)
> +static long convert_addr_from_pdu(Octstr *id, Octstr *addr, long ton, long npi, Octstr *alt_addr_charset)
>  {
>      long reason = SMPP_ESME_ROK;
>      
> @@ -372,6 +375,15 @@
>  
>          break;
>      case GSM_ADDR_TON_ALPHANUMERIC:
> +        if (alt_addr_charset) {
> +            if (octstr_str_case_compare(alt_addr_charset, "gsm") == 0)
> +				charset_gsm_to_latin1(addr);
> +            else {
> +                if (charset_convert(addr, octstr_get_cstr(alt_addr_charset), SMPP_CHARSET) != 0)
> +                    error(0, "Failed to convert address from charset <%s> to <%s>, will send as is.",
> +                            octstr_get_cstr(alt_addr_charset), SMPP_CHARSET);
> +            }
> +        }
>          if (octstr_len(addr) > 11) {
>              /* alphanum sender, max. allowed length is 11 (according to GSM specs) */
>              error(0, "SMPP[%s]: Mallformed addr `%s', alphanum length greater 11 chars. ",
> @@ -416,7 +428,7 @@
>      ton = pdu->u.deliver_sm.source_addr_ton;
>      npi = pdu->u.deliver_sm.source_addr_npi;
>      /* check source addr */
> -    if ((*reason = convert_addr_from_pdu(smpp->conn->id, pdu->u.deliver_sm.source_addr, ton, npi)) != SMPP_ESME_ROK)
> +    if ((*reason = convert_addr_from_pdu(smpp->conn->id, pdu->u.deliver_sm.source_addr, ton, npi, smpp->alt_addr_charset)) != SMPP_ESME_ROK)
>          goto error;
>      msg->sms.sender = pdu->u.deliver_sm.source_addr;
>      pdu->u.deliver_sm.source_addr = NULL;
> @@ -437,7 +449,7 @@
>      ton = pdu->u.deliver_sm.dest_addr_ton;
>      npi = pdu->u.deliver_sm.dest_addr_npi;
>      /* check destination addr */
> -    if ((*reason = convert_addr_from_pdu(smpp->conn->id, pdu->u.deliver_sm.destination_addr, ton, npi)) != SMPP_ESME_ROK)
> +    if ((*reason = convert_addr_from_pdu(smpp->conn->id, pdu->u.deliver_sm.destination_addr, ton, npi, smpp->alt_addr_charset)) != SMPP_ESME_ROK)
>          goto error;
>      msg->sms.receiver = pdu->u.deliver_sm.destination_addr;
>      pdu->u.deliver_sm.destination_addr = NULL;
> @@ -576,7 +588,7 @@
>      ton = pdu->u.data_sm.source_addr_ton;
>      npi = pdu->u.data_sm.source_addr_npi;
>      /* check source addr */
> -    if ((*reason = convert_addr_from_pdu(smpp->conn->id, pdu->u.data_sm.source_addr, ton, npi)) != SMPP_ESME_ROK)
> +    if ((*reason = convert_addr_from_pdu(smpp->conn->id, pdu->u.data_sm.source_addr, ton, npi, smpp->alt_addr_charset)) != SMPP_ESME_ROK)
>          goto error;
>      msg->sms.sender = pdu->u.data_sm.source_addr;
>      pdu->u.data_sm.source_addr = NULL;
> @@ -597,7 +609,7 @@
>      ton = pdu->u.data_sm.dest_addr_ton;
>      npi = pdu->u.data_sm.dest_addr_npi;
>      /* check destination addr */
> -    if ((*reason = convert_addr_from_pdu(smpp->conn->id, pdu->u.data_sm.destination_addr, ton, npi)) != SMPP_ESME_ROK)
> +    if ((*reason = convert_addr_from_pdu(smpp->conn->id, pdu->u.data_sm.destination_addr, ton, npi, smpp->alt_addr_charset)) != SMPP_ESME_ROK)
>          goto error;
>      msg->sms.receiver = pdu->u.data_sm.destination_addr;
>      pdu->u.data_sm.destination_addr = NULL;
> @@ -749,6 +761,15 @@
>              if (!octstr_check_range(pdu->u.submit_sm.source_addr, 1, 256, gw_isdigit)) {
>                  pdu->u.submit_sm.source_addr_ton = GSM_ADDR_TON_ALPHANUMERIC; /* alphanum */
>                  pdu->u.submit_sm.source_addr_npi = GSM_ADDR_NPI_UNKNOWN;    /* short code */
> +                if (smpp->alt_addr_charset) {
> +                    if (octstr_str_case_compare(smpp->alt_addr_charset, "gsm") == 0) {
> +                        octstr_replace(pdu->u.submit_sm.source_addr, octstr_imm("@"), octstr_imm("")); /* @ would break PDU if converted into GSM*/
> +                        charset_latin1_to_gsm(pdu->u.submit_sm.source_addr);
> +                    }
> +                    else if (charset_convert(pdu->u.submit_sm.source_addr, SMPP_CHARSET, octstr_get_cstr(smpp->alt_addr_charset)) != 0)
> +                        error(0, "Failed to convert source_addr from charset <%s> to <%s>, will send as is.",
> +                                SMPP_CHARSET, octstr_get_cstr(smpp->alt_addr_charset));
> +                }
>              } else {
>                 /* numeric sender address with + in front -> international (remove the +) */
>                 octstr_delete(pdu->u.submit_sm.source_addr, 0, 1);
> @@ -758,6 +779,15 @@
>              if (!octstr_check_range(pdu->u.submit_sm.source_addr,0, 256, gw_isdigit)) {
>                  pdu->u.submit_sm.source_addr_ton = GSM_ADDR_TON_ALPHANUMERIC;
>                  pdu->u.submit_sm.source_addr_npi = GSM_ADDR_NPI_UNKNOWN;
> +                if (smpp->alt_addr_charset) {
> +                    if (octstr_str_case_compare(smpp->alt_addr_charset, "gsm") == 0) {
> +                        octstr_replace(pdu->u.submit_sm.source_addr, octstr_imm("@"), octstr_imm("")); /* @ would break PDU if converted into GSM */
> +                        charset_latin1_to_gsm(pdu->u.submit_sm.source_addr);
> +                    }
> +                    else if (charset_convert(pdu->u.submit_sm.source_addr, SMPP_CHARSET, octstr_get_cstr(smpp->alt_addr_charset)) != 0)
> +                        error(0, "Failed to convert source_addr from charset <%s> to <%s>, will send as is.",
> +                                SMPP_CHARSET, octstr_get_cstr(smpp->alt_addr_charset));
> +                }
>              }
>          }
>      }
> @@ -1938,9 +1968,10 @@
>      long smpp_msg_id_type;
>      int autodetect_addr;
>      Octstr *alt_charset;
> +    Octstr *alt_addr_charset;
>      long connection_timeout, wait_ack, wait_ack_action;
>  
> -    my_number = alt_charset = NULL;
> +    my_number = alt_addr_charset = alt_charset = NULL;
>      transceiver_mode = 0;
>      autodetect_addr = 1;
>   
> @@ -2055,6 +2086,7 @@
>  
>      /* check for an alternative charset */
>      alt_charset = cfg_get(grp, octstr_imm("alt-charset"));
> +    alt_addr_charset = cfg_get(grp, octstr_imm("alt-addr-charset"));
>      
>      /* check for connection timeout */
>      if (cfg_get_integer(&connection_timeout, grp, octstr_imm("connection-timeout")) == -1)
> @@ -2075,7 +2107,7 @@
>                         source_addr_ton, source_addr_npi, dest_addr_ton,  
>                         dest_addr_npi, enquire_link_interval, 
>                         max_pending_submits, version, priority, validity, my_number, 
> -                       smpp_msg_id_type, autodetect_addr, alt_charset, 
> +                       smpp_msg_id_type, autodetect_addr, alt_charset, alt_addr_charset,
>                         service_type, connection_timeout, wait_ack, wait_ack_action); 
>   
>      conn->data = smpp; 
> @@ -2097,6 +2129,7 @@
>      octstr_destroy(my_number); 
>      octstr_destroy(smsc_id);
>      octstr_destroy(alt_charset); 
> +    octstr_destroy(alt_addr_charset);
>      octstr_destroy(service_type);
>  
>      conn->status = SMSCCONN_CONNECTING; 
> diff -Nru gateway.cp1252/gwlib/cfg.def gateway.alpha_gsm/gwlib/cfg.def
> --- gateway.cp1252/gwlib/cfg.def	2006-01-18 17:39:19.000000000 +0100
> +++ gateway.alpha_gsm/gwlib/cfg.def	2006-01-18 17:39:50.000000000 +0100
> @@ -365,6 +365,7 @@
>      OCTSTR(denied-prefix-regex)
>      OCTSTR(preferred-prefix-regex)
>      OCTSTR(max-error-count)
> +    OCTSTR(alt-addr-charset)
>  )
>  
>  
> 
> 
> ------------------------------------------------------------------------
> 
> diff -Nru gateway/doc/userguide/userguide.xml gateway.cp1252/doc/userguide/userguide.xml
> --- gateway/doc/userguide/userguide.xml	2005-12-26 21:13:39.000000000 +0100
> +++ gateway.cp1252/doc/userguide/userguide.xml	2006-01-18 16:17:16.000000000 +0100
> @@ -5183,10 +5183,10 @@
>       <entry>boolean</entry>
>       <entry valign="bottom">
>          If enabled, Kannel will try to convert received messages with UCS-2 charset
> -	to ISO-8859-1 or to UTF-8, simplifying external servers jobs. If Kannel is 
> +	to WINDOWS-1252 or to UTF-8, simplifying external servers jobs. If Kannel is 
>  	able to recode message, it will also change <literal>coding</literal> to 
>  	<literal>7 bits</literal> and <literal>charset</literal> to 
> -	<literal>iso-8859-1</literal> or to <literal>utf-8</literal>. 
> +	<literal>windows-1252</literal> or to <literal>utf-8</literal>. 
>       </entry></row>
>  
>      <row><entry><literal>http-request-retry</literal></entry>
> @@ -5879,7 +5879,7 @@
>  message charset: for a "normal" message, it will 
>  be "GSM" (coding=0), "binary" (coding=1) or 
>  "UTF-16BE" (coding=2). If the message was successfully
> -recoded from Unicode, it will be "ISO-8859-1"
> +recoded from Unicode, it will be "WINDOWS-1252"
>  </entry></row>
>  
>  <row><entry><literal>%u</literal></entry><entry>
> @@ -6912,7 +6912,7 @@
>     <entry><literal>string</literal></entry>
>     <entry valign="bottom">
>          Charset of text message. Used to convert to a format suitable for
> -	7 bits or to UCS-2. Defaults to ISO-8859-1 if coding is 7bits and
> +	7 bits or to UCS-2. Defaults to WINDOWS-1252 if coding is 7bits and
>  	UTF-16BE if coding is UCS-2.
>     </entry></row>
>  
> diff -Nru gateway/gw/smsbox.c gateway.cp1252/gw/smsbox.c
> --- gateway/gw/smsbox.c	2005-12-09 03:14:31.000000000 +0100
> +++ gateway.cp1252/gw/smsbox.c	2006-01-18 16:12:23.000000000 +0100
> @@ -3654,9 +3654,9 @@
>  
>  	if (coding == DC_7BIT) {
>  	    /*
> -	     * For 7 bit, convert to ISO-8859-1
> +         * For 7 bit, convert to CP1252
>  	     */
> -	    if (octstr_recode (octstr_imm ("ISO-8859-1"), charset, body) < 0) {
> +        if (charset_convert (body, octstr_get_cstr(charset), "CP1252") < 0) {
>  		resultcode = -1;
>  	    }
>  	} else if (coding == DC_UCS2) {
> diff -Nru gateway/gw/smsc/smsc_smpp.c gateway.cp1252/gw/smsc/smsc_smpp.c
> --- gateway/gw/smsc/smsc_smpp.c	2005-12-18 21:21:16.000000000 +0100
> +++ gateway.cp1252/gw/smsc/smsc_smpp.c	2006-01-18 16:12:23.000000000 +0100
> @@ -78,6 +78,8 @@
>  #include "sms.h"
>  #include "dlr.h"
>  
> +#define SMPP_CHARSET	"CP1252"
> +
>  /*
>   * Select these based on whether you want to dump SMPP PDUs as they are
>   * sent and received or not. Not dumping should be the default in at least
> @@ -490,9 +492,9 @@
>               * unless it was specified binary, ie. UDH indicator was detected
>               */
>              if (smpp->alt_charset && msg->sms.coding != DC_8BIT) {
> -                if (charset_convert(msg->sms.msgdata, octstr_get_cstr(smpp->alt_charset), "ISO-8859-1") != 0)
> +                if (charset_convert(msg->sms.msgdata, octstr_get_cstr(smpp->alt_charset), SMPP_CHARSET) != 0)
>                      error(0, "Failed to convert msgdata from charset <%s> to <%s>, will leave as is.",
> -                             octstr_get_cstr(smpp->alt_charset), "ISO-8859-1");
> +                             octstr_get_cstr(smpp->alt_charset), SMPP_CHARSET);
>                  msg->sms.coding = DC_7BIT;
>              } else { /* assume GSM 03.38 7-bit alphabet */
>                  charset_gsm_to_latin1(msg->sms.msgdata);
> @@ -640,9 +642,9 @@
>               * unless it was specified binary, ie. UDH indicator was detected
>               */
>              if (smpp->alt_charset && msg->sms.coding != DC_8BIT) {
> -                if (charset_convert(msg->sms.msgdata, octstr_get_cstr(smpp->alt_charset), "ISO-8859-1") != 0)
> +                if (charset_convert(msg->sms.msgdata, octstr_get_cstr(smpp->alt_charset), SMPP_CHARSET) != 0)
>                      error(0, "Failed to convert msgdata from charset <%s> to <%s>, will leave as is.",
> -                             octstr_get_cstr(smpp->alt_charset), "ISO-8859-1");
> +                             octstr_get_cstr(smpp->alt_charset), SMPP_CHARSET);
>                  msg->sms.coding = DC_7BIT;
>              } else { /* assume GSM 03.38 7-bit alphabet */
>                  charset_gsm_to_latin1(msg->sms.msgdata);
> @@ -845,10 +847,10 @@
>              /*
>               * convert to the given alternative charset
>               */
> -            if (charset_convert(pdu->u.submit_sm.short_message, "ISO-8859-1",
> +            if (charset_convert(pdu->u.submit_sm.short_message, SMPP_CHARSET,
>                                  octstr_get_cstr(smpp->alt_charset)) != 0)
>                  error(0, "Failed to convert msgdata from charset <%s> to <%s>, will send as is.",
> -                             "ISO-8859-1", octstr_get_cstr(smpp->alt_charset));
> +                             SMPP_CHARSET, octstr_get_cstr(smpp->alt_charset));
>          }
>      }
>