A user has kindly contributed this PR to add a useful feature to the TCP binding (old 1.x binding), to eliminate the burden of putting a transformation in the binding config string, when all you want to do is send a literal string. My preference is to follow the same pattern used by other 1.x bindings that allow either the “TRANSFORMATION(function)” syntax, or the “StringLiteral” syntax. Paolo’s PR has the syntax “(StringLiteral)”, which works fine, but the purist in me is hoping for greater consistency with other bindings. In the end, I don’t suppose it’s a big deal either way, but I wanted to get feedback from users regarding this feature and how they would like to see it configured, instead of my deciding alone to merge the current PR, or asking Paolo to rework it. Plan first; merge last!
Thoughts? Please reply here, instead of on the PR, so all the feedback is in one place.
I’ve not used it, and not looked at it, so maybe I’m speaking a bit out of turn, but I would strongly suggest to try and keep syntax the same between bindings where possible - it will make things easier for users in the long run…
I agree as well. The more the same we can keep the syntax between the different binding configs (and Things for OH 2) the easier it is for users to learn and transfer their knowledge to other bindings.
Would you agree that it might be an opportunity to compatibly expand the binding’s syntax to work in the same way as the MQTT binding, supporting default, the transformation syntax, or a string literal? I too would like to see some harmonisation for the purpose of decreasing complexity.
I like “default” or “". I’ve seen both used for similar meanings. Though I’m cautious about using "” only because in some cases (Persistence being the main one) “*” is used counter to expectations (i.e. lots of people assume the * means a wild card and will match an Item that starts with the previous string as opposed to its ream meaning of all members of this group).
Just to add, in the case where you would want to send the literal string “default”, which would be rare, you would have to use the transformation option.
I guess we should go for the harmonisation option then. Is there a more precise definition than the wiki of what is this de facto standard you wish to put in place based on the MQTT binding? From what I understand, assuming
<transformation>
is the string passed as an argument in the item definition, the algo is:
(1) if it equals default return ?
(2) if it is not in the form TEXT1(TEXT2) then return the whole string
(3) if can’t find TEXT1 as valid transformation then return the whole string
(4) if TEXT2 not valid argument of transformation TEXT1 then return the whole string
(5) return the result of the transformation TEXT1(TEXT2)
If that’s the case I don’t know what the default do. Also it’s clear that in this context there are no WARNINGS or ERRORS produced in case of typos calling for valid transformations.
Anyway, happy to propose a PR based on this.
Here is what I would suggest for the TCP/UDP binding. If the <transformation> field is:
'TEXT1(TEXT2)' : handle fully compatibly with the existing code (note the single quotes). One performance improvement you could introduce is that if the transformation string specified in the binding config is 'REGEX((.*))', then recognize it for what it would do without having to go through an actual transformation.
default : the toString() version of the State or Command
TEXT1(TEXT2) : attempt to use the transformation TEXT1 with argument TEXT2. Log a warning if unable to transform, and proceed as if default had been specified. Also, add the same optimization when you see REGEX((.*)) as described above.
any other string : use it as specified.
Note that since any string can appear as the <transformation>, there can’t be an error at the time the binding config string is parsed, since values like WRONG(THING) can’t be detected as unusable until the time it needs to be used.
For any <transformation> values you want to pass that are either the literal string default or match the TEXT1(TEXT2) format, you will have to use a real transformation in order to produce the desired result.
Does that sound to you like how the MQTT binding works? Any conflict with what I described and the semantics of how the MQTT binding works would be resolved in favor of how the MQTT binding works.
I have checked the code of the MQTT Binding in order to really stick to a ‘MQTT’ style interpretation of the <transformation>. The code is quite different from the TCP binding but anyway in a nutshell it seems to work like this:
if <transformation> is empty or default then it returns the State or the Command as a String.
if it is written like TEXT1(TEXT2) then it goes through the transformation (Note: the MQTT binding doesn’t even check that the last character is ‘)’, it just assumes it, bad for typos).
anything else and it sends back the <transformation> string as result.
I am not sure to understand the difference you make between single quotes or not being there. Also, at this stage I prefer to leave alone the REGEX case. Anyway, I have proposed a new PR here.
The single quotes would have to be supported for backward compatibility, but they seem superfluous to me and I don’t know of another binding that uses them like this. So as long as the new, MQTT-style syntax is supported as well as being backward compatible with existing users, that’s good.
Sure; that’s an optimization for another day, and also kind of pointless when default would mean the same as REGEX((.*))?
