Hi Jan:
On Wed, Jul 22, 2015 at 8:30 PM, Jan Willamowius jan@willamowius.de wrote:
I can understand the reasoning behind adding the rewrite for softphones. The room@IP (or IP##room) syntax is used a lot when dialing into MCUs for example (a lot as as in 80% of what some users dial). Since there is no consensus between the major vendors what is the 'right' syntax, most end users are utterly confused what to use. So even when the are able to dial the @, they might dial ## because they always did before...
I understand a little bit more. And I fully agree into having that translation into a softphone, and if I had to make a softphone I'll probably put it ( in the softphone ).
And I could even agree to have it into a library, but conveniently isolated and not into the base class, specially when it conflicts with some legal uses of ##. And that is not the major offender, current versions ( since 1.25.4 ) change # to % if I read the code correctly.
Changing existing behavior in a framework is a very tricky decision. And H323Plus has always put a lot of effort into maintaining compatibility.
Not so much. According to our bisections since h323+ 1.25.4 we have:
PINDEX phash = _remoteParty.Find("#"); if (phash != P_MAX_INDEX) { remoteParty.Replace("#","%"); PTRACE(4, "H323\tAdjusted " << remoteParty); }
So I cannot use # in any place in any number ( as soon as I put one, even if it is a sacrificial one at the front, it's going to get changed to % and my remoteParty is no longer able to fit into the dialedDigits choice of an AliasAddress.
We have not hit it because we base our updates in the current version. Last one was in 1.25.0. Current one is trying to be based in 1.26.5 ( we couldn't do it cause we hit another bug in the way the gkclient ras code manages ip address, due to a change which we are preparing to report ), and we have it frozen till we finish the ## / # patches, as the second one is changing existing behaviour and destroying us.
..But it is also a good thing if not every application has to code the same thing and the framework provides these bits and pieces that many applications want.
Completely agreed. Having a slew of properly maintained methods in the library is nice, keeps errors out, pervents duplicate efforts, but...,
My preferred solution would be to factor rewrites as this one into a separate endpoint method ("ConvenienceRewrites()" ?) and have a switch if its called or not. I would default the switch to not do the rewrite, but since none of my apps is hurt either way, I'll leave it to you guys to battle out the implementation. ;-)
Will do. I would also prefer the naked library to not rewrite, in fact I would default the method to a hook ( called but NOP one ), and make a derived 'softphoneEndpoint' with the fleshed out hook and some settings ( as we have three behaviours, no-rewrite, 1.25 mode, 1.26 mode, setting that would need IMO an 'applied rewrites' with some commonly used predefined values, implemented in any sensible way )
Also, as I previously stated, the hook method seems like a great point to put it for me, even if the default one destroys #. This means I can maintain a single patch to deactivate itand be reasonable confindent any future number destroying backwards incompatible transformation have a place to be put which will not need new patches ( as I would patch away the method call or derive and zap it ).
I'll wait for some feedback from Simon and do what I can to have a better, greater and prettier h323+, as I think this kind of changes are bad for its future ( they steer it towards being a niche library for softphones and users who are incapable of transforming the numbers in the proper place ( I mean 'softphone developers who are not able to apply the transformations before passing alias/numbers to the library' ), I'm all for helping them as long as it does not hurt other users ).
Francisco Olarte.