Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make input formatting for transformations consistent everywhere #4203

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Make input formatting for transformations consistent everywhere #4203

wants to merge 1 commit into from
+246 −68

Conversation

lolodomo
Copy link
Contributor

@lolodomo lolodomo commented Apr 29, 2024 •
edited

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 #4101
Also related to discussion in openhab/openhab-addons#13777

Signed-off-by: Laurent Garnier lg.hc@free.fr

@lolodomo lolodomo requested a review from a team as a code owner April 29, 2024 08:58
@lolodomo lolodomo marked this pull request as draft April 29, 2024 08:59
@lolodomo lolodomo force-pushed the transform_state_format branch 4 times, most recently from 93a78f3 to 238d91e Compare April 29, 2024 10:09
This was referenced May 3, 2024
Merged
Open
@lolodomo lolodomo force-pushed the transform_state_format branch 5 times, most recently from 53a4385 to a2b7e74 Compare May 5, 2024 09:38
@lolodomo lolodomo force-pushed the transform_state_format branch 3 times, most recently from 50eec80 to 93376fb Compare May 6, 2024 10:35
@lolodomo lolodomo changed the title Use item state formatter to format input of transformation Make input formatting for transformations consistent everywhere May 6, 2024
@lolodomo
Copy link
Contributor Author

lolodomo commented May 6, 2024 •
edited

With the following items:

Number Test01 "01 0 without MAP" [ "Status", "Light" ]
Number Test02 "02 0 with MAP found entry [MAP(en.map):_%d_]" [ "Status", "Light" ]
Number Test03 "03 1 with MAP not found entry [MAP(en2.map):_%d_]" [ "Status", "Light" ]
String Test04 "04 high without MAP" [ "Status", "Light" ]
String Test05 "05 high with MAP found entry [MAP(en.map):_%s_]" [ "Status", "Light" ]
String Test06 "06 low with MAP not found entry [MAP(en2.map):_%s_]" [ "Status", "Light" ]
Number Test07 "07 UNDEF without MAP" [ "Status", "Light" ]
Number Test08 "08 UNDEF with MAP found entry [MAP(en.map):_%d_]" [ "Status", "Light" ]
Number Test09 "09 UNDEF with MAP not found entry [MAP(en2.map):_%d_]" [ "Status", "Light" ]
String Test10 "10 NULL without MAP" [ "Status", "Light" ]
String Test11 "11 NULL with MAP found entry [MAP(en.map):_%s_]" [ "Status", "Light" ]
String Test12 "12 NULL with MAP not found entry [MAP(en2.map):_%s_]" [ "Status", "Light" ]

and en.map containing these entries:

_0_=This is value 0
_high_=Value is high
NULL=States is NULL
UNDEF=State is UNDEF
-=NULL or UNDEF
_-_=State is NULL or UNDEF

and en2.map containing only:

CLOSED=closed
OPEN=open

and with sitemap:

    Frame label="MAP & NULL state" {
        Text item=Test01
        Text item=Test02
        Text item=Test03
        Text item=Test04
        Text item=Test05
        Text item=Test06
        Text item=Test07
        Text item=Test08
        Text item=Test09
        Text item=Test10
        Text item=Test11
        Text item=Test12
    }

The result in Main UI (property card) is now:
image

The result in sitemap UI is now:
image

You can see that the result is consistent between Main UI and sitemap UI even if each UI has its own way to display a NULL or UNDEF state. Main UI is displaying an empty string for a NULL state while sitemap UI is displaying "-" for NULL and UNDEF states.
Note that there is a little bug in Main UI as the displayed value for item Test11 should be "State is NULL". It is correctly displayed in the item page but not in a property card:
image

As a comparison, here is the result in Main UI before this PR:
image

And the result in sitemap UI before this PR:
image

The noticeable changes:

  1. Main UI is now displaying the expected value for item Test02
  2. Sitemap UI is now displaying the raw state value if the transformation failed (items Test03 and Test06)
  3. Main UI and istemap UI are now considering transformation for NULL and UNDEF states (items Test08 and Test11)
  4. Sitemap UI is now displaying "-" for NULL and UNDEF states when transformation failed (items Test09 and Test12)

@lolodomo lolodomo marked this pull request as ready for review May 6, 2024 11:32
@lolodomo
Copy link
Contributor Author

lolodomo commented May 6, 2024

This is now ready for review, I have updated the initial message and I provided screenshots of Main UI and sitemap UI (Baisc UI) showing the different cases when transformation is used.

@lolodomo lolodomo force-pushed the transform_state_format branch 2 times, most recently from ec232f8 to 3dcd04c Compare May 6, 2024 11:37
@lolodomo
Copy link
Contributor Author

lolodomo commented May 6, 2024 •
edited

The consequence for users of these changes is that now they will have to consider input value "NULL" and "UNDEF" in their transformations. It will impact mainly sitemap UIs users as they could have considered "-" as input value rather than "NULL" and "UNDEF". Users using only Main UI should probably be not impacted, they will just be happy with the fix for use case Test02.

@lolodomo
Make input formatting for transformations consistent everywhere
653fad3 
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@J-N-K J-N-K J-N-K left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Different strings passed to JS state transform depending on UI where Number:Time Item is being rendered
2 participants
@lolodomo @J-N-K