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

 

Reply via email to