Hi, I would naturally suggest we go for option *(a) *and allow users to set their values. Of course only advance users would want to work with this value anyway.
My reason is simple, there is no much sense in creating a non-boolen config value and then limit users on what they put there. Truth is, I have already seen an SMSC requesting for a value other than 0 and 3. -- Regards Kenny On Thu, Aug 18, 2011 at 5:57 PM, Alexander Malysh <amal...@kannel.org>wrote: > Hi, > > I don't see this as issue. The user ether let it default or try to change > in agreement with SMSC operator. If user will try some random values > then operator will just shutdown his account. If you ask me, I don't see > any need to allow changing default esm-class. But if we see buggy SMSCs > that don't accept STORE & FORWARD mode then I don't know what to expect > here. Maybe some drunk developer implement SMS as datagram mode, > who knows :-) > > We have 2 options: > a) allow user to set any values and write in the docs that this may be > dangerous > b) we restrict user to only have two options 0 (default mode) and 3 (store > & forward), then boolean value should be enough > > Alex > > Am 18.08.2011 um 12:33 schrieb Nikos Balkanas: > > Hi Alex, > > I don't agree that this should be left open to the user. A few may know > what to do with the spec. However, a lot will play around with the values > until they get them working. Consider that a user sets this to DATAGRAM, and > kannel generates a store and forward pdu with DATAGRAM esm_class. It doesn't > hurt kannel, but what about the SMSc that receives it? It could possibly > generate a core dump if the implementation is widely different, resulting in > loss of service. Do you really want to leave that open? > > BR, > Nikos > > On Thu, Aug 18, 2011 at 11:31 AM, Alexander Malysh <amal...@kannel.org>wrote: > >> 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> >> >> > >
