-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[influxdb] Partial ModifiablePersistenceService support #12935
Conversation
<bundle start-level="79">mvn:org.openhab.addons.bundles/org.openhab.persistence.influxdb/${project.version}</bundle> | ||
<configfile finalname="${openhab.conf}/services/influxdb.cfg" override="false">mvn:org.openhab.addons.features.karaf/org.openhab.addons.features.karaf.openhab-addons-external/${project.version}/cfg/influxdb | ||
</configfile> |
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.
What is the reason for these changes?
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.
It was to start the persistence service before the bindings so they could make use of it. But the same night another pr with changes to core appeared rendering this change obsolete (when/if accepted)
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.
Should in fact all persistence bundles start before level 80?
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.
Just checked, without exception all persistence bundles start at level 80.
...xdb/src/test/java/org/openhab/persistence/influxdb/internal/ItemToStorePointCreatorTest.java
Outdated
Show resolved
Hide resolved
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/panasonic-comfort-cloud-binding/133848/26 |
@seime : you answered to comments but you pushed no change ? |
I was hoping this PR would be rendered obsolete by openhab/openhab-core#3000, but that one appears to be stuck :( |
@seime - would you like to pursue this PR to have a solution until core will be enhanced in this regard? |
f071a63
to
e5a2288
Compare
Signed-off-by: Arne Seime <[email protected]>
Signed-off-by: Arne Seime <[email protected]>
e5a2288
to
4564854
Compare
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/using-persistenceservice-in-binding/143477/10 |
Signed-off-by: Arne Seime [email protected]
Partial support for persisting values with a specific timestamp by implementing
ModifiablePersistenceService
instead ofQueryablePersistenceService
.By also changing the startlevel to 79 (before bindings), it allows the persistence service to be called from bindings to persist previous (ie offline sensor time series being reported when sensor is offline) or future time series data such as electricity prices or weather forecasts.
A better approach would be to build this into the framework, but this seems to be a larger and quite more complex job unfortunately. See openhab/openhab-core#844
EDIT: Someone just created a PR to handle this properly in core; openhab/openhab-core#3000