Making SMPP esm_class configurable?

Alan McNatty alan at catalyst.net.nz
Mon Aug 15 02:22:23 CEST 2011


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





More information about the devel mailing list