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

Settings -> Items keeps refreshing when items are added by rules, making it impossible to use. #2819

Closed
SkyLined opened this issue Oct 22, 2024 · 15 comments
Labels
bug Something isn't working main ui Main UI wontfix This will not be worked on

Comments

@SkyLined
Copy link

The problem

I've recently added a rule that runs frequently and creates and destroys new String items to track various things. I've found that the main Items UI (Settings -> Items) appears to automatically refresh every time this happens. The UI is not providing any information when it is being refreshed, as it is showing what I would describe as a loading placeholder instead of the Items.

After each such fresh, any "Search" value I've entered is ignore if there are any items matching it. The value is still there in the input field but if any Items match, all Items are listed instead of only the Items that match value I have entered. If no items matches, "Nothing found" will be displayed as expected.

If you happen to by typing in the "Search" field during a refresh, some characters you type may disappear. It looks like the value is set after the refresh completes to the value it had before the refresh started.

Expected behavior

  • Since there is a manual "REFRESH" button at the bottom of the page, I do not expect an auto-refresh. I expect the page to load the list once, filter this list if I type in search but never update the list until I manually click the "REFRESH" button.
  • After a refresh, the "Search" filter should work as expected, i.e. only Items that match it should be shown.
  • When typing in the "Search" field during a refresh, all changes should to the value in the field should remain, i.e. a refresh should not modify the value of the "Search" field.

Steps to reproduce

  1. Create a rule that runs every second
  2. Have the rule add an item named "test" if one does not exist and remove this item if it does exist.
  3. Enable the rule, so a string items is created every other second and delete again a second later.
  4. Visit the Settings -> Items page to see that it automatically gets refreshed frequently.
  5. Type something in the search field that matches some but not all items; notice that you will see all items after a refresh.
  6. Type a very long string in the search field while refreshes are taking place; notice that some characters you type are removed again.

Your environment

The requested information contains a lot of irrelevant data that I do not wish to share. If you need anything specific, please do let me know and I will provide it.

Browser console

No relevant messages or errors

Browser network traffic

Each refresh results in one or more requests for /rest/items?metadata=semantics.

Additional information

@SkyLined SkyLined added bug Something isn't working main ui Main UI labels Oct 22, 2024
@rkoshak
Copy link

rkoshak commented Oct 22, 2024

I've recently added a rule that runs frequently and creates and destroys new String items to track various things.

For starters this is probably an anti-pattern. It's defnintely not an expected way to use Items. Alternatives include Item metadata, the shared chache, and a generic storage registry that can be accessed from rules (I've done it once but can't remember how I did it and would have to look through my git history to find it, assuming I ever checked it in). Item metadata and the registry are permament and the cache is ephemeral (i.e. it gets cleared on an OH restart).

I'm not saying that there are not improvements that may be needed for the Items page. It should handle new Items being created better than this. But your specific use case I am certain is far beyond anything that the Items settings page was designed to handle. In architectural terms, Items are expected to be reltaviely static and permament. You are not adding and removing devices once per second in a typical home environment.

But I am saying that the Items settings page may not be the only place you run into problems with Items changing like this this fast.

@SkyLined
Copy link
Author

SkyLined commented Oct 22, 2024

For starters this is probably an anti-pattern.

I know Items are not ideal for this. The only reason I am doing this is as a work around because the shared cache is broken too (see openhab/openhab-core#4413). Using Items appears to allow me to avoid triggering concurrency issues in JavaScript when multiple rules in multiple threads are trying to communicate.

But your specific use case I am certain is far beyond anything that the Items settings page was designed to handle.

Bugs are always found in edge cases or they would not end up being shipped in release :).

You are not adding and removing devices once per second in a typical home environment.

The repro steps make the problem as easy to reproduce as quickly as possible. If you do this every few minutes or so, you will still encounter the issue, just less often. I highly doubt anyone else that experiences it will understand what is going on and report it: it's likely to happen only very occasionally at random times and does not have a clear cause or error report to help understand what happened.

But I am saying that the Items settings page may not be the only place you run into problems with Items changing like this this fast.

I do hope that the code is reliable and doesn't break in unexpected ways if I do anything slightly different than most users. I've tried many other home automation platforms before openHAB and moved away from each because they were unreliable and didn't provide any useful feedback when things didn't work.

@rkoshak
Copy link

rkoshak commented Oct 22, 2024

