Different strings passed to JS state transform depending on UI where Number:Time Item is being rendered
Closed this issue · 19 comments
Applies to
- Main UI Item state display, AND
- Basic UI Sitemap Text element state display
Actual Behaviour
OH passes DIFFERENT strings, (either a QuantityType compatible string, or ii) a DateTimeType string), to a JavaScript transform for rendering a Number:Time Item state depending on whether the item is being displayed via Main UI or via a Basic UI sitemap Text element.
Expected Behaviour
OH should pass the SAME string to the JavaScript transform for rendering an Item state REGARDLESS of whether the item is being displayed via Main UI or via a Basic UI sitemap Text element.
Precondition
An Item is defined using a JavaScript transform to format its state description.
// item
Number:Time System_CPU_Uptime "System CPU Uptime [JS(24g-uptime.js):%s]" <time> {channel="systeminfo:computer:g24:cpu#uptime"}
// sitemap
Frame label="System Information" {
Text item=System_CPU_Uptime
}
Test Cases
- Main UI is used to display the Item state => Result: OH passes a QuantityType compatible string e.g. "
81612 s" to the JS transform. - Basic UI Text element is used to display the Item state => Result: OH passes a DateTimeType string e.g. "
1970-01-01T22:40:12Z" to the JS transform.
Reference
See forum thread
So MainUI is using the displayState from the SSE event stream, whereas sitemaps have their own event stream.
The displayState for Main UI is created here:
The sitemap event stream state is created here:
Now it is needed to further track down where and how the transformation service is called.
Which means this is in fact a core issue and no UI issue.
BasicUI is only a frontend for sitemaps, core is where the interesting stuff regarding sitemaps happens.
this is in fact a core issue and no UI issue.
@florian-h05 thanks for the insight. Could you please move the issue to openhab-core as I don't have rights to do that? TIA
Unfortunately I also don't have the rights at the core repo.
@kaikreuzer Can you please move this issue?
For sitemap UIs, I believe the item state is formatted at this place before applying the transformation:
As you can see, "state.format(value)" is used where value is "%s" extracted from "[[JS(24g-uptime.js):%s]". It explains why a date time format is passed to the transformation.
Main UI is using "state.toString()" instead.
It makes sense to me to consider the pattern like it is done in sitemap UIs, it allows controlling the format of value passed to the transformation. I mean you could use something else than %s if you want a specific format passed to the transformation.
Apparently Main UI is ignoring this pattern.
As a workaround, replacing "%s" by "%d s" should allow having the same value passed to the transformation in both cases.
It makes sense to me to consider the pattern like it is done in sitemap UIs, it allows controlling the format of value passed to the transformation. I mean you could use something else than %s if you want a specific format passed to the transformation.
While true, I don't like the state description pattern being used both internally with some logic, and for representation. It tends to create conflict and confusion down the line.
I also think the formatting logic for Number:Time was wrong and should not rely on a conversion to DateTime. Number:Time is to express a duration, not a date and/or time. See #4169.
@andrewfg @florian-h05 @mherwege : if you are ok, I update the class SseItemStatesEventBuilder to align the behaviours.
When using %s as pattern, there will be no difference in behaviour, correct?
In that case I’m fine with aligning the behaviour.
Is there still a difference if #4169 would be applied? I still think the format() method for Quantity Time produces the wrong result today, leading to this difference. It should not generate by default something that looks like a date/time, it should be a duration. There is specific handling for that and I think it is wrong.
When using %s as pattern, there will be no difference in behaviour, correct?
In that case I’m fine with aligning the behaviour.
Normally, when there is no transformation and %s is set as pattern, yes, options should have higher priority. But I will verify.
Is there still a difference if #4169 would be applied? I still think the format() method for Quantity Time produces the wrong result today, leading to this difference. It should not generate by default something that looks like a date/time, it should be a duration. There is specific handling for that and I think it is wrong.
It looks like a different problem.
My intention here is just to replace "state.toString()" by "state.format(pattern)" to have the same behaviour and so taking into account the pattern before calling the transformation.
But yes format("%s") has probably to be updated for time duration to not return a datetime. This is probably something to change in a separate PR. Maybe, it is what you did in 4169 (sorry I have not checked) ?
@mherwege : I believe this is the place where you would like to change format for QuantityType ?
But yes format("%s") has probably to be updated for time duration to not return a datetime. This is probably something to change in a separate PR. Maybe, it is what you did in 4169 (sorry I have not checked) ?
That’s indeed what I did there, in the code you link to.
So with the change I propose to implement + your change, Main UI and Basic UI will pass the same value to the transformation (my change that will call format for both UIs) and this value will look like "81612 s" for a Number:Time and pattern %s (your change).
So with the change I propose to implement + your change, Main UI and Basic UI will pass the same value to the transformation (my change that will call
formatfor both UIs) and this value will look like "81612 s" for a Number:Time and pattern %s (your change).
Yes, that is my understanding, whereby the unit in the value can be different depending on the unit set for the item.
The change is not as easy as I have imagined.
For information, in MainUI, the pattern is already applied to the state before calling the transformation. So same principle
as for sitemap UIs. It is just done in a slightly different way.
In fact, the problem is in class TransformationHelper that takes the item state as a string.
My previous suggestion to use "%d s" as final pattern should not work due to the way it is currently implemented.
I guess the current implementation only works when the final pattern contains %s.
This is due to this line:
As a fast fix, I could duplicate the method TransformationHelper.transform and have the new one with "State state" as parameter.
But as the method is used in other places, we have to take care of consistency. I will have to analyse everywhere where the method is called.
This issue has been mentioned on openHAB Community. There might be relevant details there:
https://community.openhab.org/t/oh4-number-of-type-java-lang-string-is-not-supported/156599/3