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.

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

Removing state presentation from item (file) ignored until OH restart; change default when state presentation is missing #2251

Closed
2 of 5 tasks
dilyanpalauzov opened this issue Jan 1, 2024 · 13 comments · Fixed by openhab/openhab-core#4068
Labels
bug Something isn't working

Comments

@dilyanpalauzov
Copy link

dilyanpalauzov commented Jan 1, 2024

Which UI are you reporting an issue for?

  • Basic UI
  • Android app
  • HABPanel
  • HABot
  • CometVisu

The problem

In OH 4.1 I create in an .items file

Number:ElectricPotential  blub "Trup"

Then I set its value to 7:

$ curl -X POST --header "Content-Type: text/plain" --header "Accept: application/json" -d 7 http://IP:8080/rest/items/blub
$ curl -X GET --header "Content-Type: text/plain" --header "Accept: application/json" http://IP:8080/rest/items/blub
{"link":"http://192.168.0.176:8080/rest/items/blub","state":"7 V","unitSymbol":"V","editable":false,"type":"Number:ElectricPotential","name":"blub","label":"Trup","tags":[],"groupNames":[]}

Now I insert this item as Setpoint in a .sitemap:

Setpoint item=blub minValue=-3 maxValue=9 step=1

OpenHAB-Android does not show the current value, until I tap on the item and a horizontal slider is shown.

BasicUI never shows the current value.

After I change the .items definition to

Number:ElectricPotential  blub "Trup[%s]"

both BasicUI and OpenHAB-Android show the current value. I revert the .items definition to its initial value

Number:ElectricPotential  blub "Trup"

As long as I do not restart OpenHAB, both BasicUI and openHAB-Andoid continue to show the current value (7) in the sitemap.

I observe the same behaviour when I use Text instead of Setpoint.

When I use not a Number, but a Switch item:

Switch                    rr "Kr"  // .items file

$ curl -X POST --header "Content-Type: text/plain" --header "Accept: application/json" -d ON http://192.168.0.176:8080/rest/items/rr
$ curl -X GET --header "Content-Type: text/plain" --header "Accept: application/json"  http://192.168.0.176:8080/rest/items/rr
{"link":"http://192.168.0.176:8080/rest/items/rr","state":"ON","editable":false,"type":"Switch","name":"rr","label":"Kr","tags":[],"groupNames":[]}

Switch item=rr // .sitemap file

In the sitemap the value is not explicitly spelled, as expected, as the toggle provides sufficient information about the state. When I change the sitemap element to

Text item=rr

thus to a switch in a read-only state, the current value is still not rendered. After I change the .items definition to

Switch                    rr "Kr[%s]"

BasicUI and Android-OpenHAB render the value of the text item. Then I alter the sitemap element back to Switch:

Switch item=rr

Now the user interface shows a toggle with the current value and in addition spells as text the current value.

Expected behavior

  • As tests demonstrate, the change of removing from the .items definition [%s] is ignored, as the presentation in the UI does not change anyhow after removing [%s]. But the presentation is different, when OpenHAB is booted without the [%s]. This is obviously a bug.

https://www.openhab.org/docs/configuration/items.html#state-presentation does indeed state, that missing both square brackets and state presentation will not show the current value to the user. I think this is a suboptimal default. The default shall be to show the current value and unit to the user, unless the item is Switch, Diagram. In my use case, depending on other adjustments a switch item can be read-only, in which case I present it as Text, or read-write, when I present it in a sitemap as Switch sitemap element. I want to have in both conditions the value rendered in a non-redundant way for the same item. Thus only a toggle in Switch-sitemap-element and only text in Text element.

  • Please change the default for .items without square brackets and state presentation, which items appear in a sitemap without a label, to show using some default their current value and unit, unless the item is a Switch. This is a more meaningful default than the current one. Right now to get it useful, information has to be presented redundantly, e.g. for Number:ElectricPotential with default unit of Volts, the configuration has to contain explicit V in the state presentation. For me having a Setpoint, which by default does not show its current value, but let the user change the current value, makes no sense. The minority, who does not want to have a state presentation, can still add explict [] in the .items definiton.
