Hi Alan, sorry to write my response late (I was on vacation), but I don't think we need some restriction on configured esm-class. If we allow to change esm-class in the config, IMHO there is no need to restrict it's value, because the user know what he does.
For cannel it's equal, which value to set as esm-class because kannel doesn't interpreting it. Thanks, Alex Am 15.08.2011 um 09:35 schrieb Alan McNatty: > 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 <a...@catalyst.net.nz> >> 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: 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 >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>> >>>> >>>> >>> >>> >>> >>> >>> >> >> >> > > <esm_class.patch>
