Here is the new version...
> -----Original Message----- > From: [email protected] > [mailto:[email protected]] On Behalf Of Franck LAMASUTA > Sent: Monday, June 22, 2009 4:53 PM > To: [email protected] > Subject: RE: [PATCH] custom MO Parameters for smsc-http > > Thanks for your comments. > > First comment regarding the variable name: > The new variable is closely related to status-success-regex > which doesn't start with "generic-". > But ok, I will change it if you think it's better... > > > Second comment regarding pmatch[1].rm_eo: > The test seems to be useless according to regexec() documentation. > But it's cheap so it can't hurt. > > I will update userguide.xml also. > > Franck > > > > > -----Original Message----- > > From: Alejandro Guerrieri [mailto:[email protected]] > > Sent: Monday, June 22, 2009 12:37 PM > > To: Alexander Malysh > > Cc: [email protected];[email protected] > > Subject: Re: [PATCH] custom MO Parameters for smsc-http > > > > +1 with the changes Alex proposed. I'd rename > > ack_foreign_id_regex as > > well, for clarity's sake. > > > > Regards, > > -- > > Alejandro Guerrieri > > [email protected] > > > > On 22/06/2009, at 12:26, Alexander Malysh wrote: > > > > > Hi, > > > > > > patch looks ok. Here some minor comments: > > > > > > + OCTSTR(ack-foreign-id-regex) > > > would it be better to name it something like generic-foreign-id- > > > regex? so user know it belongs only to generic http... > > > > > > + if (pmatch[1].rm_so != -1) { > > > better to check the end as well -> if (pmatch[1].rm_eo != -1 && > > > pmatch[1].rm_so != -1) > > > > > > Otherwise patch looks good. > > > Please make patch to userguide and I will commit it to CVS. > > > > > > Thanks, > > > Alex > > > > > > Am 19.06.2009 um 14:40 schrieb Franck LAMASUTA: > > > > > >> Here it is, implemented with second idea... > > >> > > >> I did a small optimization of previous gw_regex_exec() calls in > > >> generic_parse_reply(). > > >> I have also fixed 3 debug() calls in generic_receive_sms() > > >> ("smsc.http.kannel" => "smsc.http.generic"). > > >> > > >> Waiting for your comments... > > >> > > >> Franck > > >> > > >> > > >> > > >>> -----Original Message----- > > >>> From: Alejandro Guerrieri [mailto:[email protected]] > > >>> Sent: Wednesday, June 10, 2009 3:08 PM > > >>> To: Alexander Malysh > > >>> Cc: [email protected];[email protected] > > >>> Subject: Re: [PATCH] custom MO Parameters for smsc-http > > >>> > > >>> Me too, second one is far more flexible and clean. > > >>> -- > > >>> Alejandro Guerrieri > > >>> [email protected] > > >>> > > >>> > > >>> > > >>> On 10/06/2009, at 14:31, Alexander Malysh wrote: > > >>> > > >>>> Hi, > > >>>> > > >>>> I'm for the second idea, regex rocks ;) > > >>>> > > >>>> Thanks, > > >>>> Alex > > >>>> > > >>>> Am 10.06.2009 um 12:37 schrieb Franck LAMASUTA: > > >>>> > > >>>>> It's not so urgent but I could try, it could be interesting... > > >>>>> > > >>>>> I have 2 ideas so far... > > >>>>> > > >>>>> > > >>>>> First idea > > >>>>> ========== > > >>>>> > > >>>>> I could add 2 new config parameters to define 2 regex: > > >>>>> - the first one, to find the start of the foreign id. > > >>>>> - the second one, to find the end of the foreign id. > > >>>>> > > >>>>> For Claro, they could be defined like this: > > >>>>> foreign-id-regex-begin = "<transaction-id>" > > >>>>> foreign-id-regex-end = "</transaction-id>" > > >>>>> > > >>>>> For Clickatell, they could be defined like this: > > >>>>> foreign-id-regex-begin = "ID: " > > >>>>> foreign-id-regex-end = " " > > >>>>> > > >>>>> For Brunet, they could be defined like this: > > >>>>> foreign-id-regex-begin = "MessageId=" > > >>>>> foreign-id-regex-end = " " > > >>>>> > > >>>>> In generic_parse_reply(), only in case of success, > > >>> regex-begin will > > >>>>> be used to find the beginning of the id. Starting from > > >>> there, regex- > > >>>>> end will be used to find the end of the id. If regex-end > > >>> can't find > > >>>>> a match, the id will be read until the end of text > (body) and an > > >>>>> ending CRLF will be removed. > > >>>>> Then, the id will be used to call dlr_add() (like done in > > >>>>> clickatell_parse_reply()). > > >>>>> > > >>>>> > > >>>>> Second idea > > >>>>> =========== > > >>>>> > > >>>>> It's basically the same idea except that only one regex > > >>> will be used. > > >>>>> > > >>>>> For Claro, it could be defined like this: > > >>>>> foreign-id-regex = "<transaction-id>(.*)</transaction-id>" > > >>>>> > > >>>>> For Clickatell, it could be defined like this (if an > id contains > > >>>>> only digits): > > >>>>> foreign-id-regex = "ID: ([0-9]+)" > > >>>>> > > >>>>> The offsets given by gw_regex_exec() in pmatch[1] could be > > >>> used to > > >>>>> extract the id. > > >>>>> However, it could be more difficult to understand for > people who > > >>>>> are not regex experts... > > >>>>> > > >>>>> > > >>>>> Any comment, opinion, suggestion or better idea? :) > > >>>>> > > >>>>> Franck > > >>>>> > > >>>>> > > >>>>> > > >>>>> > > >>>>>> From: Alexander Malysh > > >>>>>> Sent: Tuesday, June 09, 2009 6:05 PM > > >>>>>> > > >>>>>> > > >>>>>> Am 09.06.2009 um 17:55 schrieb Alejandro Guerrieri: > > >>>>>> > > >>>>>>> Franck, > > >>>>>>> > > >>>>>>> The dlr part is inherited from the "kannel" smsc > > format. In the > > >>>>>>> actual implementation there's no way to grab the remote > > >>> message id > > >>>>>>> from a request, and it'd make a lot of sense to be > > able to do > > >>>>>>> it. > > >>>>>>> > > >>>>>>> I can try adding it, but don't refrain to do it so > > >>> yourself if you > > >>>>>>> feel the urge (you'll learn a lot about the code in the > > >>> process!). > > >>>>>> > > >>>>>> yep, Alex is right here :) and we need more contributors :) > > >>>>>> > > >>>>>>> > > >>>>>>> Opinions? > > >>>>>>> -- > > >>>>>>> Alejandro Guerrieri > > >>>>>>> [email protected] > > >>>>>>> > > >>>>>>> > > >>>>>>> > > >>>>>>> On 09/06/2009, at 17:47, Franck LAMASUTA wrote: > > >>>>>>> > > >>>>>>>> Hi Alejandro , > > >>>>>>>> > > >>>>>>>> I work with Hervé who posted to this list the 25th of May > > >>>>>>>> (Claro > > >>>>>>>> http communication). > > >>>>>>>> It seems the patch you've done on the generic part > could be > > >>>>>>>> very > > >>>>>>>> useful for us to manage our communications with Claro. It > > >>>>>> could be > > >>>>>>>> better than a specific implementation for Claro. > > >>>>>>>> > > >>>>>>>> However, I don't understand how your patch manages a DLR: > > >>>>>>>> In generic_receive_sms(), you call dlr_find() (line > > 1674 of the > > >>>>>>>> patched source) to retrieve a previous record stored > > in the DLR > > >>>>>>>> storage. > > >>>>>>>> It seems fine except that dlr_add() is never called in > > >>>>>>>> generic_parse_reply(), so I guess dlr_find() will always > > >>>>>> return NULL. > > >>>>>>>> Am I wrong??? > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> In fact, when we submit a MT SMS to Claro with this request > > >>>>>>>> > > >>>>>> > http://retail.mds.claro.com.br/MSE/api?profile=xxxx&pwd=xxxx&m > > >>>>> ode=assync-delivery&ANUM=3333&BNUM=1234567&TEXT=test > > >>>>>>>> > > >>>>>>>> we get a HTTP code 200 with this body: > > >>>>>>>> > > >>>>>>>> <?xml version="1.0" encoding="UTF-8" ?> > > >>>>>>>> <mse-response> > > >>>>>>>> <status-code>0</status-code> > > >>>>>>>> <profile>profleID</profile> > > >>>>>>>> <transaction-id>1020606201622668099</transaction-id> > > >>>>>>>> </mse-response> > > >>>>>>>> > > >>>>>>>> So I guess that the transaction-id should be used in > > >>>>>>>> generic_parse_reply() to call dlr_add(). > > >>>>>>>> Unfortunately, the current implementation does not allow > > >>>>>> to manage > > >>>>>>>> such an id, it only allows to search if the request was > > >>>>>> successful > > >>>>>>>> or not through the regex. > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> If all my hypothesis are right... :-) > > >>>>>>>> I could try to patch generic_parse_reply() to manage > > also the > > >>>>>>>> id > > >>>>>>>> through another regex. It will be my first Kannel > patch! ;-) > > >>>>>>>> If you prefer to do it, no problem, just let me know. > > >>>>>>>> > > >>>>>>>> If I'm wrong, please clarify how it works. > > >>>>>>>> > > >>>>>>>> Regards, > > >>>>>>>> Franck > > >>>>>>>> > > >>>>>>>> > > >>>>>>> > > >>>>>>> > > >>>>>> > > >>>>>> > > >>>>> > > >>>>> > > >>>> > > >>>> > > >>> > > >>> > > >> <kannel_http-generic.patch> > > > > > > > > > > > > > > > >
kannel_http-generic_2.patch
Description: Binary data
