Re: Making SMPP esm_class configurable?
Alan McNatty <[email protected]>
| Newsgroups | gmane.comp.mobile.kannel.devel |
|---|---|
| Organization | Catalyst IT Limited |
| Message-ID | <1313393735.3462.5.camel@eos> |
Thanks Nikos, That's good news since, thanks to Edgard for pointing out, I forgot to include gwlib/cfg.def in the patch. Updated patch attached. Cheers, Alan On Mon, 2011-08-15 at 04:29 +0300, Nikos Balkanas wrote: > I guess Alex M is busy. There are a few patches prior to yours that > wait for commission. Don't worry, he never misses a patch. He is the > gateway maintainer. > > > > BR, > Nikos > > On Mon, Aug 15, 2011 at 3:22 AM, Alan McNatty <[email protected]> > wrote: > Thanks Rene, > > Can anyone with commit access give a final nod of acceptance? > > On Tue, 2011-08-09 at 21:08 +0200, Rene Kluwen wrote: > > +1 for me as well. > > > > > > > > From: [email protected] > [mailto:[email protected]] On > > Behalf Of Nikos Balkanas > > Sent: Tuesday, 09 August, 2011 02:11 > > To: Alan McNatty > > Cc: [email protected] > > Subject: Re: Making SMPP esm_class configurable? > > > > > > > > > > Looks great! +1. > > > > > > Nikos > > > > > > > > > > On Tue, Aug 9, 2011 at 2:59 AM, Alan McNatty > <[email protected]> > > wrote: > > > > Thanks again Nikos, > > > > Yeah 0 and 3 are all I'm interested in - wasn't sure if > others wanted > > support for non-compliant things. > > > > Made changes as you suggested - please check out the > attached patch. > > > > Cheers, > > Alan > > > > On Mon, 2011-08-08 at 05:39 +0300, Nikos Balkanas wrote: > > > Hi Alan, > > > > > > > > > Currently kannel doesn't support more modes. Therefore > test should > > > more specific: > > > > > > > > > else if (esm_class != SMPP_STORE... && esm_class != > DEFAULT...)) // > > > use constants > > > > > > > > > Also I see no need for panicking over a single smsc: > > > > > > > > > error(0, "SMPP: Invalid esm_class mode \"5\" in > configuration. > > > Switching to \"Store and Forward\"); > > > > > > > > > There are still many hexadecimal references to the > userguide, and it > > > doesn't restrict choices. Therefore, I suggest the > following text: > > > > > > > > > Value for esm_class according to the SMPP spec. Accepted > values are > > 0 > > > (Default smsc mode) and 3 (Store and Forward). Defaults to > 3. > > > > > > > > > HTH, > > > Nikos > > > > > > On Mon, Aug 8, 2011 at 5:01 AM, Alan McNatty > <[email protected]> > > > wrote: > > > Thanks Nikos, > > > > > > See attached. > > > > > > Also just wanted to check thoughts the range check > (possibly > > > remove and > > > leave it open?). > > > > > > i.e. > > > + else if (esm_class > 0x03 || esm_class < 0) > > > + panic(0, "SMPP: Invalid esm_class > directive in > > > configuration."); > > > > > > > > > On Mon, 2011-08-08 at 04:44 +0300, Nikos Balkanas > wrote: > > > > Hi Alan, > > > > > > > > > > > > Patch looks good. userguide needs some changes: > > > > > > > > > > > > 1) Capitalize after periods (For example, For > default > > mode) > > > > 2) In configuration the value should be integer, > not hex > > (3 > > > not 03). > > > > cfg_get_integer doesn't understand hex (0xA5). > > > > > > > > > > > > +1 > > > > > > > > > > > > > > > > Nikos > > > > > > > > On Mon, Aug 8, 2011 at 4:02 AM, Alan McNatty > > > <[email protected]> > > > > wrote: > > > > patch attached. > > > > > > > > Votes / comments, etc? > > > > > > > > On Wed, 2011-08-03 at 09:39 +1200, Alan > McNatty > > > wrote: > > > > > Thanks Nikos/Alex for the feedback - I > will work > > > on config > > > > patch. > > > > > > > > > > On Tue, 2011-08-02 at 23:10 +0300, > Nikos > > Balkanas > > > wrote: > > > > > > Hi Alan, > > > > > > > > > > > > Just to clarify on what Alex says. > Some other > > > modes that > > > > the SMSc may > > > > > > support in default mode, are: > > > > > > > > > > > > Datagram: UDP based, immediate best > effort > > high > > > throughput > > > > transmition with > > > > > > no retried, validity period or > storage. > > Similar > > > to UDP. > > > > > > Forward: Single transaction based, > for > > real-time > > > > applications, i.e. parking > > > > > > tickets, without storage, where > result is > > > returned in > > > > response. > > > > > > > > > > > > Kannel doesn't support those, only > reliable > > > store and > > > > forward. Therefore the > > > > > > default mode wouldn't be > appropriate. > > > > > > Configuration would be fine for > those buggy > > > SMScs, that do > > > > store and > > > > > > forward, but do not accept it as an > option. > > > > > > > > > > > > BR, > > > > > > Nikos > > > > > > > > > > > > ----- Original Message ----- > > > > > > From: "Alexander Malysh" > <[email protected]> > > > > > > To: "Alan McNatty" > <[email protected]> > > > > > > Cc: "Nikos Balkanas" > <[email protected]>; > > > > <[email protected]> > > > > > > Sent: Tuesday, August 02, 2011 12:29 > PM > > > > > > Subject: Re: Making SMPP esm_class > > configurable? > > > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > please don't change default because > we want > > that > > > SMSC > > > > _store_ and _forward_ > > > > > > our message that > > > > > > is what we also tell SMSC. This > works in 99% > > > cases but > > > > sometimes buggy SMSCs > > > > > > don't accept this > > > > > > and rejects messages. > > > > > > > > > > > > Please keep default as is and make > config > > option > > > for buggy > > > > SMSCs. > > > > > > > > > > > > Thanks, > > > > > > Alex > > > > > > > > > > > > Am 02.08.2011 um 06:11 schrieb Alan > McNatty: > > > > > > > > > > > > > Sorry that should be > > > ESM_CLASS_SUBMIT_DEFAULT_SMSC_MODE. > > > > > > > > > > > > > > Index: gw/smsc/smsc_smpp.c > > > > > > > > > > > > > > > > > =================================================================== > > > > > > > --- gw/smsc/smsc_smpp.c (revision > 4913) > > > > > > > +++ gw/smsc/smsc_smpp.c (working > copy) > > > > > > > @@ -876,7 +876,7 @@ > > > > > > > * set the esm_class field > > > > > > > * default is store and > forward, plus > > udh > > > and rpi if > > > > requested > > > > > > > */ > > > > > > > - pdu->u.submit_sm.esm_class = > > > > > > > > ESM_CLASS_SUBMIT_STORE_AND_FORWARD_MODE; > > > > > > > + pdu->u.submit_sm.esm_class = > > > > ESM_CLASS_SUBMIT_DEFAULT_SMSC_MODE; > > > > > > > if > (octstr_len(msg->sms.udhdata)) > > > > > > > pdu->u.submit_sm.esm_class > = > > > > pdu->u.submit_sm.esm_class | > > > > > > > > ESM_CLASS_SUBMIT_UDH_INDICATOR; > > > > > > > > > > > > > > On Tue, 2011-08-02 at 15:59 +1200, > Alan > > > McNatty wrote: > > > > > > >> Hi Nikos, > > > > > > >> > > > > > > >> You mean simply change the > default: > > > > > > >> > > > > > > >> Index: gw/smsc/smsc_smpp.c > > > > > > >> > > > > > > > > > > =================================================================== > > > > > > >> --- gw/smsc/smsc_smpp.c (revision > 4913) > > > > > > >> +++ gw/smsc/smsc_smpp.c (working > copy) > > > > > > >> @@ -876,7 +876,7 @@ > > > > > > >> * set the esm_class field > > > > > > >> * default is store and > forward, plus > > udh > > > and rpi > > > > if requested > > > > > > >> */ > > > > > > >> - pdu->u.submit_sm.esm_class = > > > > > > >> > ESM_CLASS_SUBMIT_STORE_AND_FORWARD_MODE; > > > > > > >> + pdu->u.submit_sm.esm_class = > > > > ESM_CLASS_DEFAULT_SMSC_MODE; > > > > > > >> if > (octstr_len(msg->sms.udhdata)) > > > > > > >> > pdu->u.submit_sm.esm_class = > > > > pdu->u.submit_sm.esm_class | > > > > > > >> > ESM_CLASS_SUBMIT_UDH_INDICATOR; > > > > > > >> > > > > > > >> Anyone think we should have a > config > > option? > > > Or just > > > > happy to run with > > > > > > >> he above. I need to test myself > but is this > > > likely to > > > > be a compatibility > > > > > > >> breaker for anyone? > > > > > > >> > > > > > > >> Cheers, > > > > > > >> Alan > > > > > > >> > > > > > > >> On Mon, 2011-08-01 at 07:13 > +0300, Nikos > > > Balkanas > > > > wrote: > > > > > > >>> Hi Alan, > > > > > > >>> > > > > > > >>> According to the spec SMPP 5.0, > p 125, > > > > > > >>> > ESM_CLASS_SUBMIT_DEFAULT_SMSC_MODE is > > > > > > >>> the default esm class. That part > should be > > > patched in. > > > > As far as making > > > > > > >>> it > > > > > > >>> configurable, I have no > objections to it. > > A > > > few people > > > > over the years > > > > > > >>> have > > > > > > >>> had to manually patch it in. > > > > > > >>> > > > > > > >>> BR, > > > > > > >>> Nikos > > > > > > >>> ----- Original Message ----- > > > > > > >>> From: "Alan McNatty" > > <[email protected]> > > > > > > >>> To: <[email protected]> > > > > > > >>> Sent: Monday, August 01, 2011 > 6:21 AM > > > > > > >>> Subject: Making SMPP esm_class > > configurable? > > > > > > >>> > > > > > > >>> > > > > > > >>>> Hi All, > > > > > > >>>> > > > > > > >>>> I found a thread on this from > back in Feb > > > 2005 > > > > (having received a query > > > > > > >>>> from provided now myself) .. > last word by > > > Alejandro > > > > and a lukewarm > > > > > > >>>> (+0 - > > > > > > >>>> +1) comment from Stipe about > committing > > if > > > patch > > > > provided. I would > > > > > > >>>> provide a config patch if > anyone would > > vote > > > in it's > > > > favour? > > > > > > >>>> > > > > > > >>>> Consider: > > > > > > >>>> > > > > > > >>>> gw/smsc/smsc_smpp.c > > > > > > >>>> 875 /* > > > > > > >>>> 876 * set the esm_class > field > > > > > > >>>> 877 * default is store and > forward, > > > plus udh and > > > > rpi if requested > > > > > > >>>> 878 */ > > > > > > >>>> 879 > pdu->u.submit_sm.esm_class = > > > > > > >>>> > ESM_CLASS_SUBMIT_STORE_AND_FORWARD_MODE; > > > > > > >>>> > > > > > > >>>> But the 'default' is surely > > > > ESM_CLASS_SUBMIT_DEFAULT_SMSC_MODE, no? > > > > > > >>>> > > > > > > >>>> Cheers, > > > > > > >>>> Alan > > > > > > >>>> > > > > > > >>>> > > > > > > >>>> > > > > > > >>>> > > > > > > >>> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
esm_class.patch
(text/x-patch, 4.2 KB)
Index: gwlib/cfg.def
===================================================================
--- gwlib/cfg.def (revision 4916)
+++ gwlib/cfg.def (working copy)
@@ -369,6 +369,7 @@
OCTSTR(bind-addr-ton)
OCTSTR(bind-addr-npi)
OCTSTR(service-type)
+ OCTSTR(esm-class)
OCTSTR(source-addr-autodetect)
OCTSTR(enquire-link-interval)
OCTSTR(max-pending-submits)
Index: doc/userguide/userguide.xml
===================================================================
--- doc/userguide/userguide.xml (revision 4916)
+++ doc/userguide/userguide.xml (working copy)
@@ -3573,6 +3573,14 @@
try to send the message to the recipient. Defined in minutes.
</entry></row>
+ <row><entry><literal>esm-class</literal></entry>
+ <entry><literal>number</literal></entry>
+ <entry valign="bottom">
+ Change the "esm_class" parameter sent from Kannel.
+ Accepted values are 0 (Default SMSC Mode) and 3 (Store and Forward).
+ Defaults to 3.
+ </entry></row>
+
</tbody></tgroup></informaltable>
</sect2>
Index: gw/smsc/smsc_smpp.c
===================================================================
--- gw/smsc/smsc_smpp.c (revision 4916)
+++ gw/smsc/smsc_smpp.c (working copy)
@@ -169,6 +169,7 @@
long connection_timeout;
long wait_ack;
int wait_ack_action;
+ int esm_class;
Load *load;
SMSCConn *conn;
} SMPP;
@@ -222,7 +223,7 @@
Octstr *my_number, int smpp_msg_id_type,
int autodetect_addr, Octstr *alt_charset, Octstr *alt_addr_charset,
Octstr *service_type, long connection_timeout,
- long wait_ack, int wait_ack_action)
+ long wait_ack, int wait_ack_action, int esm_class)
{
SMPP *smpp;
@@ -269,6 +270,7 @@
smpp->ssl_client_certkey_file = NULL;
smpp->load = load_create_real(0);
load_add_interval(smpp->load, 1);
+ smpp->esm_class = esm_class;
return smpp;
}
@@ -876,7 +878,7 @@
* set the esm_class field
* default is store and forward, plus udh and rpi if requested
*/
- pdu->u.submit_sm.esm_class = ESM_CLASS_SUBMIT_STORE_AND_FORWARD_MODE;
+ pdu->u.submit_sm.esm_class = smpp->esm_class;
if (octstr_len(msg->sms.udhdata))
pdu->u.submit_sm.esm_class = pdu->u.submit_sm.esm_class |
ESM_CLASS_SUBMIT_UDH_INDICATOR;
@@ -2204,6 +2206,7 @@
Octstr *alt_charset;
Octstr *alt_addr_charset;
long connection_timeout, wait_ack, wait_ack_action;
+ long esm_class;
my_number = alt_addr_charset = alt_charset = NULL;
transceiver_mode = 0;
@@ -2336,17 +2339,25 @@
if (cfg_get_integer(&wait_ack_action, grp, octstr_imm("wait-ack-expire")) == -1)
wait_ack_action = SMPP_WAITACK_REQUEUE;
-
- if (wait_ack_action > 0x03 || wait_ack_action < 0)
+ else if (wait_ack_action > 0x03 || wait_ack_action < 0)
panic(0, "SMPP: Invalid wait-ack-expire directive in configuration.");
+ if (cfg_get_integer(&esm_class, grp, octstr_imm("esm-class")) == -1) {
+ esm_class = ESM_CLASS_SUBMIT_STORE_AND_FORWARD_MODE;
+ } else if ( esm_class != ESM_CLASS_SUBMIT_DEFAULT_SMSC_MODE &&
+ esm_class != ESM_CLASS_SUBMIT_STORE_AND_FORWARD_MODE ) {
+ error(0, "SMPP: Invalid esm_class mode '%ld' in configuration. Switching to \"Store and Forward\".",
+ esm_class);
+ esm_class = ESM_CLASS_SUBMIT_STORE_AND_FORWARD_MODE;
+ }
+
smpp = smpp_create(conn, host, port, receive_port, system_type,
username, password, address_range,
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, alt_addr_charset,
- service_type, connection_timeout, wait_ack, wait_ack_action);
+ service_type, connection_timeout, wait_ack, wait_ack_action, esm_class);
cfg_get_integer(&smpp->bind_addr_ton, grp, octstr_imm("bind-addr-ton"));
cfg_get_integer(&smpp->bind_addr_npi, grp, octstr_imm("bind-addr-npi"));