Oh, my original patch was missing the smppbox-cfg.def part (in attachment) so currently you can't specify any of the new vars, otherwise smppbox doesn't start. One thing I noticed is that you committed my patch with vars using shorter names: src-addr-npi, etc, while the svn doc uses longer names for them: source-addr-npi, dest-addr-npi and so on. Either the source code needs correction to match the documentation or later is messed up :)
2010/7/10 Rene Kluwen <rene.klu...@chimit.nl>: > Right away I also checked in your ton/npi patch. > > == Rene > > -----Original Message----- > From: Victor Luchitz [mailto:vluch...@gmail.com] > Sent: vrijdag 9 juli 2010 23:19 > To: Rene Kluwen > Subject: Re: smppbox code questions > > Yeah, I was thinking about this "hack" as well, but it's going to > create more problems than it solves. Btw, why does smppbox use > system-type as boxc_id instead of ESME's login name? That forces > EMSE's to have distinct system-type values, while almost all SMSC'es > I've seen so far allow connections with empty system-type string, for > example. > > 2010/7/10 Rene Kluwen <rene.klu...@chimit.nl>: >> Heh... I think the way it works now is best for the average user. >> But I am sure you are competent enough to change it to your own needs. >> One "hack" that you can make is make the system-type of the client the same >> as your smsc-id in your kannel.conf. >> This is of course not recommended for most persons, but it might work for >> you. >> >> == Rene >> >> >> -----Original Message----- >> From: Victor Luchitz [mailto:vluch...@gmail.com] >> Sent: vrijdag 9 juli 2010 22:39 >> To: Rene Kluwen >> Subject: Re: smppbox code questions >> >> I have a pretty good idea how it works, it's just that the way it >> works doesn't suit my needs ;) >> >> 2010/7/9 Rene Kluwen <rene.klu...@chimit.nl>: >>> Surely this is relevant. >>> >>> Smppbox is not interested in bearerbox generated dlr's. It just needs to >>> "dlr_find" the dlr's that it added itself via dlr_add. >>> Bearerbox takes care of its own dlr's. Smppbox also takes care of its own >>> dlr's. >>> >>> I think you should re-read the code again to see how it works. >>> >>> == Rene >>> >>> -----Original Message----- >>> From: Victor Luchitz [mailto:vluch...@gmail.com] >>> Sent: vrijdag 9 juli 2010 15:35 >>> To: devel@kannel.org; Rene Kluwen >>> Subject: Re: smppbox code questions >>> >>> Yes, but how is this relevant? I mean, there are two possibilities to >>> make smppbox be aware of bearerbox-generated DLRs: >>> >>> 1) use boxc_id as smsc-id in dlr_add in bearerbox and then pass the >>> report_mo message to smppbox without issuing dlr_find in bearerbox >>> 2) use "proper"/parent smsc-id in smppbox >>> >>> The whole issue arises from the need to pass SMSC-related DLR's to >>> smppbox without the later issuing any DLR's itself. For example, a MT >>> message may fail to be delivered due to insufficient funds on user's >>> account. >>> >>> 2010/7/9 Rene Kluwen <rene.klu...@chimit.nl>: >>>> If bearerbox sends a report_mo, then it should include a status (dlr type) >>>> as well. >>>> Or am I wrong? >>>> >>>> == Rene >>>> >>>> -----Original Message----- >>>> From: Victor Luchitz [mailto:vluch...@gmail.com] >>>> Sent: vrijdag 9 juli 2010 14:24 >>>> To: devel@kannel.org; Rene Kluwen >>>> Subject: Re: smppbox code questions >>>> >>>> Unfortunately there's currently no way to add a SMSC_SUCCESS or >>>> SMSC_FAIL DLR in smppbox so I have/need to do that in bearerbox. But >>>> oh, well, I'll just go with boxc_id as smsc-id and live with this >>>> fact. >>>> >>>> 2010/7/9 Rene Kluwen <rene.klu...@chimit.nl>: >>>>> But bearerbox inserts it's own dlr's. As does smppbox. >>>>> So bearerbox will find their dlr's. And smppbox will do also. >>>>> >>>>> == Rene >>>>> >>>>> -----Original Message----- >>>>> From: Victor Luchitz [mailto:vluch...@gmail.com] >>>>> Sent: vrijdag 9 juli 2010 13:55 >>>>> To: Rene Kluwen >>>>> Subject: Re: smppbox code questions >>>>> >>>>> Well, this is going against the logic in bearerbox. For example, if >>>>> you pass a DLR via standard Kannel HTTP protocol, bearerbox will try >>>>> to find a matching DLR using its own smsc-id, upon failing to do, it >>>>> won't pass the DLR to smppbox either. >>>>> >>>>> 2010/7/9 Rene Kluwen <rene.klu...@chimit.nl>: >>>>>> The first parameter (smsc_id) is to determine "who's" dlr it is to begin >>>>>> with. So in short: To which smsc it belongs. >>>>>> Because smppbox does things the other way around, it passes the boxc_id >>>>>> variable. So if two boxes happen to have the same "ts" (which can in >>>>>> theory happen) the value is used to distinguish to which box_id it >>>>>> belongs. >>>>>> >>>>>> == Rene >>>>>> >>>>>> -----Original Message----- >>>>>> From: Victor Luchitz [mailto:vluch...@gmail.com] >>>>>> Sent: vrijdag 9 juli 2010 11:12 >>>>>> To: devel@kannel.org; Rene Kluwen >>>>>> Subject: Re: smppbox code questions >>>>>> >>>>>> On a side note, why does smppbox use boxc_id as the first parameter >>>>>> passed to dlr_add and dlr_find? Both functions take smsc_id as the >>>>>> first argument and boxc_id value is obtained from the sms struct. >>>>>> >>>>>> 2010/7/8 Rene Kluwen <rene.klu...@chimit.nl>: >>>>>>> Done. >>>>>>> Current revision is 17. >>>>>>> >>>>>>> -----Original Message----- >>>>>>> From: devel-boun...@kannel.org [mailto:devel-boun...@kannel.org] On >>>>>>> Behalf Of Victor Luchitz >>>>>>> Sent: donderdag 8 juli 2010 15:06 >>>>>>> To: devel@kannel.org >>>>>>> Subject: Re: smppbox code questions >>>>>>> >>>>>>> Yeah, you committed the proposed change to boxc->boxc_id in revision >>>>>>> 15. What I'm asking about is the suggestion and patch I posted here: >>>>>>> http://www.kannel.org/pipermail/devel/2010-July/003653.html >>>>>>> >>>>>>> 2010/7/8 Rene Kluwen <rene.klu...@chimit.nl>: >>>>>>>> It's already in the code. >>>>>>>> Current revision is 16. >>>>>>>> >>>>>>>> == Rene >>>>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: devel-boun...@kannel.org [mailto:devel-boun...@kannel.org] On >>>>>>>> Behalf Of Victor Luchitz >>>>>>>> Sent: donderdag 8 juli 2010 7:52 >>>>>>>> To: devel@kannel.org >>>>>>>> Subject: Re: smppbox code questions >>>>>>>> >>>>>>>> Any hope this will be reviewed and committed? >>>>>>>> >>>>>>>> I'm also working on a patch that adds TLV support to smppbox but I'd >>>>>>>> like to get this one included first. >>>>>>>> >>>>>>>> 2010/7/6 Victor Luchitz <vluch...@gmail.com>: >>>>>>>>> Yup, it's working fine now. Noticed there's another memleak though: >>>>>>>>> >>>>>>>>> another octstr_destroy(msgid); call is needed right after the: >>>>>>>>> /* we could not find a corresponding dlr; nothing to send */ >>>>>>>>> line. >>>>>>>>> >>>>>>>>> I'm also attaching another patch which allows transmission of custom >>>>>>>>> error codes in DLR's in the same manner as the message text bit. >>>>>>>>> >>>>>>>>> 2010/7/6 Rene Kluwen <rene.klu...@chimit.nl>: >>>>>>>>>> I have no way of testing this here. But since either way cannot harm >>>>>>>>>> I changed it. >>>>>>>>>> Current smppbox revision is now 15. >>>>>>>>>> Could you please check out and see if this fixes your problem? >>>>>>>>>> >>>>>>>>>> == Rene >>>>>>>>>> >>>>>>>>>> -----Original Message----- >>>>>>>>>> From: devel-boun...@kannel.org [mailto:devel-boun...@kannel.org] On >>>>>>>>>> Behalf Of Victor Luchitz >>>>>>>>>> Sent: dinsdag 6 juli 2010 14:53 >>>>>>>>>> To: devel@kannel.org >>>>>>>>>> Subject: Re: smppbox code questions >>>>>>>>>> >>>>>>>>>> 1) I think this assumption is incorrect. I have the routing set up >>>>>>>>>> this way in bearerbox: >>>>>>>>>> group = smsbox-route >>>>>>>>>> smsbox-id = vma >>>>>>>>>> smsc-id = HTTP >>>>>>>>>> >>>>>>>>>> So all messages on the 'HTTP' smsc get routed to smppbox. However, >>>>>>>>>> the >>>>>>>>>> custom HTTP protocol in the above layer does not use dlr_find to >>>>>>>>>> route >>>>>>>>>> messages to a specific box for two reasons: >>>>>>>>>> >>>>>>>>>> a) wrong smsc-id is used in the query (bearerbox doesn't know that >>>>>>>>>> smppbox overrides the smsc id with system-type) so dlr_find always >>>>>>>>>> fails >>>>>>>>>> b) dlr_find removes DLR from the table and then subsequently readds >>>>>>>>>> it, which is rather stressful on the DB for no sane reason >>>>>>>>>> >>>>>>>>>> What it does instead is simply setting the sms_type to report_mo, >>>>>>>>>> leaving box_id empty as in regular MO messages. >>>>>>>>>> >>>>>>>>>> 2010/7/6 Rene Kluwen <rene.klu...@chimit.nl>: >>>>>>>>>>> To start with the last thing: >>>>>>>>>>> >>>>>>>>>>> 2) You are right. It should use the msgid's in the dlr_url from the >>>>>>>>>>> dlr instance. I changed it. >>>>>>>>>>> >>>>>>>>>>> About 1): We assume msg->boxc_id and box->boxc_id are the same in >>>>>>>>>>> this case. Otherwise the message wouldn't have ended up there. >>>>>>>>>>> >>>>>>>>>>> == Rene >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -----Original Message----- >>>>>>>>>>> From: devel-boun...@kannel.org [mailto:devel-boun...@kannel.org] On >>>>>>>>>>> Behalf Of Victor Luchitz >>>>>>>>>>> Sent: maandag 5 juli 2010 20:36 >>>>>>>>>>> To: devel@kannel.org >>>>>>>>>>> Subject: smppbox code questions >>>>>>>>>>> >>>>>>>>>>> Hello, >>>>>>>>>>> >>>>>>>>>>> I have a few questions for you regarding the handling of DLR's by >>>>>>>>>>> smppbox, which might also turn out to be bugs. >>>>>>>>>>> >>>>>>>>>>> 1) >>>>>>>>>>> In msg_to_pdu function there's a line which reads: >>>>>>>>>>> dlr = dlr_find(msg->sms.boxc_id, msgid, msg->sms.receiver, dlrtype); >>>>>>>>>>> >>>>>>>>>>> I think it's incorrect because when a DLR is stored by smppbox in >>>>>>>>>>> handle_pdu, the boxc_id it uses is that from smpp_logins file >>>>>>>>>>> (system_type). That in turn may cause dlr_find to always fail. So in >>>>>>>>>>> my opinion the correct dlr_find call is this: >>>>>>>>>>> dlr = dlr_find(box->boxc_id, msgid, msg->sms.receiver, dlrtype); >>>>>>>>>>> >>>>>>>>>>> 2) another thing I find not quite correct is the way smppbox splits >>>>>>>>>>> message ids for concatenated DLR's. Basically, what smppbox does is >>>>>>>>>>> this: >>>>>>>>>>> >>>>>>>>>>> parts = octstr_split(msg->sms.dlr_url, octstr_imm(";")); >>>>>>>>>>> msgid = gwlist_extract_first(parts); >>>>>>>>>>> ... >>>>>>>>>>> Then it loops through all elements of the 'parts' list and here is >>>>>>>>>>> where the potential problem lies. smppbox assumes that msgid for the >>>>>>>>>>> concatenated DLR is always equal to dlr_url which is not always >>>>>>>>>>> true. >>>>>>>>>>> In fact, I think it's never true for concatenated DLR's stored by >>>>>>>>>>> the >>>>>>>>>>> dlr_add call in handle_pdu. Also, for example, the 'msgid' and >>>>>>>>>>> 'dlrurls' columns in the storage table can have different maxiumum >>>>>>>>>>> lengths, allowing truncation of the msgid. Here's my proposed fix - >>>>>>>>>>> add the following bit of code to msg_to_pdu: >>>>>>>>>>> >>>>>>>>>>> gwlist_destroy(parts, octstr_destroy_item); >>>>>>>>>>> parts = octstr_split(dlr->sms.dlr_url, octstr_imm(";")); >>>>>>>>>>> gwlist_extract_first(parts); >>>>>>>>>>> >>>>>>>>>>> right above the following bit: >>>>>>>>>>> if (gwlist_len(parts) > 0) { >>>>>>>>>>> while ((msgid2 = gwlist_extract_first(parts)) != NULL) { >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -- >>>>>>>>>>> Best regards, >>>>>>>>>>> Victor Luchitz >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> Best regards, >>>>>>>>>> Victor Luchitz >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Best regards, >>>>>>>>> Victor Luchitz >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Best regards, >>>>>>>> Victor Luchitz >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Best regards, >>>>>>> Victor Luchitz >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> Best regards, >>>>>> Victor Luchitz >>>>>> >>>>>> >>>>>> >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> Best regards, >>>>> Victor Luchitz >>>>> >>>>> >>>>> >>>>> >>>> >>>> >>>> >>>> -- >>>> Best regards, >>>> Victor Luchitz >>>> >>>> >>>> >>>> >>> >>> >>> >>> -- >>> Best regards, >>> Victor Luchitz >>> >>> >>> >>> >> >> >> >> -- >> Best regards, >> Victor Luchitz >> >> >> >> > > > > -- > Best regards, > Victor Luchitz > > > > -- Best regards, Victor Luchitz
cfg.diff
Description: Binary data
