Hi Alex,

sorry for delay...

Patch looks OK but I don't like this part:
+            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("0x%08lx", 
pdu->u.submit_sm_resp.command_status), 1);

Why do you need to forward SMPP internal status to client? If all was fine and 
message was accepted it's always 0 and if
it was rejected you will receive errorcode in DLR msgdata field.
Therefore I don't really know why you need this part?

I'm +1 for this patch without dlr_status part.

Thanks,
Alexander Malysh

P.S. Don't forget to rebase your patch due to recent changes in dlr_pgsql.c :)

Am 05.11.2009 um 23:41 schrieb Alejandro Guerrieri:

> This is an expanded version of a patch I've done a couple of months ago. It 
> adds the following functionality and fix a few things in the process:
> 
> #1: Allows meta-data to be passed when sending a message to come back on the 
> DLR's (internal, intermediate and final).
> 
> ***COMPATIBILITY BREAKER***
> A new "meta-data" field is needed on the DB table.
> ***COMPATIBILITY BREAKER***
> 
> This allows, for example, to set the dlr-url on the smsbox group and then 
> pass extra parameters as meta-data (either on the ?smpp? group or you could 
> even create your own, ?dlr? for example).
> 
> #2: It creates a meta data value called "dlr_status" that comes back on the 
> internal dlr generated by kannel. The loaded value is the SMPP 
> "command_status" parameter.
> 
> #3: It fixes/cleanup code on some dlr_<dbengine>.c, for example on many 
> places %s was used inside octstr_format, and then octstr_get_cstr(var) was 
> used, where using %S would be more direct:
> 
> -        sql = octstr_format("DELETE FROM %s WHERE %s='%s' AND %s='%s' %s",
> -                            octstr_get_cstr(fields->table),
> -                            octstr_get_cstr(fields->field_smsc), 
> octstr_get_cstr(smsc),
> -                            octstr_get_cstr(fields->field_ts), 
> octstr_get_cstr(ts), sdb_get_limit_str());
> 
> +        sql = octstr_format("DELETE FROM %S WHERE %S='%S' AND %S='%S' %s",
> +                            fields->table, fields->field_smsc, smsc,
> +                            fields->field_ts, ts, sdb_get_limit_str());
>  
> #4 There was also a "LIMIT 1" on dlr_sdb.c where sdb_get_limit_str() should 
> be used instead.
> 
> 
> 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, kannel would call the following url:
> 
> 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&
> 
> 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. You're not limited to ?smpp?, you can add your own meta-data 
> groups to avoid possible conflicts.
> 
> Regarding the dlr_status, in this example gets loaded with submit_sm_resp's 
> command_status which was: 69 = 0x00000045 (Submit Failed).
> 
> Please review, I'm writing the userguide part if it goes forward.
> 
> Regards,
> --
> Alejandro Guerrieri
> [email protected]
> 
> <kannel-dlr-meta-data.diff.zip>
> 

Reply via email to