Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

Fix averageSince calculation #2405

Merged
merged 7 commits into from Dec 17, 2016
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -21,13 +21,15 @@
import org.eclipse.smarthome.core.types.State;
import org.eclipse.smarthome.model.persistence.tests.TestPersistenceService;
import org.joda.time.DateMidnight;
import org.joda.time.DateTime;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

/**
* @author Kai Kreuzer - Initial contribution and API
* @author Chris Jackson
* @author Jan N. Klug
*/
@SuppressWarnings("deprecation")
public class PersistenceExtensionsTest {
Expand Down Expand Up @@ -124,8 +126,13 @@ public void testMaximumSince() {
@Test
public void testAverageSince() {
item.setState(new DecimalType(3025));
DecimalType average = PersistenceExtensions.averageSince(item, new DateMidnight(2003, 1, 1), "test");
assertEquals("2100", average.toString());
DateMidnight startStored = new DateMidnight(2003, 1, 1);
DateMidnight endStored = new DateMidnight(2012, 1, 1);
long storedInterval = endStored.getMillis() - startStored.getMillis();
long recentInterval = DateTime.now().getMillis() - endStored.getMillis();
double expected = (2007.4994 * storedInterval + 2518.5 * recentInterval) / (storedInterval + recentInterval);
DecimalType average = PersistenceExtensions.averageSince(item, startStored, "test");
assertEquals(expected, average.doubleValue(), 0.001);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this test stable enough? If the execution is slow, the average might be calculated with a much bigger value than recentInterval already and the results might differ, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have adjusted it to 0.01 which should be good enough even for every slow execution. In my tests the difference was always around 0.0001, so a factor of 100 better than the value I have set now.

}

@Test
Expand Down
Expand Up @@ -77,7 +77,8 @@ private static String getDefaultServiceId() {
.warn("There is no default persistence service configured!");
}
} else {
LoggerFactory.getLogger(PersistenceExtensions.class).warn("PersistenceServiceRegistryImpl is not available!");
LoggerFactory.getLogger(PersistenceExtensions.class)
.warn("PersistenceServiceRegistryImpl is not available!");
}
return null;
}
Expand Down Expand Up @@ -376,6 +377,7 @@ public String getName() {
*
* @param item the item to get the average state value for
* @param timestamp the point in time from which to search for the average state value
* @param includeCurrentValue include the current value in the calculation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line should be removed again

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

* @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
Expand All @@ -400,24 +402,51 @@ public static DecimalType averageSince(Item item, AbstractInstant timestamp, Str
Iterable<HistoricItem> result = getAllStatesSince(item, timestamp, serviceId);
Iterator<HistoricItem> it = result.iterator();

DecimalType value = (DecimalType) item.getStateAs(DecimalType.class);
if (value == null) {
value = DecimalType.ZERO;
}
BigDecimal total = BigDecimal.ZERO;

BigDecimal avgValue, timeSpan;
DecimalType lastState = null, thisState = null;
BigDecimal lastTimestamp = null, thisTimestamp = null;
BigDecimal firstTimestamp = null;

BigDecimal total = value.toBigDecimal();
int quantity = 1;
while (it.hasNext()) {
State state = it.next().getState();
HistoricItem thisItem = it.next();
State state = thisItem.getState();

if (state instanceof DecimalType) {
value = (DecimalType) state;
total = total.add(value.toBigDecimal());
quantity++;
thisState = (DecimalType) state;
thisTimestamp = BigDecimal.valueOf(thisItem.getTimestamp().getTime());
if (firstTimestamp == null) {
firstTimestamp = thisTimestamp;
} else {
avgValue = (thisState.toBigDecimal().add(lastState.toBigDecimal())).divide(BigDecimal.valueOf(2),
MathContext.DECIMAL64);
timeSpan = thisTimestamp.subtract(lastTimestamp);
total = total.add(avgValue.multiply(timeSpan, MathContext.DECIMAL64));

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove this empty line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}
lastTimestamp = thisTimestamp;
lastState = thisState;
}
}
BigDecimal average = total.divide(BigDecimal.valueOf(quantity), MathContext.DECIMAL64);

return new DecimalType(average);
if (lastState != null) {
thisState = (DecimalType) item.getStateAs(DecimalType.class);
thisTimestamp = BigDecimal.valueOf((new DateTime()).getMillis());
avgValue = (thisState.toBigDecimal().add(lastState.toBigDecimal())).divide(BigDecimal.valueOf(2),
MathContext.DECIMAL64);
timeSpan = thisTimestamp.subtract(lastTimestamp);
total = total.add(avgValue.multiply(timeSpan, MathContext.DECIMAL64));
}

if (thisTimestamp != null) {
timeSpan = thisTimestamp.subtract(firstTimestamp, MathContext.DECIMAL64);

BigDecimal average = total.divide(timeSpan, MathContext.DECIMAL64);
return new DecimalType(average);
} else {
return null;
}
}

/**
Expand Down