After upgrade to openHAB 4.0.2 my rules seems to behave in a strange way, after some testing it looks like the “Less then” and “Greater then” comparison gives a inverted return value.
I made the following test rule :
var thread = Java.type('java.lang.Thread')
thread.sleep(100);
if (items.getItem('KantoorFibaroRGBWMonitorBacklight_Dimmer').state > '80') {
console.info('Test1');
} else {
console.info('Test2');
}
console.info('Test3');
The value of item “KantoorFibaroRGBWMonitorBacklight_Dimmer” is set to 100:
When i execute the script i would expect to see in my Log Viewer info text “Test1” and “Test3”, for value 100 (KantoorFibaroRGBWMonitorBacklight_Dimmer) is greater than 80.
var thread = Java.type('java.lang.Thread')
thread.sleep(100);
if (items.getItem('KantoorFibaroRGBWMonitorBacklight_Dimmer').state < '80') {
console.info('Test1');
} else {
console.info('Test2');
}
console.info('Test3');
When i execute the script again i would expect to see in my Log Viewer info text “Test2” and “Test3”, for value 100 (KantoorFibaroRGBWMonitorBacklight_Dimmer) is still greater than 80.
which returns a string. So, in this case that means '100' and not 100. When you do a comparison between strings (e.g., '100' and '80') the process looks at each character one at a time. Essentially it “alphabetizes” the strings. So because '1' comes before '8' the string '100' is less than the string '80'.
In order to do the comparison you want, you need to be working with numbers. You want the
block and to set that dropdown to numeric state. Then you can set up a comparison between that number and the number 80.
Which will give you the proper code comparison:
That seems like such a big trap for newbies! I’m sure there is a valid technical reason for it, but this isn’t intuitive.
From a casual observation: Why can’t it be designed so that the of item <X> block can accept an item or an item name, to reduce redundancy/verbosity? It doesn’t flow when you read Get numeric state of item get item item X. It would read much better if it said Get numeric state of item X. I know this is probably not the right place to discuss it, though.
@stefan.hoehn can definitely tell you better, but as I understand it, Blockly just doesn’t have great type checking. Also, Blockly is now based on the JSscripting library and in that library state returns the stringified raw state.
I know this has been discussed at length, but I don’t remember the details behind the final decision.
The only think I can add to what @JustinG has said is that there is a very strong requirement that the code produced by these blocks be simple and straight forward. Similar proposals have been rejected by maintainers in the past.
So even if it’s technically feasible (which as mentioned Blockly has some problems determining type in a lot of cases so it might not be feasible) it might be blocked because the code behind the blocks become too complex.
I do wonder though if something can be done here to make things better. Some ideas (some of these ideas could be combined):
mark the “get state of item” block as deprecated to encourage the use of the get [name] of item block instead
change the name of that block to “get state of item as string” or something like that
add in a “get numeric state of item” and “get quantity state of item” block to make it more clear that if you want to do math you need to get the state as a number
make it clear in all the blocks that return the state as a string that this is what’s happening, not just these blocks
change the default of the “get [name] of item” to “get [numeric state] of item”. If the first idea is selected, “get [state as string] of item”.
@stefan.hoehn, do you have any thoughts? Is this worth an issue?
It is tripping up a lot of users, particularly those who migrated from 3.4.
Thanks for explaining it well. I put a lot of thought into it to make it as straight forward as possible but as you mentioned I need to find out during code generation and not during runtime. The latter would bloat the code which Yannick was very much against it. A few months back we found out how we can detect the type of block that is added, which allows us to tailer the generated code but as far as variables is concerned no type is carried along so everytime a var is added to the block I need to make assumptions.
One thing to mention is that I was really trying to break as few things as possible with 4.0 which is why I tried not to drop blocks or let them behave differently.
Not sure how we would do so. Adding that to the docs is probably not effective because many people would probably overlook that but surely this can be added to the docs
Sure, that would be an easy thing.
Do you mean, make sure or document (make clear). What do you mean with “this is what is happening”. Do you mean check if there are blocks that suppose to return a string state but don’t?
I second the fact that I don’t like that either. I actually implemented much more resilience towards that, so you add either one of the blocks (getItem) and omit the other one. For some reason I must have failed there and created other side effects or regressions.
Maybe adding a “" to it or some other symbol with a note somewhere on the page saying "blocks with "” are deprecated and should be removed.
Or adding a warning level log statement that prints that to the logs when ever a deprecated block is used. Just some sort of indication that the block shouldn’t be used, obvious and annoying enough to encourage users to switch but not so much that it’s continued use become unbearable.
The blocks themselves cannot actually be removed until OH 5 I would think as that would be a breaking change. But at some point you will need to replace a block or two, may as well start thinking about that now. I’m not saying this is the best way to handle this particular situation, it’s just one option among many.
Any block that returns a state as a String should indicate that it’s returning a String in the block name itself.
That or “state as string”. Any one of the three get states should probably be the first entry. This could be useful on it’s own but it will be vital if the “get state of item” block becomes marked as deprecated so it’s obvious what block they should use instead.
I added this functionality now and I hope it will not create a new regression.
The block can now take both “get item item xxx” or (new) “item xxx” directly. I have also added some hint in the help tooltip that clarifies how to work with the block when using it through variables because Blockly has to make an assumption due to the mentioned fact that it cannot detect what is provided in the variable.
Just an idea: put all the deprecated blocks in a special Deprecated sub group on the left.
That would make it extremely clear that they are deprecated, and when you’re creating a new script you won’t even see them in the usual category so you won’t use them by mistake.
Let me think about how we could group that and thanks for the idea.
As far as the versioning is concerned I actually have no creative idea how this could be done in Blocky (and if that would be maintainable without too much effort).
