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

[persistence] averageSince shouldn't return null #1333

Closed
5iver opened this issue Jan 18, 2020 · 3 comments · Fixed by #2931
Closed

[persistence] averageSince shouldn't return null #1333

5iver opened this issue Jan 18, 2020 · 3 comments · Fixed by #2931

Comments

@5iver
Copy link

5iver commented Jan 18, 2020

The JavaDoc says that the Items state will be returned if it has not changed since the DateTime specified, but it returns null. This should be changed. Changing the docs would be easy, but I think this would be better functionality.

@cweitkamp
Copy link
Contributor

To find an answer why null is returned you have to dive into this discussion: eclipse-archived/smarthome#2405

As the calculation considers the current value - if known - imo it would make sense to return the current value instead. Maybe @J-N-K can elaborate on this topic.

I think we have to change the JavaDocs anyway because the other method says to return null if no default persistence service is available which is not the case either:

* @return the average state values since <code>timestamp</code>, <code>null</code> if the default persistence
* service is not available, or the state of the given <code>item</code> if no previous states could be
* found or if the default persistence service does not refer to an available
* {@link QueryablePersistenceService}

@cweitkamp cweitkamp changed the title PersistenceExtensions.averageSince shouldn't return null [persistence] averageSince shouldn't return null Jul 16, 2020
Rosi2143 pushed a commit to Rosi2143/openhab-core that referenced this issue Dec 26, 2020
Signed-off-by: Jerome Luckenbach <github@luckenba.ch>
@J-N-K
Copy link
Member

J-N-K commented Apr 25, 2022

The problem with returning the current value is that this might be wrong. The calculation is essentially the integral of values over time \int_{t0}^{now} y(t) dt divided by the timespan t0-now. If we don't have at least two values (one stored and the current value), we can't calculate the average. I'm not convinced that returning the current state is the correct thing to do in that case.

@J-N-K
Copy link
Member

J-N-K commented Jun 5, 2022

I re-read the old discussion on ESH and I think the consensus reached is still valid. I'll adjust the JavaDoc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants