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