Ok, done!
http://www.blogalex.com/wp-content/uploads/2009/06/kannel-http-mo-params1.patch
I've fixed the dlr-url bug _only_ on the generic part. If no
objections, I'm doing a second patch to fix it on the kannel smsc
(since it's in fact a "separate" issue).
I've also fixed a small glitch on the dlrmsg response code (I was
using the "error" status code for successful submits as well).
Last but not least, I've added url translation to the response
message, so now you can include escape codes on the response, which
may come handy on many cases (for example, to return kannel's message
id on the requests). Userguide part updated accordingly.
Regards,
--
Alejandro Guerrieri
aguerri...@kannel.org
On 09/06/2009, at 11:27, Alexander Malysh wrote:
Am 09.06.2009 um 11:23 schrieb Alejandro Guerrieri:
Alex,
Regarding the charsets, I agree it's better to tackle it now that
leave it for "later" (aka, "never" ;) ).
I'm fixing the charset issue and the dlr bugs and resending later.
thanks alex!
Regards,
--
Alejandro Guerrieri
aguerri...@kannel.org
On 09/06/2009, at 11:17, Alexander Malysh wrote:
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
