Re: Making SMPP esm_class configurable?

Ah that's great, thanks Alex.

On Wed, 2011-09-14 at 22:08 +0200, Alexander Malysh wrote:
> Hi Alan,
> 
> after the long discussion this patch commited to SVN.
> 
> Thanks,
> Alexander Malysh
> 
> 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 <[email protected]>
> >> 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: [email protected]
> >>        [mailto:[email protected]] On
> >>> Behalf Of Nikos Balkanas
> >>> Sent: Tuesday, 09 August, 2011 02:11
> >>> To: Alan McNatty
> >>> Cc: [email protected]
> >>> Subject: Re: Making SMPP esm_class configurable?
> >>> 
> >>> 
> >>> 
> >>> 
> >>> Looks great! +1.
> >>> 
> >>> 
> >>> Nikos
> >>> 
> >>> 
> >>> 
> >>> 
> >>> On Tue, Aug 9, 2011 at 2:59 AM, Alan McNatty
> >>        <[email protected]>
> >>> 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
> >>        <[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
> >>>>>>>>>>> 
> >>>>>>>>>>> 
> >>>>>>>>>>> 
> >>>>>>>>>>> 
> >>>>>>>>>> 
> >>>>>>>>> 
> >>>>>>>>> 
> >>>>>>>>> 
> >>>>>>>> 
> >>>>>>>> 
> >>>>>>>> 
> >>>>>>> 
> >>>>>> 
> >>>>>> 
> >>>>>> 
> >>>>> 
> >>>>> 
> >>>> 
> >>>> 
> >>>> 
> >>> 
> >>> 
> >>> 
> >>> 
> >>> 
> >> 
> >> 
> >> 
> > 
> > <esm_class.patch>
>