From 89d55c1d50305a43b3e56157a1e4cabbda0bdb0e Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Wed, 11 Oct 2017 11:29:01 -0700 Subject: BatteryService: Use android.hardware.health@2.0. - BatteryService talks to android.hardware.health@2.0::IHealth /default and /backup instances. - /default is optionally implemented by vendor (for launch devices) and overrides /backup if it exist. - /backup is served by healthd (for upgrading devices) and is the fallback option. Before this change, BatteryService talks to healthd in binder. This change switches to hwbinder for this communication. The information flow between them stays the same (and is augmented with a follow-up CL for BatteryManager). Test: adb logcat -e "health@2.0" Test: vts health test Test: BatteryManagerTest Bug: 62229583 Change-Id: I46c4c0746a579b76c94660d062b92b9a9c4a4dbd --- .../java/com/android/server/BatteryService.java | 153 +++++++++++++++------ .../src/com/android/server/BatteryServiceTest.java | 4 +- 2 files changed, 111 insertions(+), 46 deletions(-) diff --git a/services/core/java/com/android/server/BatteryService.java b/services/core/java/com/android/server/BatteryService.java index 47be0a704d00..1978baab7fb4 100644 --- a/services/core/java/com/android/server/BatteryService.java +++ b/services/core/java/com/android/server/BatteryService.java @@ -39,10 +39,10 @@ import android.content.pm.PackageManager; import android.hidl.manager.V1_0.IServiceManager; import android.hidl.manager.V1_0.IServiceNotification; import android.hardware.health.V2_0.HealthInfo; +import android.hardware.health.V2_0.IHealthInfoCallback; import android.hardware.health.V2_0.IHealth; import android.os.BatteryManager; import android.os.BatteryManagerInternal; -import android.os.BatteryProperties; import android.os.Binder; import android.os.FileUtils; import android.os.Handler; @@ -70,6 +70,7 @@ import java.io.PrintWriter; import java.util.Arrays; import java.util.List; import java.util.NoSuchElementException; +import java.util.concurrent.atomic.AtomicReference; /** *

BatteryService monitors the charging status, and charge level of the device @@ -108,6 +109,8 @@ public final class BatteryService extends SystemService { private static final int BATTERY_SCALE = 100; // battery capacity is a percentage + private static final long HEALTH_HAL_WAIT_MS = 1000; + // Used locally for determining when to make a last ditch effort to log // discharge stats before the device dies. private int mCriticalBatteryLevel; @@ -165,6 +168,9 @@ public final class BatteryService extends SystemService { private ActivityManagerInternal mActivityManagerInternal; + private HealthServiceWrapper mHealthServiceWrapper; + private HealthHalCallback mHealthHalCallback; + public BatteryService(Context context) { super(context); @@ -203,14 +209,7 @@ public final class BatteryService extends SystemService { @Override public void onStart() { - IBinder b = ServiceManager.getService("batteryproperties"); - final IBatteryPropertiesRegistrar batteryPropertiesRegistrar = - IBatteryPropertiesRegistrar.Stub.asInterface(b); - try { - batteryPropertiesRegistrar.registerListener(new BatteryListener()); - } catch (RemoteException e) { - // Should never happen. - } + registerHealthCallback(); mBinderService = new BinderService(); publishBinderService("battery", mBinderService); @@ -239,6 +238,49 @@ public final class BatteryService extends SystemService { } } + private void registerHealthCallback() { + mHealthServiceWrapper = new HealthServiceWrapper(); + mHealthHalCallback = new HealthHalCallback(); + // IHealth is lazily retrieved. + try { + mHealthServiceWrapper.init(mHealthHalCallback, + new HealthServiceWrapper.IServiceManagerSupplier() {}, + new HealthServiceWrapper.IHealthSupplier() {}); + } catch (RemoteException | NoSuchElementException ex) { + Slog.w(TAG, "health: cannot register callback. " + + "BatteryService will be started with dummy values. Reason: " + + ex.getClass().getSimpleName() + ": " + ex.getMessage()); + update(new HealthInfo()); + return; + } + + // init register for new service notifications, and IServiceManager should return the + // existing service in a near future. Wait for this.update() to instantiate + // the initial mHealthInfo. + long timeWaited = 0; + synchronized (mLock) { + long beforeWait = SystemClock.uptimeMillis(); + while (mHealthInfo == null && + (timeWaited = SystemClock.uptimeMillis() - beforeWait) < HEALTH_HAL_WAIT_MS) { + try { + mLock.wait(HEALTH_HAL_WAIT_MS - timeWaited); + } catch (InterruptedException ex) { + break; + } + } + if (mHealthInfo == null) { + Slog.w(TAG, "health: Waited " + timeWaited + "ms for callbacks but received " + + "nothing. BatteryService will be started with dummy values."); + update(new HealthInfo()); + return; + } + } + + if (DEBUG) { + Slog.d(TAG, "health: Waited " + timeWaited + "ms and received the update."); + } + } + private void updateBatteryWarningLevelLocked() { final ContentResolver resolver = mContext.getContentResolver(); int defWarnLevel = mContext.getResources().getInteger( @@ -331,15 +373,15 @@ public final class BatteryService extends SystemService { } } - private void update(BatteryProperties props) { + private void update(HealthInfo info) { synchronized (mLock) { if (!mUpdatesStopped) { - mHealthInfo = new HealthInfo(); - copy(mHealthInfo, props); + mHealthInfo = info; // Process the new values. processValuesLocked(false); + mLock.notifyAll(); // for any waiters on new info } else { - copy(mLastHealthInfo, props); + copy(mLastHealthInfo, info); } } } @@ -366,24 +408,6 @@ public final class BatteryService extends SystemService { dst.energyCounter = src.energyCounter; } - // TODO(b/62229583): remove this function when BatteryProperties are completely replaced. - private static void copy(HealthInfo dst, BatteryProperties src) { - dst.legacy.chargerAcOnline = src.chargerAcOnline; - dst.legacy.chargerUsbOnline = src.chargerUsbOnline; - dst.legacy.chargerWirelessOnline = src.chargerWirelessOnline; - dst.legacy.maxChargingCurrent = src.maxChargingCurrent; - dst.legacy.maxChargingVoltage = src.maxChargingVoltage; - dst.legacy.batteryStatus = src.batteryStatus; - dst.legacy.batteryHealth = src.batteryHealth; - dst.legacy.batteryPresent = src.batteryPresent; - dst.legacy.batteryLevel = src.batteryLevel; - dst.legacy.batteryVoltage = src.batteryVoltage; - dst.legacy.batteryTemperature = src.batteryTemperature; - dst.legacy.batteryFullCharge = src.batteryFullCharge; - dst.legacy.batteryChargeCounter = src.batteryChargeCounter; - dst.legacy.batteryTechnology = src.batteryTechnology; - } - private void processValuesLocked(boolean force) { boolean logOutlier = false; long dischargeDuration = 0; @@ -962,15 +986,43 @@ public final class BatteryService extends SystemService { } } - private final class BatteryListener extends IBatteryPropertiesListener.Stub { - @Override public void batteryPropertiesChanged(BatteryProperties props) { - final long identity = Binder.clearCallingIdentity(); + private final class HealthHalCallback extends IHealthInfoCallback.Stub + implements HealthServiceWrapper.Callback { + @Override public void healthInfoChanged(HealthInfo props) { + BatteryService.this.update(props); + } + // on new service registered + @Override public void onRegistration(IHealth oldService, IHealth newService, + String instance) { + if (newService == null) return; + + try { + if (oldService != null) { + int r = oldService.unregisterCallback(this); + if (r != Result.SUCCESS) { + Slog.w(TAG, "health: cannot unregister previous callback: " + + Result.toString(r)); + } + } + } catch (RemoteException ex) { + Slog.w(TAG, "health: cannot unregister previous callback (transaction error): " + + ex.getMessage()); + } + try { - BatteryService.this.update(props); - } finally { - Binder.restoreCallingIdentity(identity); + int r = newService.registerCallback(this); + if (r != Result.SUCCESS) { + Slog.w(TAG, "health: cannot register callback: " + Result.toString(r)); + return; + } + // registerCallback does NOT guarantee that update is called + // immediately, so request a manual update here. + newService.update(); + } catch (RemoteException ex) { + Slog.e(TAG, "health: cannot register callback (transaction error): " + + ex.getMessage()); } - } + } } private final class BinderService extends Binder { @@ -1053,6 +1105,11 @@ public final class BatteryService extends SystemService { private Callback mCallback; private IHealthSupplier mHealthSupplier; + private final Object mLastServiceSetLock = new Object(); + // Last IHealth service received. + // set must be also be guarded with mLastServiceSetLock to ensure ordering. + private final AtomicReference mLastService = new AtomicReference<>(); + /** * init should be called after constructor. For testing purposes, init is not called by * constructor. @@ -1109,7 +1166,7 @@ public final class BatteryService extends SystemService { * into service. * @param instance instance name. */ - void onRegistration(IHealth service, String instance); + void onRegistration(IHealth oldService, IHealth newService, String instance); } /** @@ -1117,14 +1174,18 @@ public final class BatteryService extends SystemService { * Must not return null; throw {@link NoSuchElementException} if a service is not available. */ interface IServiceManagerSupplier { - IServiceManager get() throws NoSuchElementException, RemoteException; + default IServiceManager get() throws NoSuchElementException, RemoteException { + return IServiceManager.getService(); + } } /** * Supplier of services. * Must not return null; throw {@link NoSuchElementException} if a service is not available. */ interface IHealthSupplier { - IHealth get(String instanceName) throws NoSuchElementException, RemoteException; + default IHealth get(String name) throws NoSuchElementException, RemoteException { + return IHealth.getService(name); + } } private class Notification extends IServiceNotification.Stub { @@ -1134,9 +1195,13 @@ public final class BatteryService extends SystemService { if (!IHealth.kInterfaceName.equals(interfaceName)) return; if (!sAllInstances.contains(instanceName)) return; try { - IHealth service = mHealthSupplier.get(instanceName); - Slog.i(TAG, "health: new instance registered " + instanceName); - mCallback.onRegistration(service, instanceName); + // ensures the order of multiple onRegistration on different threads. + synchronized (mLastServiceSetLock) { + IHealth newService = mHealthSupplier.get(instanceName); + IHealth oldService = mLastService.getAndSet(newService); + Slog.i(TAG, "health: new instance registered " + instanceName); + mCallback.onRegistration(oldService, newService, instanceName); + } } catch (NoSuchElementException | RemoteException ex) { Slog.e(TAG, "health: Cannot get instance '" + instanceName + "': " + ex.getMessage() + ". Perhaps no permission?"); diff --git a/services/tests/servicestests/src/com/android/server/BatteryServiceTest.java b/services/tests/servicestests/src/com/android/server/BatteryServiceTest.java index 5d1d07bf42a2..daaad7a8c0b6 100644 --- a/services/tests/servicestests/src/com/android/server/BatteryServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/BatteryServiceTest.java @@ -98,14 +98,14 @@ public class BatteryServiceTest extends AndroidTestCase { public void testWrapPreferVendor() throws Exception { initForInstances(VENDOR, HEALTHD); mWrapper.init(mCallback, mManagerSupplier, mHealthServiceSupplier); - verify(mCallback).onRegistration(same(mMockedHal), eq(VENDOR)); + verify(mCallback).onRegistration(same(null), same(mMockedHal), eq(VENDOR)); } @SmallTest public void testUseHealthd() throws Exception { initForInstances(HEALTHD); mWrapper.init(mCallback, mManagerSupplier, mHealthServiceSupplier); - verify(mCallback).onRegistration(same(mMockedHal), eq(HEALTHD)); + verify(mCallback).onRegistration(same(null), same(mMockedHal), eq(HEALTHD)); } @SmallTest -- cgit v1.2.3-59-g8ed1b From 1fd86f4c058c6f2f31df6489049f7ab6fbeee717 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Mon, 9 Oct 2017 16:50:33 -0700 Subject: BatteryService serves IBatteryPropertiesRegistrar. This binder service is originally served by healthd. To allow BatteryManager to continue to work, BatteryService implements this binder service. Test: BatteryManagerTest Bug: 62229583 Change-Id: I9c97b3936546740c6d63060899fe50c5c4c957bd --- core/java/android/os/BatteryProperty.java | 7 +++ .../java/com/android/server/BatteryService.java | 67 ++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/core/java/android/os/BatteryProperty.java b/core/java/android/os/BatteryProperty.java index 84119bdc126d..b7e7b17729f9 100644 --- a/core/java/android/os/BatteryProperty.java +++ b/core/java/android/os/BatteryProperty.java @@ -43,6 +43,13 @@ public class BatteryProperty implements Parcelable { return mValueLong; } + /** + * @hide + */ + public void setLong(long val) { + mValueLong = val; + } + /* * Parcel read/write code must be kept in sync with * frameworks/native/services/batteryservice/BatteryProperty.cpp diff --git a/services/core/java/com/android/server/BatteryService.java b/services/core/java/com/android/server/BatteryService.java index 1978baab7fb4..46b671bd1419 100644 --- a/services/core/java/com/android/server/BatteryService.java +++ b/services/core/java/com/android/server/BatteryService.java @@ -41,8 +41,10 @@ import android.hidl.manager.V1_0.IServiceNotification; import android.hardware.health.V2_0.HealthInfo; import android.hardware.health.V2_0.IHealthInfoCallback; import android.hardware.health.V2_0.IHealth; +import android.hardware.health.V2_0.Result; import android.os.BatteryManager; import android.os.BatteryManagerInternal; +import android.os.BatteryProperty; import android.os.Binder; import android.os.FileUtils; import android.os.Handler; @@ -58,6 +60,7 @@ import android.os.UserHandle; import android.provider.Settings; import android.service.battery.BatteryServiceDumpProto; import android.util.EventLog; +import android.util.MutableInt; import android.util.Slog; import android.util.proto.ProtoOutputStream; @@ -170,6 +173,7 @@ public final class BatteryService extends SystemService { private HealthServiceWrapper mHealthServiceWrapper; private HealthHalCallback mHealthHalCallback; + private BatteryPropertiesRegistrar mBatteryPropertiesRegistrar; public BatteryService(Context context) { super(context); @@ -213,6 +217,8 @@ public final class BatteryService extends SystemService { mBinderService = new BinderService(); publishBinderService("battery", mBinderService); + mBatteryPropertiesRegistrar = new BatteryPropertiesRegistrar(); + publishBinderService("batteryproperties", mBatteryPropertiesRegistrar); publishLocalService(BatteryManagerInternal.class, new LocalService()); } @@ -1043,6 +1049,63 @@ public final class BatteryService extends SystemService { } } + // Reduced IBatteryPropertiesRegistrar that only implements getProperty for usage + // in BatteryManager. + private final class BatteryPropertiesRegistrar extends IBatteryPropertiesRegistrar.Stub { + public void registerListener(IBatteryPropertiesListener listener) { + Slog.e(TAG, "health: must not call registerListener on battery properties"); + } + public void unregisterListener(IBatteryPropertiesListener listener) { + Slog.e(TAG, "health: must not call unregisterListener on battery properties"); + } + public int getProperty(int id, final BatteryProperty prop) throws RemoteException { + IHealth service = mHealthServiceWrapper.getLastService(); + final MutableInt outResult = new MutableInt(Result.NOT_SUPPORTED); + switch(id) { + case BatteryManager.BATTERY_PROPERTY_CHARGE_COUNTER: + service.getChargeCounter((int result, int value) -> { + outResult.value = result; + if (result == Result.SUCCESS) prop.setLong(value); + }); + break; + case BatteryManager.BATTERY_PROPERTY_CURRENT_NOW: + service.getCurrentNow((int result, int value) -> { + outResult.value = result; + if (result == Result.SUCCESS) prop.setLong(value); + }); + break; + case BatteryManager.BATTERY_PROPERTY_CURRENT_AVERAGE: + service.getCurrentAverage((int result, int value) -> { + outResult.value = result; + if (result == Result.SUCCESS) prop.setLong(value); + }); + break; + case BatteryManager.BATTERY_PROPERTY_CAPACITY: + service.getCapacity((int result, int value) -> { + outResult.value = result; + if (result == Result.SUCCESS) prop.setLong(value); + }); + break; + case BatteryManager.BATTERY_PROPERTY_STATUS: + service.getChargeStatus((int result, int value) -> { + outResult.value = result; + if (result == Result.SUCCESS) prop.setLong(value); + }); + break; + case BatteryManager.BATTERY_PROPERTY_ENERGY_COUNTER: + service.getEnergyCounter((int result, long value) -> { + outResult.value = result; + if (result == Result.SUCCESS) prop.setLong(value); + }); + break; + } + return outResult.value; + } + public void scheduleUpdate() { + Slog.e(TAG, "health: must not call scheduleUpdate on battery properties"); + } + } + private final class LocalService extends BatteryManagerInternal { @Override public boolean isPowered(int plugTypeSet) { @@ -1117,6 +1180,10 @@ public final class BatteryService extends SystemService { HealthServiceWrapper() { } + IHealth getLastService() { + return mLastService.get(); + } + /** * Start monitoring registration of new IHealth services. Only instances that are in * {@code sAllInstances} and in device / framework manifest are used. This function should -- cgit v1.2.3-59-g8ed1b