Re: Making SMPP esm_class configurable?

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
> >         > > >>>>
> >         > > >>>>
> >         > > >>>>
> >         > > >>>>
> >         > > >>>
> >         > > >>
> >         > > >>
> >         > > >>
> >         > > >
> >         > > >
> >         > > >
> >         > >
> >         >
> >         >
> >         >
> >
> >
>
>