Re: [PATCH] Pass meta-data from message to dlrs

Ok, silly diff errors aside (I wasn't including dlr_p.h :P), here's  
the new patches also changing the error code format as Alex suggested.



Userguide patch coming later.

Just a few more ideas to consider:

1. Adding one more called "dlr_status_text" with the text description  
from smpp_error_to_string(pdu->u.submit_sm_resp.command_status). Not  
sure if it makes sense, this could be done at the application level  
with a simple table...

2. Adding one more called "dlr_sequence" with the sequence_number  
parameter.

3. Extending patch #1 adding the meta-data column on all DB  
implementations, so meta-data could be passed and stored on dlr's  
other than the first one generated by kannel.

Regards,
--
Alejandro Guerrieri
[email protected]



On 03/09/2009, at 23:33, Alexander Malysh wrote:

> Hi Alex,
>
> first patch seems still broken:
> +    dlr->meta_data = (msg->sms.meta_data ? octstr_duplicate(msg- 
> >sms.meta_data) : octstr_create(""));
>
> we don't have meta_data in dlr struct. otherwise +1
>
> second patch:
> +            if (msg->sms.meta_data == NULL)
> +                msg->sms.meta_data = octstr_create("");
> +            meta_data_set_value(msg->sms.meta_data, "smpp",  
> octstr_imm("dlr_status"),
> +                                octstr_format("%d", pdu- 
> >u.submit_sm_resp.command_status), 1);
> +
>
> Would it it not better to have error code formated as smpp-tlv group  
> and as error message?
> +                                octstr_format("0x%08lx", pdu- 
> >u.submit_sm_resp.command_status), 1);
>
> Thanks,
> Alexander Malysh
>
>
> Am 03.09.2009 um 21:13 schrieb Alejandro Guerrieri:
>
>> Oh, I see, I've attached the ~ backup file from vim. This are the  
>> right files, just in case:
>>
>> <kannel-dlr-command-status.diff>
>> <kannel-dlr-meta-data.diff>
>>
>> --
>> Alejandro Guerrieri
>> [email protected]
>>
>>
>>
>> On 03/09/2009, at 21:06, Alejandro Guerrieri wrote:
>>
>>> Oh, that must have filtered into the patch. Weird :P
>>>
>>> Sorry, it's safe to remove it.
>>>
>>> Regards,
>>> --
>>> Alejandro Guerrieri
>>> [email protected]
>>>
>>>
>>>
>>> On 03/09/2009, at 20:56, Gustavo Mohme C. wrote:
>>>
>>>> Hi Alejandro,
>>>> I get this error after applying your patch.
>>>>
>>>> gw/dlr.c: In function ‘dlr_add’:
>>>> gw/dlr.c:352: error: ‘struct dlr_entry’ has no member named  
>>>> ‘meta_data’
>>>> make: *** [gw/dlr.o] Error 1
>>>>
>>>> After editing /gw/dlr_p.h and adding meta_data to the struct  
>>>> dlr_entry, it compiled with no complaints. Is this the correct  
>>>> thing to do?
>>>> I was patching the latest kannel snapshot....
>>>> Thanks,
>>>> Gustavo
>>>>
>>>> On Thu, Sep 3, 2009 at 3:39 AM, Alejandro Guerrieri <[email protected] 
>>>> > wrote:
>>>> Hi,
>>>>
>>>> This is a set of two patches, though they're both very simple and  
>>>> could have been mixed, I've preferred to split to honor the rule  
>>>> of not mixing features in a single patch.
>>>>
>>>> Patch #1, kannel-drl-meta-data.diff allows meta-data to be passed  
>>>> when sending a message to come back into the "fake" dlr that  
>>>> kannel generates when the smsc accepts or rejects a message. It  
>>>> could be extended to work with the different dlr engines so the  
>>>> meta-data would be also passed on the intermediate and final  
>>>> dlr's delivered by the carriers (though it would require changes  
>>>> on the database structure to add a "meta_data" column of course.  
>>>> I'm not sure this would be needed, the dlr-url could be easily  
>>>> abused to pass application-specific parameters without much hassle.
>>>>
>>>> Patch #2, kannel-dlr-command-status.diff builds on top of patch  
>>>> #1 and creates a meta data value called "dlr_status" that comes  
>>>> back on the "fake" dlr generated by kannel (This was the real  
>>>> reason why patch #1 was created). The value there is the  
>>>> "command_status" value some people on the list needs to get from  
>>>> the submit_sm_resp PDU's.
>>>>
>>>> Example usage:
>>>>
>>>> On sendsms:
>>>>
>>>> http://localhost:13013/cgi-bin/sendsms?username=kannel&password=kannel&from=12345&to=12345678&smsc=mysmsc&text=Hello&dlr-mask=31&meta-data=%3Fsmpp%3Fmy_own_field%3D1234&dlr-url=http%3A%2F%2Flocalhost%2Fx%3Fdata%3D%25D
>>>>
>>>> Notes:
>>>>
>>>> meta-data is urlencoded version of: ?smpp?my_own_field=1234
>>>> dlr-url is urlencoded version of: http://localhost/x?data=%D
>>>>
>>>> So, after applying Patch #1, kannel would call the following url:
>>>>
>>>> http://localhost/x?md=%3Fsmpp%3Fmy_own_field%3D1234
>>>>
>>>> The "md" parameter, once urldecoded would look like:
>>>>
>>>> ?smpp?my_own_field=1234
>>>>
>>>> You can pass as many parameters as you want of course and they  
>>>> would be added to meta-data along with any other fields you've  
>>>> defined on your smpp-tlv groups, etc.
>>>>
>>>> So far so good, in fact you could pass this kind of stuff on  
>>>> regular url variables, but the goal of this patch was to allow to  
>>>> add stuff back from the smsc's response.
>>>>
>>>> Now, with patch #2 also applied, the url returned would be  
>>>> something like this:
>>>>
>>>> http://localhost/x?md=%3Fsmpp%3Fmy_own_field%3D1234%26dlr_status%3D69%26
>>>>
>>>> The "md" parameter, once urldecoded would look like:
>>>>
>>>> ?smpp?my_own_field=1234&dlr_status=69&
>>>>
>>>> In this case, dlr_status gets loaded with submit_sm_resp's  
>>>> command_status which was: 69 = 0x00000045 (Submit Failed).
>>>>
>>>> The "sequence_number" could be added just as easily (not a bad  
>>>> idea imho). What do you think? "dlr_sequence" or "dlr_seq" would  
>>>> be ok?
>>>>
>>>> I'm writing the userguide patch if this goes forward of course.
>>>>
>>>> Comments?
>>>>
>>>> Regards,
>>>> --
>>>> Alejandro Guerrieri
>>>> [email protected]
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>
>