@dilyanpalauzov
Copy link
Author

dilyanpalauzov commented Jan 7, 2024

Also relevant: The documentation at https://www.openhab.org/docs/configuration/items.html#state-presentation states “If no state presentation and no square brackets are given, the Item will not provide a textual presentation of its internal state (i.e. in UIs no state is shown).” But the example for systeminfo suggests not to use state presentation/square brackets, and it still shows the current values (I tried only with CPU_Load5).

This is a contradiction: one the one side missing state presentation implies no state shown, but on the other side there is an example with state shown without explicit state presentation.

@lolodomo
Copy link
Contributor

Regarding change of config files (items, sitemap), did you see a "Loading xxx" in the logs confirming the loading of your changes and did you try Ctrl+F5 in your browser ?

Regarding the default pattern of an item, you already requested such change in openhab/openhab-core#3835.

I will explain the current algorithm (simplified version without considering [] for example):

  1. if a pattern is found in the label of the sitemap element, use it.
  2. If a pattern is found in the label of the item, use it.
  3. If a pattern is found in the linked channel, use it
  4. If the linked channel is without pattern but is for string or number, use respectively %s and %.0f as pattern
  5. if still not found, do not display any value

So for an item not linked to any channel in particular, do not expect the display of a value without setting a pattern.

That being said, when I implemented the default pattern for channels (see point 4 in my list) in 2017, I should have probably thought at item level rather than channel level.

I agree that a default pattern could be a good idea but as already mentioned there is no good default for datetime in particular because this default format should also consider locale (we are not formatting the date the same in France and in England for example).
Any change consisting in adding a default pattern will also be a breaking change to the existing behaviour.

@dilyanpalauzov
Copy link
Author

Regarding change of config files (items, sitemap), did you see a "Loading xxx" in the logs confirming the loading of your changes and did you try Ctrl+F5 in your browser ?

After I change only the item to the initial value, without [], I see in openhab log … Loading model 'abc.items'. I do refresh the browser, I load the /basicui/app?sitemap=ab address in a private tab, it still does show 7, thus as if [] has not been removed.

openhab/openhab-core#3835 is only about providing default values for DateTime items. It does reiterate the current problem of BasicUI keeping the state presentation, after it has been removed from the .items files. It discusses whether [%s] can be an acceptable default for DateTime items and in my opinion [%s] is an acceptable default, irrespective of localization. (But it should also visualize the NULL/UNDEF state in an unique way, if it currently does not). For non-DateTime items:

If the linked channel is without pattern but is for string or number, use respectively %s and %.0f as pattern

Does this step leave on purpose the units, or the default units respectively as for Volts in Number:ElectricPotential? Or it does consider the unit, and does use it in the pattern?

I suggest one more step, for items of type string or number, not linked to channels, use respectively %s and %.0f as pattern, including unit for the later, when known.

Adding that step indeed changes the default behaviour (breaking), but I think this change is good.

@lolodomo lolodomo changed the title [BasicUI] Removing state presentation ignored until OH restart; change default when state presentation is missing [UI] Removing state presentation ignored until OH restart; change default when state presentation is missing Jan 27, 2024
@lolodomo
Copy link
Contributor

lolodomo commented Jan 27, 2024

Regarding your issue with update of unmanaged items (file) not taken into account by UIs (Baisc UI and Android app), it is clearly not a bug in UIs but rather a bug in the core part. I was able to reproduce it: when adding the pattern to the item, I just have to hit F5 in my browser to get it but then when removing it, I can see using the REST API that the state description is in fact not removed from the item. It explains why UIs continue to display a value as if the old pattern was still there (as it is still there).
After saving my file containing this updated item without any pattern:

Switch TestSwitch "Test Switch"

the items REST API continues to provide this unexpected response:

{
  "link": "http://localhost:8080/rest/items/TestSwitch",
  "state": "ON",
  "stateDescription": {
    "pattern": "%s",
    "readOnly": false,
    "options": []
  },
  "editable": false,
  "type": "Switch",
  "name": "TestSwitch",
  "label": "Test Switch",
  "tags": [],
  "groupNames": []
}

