Hi! Yesterday we were talking about the same thing, that an ACK is an ACK, so you don't really need the error code there. But IMHO it anyway makes it more tidy to use the same approach for both ACK and NACK, providing the same syntax on both. It won't provide any real benefit to anyone, but looks more tidy to me to do it the same way on both.
Regarding obtaining the protocol, the discussion was that you could be load balancing between different protocols, so the smsc_id would be the same for all, and you might not have a way of mapping the correct errors, since they may collide between the different protocols. I'm not sure how probable a setup like that one might be, seems quit rare, but from a technical point of view it's perfectly possible. Now, wouldn't it maybe make more sense to implement a get_protocol callback function like you said, and be able to retrieve its value via some new parameter (%whatever)? That way only the one who's interested on it uses it, and we don't modify what we already have which can break some implementations. Another option could be the one I mentioned, where the protocol shows up on meta-data, and we would need to make sure it shows up on all cases. Seems a bit ugly to me to obtain it that way, but it would work... Juan On Thu, Oct 25, 2012 at 2:01 PM, Alexander Malysh <amal...@kannel.org>wrote: > Hi Alex, > > this patch is no go, because you put SMPP specific handling into abstract > bb_smscconn. > > As to the discussion itself. We use error codes in our apps as well and we > were never in the > situation that we have to know protocol from the meta-data or message > itself. We have mapping > of SMSC name and which protocol it uses BUT we this only as exception. > > We can put protocol into NACK with error code but it make no sense for me > to put protocol into ACK as well as put > error code into ACK. ACK means accepted and I don't see any sense to > forward protocol specific ack error code > to the application layer. Am I missing something then please clarify why > do you need numeric value for ACK? > > If we decide to put protocol into NACK message then we should implement > additional callback function (get_protocol) or > something like this and then write it out to application layer but not as > you did it with mixing SMSC abstract layer and implementation > specific things. > > Alex > > Am 25.10.2012 um 00:56 schrieb Alejandro Guerrieri < > alejandro.guerri...@gmail.com>: > > Hi, > > Attached is a patch that allows passing the command_status on SMPP as > meta-data, on the smpp_error_code parameter, so on the logs (and dlr-url) > you'd get something like: > > *[META:?smpp?smpp_error_code=72&]* > * > * > This would be command status value (which is 0 for accepted messages and a > positive integer for any other outcome). > > This is based on some code Donald Jackson wrote for us time ago, and while > this might prove useful for some people, after speaking with Stipe he > mentioned that we can achieve similar results by parsing the %A parameter > on the dlr-url. In that case, the possible outcomes right now are: > > *ACK/* > *NACK/0xXXXXXXXX/Error text* > > In this case, for an accepted message, no numeric code is given. > For a rejected message, 0xXXXXXXXX contains the hex value of the > command_status > > After discussing for a little bit, Stipe pretty much talked us out of the > idea of our patch because it's non-generic (only works for SMPP). > > However, %A is far from perfect as well: > > 1. It's inconsistent. No numeric value is provided on ACK's. > 2. You cannot tell the protocol, so if you're load-balancing binds with > different protocols you'd get different error codes with no way to > correlate them by protocol. > 3. It's not documented either. > > After a very healthy debate, we thought of the following approach, keeping > the %A method: > > *ACK/SMPP:0x00000000/OK* > *NACK/SMPP:0x00000048/Invalid Source address TON* > > This slightly breaks compatibility if you were already parsing the error > code on the NACK's or doing an exact match on "ACK/" but imho is not a big > deal. > > Thoughts? Ideas? Is it something other people would be interested into? > > Regards, > > Alex > <kannel-command-status.patch> > > >
