Different strings passed to JS state transform depending on UI where Number:Time Item is being rendered #4101

andrewfg · 2024-02-18T17:19:40Z

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

The text was updated successfully, but these errors were encountered:

florian-h05 · 2024-02-18T21:35:19Z

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:

openhab-core/bundles/org.openhab.core.io.rest.sse/src/main/java/org/openhab/core/io/rest/sse/internal/SseItemStatesEventBuilder.java

Line 105 in e628f75

private @Nullable String getDisplayState(Item item, Locale locale) {

The sitemap event stream state is created here:

openhab-core/bundles/org.openhab.core.io.rest.sitemap/src/main/java/org/openhab/core/io/rest/sitemap/internal/PageChangeListener.java

Line 211 in b64e972

    
           private SitemapWidgetEvent constructSitemapEventForWidget(Item item, State state, Widget widget) {

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.

andrewfg · 2024-02-18T23:00:12Z

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

florian-h05 · 2024-02-19T16:55:06Z

Unfortunately I also don't have the rights at the core repo.
@kaikreuzer Can you please move this issue?

lolodomo · 2024-04-15T11:29:15Z

For sitemap UIs, I believe the item state is formatted at this place before applying the transformation:

https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core.ui/src/main/java/org/openhab/core/ui/internal/items/ItemUIRegistryImpl.java#L461

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.

lolodomo · 2024-04-15T11:36:15Z

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.

lolodomo · 2024-04-15T11:48:27Z

As a workaround, replacing "%s" by "%d s" should allow having the same value passed to the transformation in both cases.

mherwege · 2024-04-15T13:45:56Z

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.

lolodomo · 2024-04-20T08:05:01Z

@andrewfg @florian-h05 @mherwege : if you are ok, I update the class SseItemStatesEventBuilder to align the behaviours.

florian-h05 · 2024-04-20T10:14:37Z

When using %s as pattern, there will be no difference in behaviour, correct?
In that case I’m fine with aligning the behaviour.

mherwege · 2024-04-20T11:21:45Z

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.

lolodomo · 2024-04-20T11:22:15Z

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.

lolodomo · 2024-04-20T11:38:09Z

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) ?

lolodomo · 2024-04-20T12:05:09Z

@mherwege : I believe this is the place where you would like to change format for QuantityType ?

https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core/src/main/java/org/openhab/core/library/types/QuantityType.java#L373

mherwege · 2024-04-20T12:09:13Z

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.

lolodomo · 2024-04-20T12:18:31Z

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).

mherwege · 2024-04-20T13:05:02Z

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).

Yes, that is my understanding, whereby the unit in the value can be different depending on the unit set for the item.

lolodomo · 2024-04-21T07:18:30Z

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.

lolodomo · 2024-04-21T17:13:11Z

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:

https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core.transform/src/main/java/org/openhab/core/transform/TransformationHelper.java#L167

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.

The idea here is to use state.format(format) instead of String.format(format, state.toString()) Related to openhab#4101 Signed-off-by: Laurent Garnier <lg.hc@free.fr>

The idea here is to use state.format(format) instead of String.format(format, state.toString()) As a consequence, the input value passed to the transformation is the same in all UIs (Main UI and sitemap UIs). Fix openhab#4101 Signed-off-by: Laurent Garnier <lg.hc@free.fr>

The idea here is to use state.format(format) instead of String.format(format, state.toString()) As a consequence, the input value passed to the transformation is the same in all UIs (Main UI and sitemap UIs). Also make sure to call transformation even for NULL and UNDEF states but without applying format to the input value. Fix openhab#4101 Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Use item state formatter to format input of transformation, meaning using state.format(format) instead of String.format(format, state.toString()) This was already the case in sitemap API but not in other APIs used by Main UI. Make sure to call transformation even for NULL and UNDEF states. It was not the case in one API used by Main UI. When calling transformation and state is NULL or UNDEF, do not apply format to the input value and do not replace by "-". That leans the transformation will be called with "NULL" or "UNDEF". Sitemap API was calling the transformation using a pattern containing "-". Fix openhab#4101 Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Use item state formatter to format input of transformation, meaning using state.format(format) instead of String.format(format, state.toString()) This was already the case in sitemap API but not in other APIs used by Main UI. Make sure to call transformation even for NULL and UNDEF states. It was not the case in one API used by Main UI. When calling transformation and state is NULL or UNDEF, do not apply format to the input value and do not replace by "-". That leans the transformation will be called with "NULL" or "UNDEF". Sitemap API was calling the transformation using a pattern containing "-". Fix openhab#4101 Also related to discussion in openhab/openhab-addons#13777 Signed-off-by: Laurent Garnier <lg.hc@free.fr>

