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: devel-boun...@kannel.org [mailto:devel-boun...@kannel.org] On > Behalf Of Nikos Balkanas > Sent: Tuesday, 09 August, 2011 02:11 > To: Alan McNatty > Cc: devel@kannel.org > Subject: Re: Making SMPP esm_class configurable? > > > > > Looks great! +1. > > > Nikos > > > > > On Tue, Aug 9, 2011 at 2:59 AM, Alan McNatty <a...@catalyst.net.nz> > 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 <a...@catalyst.net.nz> > > 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 > > <a...@catalyst.net.nz> > > > 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" <amal...@kannel.org> > > > > > To: "Alan McNatty" <a...@catalyst.net.nz> > > > > > Cc: "Nikos Balkanas" <nbalka...@gmail.com>; > > > <de...@vm1.kannel.org> > > > > > 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" > <a...@catalyst.net.nz> > > > > > >>> To: <devel@kannel.org> > > > > > >>> 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 > > > > > >>>> > > > > > >>>> > > > > > >>>> > > > > > >>>> > > > > > >>> > > > > > >> > > > > > >> > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
