add feature sumSince to persistence extensions #376
Conversation
Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
| while(it.hasNext()) { | ||
| State state = it.next().getState(); | ||
| if (state instanceof DecimalType) { | ||
| sum += ((DecimalType) state).doubleValue(); |
There was a problem hiding this comment.
Why do you use the (unprecise) double value here instead of summing up the (exact) BigDecimals?
|
Thanks for this PR! It seems as if you do not have signed the CLA, right? Could you please do this? (see https://wiki.eclipse.org/Development_Resources/Contributing_via_Git#Eclipse_Foundation_Contributor_License_Agreement for instructions) |
|
CLA is there now, thanks! So I am only waiting for an answer on #376 (comment) now. |
|
Because all other functions like averageSince do the same. Its about consistency. Sorry fot not coming back to this issue earlier. I did not receive your update. |
Well, minimum and maximum work with the precise values. average obviously anyhow results in some "unprecise" value since it does floating point arithmetics. But a sum of exact values stays an exact value - hence I would expect here not having to deal with possible rounding issues etc. Since we have the precision, why not use it here? |
|
@J-N-K Would you be ok with using BigDecimals here and update the PR accordingly? We could then merge it. |
|
it would be great if you could sync the change also against OH1. Thanks @J-N-K |
|
Using BigDecimals is ok for me, but I will not be able to update this PR unless I have solved some more important issues for me. |
|
No worries, there's no need to rush. |
|
Is there a process to merge changes to the core OH1 with ESH. The persistence extensions seem to have had significant work done since the code was released to Eclipse. Or is it a case of having to pull request each OH1 core update ? you have to assume if there is a change in OH1 then its a requirement that a user wanted. Im not sure how migrating users from OH1 to OH2 will be feasible without ESH/OH2 having all the functionality of OH1. I personally make use of lastUpdate in rules on OH1, but ESH doesnt have this in its persistence extension, hence my question.am i barking up the wrong tree ? (or is that subtree :) ) |
|
@mccaffm You are fully right, there must be no changes in the openHAB 1 runtime that are not reflected in Eclipse SmartHome as well as otherwise the backward compatibility will be lost. To guarantee this, we communicated a long while ago that the openHAB 1 runtime is considered to be feature-frozen and only critical patches should be done to it. @teichsta tracks all changes that are done to the openHAB 1 core and makes sure that they also end up in Eclipse SmartHome, either by migrating them himself or by asking the contributor of the change to do so prior to closing/merging the issue in openHAB 1. At least this is the agreed process. If you feel (and from your comment you seem to) that some features got lost on the way, please immediately tell @teichsta about it - the best way to do this is to (re-)open an issue in the openHAB 1 issue tracker and mark it as critical. Thanks! |
|
@mccaffm any specific case you are referring to? |
|
Just compare the persistence extensions between oh1 and ESH. Unless I'm missing something lots of features are missing. M On 8 Oct 2015 21:52, at 21:52, "Thomas Eichstädt-Engelen" notifications@github.com wrote:
|
|
Am I going mad then ?? it wouldn't be the first time |
no, you are right, there is a diff. Will have a look and come back to … Update: it looks like #1471 and #2232 didn't find their way to ESH. @clinique and @edog1973 could you please also contribute your changes to ESH or @J-N-K could incorporate their enhancements in this PR as well? Thanks in advance, Thomas E.-E. |
great, thanks! |
|
@J-N-K Could you update this PR? |
* added sumSince (fixes eclipse-archived#1843 replaces eclipse-archived#376) * changed evolutionRate to use BigDecimal instead of double * changed deltaSince to use BigDecimal instead of double * added check in evolutionRate method to avoid divide-by-zero error * updated all JavaDoc headers to reflect the odd variety of return values and their reasons, in hopes of spurring discussion as to whether and how to tighten and simplify the PersistenceExtensions. Also-By: Jan N. Klug jan.n.klug@rub.de Signed-off-by: John Cocula <john@cocula.com>
* changed averageSince to use BigDecimal instead of double * added sumSince (fixes #1843 replaces #376) * changed evolutionRate to use BigDecimal instead of double * changed deltaSince to use BigDecimal instead of double * added check in evolutionRate method to avoid divide-by-zero error * updated all JavaDoc headers to reflect the odd variety of return values and their reasons, in hopes of spurring discussion as to whether and how to tighten and simplify the PersistenceExtensions. Also-By: Jan N. Klug jan.n.klug@rub.de Signed-off-by: John Cocula <john@cocula.com> * Changed modifier order to recommended order per @maggu2810 Signed-off-by: John Cocula <john@cocula.com>
Signed-off-by: Jan N. Klug jan.n.klug@rub.de