Re: [PATCH] custom MO Parameters for smsc-http
Alejandro Guerrieri <[email protected]>
| Newsgroups | gmane.comp.mobile.kannel.devel |
|---|---|
| Message-ID | <[email protected]> |
I'll do it, no problem. Regards, -- Alejandro Guerrieri [email protected] On 09/06/2009, at 16:30, Alexander Malysh wrote: > now it's perfect ;) > > would you like to commit or should I commit it? > > Thanks, > Alex > > Am 09.06.2009 um 16:06 schrieb Alejandro Guerrieri: > >> "ups" fixed :D >> >> http://www.blogalex.com/wp-content/uploads/2009/06/kannel-http-mo-params2.patch >> >> Regards, >> -- >> Alejandro Guerrieri >> [email protected] >> >> >> >> On 09/06/2009, at 15:40, Alejandro Guerrieri wrote: >> >>> Oh, yes, sorry :D >>> >>> I'd rather duplicate the msg... >>> -- >>> Alejandro Guerrieri >>> [email protected] >>> >>> >>> >>> On 09/06/2009, at 15:38, Alejandro Guerrieri wrote: >>> >>>> Yes, I have to move that afterwards, otherwise the >>>> urltrans_fill_escape_codes call would panic: >>>> >>>> 2009-06-09 12:46:13 [15580] [6] PANIC: gwlib/octstr.c:2488: >>>> seems_valid_real: Assertion `ostr->len >= 0' failed. (Called from >>>> gwlib/octstr.c:1552:octstr_split_words.) >>>> ... >>>> 2009-06-09 12:46:13 [15580] [6] PANIC: 0 >>>> bearerbox 0x0009136d gw_panic + 253 >>>> 2009-06-09 12:46:13 [15580] [6] PANIC: 1 >>>> bearerbox 0x000925de log_thread_to + 750 >>>> 2009-06-09 12:46:13 [15580] [6] PANIC: 2 >>>> bearerbox 0x0009b360 octstr_split_words >>>> + 48 >>>> 2009-06-09 12:46:13 [15580] [6] PANIC: 3 >>>> bearerbox 0x000191e2 >>>> urltrans_fill_escape_codes + 114 >>>> 2009-06-09 12:46:13 [15580] [6] PANIC: 4 >>>> bearerbox 0x0005d91b smsc_fake_create + >>>> 21803 >>>> >>>> Probably the bb_smscconn_receive call destroys the msg structure? >>>> >>>> Regards, >>>> -- >>>> Alejandro Guerrieri >>>> [email protected] >>>> >>>> >>>> >>>> On 09/06/2009, at 15:16, Alexander Malysh wrote: >>>> >>>>> only one last thing: >>>>> >>>>> + msg->sms.account = octstr_duplicate(account); >>>>> + msg->sms.binfo = octstr_duplicate(binfo); >>>>> + if (ret == -1) { >>>>> + retmsg = octstr_create("Not accepted"); >>>>> + retstatus = fm->status_error; >>>>> + } else { >>>>> + retmsg = urltrans_fill_escape_codes(fm- >>>>> >message_sent, msg); >>>>> + retstatus = fm->status_sent; >>>>> + } >>>>> + ret = bb_smscconn_receive(conn, msg); >>>>> + } >>>>> >>>>> are you sure you want check ret before bb_smscconn_receive ? ;) >>>>> >>>>> Thanks, >>>>> Alex >>>>> >>>>> Am 09.06.2009 um 13:23 schrieb Alejandro Guerrieri: >>>>> >>>>>> 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 >>>>>> [email protected] >>>>>> >>>>>> >>>>>> >>>>>> 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 >>>>>>>> [email protected] >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> 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 >>>>>>>>>> [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? >>>>>>>>>> >>>>>>>>> >>>>>>>>> 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 >>>>>>>>>>>> [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] >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>>> >>> >> >