Use item state formatter to format input of transformation, meaning using state.format(format) instead of String.format(format, state.toString()) This was already the case in sitemap API but not in other APIs used by Main UI. Make sure to call transformation even for NULL and UNDEF states. It was not the case in one API used by Main UI. When calling transformation and state is NULL or UNDEF, do not apply format to the input value and do not replace by "-". That means the transformation will be called with "NULL" or "UNDEF". Sitemap API was calling the transformation using a pattern containing "-". Fix openhab#4101 Also related to discussion in openhab/openhab-addons#13777 Signed-off-by: Laurent Garnier <lg.hc@free.fr>

openhab-bot · 2024-06-06T14:46:36Z

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

andrewfg added the bug An unexpected problem or unintended behavior of the Core label Feb 18, 2024

andrewfg changed the title ~~Core passes DIFFERENT strings to JS state transform depending on UI where Item is being rendered~~ Core passes DIFFERENT strings to JS state transform depending on UI where Number:Time Item is being rendered Feb 18, 2024

andrewfg changed the title ~~Core passes DIFFERENT strings to JS state transform depending on UI where Number:Time Item is being rendered~~ DIFFERENT strings passed to JS state transform depending on UI where Number:Time Item is being rendered Feb 18, 2024

andrewfg changed the title ~~DIFFERENT strings passed to JS state transform depending on UI where Number:Time Item is being rendered~~ Different strings passed to JS state transform depending on UI where Number:Time Item is being rendered Feb 18, 2024

kaikreuzer transferred this issue from openhab/openhab-webui Feb 19, 2024

lolodomo linked a pull request Apr 29, 2024 that will close this issue

Make input formatting for transformations consistent everywhere #4203

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Different strings passed to JS state transform depending on UI where Number:Time Item is being rendered #4101

Different strings passed to JS state transform depending on UI where Number:Time Item is being rendered #4101

andrewfg commented Feb 18, 2024 •

edited

florian-h05 commented Feb 18, 2024 •

edited

andrewfg commented Feb 18, 2024

florian-h05 commented Feb 19, 2024

lolodomo commented Apr 15, 2024 •

edited

lolodomo commented Apr 15, 2024

lolodomo commented Apr 15, 2024

mherwege commented Apr 15, 2024 •

edited

lolodomo commented Apr 20, 2024 •

edited

florian-h05 commented Apr 20, 2024

mherwege commented Apr 20, 2024 •

edited

lolodomo commented Apr 20, 2024

lolodomo commented Apr 20, 2024 •

edited

lolodomo commented Apr 20, 2024 •

edited

mherwege commented Apr 20, 2024

lolodomo commented Apr 20, 2024 •

edited

mherwege commented Apr 20, 2024 •

edited

lolodomo commented Apr 21, 2024

lolodomo commented Apr 21, 2024

openhab-bot commented Jun 6, 2024

Different strings passed to JS state transform depending on UI where Number:Time Item is being rendered #4101

Different strings passed to JS state transform depending on UI where Number:Time Item is being rendered #4101

Comments

andrewfg commented Feb 18, 2024 • edited

Applies to

Actual Behaviour

Expected Behaviour

Precondition

Test Cases

Reference

florian-h05 commented Feb 18, 2024 • edited

andrewfg commented Feb 18, 2024

florian-h05 commented Feb 19, 2024

lolodomo commented Apr 15, 2024 • edited

lolodomo commented Apr 15, 2024

lolodomo commented Apr 15, 2024

mherwege commented Apr 15, 2024 • edited

lolodomo commented Apr 20, 2024 • edited

florian-h05 commented Apr 20, 2024

mherwege commented Apr 20, 2024 • edited

lolodomo commented Apr 20, 2024

lolodomo commented Apr 20, 2024 • edited

lolodomo commented Apr 20, 2024 • edited

mherwege commented Apr 20, 2024

lolodomo commented Apr 20, 2024 • edited

mherwege commented Apr 20, 2024 • edited

lolodomo commented Apr 21, 2024

lolodomo commented Apr 21, 2024

openhab-bot commented Jun 6, 2024

andrewfg commented Feb 18, 2024 •

edited

florian-h05 commented Feb 18, 2024 •

edited

lolodomo commented Apr 15, 2024 •

edited

mherwege commented Apr 15, 2024 •

edited

lolodomo commented Apr 20, 2024 •

edited

mherwege commented Apr 20, 2024 •

edited

lolodomo commented Apr 20, 2024 •

edited

lolodomo commented Apr 20, 2024 •

edited

lolodomo commented Apr 20, 2024 •

edited

mherwege commented Apr 20, 2024 •

edited