The problem is in the handling of updated items (by the core framework) and more precisely the state description which is not removed.
Note that I can see in logs that my items file was reloaded, so it is not a problem with file change ignored but really something wrong when loading the updated file.

@openhab/core-maintainers : please move this issue in core repo.

@lolodomo lolodomo changed the title [UI] Removing state presentation ignored until OH restart; change default when state presentation is missing Removing state presentation ignored until OH restart; change default when state presentation is missing Jan 27, 2024
@lolodomo lolodomo changed the title Removing state presentation ignored until OH restart; change default when state presentation is missing Removing state presentation from item (file) ignored until OH restart; change default when state presentation is missing Jan 27, 2024
@lolodomo
Copy link
Contributor

lolodomo commented Jan 27, 2024

I don't know if this is new or not but Main UI does not display the pattern defined in file.
For example, here is my item defined in file:

String TestString "Test String [%s]"

I don't find this pattern in Main UI:
image

image

image

I have the strange feeling it was shown in Main UI in the past...
@florian-h05 : is it something that was changed ?

PS: here is the result of REST API:

{
  "link": "http://localhost:8080/rest/items/TestString",
  "state": "NULL",
  "stateDescription": {
    "pattern": "%s",
    "readOnly": false,
    "options": []
  },
  "editable": false,
  "type": "String",
  "name": "TestString",
  "label": "Test String",
  "tags": [],
  "groupNames": []
}

@lolodomo
Copy link
Contributor

lolodomo commented Jan 27, 2024

Just to summarize the problem: when a state description (pattern) is removed from an unmanaged item (from the file), the state description is in fact not removed and still there in the items registry after items file reloading.

@J-N-K : can you please help me going in the right direction ? Can you confirm that state descriptions are handled by core framework as metadata and stored in the metadata registry ? When an item is updated, I can see in the item registry that the method onUpdateElement calls injectServices which seems to restore metadata from the metadata registry. Could it be that state description was not removed from the metadata registry ?
https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ItemRegistryImpl.java#L259

@florian-h05
Copy link
Contributor

I have the strange feeling it was shown in Main UI in the past...
@florian-h05 : is it something that was changed ?

No, that shouldn't have changed in the last time.
IIRC correctly state descriptions determined by the square brackets in the Item label were never shown in the UI, only those defined in metadata though the metadata list.

@lolodomo
Copy link
Contributor

lolodomo commented Jan 27, 2024

I can also see this particular case in removed done only for managed provider. Is it normal to not call it also for unmanaged items:
https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ItemRegistryImpl.java#L411

@lolodomo
Copy link
Contributor

lolodomo commented Jan 27, 2024

The bug is really severe. In fact, when you just add/remove a pattern to the label of the item, the item registry is even not updated. When adding the pattern to the label, the metadata registry is certainly updated (when the file is loaded) while it is not when removing it (this is probably the bug).

All this leads me to the GenericItemProvider.

@lolodomo
Copy link
Contributor

Ok, I found and fixed the issue.

@lolodomo
Copy link
Contributor

@dilyanpalauzov : this was a great bug detection. Very strange that no one detected it earlier. As this issue will be closed when the fix will be merged, please create a new feature request in core repo for your request of default patterns, or even better simply enhance your existing feature request about default pattern for DateTime to make it more generic.

J-N-K pushed a commit to openhab/openhab-core that referenced this issue Jan 27, 2024
…4068)

* Remove state description when loading an item with a removed pattern

Fix openhab/openhab-webui#2251

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

* Extended integration test testSquareBracketsInFormat

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

---------

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
@lolodomo
Copy link
Contributor

lolodomo commented Jan 27, 2024

Closed by openhab/openhab-core#4068

@dilyanpalauzov
Copy link
Author

Closed by openhab/openhab1-addons#4068

Should have been closed by openhab/openhab-core#4068.

J-N-K added a commit to openhab/openhab-core that referenced this issue Jan 27, 2024
…4068)

* Remove state description when loading an item with a removed pattern

Fix openhab/openhab-webui#2251

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

* Extended integration test testSquareBracketsInFormat

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

---------

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
(cherry picked from commit a957122)
Signed-off-by: Jan N. Klug <github@klug.nrw>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants