-
-
Notifications
You must be signed in to change notification settings - Fork 431
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
Implemented start level service #1914
Conversation
Signed-off-by: Kai Kreuzer <[email protected]>
Signed-off-by: Kai Kreuzer <[email protected]>
Signed-off-by: Kai Kreuzer <[email protected]>
Signed-off-by: Kai Kreuzer <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your awesome work on this! I'm very happy to see this being addressed just in time for OH3. 😃
I only have a few comments and think we should then proceed with merging this so we can start testing it and it makes it into the RC.
Can you also have a look at the failing tests?
...enhab.core.automation/src/main/java/org/openhab/core/automation/internal/RuleEngineImpl.java
Outdated
Show resolved
Hide resolved
bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ItemRegistryImpl.java
Show resolved
Hide resolved
Signed-off-by: Kai Kreuzer <[email protected]>
Signed-off-by: Kai Kreuzer <[email protected]>
Yep, I'm on it. |
Might become one of the major improvements of OH3 over OH2. I like. |
@wborn Tests succeeded! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot! 👍 Let's merge it so we can start testing it.
Let's do testing (and fixing 🤔) until tomorrow evening... |
@kaikreuzer could you confirm the "system started" rules still have to configured with start level 20? I just implemented some UI helper to abstract this when adding these sorts of triggers yesterday. |
@ghys Anything below 50 will do. |
Got it. So maybe offer these as options in a list? (or those above 50)
(this could also go to the configParameterDescription of the SystemStartlevelTrigger) |
Options above 40 make sense, yes. And indeed, they should be provided as configParameterDescription, I didn't do this yet. |
After first startup of snapshot 2066, I noticed in UI that no rules are loaded. I also noticed that I have no item state changes logged. |
Same for me. On the second startup, everything went well, though. |
Yes, second startup looks good, item/thing logging is back, I see no rule errors in log, my "system started" rule is run and without error, all rules are listed in UI. |
Sounds great - first sign that I didn't completely destroy the runtime ;-) For me all further starts (2-4 by now) went smooth as well. I guess the first start is our usual problem that the system runs amok directly after doing an update. |
After few other restarts, I finally got errors of this kind:
This is failing on lines of this kind:
while the rule trigger is "item xxxx changed". |
Another example, this time with ON instead of NULL:
Line 96 is:
It looks like errors are related to |
Errors are certainly occuring at first startup of the rule, while previous state is NULL. |
I even got this one:
One time again, it is related to previousState for a rule with a trigger "item xxx changed". Regarding the run of the rule with "system started", I have always seen this rule run correctly. This case is now working perfectly. |
@lolodomo Are those problems with the |
I will check again but I guess it is only temporary during startup. |
This would clearly reduce the severity of the issue! |
@kaikreuzer : as you provided several fixes since my tests yesterday, I will try again when a new snapshot is available and create an issue if I still see problems. |
@kaikreuzer : just another positive feedback. With snapshot 2085, I was able to restart openHAB server 5 times without any errors triggered by rules. Super. No need for me to open another issue ;) |
Excellent 🎉 |
Hi @kaikreuzer, just a small question without digging deeper into the code changes myself: |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/oh3-events-log-stays-empty/111043/12 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/oh3-events-log-stays-empty/111043/16 |
@maggu2810 Yes, reaching certain start levels requires certain components to be available in the distro. |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/publishmqtt-not-working-as-advertised-in-oh3/115502/13 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/touch-is-not-working-for-reloading-rules/118605/5 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/rule-engine-not-yet-started-not-executing-rule/129299/6 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/when-system-started-rule/129779/20 |
Fixes openhab#1637 Fixes openhab#1777 Fixes openhab#1734 Fixes openhab#1823 Fixes openhab#1808 Signed-off-by: Kai Kreuzer <[email protected]> GitOrigin-RevId: 076ac3f
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/startup-order-of-bundles-and-rules/153284/1 |
This finally implements eclipse-archived/smarthome#1896, fixes a couple of issues that currently exist due to the unpredictable startup sequence and it should also improve the startup performance, since activities do not have to be done multiple times in order to deal with lazy loaded stuff.
This service combines different ReadyMarkers into a new start level ready marker and thus lets other services depend on those, without having to know about the single markers.
This brings an important decoupling, since the set of markers for a certain start level might depend on the individual set up-
The start level service is therefore configurable (see here, so that users have a chance to adapt the conditions upon a certain start level is reached.
Start levels are defined as values between 0 and 100. They carry the following semantics:
Note: Levels 70 and 80 are not yet implemented in this PR, but should be added at a later stage.
While still being in a level <50, no events will be logged out anymore, which reduces the logs wrt "item added" and "link added" events tremendously.
I have also further improvements in mind like automatically registering what bundles are installed and for which markers needs to be waited, so that there is less necessity to adapt the configuration, if anybody does not use a "standard" setup of openHAB. Note that for anybody using the normal distro and not removing any essential parts of it, there should be no need to configure anything with the current implementation.
Fixes #1637
Fixes #1777
Fixes #1734
Fixes #1823
Fixes #1808
Might also fix #1365
Might also fix #1829
Signed-off-by: Kai Kreuzer [email protected]