Hi Alejandro, Your patch on the generic part uses the same fields than the specific Kannel code: username, password, from, to, text, mwi, deferred. However you added foreign_id but it's actually never used... Could you clarify?
I'm working on a new patch and I would like to know if I should take this variable into account or if I can remove it... ;) Thanks. Franck > -----Original Message----- > From: devel-boun...@kannel.org > [mailto:devel-boun...@kannel.org] On Behalf Of Alejandro Guerrieri > Sent: Tuesday, June 09, 2009 4:06 PM > To: Alejandro Guerrieri > Cc: Kannel Devel > Subject: Re: [PATCH] custom MO Parameters for smsc-http > > "ups" fixed :D > > http://www.blogalex.com/wp-content/uploads/2009/06/kannel-http > -mo-params2.patch > > Regards, > -- > Alejandro Guerrieri > aguerri...@kannel.org > > > > On 09/06/2009, at 15:40, Alejandro Guerrieri wrote: > > > Oh, yes, sorry :D > > > > I'd rather duplicate the msg... > > -- > > Alejandro Guerrieri > > aguerri...@kannel.org > > > > > > > > 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 > >> aguerri...@kannel.org > >> > >> > >> > >> 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 > >>>> 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 > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>>> > >>> > >> > >> > > > > > >