the shared cache is broken too (see openhab/openhab-core#4413).

This is a limitation of GraalVM JS. It simply does not support multithreaded operations, full stop. However, JS Scripting is the only rules language with this limitation. Rules DSL, Groovy, jRuby, and even Jython are alternatives that do not have this limitation. You can avoid this problem now by using a different rules langauge.

Note that Blockly "compiles" to JS Scripting.

I do hope that the code is reliable and doesn't break in unexpected ways if I do anything slightly different than most users. I've tried many other home automation platforms before openHAB and moved away from each because they were unreliable and didn't provide any useful feedback when things didn't work.

We do our best but if that's not good enough for you 🤷‍♂️ . It's an all volunteer effort and we do not begrudge anyone choosing another solution if that works better for them.

@SkyLined
Copy link
Author

SkyLined commented Oct 22, 2024

You can avoid this problem now by using a different rules langauge.

Thanks, I'll have a look. I may try to create a stub or wrapper to access things from JavaScript through one of these languages. I'm hoping to work around this issue without having to learn a new programming language and porting all my scripts.

We do our best but if that's not good enough for you 🤷‍♂️

I did not mean to offend. I was simply surprised at your suggestion that openHAB is not reliable. I've found openHAB to be much more reliable than other options I've tried and the few issues I did find so far were reasonably straight forward to root-cause and reproduce, and understandable oversights from a programmer's point of view. Nothing so far suggested that I would run into problems if I used its features slightly differently than the average user. I'm hoping this is an exception :)

@florian-h05
Copy link
Contributor

I've recently added a rule that runs frequently and creates and destroys new String items to track various things.

That’s not really how it is supposed to be used … but I understand why you do.
Putting in another workaround for the shared cache issue: Have all rules that need to access the audio file queue in a single JS file and use a variable outside of the rule callbacks instead of the shared cache. This is easier to use and is synchronised by the script engine’s synchronisation itself.

The UI is not providing any information when it is being refreshed, as it is showing what I would describe as a loading placeholder instead of the Items.

It is just reloading the list if a Item was added, updated or removed. While loading, the loading skeleton is shown. As long as people don’t add and remove Items dynamically, I haven’t heard somebody complaining about this before. That being said this doesn’t mean there is no room for improvement.

After each such fresh, any "Search" value I've entered is ignore if there are any items matching it.

That shouldn’t happen and needs to be fixed.

If you happen to by typing in the "Search" field during a refresh, some characters you type may disappear. It looks like the value is set after the refresh completes to the value it had before the refresh started.

Expected behaviour, but of course not good.

@rkoshak
Copy link

rkoshak commented Oct 23, 2024

I did not mean to offend. I was simply surprised at your suggestion that openHAB is not reliable.

I did not suggest OH is unreliable. But it definitely has a specific context in mind in how it's designed and built. If you go outside of that intended use YMMV. It may work great, iut may completely blow up.

As long as people don’t add and remove Items dynamically, I haven’t heard somebody complaining about this before.

This isn't something that most people do that much. Usually when they do it's at load time of the rule(s) and not periodically after the rule has loaded and it usually "create this Item if it doesn't exist", not create this Item and destory this other Item kind of thing. So this behavior in the UI wouldn't be noticed.

I agree there is room for improvement here in MainUI but I also think that using Items in this way should not be encouraged. We have lot of other options for storing ephemeral data like this which should be less disruptive overall compared to dynamically creating and destroying Items all the time and if any of those are not working as they should, we should address that use case there.

Maybe I do need to go back and see how to access and use the generaic data registry and add using that to the docs somehow. I'm assuming registry access is already thread safe so JS shouldn't have any problem with that like it currently does with the cache.
tl;dr, MainUI should work better but an alternative approach to creating and destroying Items should also be found and used instead.

@florian-h05
Copy link
Contributor

Since there is a manual "REFRESH" button at the bottom of the page, I do not expect an auto-refresh. I expect the page to load the list once, filter this list if I type in search but never update the list until I manually click the "REFRESH" button.

The refresh button is there to allow a manual refresh for updating the Item states. As Items are not supposed to be added and removed dynamically from scripts, the automatic refresh is no problem normally. I won't change that behavior therefore.

After each such fresh, any "Search" value I've entered is ignore if there are any items matching it. The value is still there in the input field but if any Items match, all Items are listed instead of only the Items that match value I have entered. If no items matches, "Nothing found" will be displayed as expected.

Just looking into this issue, and I cannot reproduce it at all, i.e.

After a refresh, the "Search" filter should work as expected, i.e. only Items that match it should be shown.

this is working exactly like that for me.

When typing in the "Search" field during a refresh, all changes should to the value in the field should remain, i.e. a refresh should not modify the value of the "Search" field.

As the search has to be re-initialised on refresh to properly work, this cannot be fixed.

