power: Discard ALS data at resume
Currently, if we suspend at indoor light and resume in
completely dark environment, it takes about 10 seconds
for exponential smoothing algorithm to make powerd know
that we are in complete darkness.
This CL fixes that by discard previos data at resume
as ambient light data before suspend won't be relevant.
BUG=b:138666654
TEST=Unit test. manual test on chromebook.
Change-Id: I47a0090ed7e35bf3b5b8e77af82ef00108b3dbd3
Signed-off-by: Puthikorn Voravootivat <puthik@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1727090
Legacy-Commit-Queue: Commit Bot <commit-bot@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
(cherry picked from commit 1590e186377bfc4a379e77f437e8061a921cc2ed)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/1744576
diff --git a/power_manager/powerd/policy/ambient_light_handler.cc b/power_manager/powerd/policy/ambient_light_handler.cc
index f775917..4e56f4f 100644
--- a/power_manager/powerd/policy/ambient_light_handler.cc
+++ b/power_manager/powerd/policy/ambient_light_handler.cc
@@ -152,6 +152,10 @@
}
}
+void AmbientLightHandler::HandleResume() {
+ hysteresis_state_ = HysteresisState::RESUMING;
+}
+
std::string AmbientLightHandler::GetRecentReadingsString() const {
std::string str;
for (int i = 0; i < recent_lux_readings_.size(); ++i) {
@@ -167,6 +171,12 @@
system::AmbientLightSensorInterface* sensor) {
DCHECK_EQ(sensor, sensor_);
+ // Discard first reading after resume as it is probably cached value.
+ if (hysteresis_state_ == HysteresisState::RESUMING) {
+ hysteresis_state_ = HysteresisState::IMMEDIATE;
+ return;
+ }
+
const int raw_lux = sensor_->GetAmbientLightLux();
if (raw_lux < 0) {
LOG(WARNING) << "Sensor doesn't have valid value";
diff --git a/power_manager/powerd/policy/ambient_light_handler.h b/power_manager/powerd/policy/ambient_light_handler.h
index dc27e1f..4341f7e 100644
--- a/power_manager/powerd/policy/ambient_light_handler.h
+++ b/power_manager/powerd/policy/ambient_light_handler.h
@@ -101,6 +101,9 @@
// Should be called when the power source changes.
void HandlePowerSourceChange(PowerSource source);
+ // Should be called when resuming.
+ void HandleResume();
+
// Returns a string containing recent ALS readings, space-separated and
// newest-to-oldest. Public so it can be called by tests.
std::string GetRecentReadingsString() const;
@@ -136,6 +139,8 @@
// The brightness should be adjusted immediately after the next sensor
// reading.
IMMEDIATE,
+ // Discard next sensor reading and go to |IMMEDIATE| state.
+ RESUMING,
};
// Returns the current target backlight brightness percent based on
diff --git a/power_manager/powerd/policy/ambient_light_handler_test.cc b/power_manager/powerd/policy/ambient_light_handler_test.cc
index 9fab3df..bff6add 100644
--- a/power_manager/powerd/policy/ambient_light_handler_test.cc
+++ b/power_manager/powerd/policy/ambient_light_handler_test.cc
@@ -161,6 +161,30 @@
delegate_.cause());
}
+TEST_F(AmbientLightHandlerTest, HandleResume) {
+ steps_pref_ = "20.0 -1 40\n50.0 20 80\n100.0 60 -1";
+ initial_lux_ = 50;
+ initial_brightness_percent_ = 60.0;
+ als_smoothing_constant_ = 0.2;
+ Init();
+ EXPECT_LT(delegate_.percent(), 0.0);
+
+ // The middle step should be used as soon as a light reading is received.
+ UpdateSensor(50); // smooth_lux_ = 50
+ EXPECT_DOUBLE_EQ(50.0, delegate_.percent());
+ EXPECT_EQ(AmbientLightHandler::BrightnessChangeCause::AMBIENT_LIGHT,
+ delegate_.cause());
+
+ // Contrary to SmoothedLux Test, this time 1 readings at 10 lux should make
+ // brightness to go to lower level
+ handler_.HandleResume();
+ UpdateSensor(50); // First reading is discard as it is probably cached value.
+ UpdateSensor(10);
+ EXPECT_DOUBLE_EQ(20.0, delegate_.percent());
+ EXPECT_EQ(AmbientLightHandler::BrightnessChangeCause::AMBIENT_LIGHT,
+ delegate_.cause());
+}
+
TEST_F(AmbientLightHandlerTest, PowerSources) {
// Define a single target percent in the bottom step and separate AC and
// battery targets for the middle and top steps.
diff --git a/power_manager/powerd/policy/internal_backlight_controller.cc b/power_manager/powerd/policy/internal_backlight_controller.cc
index 10ae5a1..d1dfd7f 100644
--- a/power_manager/powerd/policy/internal_backlight_controller.cc
+++ b/power_manager/powerd/policy/internal_backlight_controller.cc
@@ -403,6 +403,9 @@
VLOG(1) << (suspended ? "Suspending" : "Unsuspending") << " backlight";
suspended_ = suspended;
UpdateState(BacklightBrightnessChange_Cause_OTHER);
+
+ if (!suspended && use_ambient_light_)
+ ambient_light_handler_->HandleResume();
}
void InternalBacklightController::SetShuttingDown(bool shutting_down) {
diff --git a/power_manager/powerd/policy/keyboard_backlight_controller.cc b/power_manager/powerd/policy/keyboard_backlight_controller.cc
index 27a6809..17c1ff8 100644
--- a/power_manager/powerd/policy/keyboard_backlight_controller.cc
+++ b/power_manager/powerd/policy/keyboard_backlight_controller.cc
@@ -302,6 +302,9 @@
suspended_ = suspended;
UpdateState(suspended ? Transition::INSTANT : Transition::FAST,
BacklightBrightnessChange_Cause_OTHER);
+
+ if (!suspended && ambient_light_handler_.get())
+ ambient_light_handler_->HandleResume();
}
void KeyboardBacklightController::SetShuttingDown(bool shutting_down) {
