From 5effd856f5e7ed5041c843fd16b08b8ee8414f03 Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Wed, 11 Aug 2021 14:55:58 -0700 Subject: Add MultiStateCounter This native object is used to track values per-state. For example, if the state changes from 0 to 1 between two updateValue calls, the delta between the values is distributed to the states 0 and 1 in accordance with the time spent in those states. Bug: 197162116 Test: atest libbattery_test Change-Id: Ie304db5c93f4aa9676d12d0a8ab53b6867b24fff --- libs/battery/MultiStateCounterTest.cpp | 105 +++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 libs/battery/MultiStateCounterTest.cpp (limited to 'libs/battery/MultiStateCounterTest.cpp') diff --git a/libs/battery/MultiStateCounterTest.cpp b/libs/battery/MultiStateCounterTest.cpp new file mode 100644 index 0000000000..942d5cadf5 --- /dev/null +++ b/libs/battery/MultiStateCounterTest.cpp @@ -0,0 +1,105 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * Android BPF library - public API + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include "MultiStateCounter.h" + +namespace android { +namespace battery { + +typedef MultiStateCounter DoubleMultiStateCounter; + +template <> +bool DoubleMultiStateCounter::delta(const double& previousValue, const double& newValue, + double* outValue) const { + *outValue = newValue - previousValue; + return *outValue >= 0; +} + +template <> +void DoubleMultiStateCounter::add(double* value1, const double& value2, const uint64_t numerator, + const uint64_t denominator) const { + if (numerator != denominator) { + // The caller ensures that denominator != 0 + *value1 += value2 * numerator / denominator; + } else { + *value1 += value2; + } +} + +template <> +std::string DoubleMultiStateCounter::valueToString(const double& v) const { + return std::to_string(v); +} + +class MultiStateCounterTest : public testing::Test {}; + +TEST_F(MultiStateCounterTest, constructor) { + DoubleMultiStateCounter testCounter(3, 1, 0, 1000); + testCounter.setState(1, 2000); + testCounter.updateValue(3.14, 3000); + + EXPECT_DOUBLE_EQ(0, testCounter.getCount(0)); + EXPECT_DOUBLE_EQ(3.14, testCounter.getCount(1)); + EXPECT_DOUBLE_EQ(0, testCounter.getCount(2)); +} + +TEST_F(MultiStateCounterTest, stateChange) { + DoubleMultiStateCounter testCounter(3, 1, 0, 0); + testCounter.setState(2, 1000); + testCounter.updateValue(6.0, 3000); + + EXPECT_DOUBLE_EQ(0, testCounter.getCount(0)); + EXPECT_DOUBLE_EQ(2.0, testCounter.getCount(1)); + EXPECT_DOUBLE_EQ(4.0, testCounter.getCount(2)); +} + +TEST_F(MultiStateCounterTest, timeAdjustment_setState) { + DoubleMultiStateCounter testCounter(3, 1, 0, 0); + testCounter.setState(2, 2000); + + // Time moves back + testCounter.setState(1, 1000); + testCounter.updateValue(6.0, 3000); + + EXPECT_DOUBLE_EQ(0, testCounter.getCount(0)); + + // We were in state 1 from 0 to 2000, which was erased because the time moved back. + // Then from 1000 to 3000, so we expect the count to be 6 * (2000/3000) + EXPECT_DOUBLE_EQ(4.0, testCounter.getCount(1)); + + // No time was effectively accumulated for state 2, because the timestamp moved back + // while we were in state 2. + EXPECT_DOUBLE_EQ(0, testCounter.getCount(2)); +} + +TEST_F(MultiStateCounterTest, timeAdjustment_updateValue) { + DoubleMultiStateCounter testCounter(1, 0, 0, 0); + testCounter.updateValue(6.0, 2000); + + // Time moves back. The negative delta from 2000 to 1000 is ignored + testCounter.updateValue(8.0, 1000); + testCounter.updateValue(11.0, 3000); + + // The total accumulated count is: + // 6.0 // For the period 0-2000 + // +(11.0-8.0) // For the period 1000-3000 + EXPECT_DOUBLE_EQ(9.0, testCounter.getCount(0)); +} + +} // namespace battery +} // namespace android -- cgit v1.2.3-59-g8ed1b From 12aaf8e35911cd5f036917de9a6f388d611ed51c Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Fri, 3 Sep 2021 19:07:23 -0700 Subject: Simplify initialization and add setValue to support parceling Bug: 197162116 Test: atest libbattery_test Change-Id: I4278206eab049d714c5278e6b10ba3155e17142f --- libs/battery/LongArrayMultiStateCounter.cpp | 2 +- libs/battery/LongArrayMultiStateCounterTest.cpp | 19 ++++- libs/battery/MultiStateCounter.h | 107 +++++++++++++++--------- libs/battery/MultiStateCounterTest.cpp | 35 ++++++-- 4 files changed, 116 insertions(+), 47 deletions(-) (limited to 'libs/battery/MultiStateCounterTest.cpp') diff --git a/libs/battery/LongArrayMultiStateCounter.cpp b/libs/battery/LongArrayMultiStateCounter.cpp index 68e088394a..125cfaffa4 100644 --- a/libs/battery/LongArrayMultiStateCounter.cpp +++ b/libs/battery/LongArrayMultiStateCounter.cpp @@ -62,7 +62,7 @@ void LongArrayMultiStateCounter::add(std::vector* value1, template <> std::string LongArrayMultiStateCounter::valueToString(const std::vector& v) const { std::stringstream s; - s << "{ "; + s << "{"; bool first = true; for (uint64_t n : v) { if (!first) { diff --git a/libs/battery/LongArrayMultiStateCounterTest.cpp b/libs/battery/LongArrayMultiStateCounterTest.cpp index 24cb437eaa..e4e6b2a49f 100644 --- a/libs/battery/LongArrayMultiStateCounterTest.cpp +++ b/libs/battery/LongArrayMultiStateCounterTest.cpp @@ -24,7 +24,9 @@ namespace battery { class LongArrayMultiStateCounterTest : public testing::Test {}; TEST_F(LongArrayMultiStateCounterTest, stateChange) { - LongArrayMultiStateCounter testCounter(2, 0, std::vector(4), 1000); + LongArrayMultiStateCounter testCounter(2, std::vector(4)); + testCounter.updateValue(std::vector({0, 0, 0, 0}), 1000); + testCounter.setState(0, 1000); testCounter.setState(1, 2000); testCounter.updateValue(std::vector({100, 200, 300, 400}), 3000); @@ -34,7 +36,9 @@ TEST_F(LongArrayMultiStateCounterTest, stateChange) { } TEST_F(LongArrayMultiStateCounterTest, accumulation) { - LongArrayMultiStateCounter testCounter(2, 0, std::vector(4), 1000); + LongArrayMultiStateCounter testCounter(2, std::vector(4)); + testCounter.updateValue(std::vector({0, 0, 0, 0}), 1000); + testCounter.setState(0, 1000); testCounter.setState(1, 2000); testCounter.updateValue(std::vector({100, 200, 300, 400}), 3000); testCounter.setState(0, 4000); @@ -50,5 +54,16 @@ TEST_F(LongArrayMultiStateCounterTest, accumulation) { EXPECT_EQ(std::vector({70, 120, 170, 220}), testCounter.getCount(1)); } +TEST_F(LongArrayMultiStateCounterTest, toString) { + LongArrayMultiStateCounter testCounter(2, std::vector(4)); + testCounter.updateValue(std::vector({0, 0, 0, 0}), 1000); + testCounter.setState(0, 1000); + testCounter.setState(1, 2000); + testCounter.updateValue(std::vector({100, 200, 300, 400}), 3000); + + EXPECT_STREQ("[0: {50, 100, 150, 200}, 1: {50, 100, 150, 200}] updated: 3000 currentState: 1", + testCounter.toString().c_str()); +} + } // namespace battery } // namespace android diff --git a/libs/battery/MultiStateCounter.h b/libs/battery/MultiStateCounter.h index 9f56b29dfb..40de068a95 100644 --- a/libs/battery/MultiStateCounter.h +++ b/libs/battery/MultiStateCounter.h @@ -51,15 +51,18 @@ class MultiStateCounter { State* states; public: - MultiStateCounter(uint16_t stateCount, state_t initialState, const T& emptyValue, - time_t timestamp); + MultiStateCounter(uint16_t stateCount, const T& emptyValue); virtual ~MultiStateCounter(); void setState(state_t state, time_t timestamp); + void setValue(state_t state, const T& value); + void updateValue(const T& value, time_t timestamp); + uint16_t getStateCount(); + const T& getCount(state_t state); std::string toString(); @@ -86,14 +89,13 @@ private: // Since MultiStateCounter is a template, the implementation must be inlined. template -MultiStateCounter::MultiStateCounter(uint16_t stateCount, state_t initialState, - const T& emptyValue, time_t timestamp) +MultiStateCounter::MultiStateCounter(uint16_t stateCount, const T& emptyValue) : stateCount(stateCount), - currentState(initialState), - lastStateChangeTimestamp(timestamp), + currentState(0), + lastStateChangeTimestamp(-1), emptyValue(emptyValue), lastValue(emptyValue), - lastUpdateTimestamp(timestamp), + lastUpdateTimestamp(-1), deltaValue(emptyValue) { states = new State[stateCount]; for (int i = 0; i < stateCount; i++) { @@ -109,53 +111,68 @@ MultiStateCounter::~MultiStateCounter() { template void MultiStateCounter::setState(state_t state, time_t timestamp) { - if (timestamp >= lastStateChangeTimestamp) { - states[currentState].timeInStateSinceUpdate += timestamp - lastStateChangeTimestamp; - } else { - ALOGE("setState is called with an earlier timestamp: %lu, previous timestamp: %lu\n", - (unsigned long)timestamp, (unsigned long)lastStateChangeTimestamp); - // The accumulated durations have become unreliable. For example, if the timestamp - // sequence was 1000, 2000, 1000, 3000, if we accumulated the positive deltas, - // we would get 4000, which is greater than (last - first). This could lead to - // counts exceeding 100%. - for (int i = 0; i < stateCount; i++) { - states[i].timeInStateSinceUpdate = 0; + if (lastStateChangeTimestamp >= 0) { + if (timestamp >= lastStateChangeTimestamp) { + states[currentState].timeInStateSinceUpdate += timestamp - lastStateChangeTimestamp; + } else { + ALOGE("setState is called with an earlier timestamp: %lu, previous timestamp: %lu\n", + (unsigned long)timestamp, (unsigned long)lastStateChangeTimestamp); + // The accumulated durations have become unreliable. For example, if the timestamp + // sequence was 1000, 2000, 1000, 3000, if we accumulated the positive deltas, + // we would get 4000, which is greater than (last - first). This could lead to + // counts exceeding 100%. + for (int i = 0; i < stateCount; i++) { + states[i].timeInStateSinceUpdate = 0; + } } } currentState = state; lastStateChangeTimestamp = timestamp; } +template +void MultiStateCounter::setValue(state_t state, const T& value) { + states[state].counter = value; +} + template void MultiStateCounter::updateValue(const T& value, time_t timestamp) { // Confirm the current state for the side-effect of updating the time-in-state // counter for the current state. setState(currentState, timestamp); - if (timestamp > lastUpdateTimestamp) { - if (delta(lastValue, value, &deltaValue)) { - time_t timeSinceUpdate = timestamp - lastUpdateTimestamp; - for (int i = 0; i < stateCount; i++) { - time_t timeInState = states[i].timeInStateSinceUpdate; - if (timeInState) { - add(&states[i].counter, deltaValue, timeInState, timeSinceUpdate); - states[i].timeInStateSinceUpdate = 0; + if (lastUpdateTimestamp >= 0) { + if (timestamp > lastUpdateTimestamp) { + if (delta(lastValue, value, &deltaValue)) { + time_t timeSinceUpdate = timestamp - lastUpdateTimestamp; + for (int i = 0; i < stateCount; i++) { + time_t timeInState = states[i].timeInStateSinceUpdate; + if (timeInState) { + add(&states[i].counter, deltaValue, timeInState, timeSinceUpdate); + states[i].timeInStateSinceUpdate = 0; + } } + } else { + std::stringstream str; + str << "updateValue is called with a value " << valueToString(value) + << ", which is lower than the previous value " << valueToString(lastValue) + << "\n"; + ALOGE("%s", str.str().c_str()); } - } else { - std::stringstream str; - str << "updateValue is called with a value " << valueToString(value) - << ", which is lower than the previous value " << valueToString(lastValue) << "\n"; - ALOGE("%s", str.str().c_str()); + } else if (timestamp < lastUpdateTimestamp) { + ALOGE("updateValue is called with an earlier timestamp: %lu, previous timestamp: %lu\n", + (unsigned long)timestamp, (unsigned long)lastUpdateTimestamp); } - } else if (timestamp < lastUpdateTimestamp) { - ALOGE("updateValue is called with an earlier timestamp: %lu, previous timestamp: %lu\n", - (unsigned long)timestamp, (unsigned long)lastUpdateTimestamp); } lastValue = value; lastUpdateTimestamp = timestamp; } +template +uint16_t MultiStateCounter::getStateCount() { + return stateCount; +} + template const T& MultiStateCounter::getCount(state_t state) { return states[state].counter; @@ -164,17 +181,29 @@ const T& MultiStateCounter::getCount(state_t state) { template std::string MultiStateCounter::toString() { std::stringstream str; - str << "currentState: " << currentState - << " lastStateChangeTimestamp: " << lastStateChangeTimestamp - << " lastUpdateTimestamp: " << lastUpdateTimestamp << " states: ["; + str << "["; for (int i = 0; i < stateCount; i++) { if (i != 0) { str << ", "; } - str << i << ": time: " << states[i].timeInStateSinceUpdate - << " counter: " << valueToString(states[i].counter); + str << i << ": " << valueToString(states[i].counter); + if (states[i].timeInStateSinceUpdate > 0) { + str << " timeInStateSinceUpdate: " << states[i].timeInStateSinceUpdate; + } } str << "]"; + if (lastUpdateTimestamp >= 0) { + str << " updated: " << lastUpdateTimestamp; + } + if (lastStateChangeTimestamp >= 0) { + str << " currentState: " << currentState; + if (lastStateChangeTimestamp > lastUpdateTimestamp) { + str << " stateChanged: " << lastStateChangeTimestamp; + } + } else { + str << " currentState: none"; + } + return str.str(); } diff --git a/libs/battery/MultiStateCounterTest.cpp b/libs/battery/MultiStateCounterTest.cpp index 942d5cadf5..87c80c53d3 100644 --- a/libs/battery/MultiStateCounterTest.cpp +++ b/libs/battery/MultiStateCounterTest.cpp @@ -49,8 +49,9 @@ std::string DoubleMultiStateCounter::valueToString(const double& v) const { class MultiStateCounterTest : public testing::Test {}; TEST_F(MultiStateCounterTest, constructor) { - DoubleMultiStateCounter testCounter(3, 1, 0, 1000); - testCounter.setState(1, 2000); + DoubleMultiStateCounter testCounter(3, 0); + testCounter.updateValue(0, 0); + testCounter.setState(1, 0); testCounter.updateValue(3.14, 3000); EXPECT_DOUBLE_EQ(0, testCounter.getCount(0)); @@ -59,7 +60,9 @@ TEST_F(MultiStateCounterTest, constructor) { } TEST_F(MultiStateCounterTest, stateChange) { - DoubleMultiStateCounter testCounter(3, 1, 0, 0); + DoubleMultiStateCounter testCounter(3, 0); + testCounter.updateValue(0, 0); + testCounter.setState(1, 0); testCounter.setState(2, 1000); testCounter.updateValue(6.0, 3000); @@ -69,7 +72,9 @@ TEST_F(MultiStateCounterTest, stateChange) { } TEST_F(MultiStateCounterTest, timeAdjustment_setState) { - DoubleMultiStateCounter testCounter(3, 1, 0, 0); + DoubleMultiStateCounter testCounter(3, 0); + testCounter.updateValue(0, 0); + testCounter.setState(1, 0); testCounter.setState(2, 2000); // Time moves back @@ -88,7 +93,9 @@ TEST_F(MultiStateCounterTest, timeAdjustment_setState) { } TEST_F(MultiStateCounterTest, timeAdjustment_updateValue) { - DoubleMultiStateCounter testCounter(1, 0, 0, 0); + DoubleMultiStateCounter testCounter(1, 0); + testCounter.updateValue(0, 0); + testCounter.setState(0, 0); testCounter.updateValue(6.0, 2000); // Time moves back. The negative delta from 2000 to 1000 is ignored @@ -101,5 +108,23 @@ TEST_F(MultiStateCounterTest, timeAdjustment_updateValue) { EXPECT_DOUBLE_EQ(9.0, testCounter.getCount(0)); } +TEST_F(MultiStateCounterTest, toString) { + DoubleMultiStateCounter testCounter(2, 0); + + EXPECT_STREQ("[0: 0.000000, 1: 0.000000] currentState: none", testCounter.toString().c_str()); + + testCounter.updateValue(0, 0); + testCounter.setState(1, 0); + testCounter.setState(1, 2000); + EXPECT_STREQ("[0: 0.000000, 1: 0.000000 timeInStateSinceUpdate: 2000]" + " updated: 0 currentState: 1 stateChanged: 2000", + testCounter.toString().c_str()); + + testCounter.updateValue(3.14, 3000); + + EXPECT_STREQ("[0: 0.000000, 1: 3.140000] updated: 3000 currentState: 1", + testCounter.toString().c_str()); +} + } // namespace battery } // namespace android -- cgit v1.2.3-59-g8ed1b From 99d7c712ddc714e034aa5e114502c71772ef6abd Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Fri, 3 Sep 2021 19:07:23 -0700 Subject: Add reset() and setEnabled() to MultiStateCounter Bug: 197162116 Test: atest libbattery_test Change-Id: I14006af5e6a3e16d7849beb6def53de77b15b7bc --- libs/battery/MultiStateCounter.h | 87 ++++++++++++++++++++++++---------- libs/battery/MultiStateCounterTest.cpp | 77 ++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 24 deletions(-) (limited to 'libs/battery/MultiStateCounterTest.cpp') diff --git a/libs/battery/MultiStateCounter.h b/libs/battery/MultiStateCounter.h index 40de068a95..e1ee07ccf1 100644 --- a/libs/battery/MultiStateCounter.h +++ b/libs/battery/MultiStateCounter.h @@ -42,6 +42,7 @@ class MultiStateCounter { T lastValue; time_t lastUpdateTimestamp; T deltaValue; + bool isEnabled; struct State { time_t timeInStateSinceUpdate; @@ -55,12 +56,16 @@ public: virtual ~MultiStateCounter(); + void setEnabled(bool enabled, time_t timestamp); + void setState(state_t state, time_t timestamp); void setValue(state_t state, const T& value); void updateValue(const T& value, time_t timestamp); + void reset(); + uint16_t getStateCount(); const T& getCount(state_t state); @@ -96,7 +101,8 @@ MultiStateCounter::MultiStateCounter(uint16_t stateCount, const T& emptyValue emptyValue(emptyValue), lastValue(emptyValue), lastUpdateTimestamp(-1), - deltaValue(emptyValue) { + deltaValue(emptyValue), + isEnabled(true) { states = new State[stateCount]; for (int i = 0; i < stateCount; i++) { states[i].timeInStateSinceUpdate = 0; @@ -110,8 +116,27 @@ MultiStateCounter::~MultiStateCounter() { }; template -void MultiStateCounter::setState(state_t state, time_t timestamp) { +void MultiStateCounter::setEnabled(bool enabled, time_t timestamp) { + if (enabled == isEnabled) { + return; + } + + if (!enabled) { + // Confirm the current state for the side-effect of updating the time-in-state + // counter for the current state. + setState(currentState, timestamp); + } + + isEnabled = enabled; + if (lastStateChangeTimestamp >= 0) { + lastStateChangeTimestamp = timestamp; + } +} + +template +void MultiStateCounter::setState(state_t state, time_t timestamp) { + if (isEnabled && lastStateChangeTimestamp >= 0) { if (timestamp >= lastStateChangeTimestamp) { states[currentState].timeInStateSinceUpdate += timestamp - lastStateChangeTimestamp; } else { @@ -137,37 +162,51 @@ void MultiStateCounter::setValue(state_t state, const T& value) { template void MultiStateCounter::updateValue(const T& value, time_t timestamp) { - // Confirm the current state for the side-effect of updating the time-in-state - // counter for the current state. - setState(currentState, timestamp); - - if (lastUpdateTimestamp >= 0) { - if (timestamp > lastUpdateTimestamp) { - if (delta(lastValue, value, &deltaValue)) { - time_t timeSinceUpdate = timestamp - lastUpdateTimestamp; - for (int i = 0; i < stateCount; i++) { - time_t timeInState = states[i].timeInStateSinceUpdate; - if (timeInState) { - add(&states[i].counter, deltaValue, timeInState, timeSinceUpdate); - states[i].timeInStateSinceUpdate = 0; + // If the counter is disabled, we ignore the update, except when the counter got disabled after + // the previous update, in which case we still need to pick up the residual delta. + if (isEnabled || lastUpdateTimestamp < lastStateChangeTimestamp) { + // Confirm the current state for the side-effect of updating the time-in-state + // counter for the current state. + setState(currentState, timestamp); + + if (lastUpdateTimestamp >= 0) { + if (timestamp > lastUpdateTimestamp) { + if (delta(lastValue, value, &deltaValue)) { + time_t timeSinceUpdate = timestamp - lastUpdateTimestamp; + for (int i = 0; i < stateCount; i++) { + time_t timeInState = states[i].timeInStateSinceUpdate; + if (timeInState) { + add(&states[i].counter, deltaValue, timeInState, timeSinceUpdate); + states[i].timeInStateSinceUpdate = 0; + } } + } else { + std::stringstream str; + str << "updateValue is called with a value " << valueToString(value) + << ", which is lower than the previous value " << valueToString(lastValue) + << "\n"; + ALOGE("%s", str.str().c_str()); } - } else { - std::stringstream str; - str << "updateValue is called with a value " << valueToString(value) - << ", which is lower than the previous value " << valueToString(lastValue) - << "\n"; - ALOGE("%s", str.str().c_str()); + } else if (timestamp < lastUpdateTimestamp) { + ALOGE("updateValue is called with an earlier timestamp: %lu, previous: %lu\n", + (unsigned long)timestamp, (unsigned long)lastUpdateTimestamp); } - } else if (timestamp < lastUpdateTimestamp) { - ALOGE("updateValue is called with an earlier timestamp: %lu, previous timestamp: %lu\n", - (unsigned long)timestamp, (unsigned long)lastUpdateTimestamp); } } lastValue = value; lastUpdateTimestamp = timestamp; } +template +void MultiStateCounter::reset() { + lastStateChangeTimestamp = -1; + lastUpdateTimestamp = -1; + for (int i = 0; i < stateCount; i++) { + states[i].timeInStateSinceUpdate = 0; + states[i].counter = emptyValue; + } +} + template uint16_t MultiStateCounter::getStateCount() { return stateCount; diff --git a/libs/battery/MultiStateCounterTest.cpp b/libs/battery/MultiStateCounterTest.cpp index 87c80c53d3..319ba76a4f 100644 --- a/libs/battery/MultiStateCounterTest.cpp +++ b/libs/battery/MultiStateCounterTest.cpp @@ -71,6 +71,83 @@ TEST_F(MultiStateCounterTest, stateChange) { EXPECT_DOUBLE_EQ(4.0, testCounter.getCount(2)); } +TEST_F(MultiStateCounterTest, setEnabled) { + DoubleMultiStateCounter testCounter(3, 0); + testCounter.updateValue(0, 0); + testCounter.setState(1, 0); + testCounter.setEnabled(false, 1000); + testCounter.setState(2, 2000); + testCounter.updateValue(6.0, 3000); + + // In state 1: accumulated 1000 before disabled, that's 6.0 * 1000/3000 = 2.0 + // In state 2: 0, since it is still disabled + EXPECT_DOUBLE_EQ(0, testCounter.getCount(0)); + EXPECT_DOUBLE_EQ(2.0, testCounter.getCount(1)); + EXPECT_DOUBLE_EQ(0, testCounter.getCount(2)); + + // Should have no effect since the counter is disabled + testCounter.setState(0, 3500); + + // Should have no effect since the counter is disabled + testCounter.updateValue(10.0, 4000); + + EXPECT_DOUBLE_EQ(0, testCounter.getCount(0)); + EXPECT_DOUBLE_EQ(2.0, testCounter.getCount(1)); + EXPECT_DOUBLE_EQ(0, testCounter.getCount(2)); + + testCounter.setState(2, 4500); + + // Enable the counter to partially accumulate deltas for the current state, 2 + testCounter.setEnabled(true, 5000); + testCounter.setEnabled(false, 6000); + testCounter.setEnabled(true, 7000); + testCounter.updateValue(20.0, 8000); + + // The delta is 10.0 over 5000-3000=2000. + // Counter has been enabled in state 2 for (6000-5000)+(8000-7000) = 2000, + // so its share is (20.0-10.0) * 2000/(8000-4000) = 5.0 + EXPECT_DOUBLE_EQ(0, testCounter.getCount(0)); + EXPECT_DOUBLE_EQ(2.0, testCounter.getCount(1)); + EXPECT_DOUBLE_EQ(5.0, testCounter.getCount(2)); + + testCounter.reset(); + testCounter.setState(0, 0); + testCounter.updateValue(0, 0); + testCounter.setState(1, 2000); + testCounter.setEnabled(false, 3000); + testCounter.updateValue(200, 5000); + + // 200 over 5000 = 40 per second + // Counter was in state 0 from 0 to 2000, so 2 sec, so the count should be 40 * 2 = 80 + // It stayed in state 1 from 2000 to 3000, at which point the counter was disabled, + // so the count for state 1 should be 40 * 1 = 40. + // The remaining 2 seconds from 3000 to 5000 don't count because the counter was disabled. + EXPECT_DOUBLE_EQ(80.0, testCounter.getCount(0)); + EXPECT_DOUBLE_EQ(40.0, testCounter.getCount(1)); + EXPECT_DOUBLE_EQ(0, testCounter.getCount(2)); +} + +TEST_F(MultiStateCounterTest, reset) { + DoubleMultiStateCounter testCounter(3, 0); + testCounter.updateValue(0, 0); + testCounter.setState(1, 0); + testCounter.updateValue(2.72, 3000); + + testCounter.reset(); + + EXPECT_DOUBLE_EQ(0, testCounter.getCount(0)); + EXPECT_DOUBLE_EQ(0, testCounter.getCount(1)); + EXPECT_DOUBLE_EQ(0, testCounter.getCount(2)); + + // Assert that we can still continue accumulating after a reset + testCounter.updateValue(0, 4000); + testCounter.updateValue(3.14, 5000); + + EXPECT_DOUBLE_EQ(0, testCounter.getCount(0)); + EXPECT_DOUBLE_EQ(3.14, testCounter.getCount(1)); + EXPECT_DOUBLE_EQ(0, testCounter.getCount(2)); +} + TEST_F(MultiStateCounterTest, timeAdjustment_setState) { DoubleMultiStateCounter testCounter(3, 0); testCounter.updateValue(0, 0); -- cgit v1.2.3-59-g8ed1b From 8940e5d69574cb4e068758e12fa15794114ff2ff Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Thu, 23 Sep 2021 14:56:30 -0700 Subject: Add MultiStateCounter.addValue and make updateValue return delta Bug: 197162116 Test: atest libbattery_test Change-Id: I790ed0b805a88aa6ee9659f8494af8edf693d931 --- libs/battery/MultiStateCounter.h | 24 +++++++++++++++++++++--- libs/battery/MultiStateCounterTest.cpp | 24 ++++++++++++++++++++++-- 2 files changed, 43 insertions(+), 5 deletions(-) (limited to 'libs/battery/MultiStateCounterTest.cpp') diff --git a/libs/battery/MultiStateCounter.h b/libs/battery/MultiStateCounter.h index e1ee07ccf1..112fb85051 100644 --- a/libs/battery/MultiStateCounter.h +++ b/libs/battery/MultiStateCounter.h @@ -62,7 +62,13 @@ public: void setValue(state_t state, const T& value); - void updateValue(const T& value, time_t timestamp); + /** + * Updates the value for the current state and returns the delta from the previously + * set value. + */ + const T& updateValue(const T& value, time_t timestamp); + + void addValue(const T& value); void reset(); @@ -161,7 +167,7 @@ void MultiStateCounter::setValue(state_t state, const T& value) { } template -void MultiStateCounter::updateValue(const T& value, time_t timestamp) { +const T& MultiStateCounter::updateValue(const T& value, time_t timestamp) { // If the counter is disabled, we ignore the update, except when the counter got disabled after // the previous update, in which case we still need to pick up the residual delta. if (isEnabled || lastUpdateTimestamp < lastStateChangeTimestamp) { @@ -195,6 +201,16 @@ void MultiStateCounter::updateValue(const T& value, time_t timestamp) { } lastValue = value; lastUpdateTimestamp = timestamp; + return deltaValue; +} + +template +void MultiStateCounter::addValue(const T& value) { + if (!isEnabled) { + return; + } + + add(&states[currentState].counter, value, 1 /* numerator */, 1 /* denominator */); } template @@ -242,7 +258,9 @@ std::string MultiStateCounter::toString() { } else { str << " currentState: none"; } - + if (!isEnabled) { + str << " disabled"; + } return str.str(); } diff --git a/libs/battery/MultiStateCounterTest.cpp b/libs/battery/MultiStateCounterTest.cpp index 319ba76a4f..848fd10d15 100644 --- a/libs/battery/MultiStateCounterTest.cpp +++ b/libs/battery/MultiStateCounterTest.cpp @@ -52,11 +52,12 @@ TEST_F(MultiStateCounterTest, constructor) { DoubleMultiStateCounter testCounter(3, 0); testCounter.updateValue(0, 0); testCounter.setState(1, 0); - testCounter.updateValue(3.14, 3000); + double delta = testCounter.updateValue(3.14, 3000); EXPECT_DOUBLE_EQ(0, testCounter.getCount(0)); EXPECT_DOUBLE_EQ(3.14, testCounter.getCount(1)); EXPECT_DOUBLE_EQ(0, testCounter.getCount(2)); + EXPECT_DOUBLE_EQ(3.14, delta); } TEST_F(MultiStateCounterTest, stateChange) { @@ -177,12 +178,31 @@ TEST_F(MultiStateCounterTest, timeAdjustment_updateValue) { // Time moves back. The negative delta from 2000 to 1000 is ignored testCounter.updateValue(8.0, 1000); - testCounter.updateValue(11.0, 3000); + double delta = testCounter.updateValue(11.0, 3000); // The total accumulated count is: // 6.0 // For the period 0-2000 // +(11.0-8.0) // For the period 1000-3000 EXPECT_DOUBLE_EQ(9.0, testCounter.getCount(0)); + + // 11.0-8.0 + EXPECT_DOUBLE_EQ(3.0, delta); +} + +TEST_F(MultiStateCounterTest, addValue) { + DoubleMultiStateCounter testCounter(1, 0); + testCounter.updateValue(0, 0); + testCounter.setState(0, 0); + testCounter.updateValue(6.0, 2000); + + testCounter.addValue(8.0); + + EXPECT_DOUBLE_EQ(14.0, testCounter.getCount(0)); + + testCounter.setEnabled(false, 3000); + testCounter.addValue(888.0); + + EXPECT_DOUBLE_EQ(14.0, testCounter.getCount(0)); } TEST_F(MultiStateCounterTest, toString) { -- cgit v1.2.3-59-g8ed1b From 4b238fb0a9bd5533651030faf22391675fc88024 Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Tue, 9 Nov 2021 11:52:08 -0800 Subject: Reset time-since-update if the tracked value is nonmonotonic The absence of this reset led to timeInStateSinceUpdate exceeding total timeSinceUpdate. This resulted in inflated counts, which are smeared using the `delta * timeInState / timeSinceUpdate` formula. So, instead of assigning a portion of the time to a specific state it would assign a multiple of the time to it. Bug: 204087731 Test: atest libbattery_test Change-Id: I7805d5c6aa314f4030c8a1ac9541f2d439a471cb --- libs/battery/MultiStateCounter.h | 8 ++++++++ libs/battery/MultiStateCounterTest.cpp | 23 ++++++++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletion(-) (limited to 'libs/battery/MultiStateCounterTest.cpp') diff --git a/libs/battery/MultiStateCounter.h b/libs/battery/MultiStateCounter.h index 03e1f2a022..a2b59a877f 100644 --- a/libs/battery/MultiStateCounter.h +++ b/libs/battery/MultiStateCounter.h @@ -195,10 +195,18 @@ const T& MultiStateCounter::updateValue(const T& value, time_t timestamp) { << ", which is lower than the previous value " << valueToString(lastValue) << "\n"; ALOGE("%s", str.str().c_str()); + + for (int i = 0; i < stateCount; i++) { + states[i].timeInStateSinceUpdate = 0; + } } } else if (timestamp < lastUpdateTimestamp) { ALOGE("updateValue is called with an earlier timestamp: %lu, previous: %lu\n", (unsigned long)timestamp, (unsigned long)lastUpdateTimestamp); + + for (int i = 0; i < stateCount; i++) { + states[i].timeInStateSinceUpdate = 0; + } } } } diff --git a/libs/battery/MultiStateCounterTest.cpp b/libs/battery/MultiStateCounterTest.cpp index 848fd10d15..876bf745a8 100644 --- a/libs/battery/MultiStateCounterTest.cpp +++ b/libs/battery/MultiStateCounterTest.cpp @@ -176,7 +176,7 @@ TEST_F(MultiStateCounterTest, timeAdjustment_updateValue) { testCounter.setState(0, 0); testCounter.updateValue(6.0, 2000); - // Time moves back. The negative delta from 2000 to 1000 is ignored + // Time moves back. The delta over the negative interval from 2000 to 1000 is ignored testCounter.updateValue(8.0, 1000); double delta = testCounter.updateValue(11.0, 3000); @@ -189,6 +189,27 @@ TEST_F(MultiStateCounterTest, timeAdjustment_updateValue) { EXPECT_DOUBLE_EQ(3.0, delta); } +TEST_F(MultiStateCounterTest, updateValue_nonmonotonic) { + DoubleMultiStateCounter testCounter(2, 0); + testCounter.updateValue(0, 0); + testCounter.setState(0, 0); + testCounter.updateValue(6.0, 2000); + + // Value goes down. The negative delta from 6.0 to 4.0 is ignored + testCounter.updateValue(4.0, 3000); + + // Value goes up again. The positive delta from 4.0 to 7.0 is accumulated. + double delta = testCounter.updateValue(7.0, 4000); + + // The total accumulated count is: + // 6.0 // For the period 0-2000 + // +(7.0-4.0) // For the period 3000-4000 + EXPECT_DOUBLE_EQ(9.0, testCounter.getCount(0)); + + // 7.0-4.0 + EXPECT_DOUBLE_EQ(3.0, delta); +} + TEST_F(MultiStateCounterTest, addValue) { DoubleMultiStateCounter testCounter(1, 0); testCounter.updateValue(0, 0); -- cgit v1.2.3-59-g8ed1b From 40dcce273ff440b169149ee029e019f7fc9b4e66 Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Fri, 10 Dec 2021 15:38:17 -0800 Subject: Add incrementValue method Bug: 191921016 Test: atest libbattery Change-Id: Ia10999854eef99d47b7968d23881a39d9976be24 --- libs/battery/MultiStateCounter.h | 26 ++++++++++++++++++++++---- libs/battery/MultiStateCounterTest.cpp | 20 ++++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) (limited to 'libs/battery/MultiStateCounterTest.cpp') diff --git a/libs/battery/MultiStateCounter.h b/libs/battery/MultiStateCounter.h index a2b59a877f..0caf005a9a 100644 --- a/libs/battery/MultiStateCounter.h +++ b/libs/battery/MultiStateCounter.h @@ -63,12 +63,24 @@ public: void setValue(state_t state, const T& value); /** - * Updates the value for the current state and returns the delta from the previously - * set value. + * Updates the value by distributing the delta from the previously set value + * among states according to their respective time-in-state. + * Returns the delta from the previously set value. */ const T& updateValue(const T& value, time_t timestamp); - void addValue(const T& value); + /** + * Updates the value by distributing the specified increment among states according + * to their respective time-in-state. + */ + void incrementValue(const T& increment, time_t timestamp); + + /** + * Adds the specified increment to the value for the current state, without affecting + * the last updated value or timestamp. Ignores partial time-in-state: the entirety of + * the increment is given to the current state. + */ + void addValue(const T& increment); void reset(); @@ -215,12 +227,18 @@ const T& MultiStateCounter::updateValue(const T& value, time_t timestamp) { return *returnValue; } +template +void MultiStateCounter::incrementValue(const T& increment, time_t timestamp) { + T newValue = lastValue; + add(&newValue, increment, 1 /* numerator */, 1 /* denominator */); + updateValue(newValue, timestamp); +} + template void MultiStateCounter::addValue(const T& value) { if (!isEnabled) { return; } - add(&states[currentState].counter, value, 1 /* numerator */, 1 /* denominator */); } diff --git a/libs/battery/MultiStateCounterTest.cpp b/libs/battery/MultiStateCounterTest.cpp index 876bf745a8..cb11a5444d 100644 --- a/libs/battery/MultiStateCounterTest.cpp +++ b/libs/battery/MultiStateCounterTest.cpp @@ -210,6 +210,26 @@ TEST_F(MultiStateCounterTest, updateValue_nonmonotonic) { EXPECT_DOUBLE_EQ(3.0, delta); } +TEST_F(MultiStateCounterTest, incrementValue) { + DoubleMultiStateCounter testCounter(2, 0); + testCounter.updateValue(0, 0); + testCounter.setState(0, 0); + testCounter.updateValue(6.0, 2000); + + testCounter.setState(1, 3000); + + testCounter.incrementValue(8.0, 6000); + + // The total accumulated count is: + // 6.0 // For the period 0-2000 + // +(8.0 * 0.25) // For the period 3000-4000 + EXPECT_DOUBLE_EQ(8.0, testCounter.getCount(0)); + + // 0 // For the period 0-3000 + // +(8.0 * 0.75) // For the period 3000-4000 + EXPECT_DOUBLE_EQ(6.0, testCounter.getCount(1)); +} + TEST_F(MultiStateCounterTest, addValue) { DoubleMultiStateCounter testCounter(1, 0); testCounter.updateValue(0, 0); -- cgit v1.2.3-59-g8ed1b