Re: [PATCH] custom MO Parameters for smsc-http

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>