TLDR; This is a won't fix, as this is only occurring when Items are used in "unsupported" ways. Let us rather focus on solving the root problem to avoid Item use in unintended ways.

@florian-h05 florian-h05 closed this as not planned Won't fix, can't repro, duplicate, stale Oct 24, 2024
@florian-h05 florian-h05 added the wontfix This will not be worked on label Oct 24, 2024
@SkyLined
Copy link
Author

SkyLined commented Oct 24, 2024

The refresh button is there to allow a manual refresh for updating the Item states.
As Items are not supposed to be added and removed dynamically from scripts, the automatic refresh is no problem normally.

If the list is not expected to change, why is it even auto-updating at all? It seems like removing the auto-update feature would fix my issue and not affect anyone, since nobody is supposed to be doing this.

Just looking into this issue, and I cannot reproduce it at all,

That is weird. Why could this work for you but not for me? I don't think a bug should be closed because it cannot quickly be reproduced be a single developer. But this is not an issue I care about enough to argue about too much.

As the search has to be re-initialised on refresh to properly work, this cannot be fixed.

It sounds like the "seach" input element is part of the refresh, when there is no reason for that. Only the list needs to be refreshed: the search input element can remain in place and allow the user to continue entering chars while the list is being updated. Once the updated list is retrieved from the server, the current value of the input field should be applied. But this is also not an issue I care about enough to argue about too much.

@florian-h05
Copy link
Contributor

florian-h05 commented Oct 24, 2024

If the list is not expected to change, why is it even auto-updating at all?

All lists are auto-updating, this has been that way when I joined the UI development - so I cannot say why it was decided to be that case. I think I can offer a compromise here: If the list changes, show a toast at the bottom of the screen asking if you want to reload.

Why could this work for you but not for me? I don't think a bug should be closed because it cannot quickly be reproduced be a single developer.

Which openHAB version do you use?
It is true that I am single developer, but as I am the main developer and not many others are contributing to all parts of the UI, it is likely me or no one working on that ...

It sounds like the "seach" input element is part of the refresh, when there is no reason for that.

I am currently working on that ... I cannot fix this, as we have to re-render the searchbar which is an external library component.

@SkyLined
Copy link
Author

SkyLined commented Oct 24, 2024

Well, I would mark these as low priority bugs personally, assuming I am the only person running into this. The thread synchronization issues are a much bigger problem for me; my work-around does not remove the issue, it just seems to make it less likely to happen.

I've already added a switch that stops the rules creating new Items when it's OFF. This kind of breaks things when it's OFF but I expect to only use is sparingly when I absolutely need to access the Items page.

@SkyLined
Copy link
Author

If you don't mind me asking: I had planned to create a system to report errors/warnings/notes to the user by creating new Items that contain relevant information and adding them to special groups. I wanted to create a pages that shows all Items in these groups, and optionally allows users to dismiss them, or detect when an error is resolved. Once dismissed/resolved, the Item would be removed from the group and deleted. I assumed this is a perfectly normal use case for Items and Groups. From the above I gather this is not what one would normally do. Any suggestions on an alternative form Items to design/implement something like this?

@florian-h05
Copy link
Contributor

The problem with this "use case" is that it is not intended to dynamically create, update and remove Items. These are stored on disk (and not in memory in contrast to Item states, which are dynamic) and we had some discussion some time ago about metadata, where it was also said that metadata are not meant to store dynamic data as well.

Any suggestions on an alternative form Items to design/implement something like this?

  1. Have an Item counting the number of errors/warnings/notes, one Item to send commands from UI to a rule and one Item to send information from rule to UI. This way, you could create a widget, that says "x warnings" with several buttons e.g. "next warning", "previous warning", "dismiss warning" and allows to click through the list of warnings.
    This would be implemented by updating the third Item on commands from the second one.
  2. Build a JSON string containing all warnings and sent it to an Item, read and parse the JSON from the Item state in the UI. You could create an array of strings, serialise it to JSON, send it to a string Item, read its state in the UI, parse from string to JSON, and use oh-repeater to show a list entry (or whatever) for each array element.

@rkoshak
Copy link

rkoshak commented Oct 24, 2024

Build a JSON string containing

I'll add that the oh-repeater widget in MainUI can parse that JSON stored in a String Item and render a card with list item widgets based on the contents of the JSON to give you a place to start looking.

@SkyLined
Copy link
Author

Thanks for the suggestions!

@florian-h05
Copy link
Contributor

#2837 fixes the flickering of the Items page on change, so it should be well usable now even with your case of dynamically adding & removing Items. Only the searchbar will flicker from now on, I haven't found a way to avoid (yet?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working main ui Main UI wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants