Alex,
Commenting inline below.
Regards,
--
Alejandro Guerrieri
[email protected]
On 09/06/2009, at 10:44, Alexander Malysh wrote:
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...
I'm not adding anything new here, the MO handling is built on the
original code for kannel_receive_sms (which was the default interface
for MO on the generic interface anyway). There's no charset handling
in there either. I agree it makes sense to have charset handling,
since an http interface should be as flexible as possible, but is it a
showstopper at this point?
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;
Idem... why is it needed for the kannel interface then?
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;
+ }
Idem... this is from kannel_receive_sms as well. If it's wrong here,
then it's wrong on the "kannel" smsc as well.
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;
True, it's a waste of CPU cycles if you're using other smsc-types
instead. Fixing it.
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]
