From f5d7e441ce29fed56cd554011360932d0f2b0123 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Thu, 21 May 2020 09:59:56 -0700 Subject: [PATCH] ThermalManagerService: initialize to THROTTLING_NONE explicitly When there is no HAL connection or HAL malfunctioning, thermal status will stay in an invalid value, which would be returned when client register status callback. This CL will opportunistically assume system is in "NONE" throttling when in above states such as no HAL connection or HAL malfunctioning. Bug: 155772239 Test: atest ThermalManagerServiceTest Test: atest PowerManagerTest Test: atest thermalmanager-test Change-Id: I9fceb5ef6f8b9a23983b7b93200f381eaf23a77b --- .../server/power/ThermalManagerService.java | 30 +++++++++---------- .../power/ThermalManagerServiceTest.java | 8 ++--- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/power/ThermalManagerService.java b/services/core/java/com/android/server/power/ThermalManagerService.java index 4f4019cdea50f..ef4954aa4e4cf 100644 --- a/services/core/java/com/android/server/power/ThermalManagerService.java +++ b/services/core/java/com/android/server/power/ThermalManagerService.java @@ -110,9 +110,6 @@ public class ThermalManagerService extends SystemService { @VisibleForTesting final TemperatureWatcher mTemperatureWatcher = new TemperatureWatcher(); - /** Invalid throttling status */ - private static final int INVALID_THROTTLING = Integer.MIN_VALUE; - public ThermalManagerService(Context context) { this(context, null); } @@ -121,8 +118,7 @@ public class ThermalManagerService extends SystemService { ThermalManagerService(Context context, @Nullable ThermalHalWrapper halWrapper) { super(context); mHalWrapper = halWrapper; - // Initialize to invalid to send status onActivityManagerReady - mStatus = INVALID_THROTTLING; + mStatus = Temperature.THROTTLING_NONE; } @Override @@ -155,11 +151,15 @@ public class ThermalManagerService extends SystemService { } mHalWrapper.setCallback(this::onTemperatureChangedCallback); if (!halConnected) { + Slog.w(TAG, "No Thermal HAL service on this device"); return; } List temperatures = mHalWrapper.getCurrentTemperatures(false, 0); final int count = temperatures.size(); + if (count == 0) { + Slog.w(TAG, "Thermal HAL reported invalid data, abort connection"); + } for (int i = 0; i < count; i++) { onTemperatureChanged(temperatures.get(i), false); } @@ -183,9 +183,6 @@ public class ThermalManagerService extends SystemService { } private void notifyStatusListenersLocked() { - if (!Temperature.isValidStatus(mStatus)) { - return; - } final int length = mThermalStatusListeners.beginBroadcast(); try { for (int i = 0; i < length; i++) { @@ -199,7 +196,7 @@ public class ThermalManagerService extends SystemService { } private void onTemperatureMapChangedLocked() { - int newStatus = INVALID_THROTTLING; + int newStatus = Temperature.THROTTLING_NONE; final int count = mTemperatureMap.size(); for (int i = 0; i < count; i++) { Temperature t = mTemperatureMap.valueAt(i); @@ -292,11 +289,7 @@ public class ThermalManagerService extends SystemService { shutdownIfNeeded(temperature); synchronized (mLock) { Temperature old = mTemperatureMap.put(temperature.getName(), temperature); - if (old != null) { - if (old.getStatus() != temperature.getStatus()) { - notifyEventListenersLocked(temperature); - } - } else { + if (old == null || old.getStatus() != temperature.getStatus()) { notifyEventListenersLocked(temperature); } if (sendStatus) { @@ -438,8 +431,7 @@ public class ThermalManagerService extends SystemService { synchronized (mLock) { final long token = Binder.clearCallingIdentity(); try { - return Temperature.isValidStatus(mStatus) ? mStatus - : Temperature.THROTTLING_NONE; + return mStatus; } finally { Binder.restoreCallingIdentity(token); } @@ -964,6 +956,12 @@ public class ThermalManagerService extends SystemService { if (ThermalStatusCode.SUCCESS == status.code) { for (android.hardware.thermal.V2_0.Temperature temperature : temperatures) { + if (!Temperature.isValidStatus( + temperature.throttlingStatus)) { + Slog.e(TAG, "Invalid status data from HAL"); + temperature.throttlingStatus = + Temperature.THROTTLING_NONE; + } ret.add(new Temperature( temperature.value, temperature.type, temperature.name, diff --git a/services/tests/servicestests/src/com/android/server/power/ThermalManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/power/ThermalManagerServiceTest.java index 615fa5c614f04..083df28b22786 100644 --- a/services/tests/servicestests/src/com/android/server/power/ThermalManagerServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/power/ThermalManagerServiceTest.java @@ -216,11 +216,11 @@ public class ThermalManagerServiceTest { verify(mEventListener1, timeout(CALLBACK_TIMEOUT_MILLI_SEC) .times(0)).notifyThrottling(any(Temperature.class)); verify(mStatusListener1, timeout(CALLBACK_TIMEOUT_MILLI_SEC) - .times(1)).onStatusChange(anyInt()); + .times(1)).onStatusChange(Temperature.THROTTLING_NONE); verify(mEventListener2, timeout(CALLBACK_TIMEOUT_MILLI_SEC) .times(0)).notifyThrottling(any(Temperature.class)); verify(mStatusListener2, timeout(CALLBACK_TIMEOUT_MILLI_SEC) - .times(1)).onStatusChange(anyInt()); + .times(1)).onStatusChange(Temperature.THROTTLING_NONE); resetListenerMock(); mService.onBootPhase(SystemService.PHASE_ACTIVITY_MANAGER_READY); ArgumentCaptor captor = ArgumentCaptor.forClass(Temperature.class); @@ -228,7 +228,7 @@ public class ThermalManagerServiceTest { .times(4)).notifyThrottling(captor.capture()); assertListEqualsIgnoringOrder(mFakeHal.mTemperatureList, captor.getAllValues()); verify(mStatusListener1, timeout(CALLBACK_TIMEOUT_MILLI_SEC) - .times(1)).onStatusChange(Temperature.THROTTLING_NONE); + .times(0)).onStatusChange(Temperature.THROTTLING_NONE); captor = ArgumentCaptor.forClass(Temperature.class); verify(mEventListener2, timeout(CALLBACK_TIMEOUT_MILLI_SEC) .times(2)).notifyThrottling(captor.capture()); @@ -236,7 +236,7 @@ public class ThermalManagerServiceTest { new ArrayList<>(Arrays.asList(mFakeHal.mSkin1, mFakeHal.mSkin2)), captor.getAllValues()); verify(mStatusListener2, timeout(CALLBACK_TIMEOUT_MILLI_SEC) - .times(1)).onStatusChange(Temperature.THROTTLING_NONE); + .times(0)).onStatusChange(Temperature.THROTTLING_NONE); } private void resetListenerMock() {