Am 09.06.2009 um 11:05 schrieb Alejandro Guerrieri:
Alex,
Commenting inline below.
Regards,
--
Alejandro Guerrieri
aguerri...@kannel.org
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?
It is not really show stopper but if it will not be added now it will
remain so for ages. It is not a problem for kannel_receive_sms because
it's
well defined interface to smsbox which uses UTF-8.
I would prefer to see it implemented (there only few line of code ;) )
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, seems both buggy then. We don't need dlr-url for DLR. Could you
please fix both interfaces?
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.
Hmm seems, I'm wrong here. This check checks for the right UDH but
allows long MO messages.
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
aguerri...@kannel.org
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
aguerri...@kannel.org