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

Hi,

Seems you don't handle charset at all. Do you assume to receive UTF-8  
in any case? I don't think it's
applicable to generic interface. At least alt-charset should be taken  
in account...

some comments to patch below...

why is dlrurl required for DLR? We have dlrurl stored in DLR storage...

+    }
+    else if (dlrurl != NULL && dlrmask != 0 && dlrmid != NULL) {
+        /* we got a DLR, and we don't require additional values */
+        Msg *dlrmsg;


hmm this check is not complete. it should be udh_len + msgdata_len but  
msgdata_len depends in coding...

+    else if (udh != NULL && octstr_len(udh) > MAX_SMS_OCTETS) {
+        error(0, "HTTP[%s]: UDH field is too long, rejected",
+              octstr_get_cstr(conn->id));
+        retmsg = octstr_create("UDH field is too long, rejected");
+        retstatus = fm->status_error;
+    }


why do you always load generic config options ?

  static void generic_send_sms(SMSCConn *conn, Msg *sms)
  {
@@ -1635,6 +1934,7 @@
      cfg_get_bool(&conndata->no_sep, cfg, octstr_imm("no-sep"));
      conndata->proxy = cfg_get(cfg, octstr_imm("system-id"));
      conndata->alt_charset = cfg_get(cfg, octstr_imm("alt-charset"));
+    conndata->fieldmap = generic_get_field_map(cfg);

---> conndata->fieldmap = NULL:

      if (conndata->send_url == NULL)
          panic(0, "HTTP[%s]: Sending not allowed. No 'send-url'  
specified.",
@@ -1693,7 +1993,7 @@
                    octstr_get_cstr(conn->id));
              goto error;
          }

--->    conndata->fieldmap = generic_get_field_map(cfg);

-        conndata->receive_sms = kannel_receive_sms; /* emulate  
sendsms interface */
+        conndata->receive_sms = generic_receive_sms; /* emulate  
sendsms interface */
          conndata->send_sms = generic_send_sms;
          conndata->parse_reply = generic_parse_reply;


Thanks,
Alex

Am 09.06.2009 um 10:14 schrieb Alejandro Guerrieri:

> Ok, here's a new version of my patch to add support for custom MO  
> parameters on the generic http-smsc.
>
> New features:
>
> 1. Support for setting the numeric response code for successful and  
> failed requests (as it is now, it always returns 202 HTTP_ACCEPTED).
> 2. Support for setting the text response for successful requests  
> (right now it returns "Sent.").
> 3. Some code cleanups (extra lines, parameter expansion after  
> authorization).
> 4. Documentation.
>
> http://www.blogalex.com/archives/192
>
> I can commit if no objections.
>
> Regards,
> --
> Alejandro Guerrieri
> [email protected]
>
>
>
> On 01/06/2009, at 22:50, Alexander Malysh wrote:
>
>> Hi,
>>
>> here some comments...
>>
>> in generic_receive_sms you first receive all values than converts  
>> it and only
>> after this done check authorization. Make no sense... first check  
>> auth then convert anything.
>>
>> +    else if (from == NULL || to == NULL || text == NULL) {
>> +
>> +        error(0, "HTTP[%s]: Insufficient args",
>>
>> why extra line?
>>
>> I think retmsg should also be configurable. Because not all  
>> gateways will accept 'Sent'  as response.
>> I think even HTTP error code for not accepted MOs/DLRs should be  
>> configurable.
>>
>> Thanks,
>> Alex
>>
>> P.S. And userguide part would be nice ;)
>>
>> Am 28.05.2009 um 23:00 schrieb Alejandro Guerrieri:
>>
>>> HI,
>>>
>>> I've finished my patch to configure the parameter names for MO on  
>>> the generic http-smsc.
>>>
>>> To use it you just add a few entries on the smsc definition, only  
>>> with the parameters you want to rename.
>>>
>>> e.g:
>>>
>>> generic-param-from = "phoneNumber"
>>> generic-param-to = "shortCode"
>>> generic-param-text = "message"
>>>
>>> More details and the patch here:
>>>
>>> http://www.blogalex.com/archives/171
>>>
>>> Of course I'm writing the userguide part if this goes forward :)
>>>
>>> Regards,
>>> --
>>> Alejandro Guerrieri
>>> [email protected]
>>>
>>>
>>
>