Merge changes from topic "health-service-wrapper-test"

* changes:
  health: Drop HealthServiceWrapperHidl.init.
  BatteryService: remove HIDL-ness
  Move BatteryServiceTest to HealthServiceWrapperTest.

services/core/java/com/android/server/BatteryService.java

@@ -17,6 +17,7 @@
 package com.android.server;
 
 import static com.android.internal.logging.nano.MetricsProto.MetricsEvent;
+import static com.android.server.health.Utils.copy;
 
 import android.annotation.Nullable;
 import android.app.ActivityManager;
@@ -26,13 +27,7 @@ import android.content.Context;
 import android.content.Intent;
 import android.database.ContentObserver;
 import android.hardware.health.V1_0.HealthInfo;
-import android.hardware.health.V2_0.IHealth;
-import android.hardware.health.V2_0.Result;
 import android.hardware.health.V2_1.BatteryCapacityLevel;
-import android.hardware.health.V2_1.Constants;
-import android.hardware.health.V2_1.IHealthInfoCallback;
-import android.hidl.manager.V1_0.IServiceManager;
-import android.hidl.manager.V1_0.IServiceNotification;
 import android.metrics.LogMaker;
 import android.os.BatteryManager;
 import android.os.BatteryManagerInternal;
@@ -44,7 +39,6 @@ import android.os.Bundle;
 import android.os.DropBoxManager;
 import android.os.FileUtils;
 import android.os.Handler;
-import android.os.HandlerThread;
 import android.os.IBatteryPropertiesRegistrar;
 import android.os.IBinder;
 import android.os.OsProtoEnums;
@@ -62,15 +56,14 @@ import android.provider.Settings;
 import android.service.battery.BatteryServiceDumpProto;
 import android.sysprop.PowerProperties;
 import android.util.EventLog;
-import android.util.MutableInt;
 import android.util.Slog;
 import android.util.proto.ProtoOutputStream;
 
-import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.app.IBatteryStats;
 import com.android.internal.logging.MetricsLogger;
 import com.android.internal.util.DumpUtils;
 import com.android.server.am.BatteryStatsService;
+import com.android.server.health.HealthServiceWrapper;
 import com.android.server.lights.LightsManager;
 import com.android.server.lights.LogicalLight;
 
@@ -82,8 +75,6 @@ import java.io.PrintWriter;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.NoSuchElementException;
-import java.util.Objects;
-import java.util.concurrent.atomic.AtomicReference;
 
 /**
  * <p>BatteryService monitors the charging status, and charge level of the device
@@ -191,7 +182,6 @@ public final class BatteryService extends SystemService {
     private ActivityManagerInternal mActivityManagerInternal;
 
     private HealthServiceWrapper mHealthServiceWrapper;
-    private HealthHalCallback mHealthHalCallback;
     private BatteryPropertiesRegistrar mBatteryPropertiesRegistrar;
     private ArrayDeque<Bundle> mBatteryLevelsEventQueue;
     private long mLastBatteryLevelChangedSentMs;
@@ -274,13 +264,9 @@ public final class BatteryService extends SystemService {
 
     private void registerHealthCallback() {
         traceBegin("HealthInitWrapper");
-        mHealthServiceWrapper = new HealthServiceWrapper();
-        mHealthHalCallback = new HealthHalCallback();
         // IHealth is lazily retrieved.
         try {
-            mHealthServiceWrapper.init(mHealthHalCallback,
+            mHealthServiceWrapper = HealthServiceWrapper.create(this::update);
-                    new HealthServiceWrapper.IServiceManagerSupplier() {},
-                    new HealthServiceWrapper.IHealthSupplier() {});
         } catch (RemoteException ex) {
             Slog.e(TAG, "health: cannot register callback. (RemoteException)");
             throw ex.rethrowFromSystemServer();
@@ -454,25 +440,6 @@ public final class BatteryService extends SystemService {
         traceEnd();
     }
 
-    private static void copy(HealthInfo dst, HealthInfo src) {
-        dst.chargerAcOnline = src.chargerAcOnline;
-        dst.chargerUsbOnline = src.chargerUsbOnline;
-        dst.chargerWirelessOnline = src.chargerWirelessOnline;
-        dst.maxChargingCurrent = src.maxChargingCurrent;
-        dst.maxChargingVoltage = src.maxChargingVoltage;
-        dst.batteryStatus = src.batteryStatus;
-        dst.batteryHealth = src.batteryHealth;
-        dst.batteryPresent = src.batteryPresent;
-        dst.batteryLevel = src.batteryLevel;
-        dst.batteryVoltage = src.batteryVoltage;
-        dst.batteryTemperature = src.batteryTemperature;
-        dst.batteryCurrent = src.batteryCurrent;
-        dst.batteryCycleCount = src.batteryCycleCount;
-        dst.batteryFullCharge = src.batteryFullCharge;
-        dst.batteryChargeCounter = src.batteryChargeCounter;
-        dst.batteryTechnology = src.batteryTechnology;
-    }
-
     private static int plugType(HealthInfo healthInfo) {
         if (healthInfo.chargerAcOnline) {
             return BatteryManager.BATTERY_PLUGGED_AC;
@@ -1184,64 +1151,6 @@ public final class BatteryService extends SystemService {
         }
     }
 
-    private final class HealthHalCallback extends IHealthInfoCallback.Stub
-            implements HealthServiceWrapper.Callback {
-        @Override public void healthInfoChanged(android.hardware.health.V2_0.HealthInfo props) {
-            android.hardware.health.V2_1.HealthInfo propsLatest =
-                    new android.hardware.health.V2_1.HealthInfo();
-            propsLatest.legacy = props;
-
-            propsLatest.batteryCapacityLevel = BatteryCapacityLevel.UNSUPPORTED;
-            propsLatest.batteryChargeTimeToFullNowSeconds =
-                Constants.BATTERY_CHARGE_TIME_TO_FULL_NOW_SECONDS_UNSUPPORTED;
-
-            BatteryService.this.update(propsLatest);
-        }
-
-        @Override public void healthInfoChanged_2_1(android.hardware.health.V2_1.HealthInfo props) {
-            BatteryService.this.update(props);
-        }
-
-        // on new service registered
-        @Override public void onRegistration(IHealth oldService, IHealth newService,
-                String instance) {
-            if (newService == null) return;
-
-            traceBegin("HealthUnregisterCallback");
-            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());
-            } finally {
-                traceEnd();
-            }
-
-            traceBegin("HealthRegisterCallback");
-            try {
-                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());
-            } finally {
-                traceEnd();
-            }
-        }
-    }
-
     private final class BinderService extends Binder {
         @Override protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
             if (!DumpUtils.checkDumpPermission(mContext, TAG, pw)) return;
@@ -1265,71 +1174,11 @@ public final class BatteryService extends SystemService {
     private final class BatteryPropertiesRegistrar extends IBatteryPropertiesRegistrar.Stub {
         @Override
         public int getProperty(int id, final BatteryProperty prop) throws RemoteException {
-            traceBegin("HealthGetProperty");
+            return mHealthServiceWrapper.getProperty(id, prop);
-            try {
-                IHealth service = mHealthServiceWrapper.getLastService();
-                if (service == null) throw new RemoteException("no health service");
-                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;
-            } finally {
-                traceEnd();
-            }
         }
         @Override
         public void scheduleUpdate() throws RemoteException {
-            mHealthServiceWrapper.getHandlerThread().getThreadHandler().post(() -> {
+            mHealthServiceWrapper.scheduleUpdate();
-                traceBegin("HealthScheduleUpdate");
-                try {
-                    IHealth service = mHealthServiceWrapper.getLastService();
-                    if (service == null) {
-                        Slog.e(TAG, "no health service");
-                        return;
-                    }
-                    service.update();
-                } catch (RemoteException ex) {
-                    Slog.e(TAG, "Cannot call update on health HAL", ex);
-                } finally {
-                    traceEnd();
-                }
-            });
         }
     }
 
@@ -1418,184 +1267,4 @@ public final class BatteryService extends SystemService {
             BatteryService.this.suspendBatteryInput();
         }
     }
-
-    /**
-     * HealthServiceWrapper wraps the internal IHealth service and refreshes the service when
-     * necessary.
-     *
-     * On new registration of IHealth service, {@link #onRegistration onRegistration} is called and
-     * the internal service is refreshed.
-     * On death of an existing IHealth service, the internal service is NOT cleared to avoid
-     * race condition between death notification and new service notification. Hence,
-     * a caller must check for transaction errors when calling into the service.
-     *
-     * @hide Should only be used internally.
-     */
-    public static final class HealthServiceWrapper {
-        private static final String TAG = "HealthServiceWrapper";
-        public static final String INSTANCE_VENDOR = "default";
-
-        private final IServiceNotification mNotification = new Notification();
-        private final HandlerThread mHandlerThread = new HandlerThread("HealthServiceHwbinder");
-        // These variables are fixed after init.
-        private Callback mCallback;
-        private IHealthSupplier mHealthSupplier;
-        private String mInstanceName;
-
-        // Last IHealth service received.
-        private final AtomicReference<IHealth> mLastService = new AtomicReference<>();
-
-        /**
-         * init should be called after constructor. For testing purposes, init is not called by
-         * constructor.
-         */
-        public HealthServiceWrapper() {
-        }
-
-        public IHealth getLastService() {
-            return mLastService.get();
-        }
-
-        /**
-         * See {@link #init(Callback, IServiceManagerSupplier, IHealthSupplier)}
-         */
-        public void init() throws RemoteException, NoSuchElementException {
-            init(/* callback= */null, new HealthServiceWrapper.IServiceManagerSupplier() {},
-                    new HealthServiceWrapper.IHealthSupplier() {});
-        }
-
-        /**
-         * Start monitoring registration of new IHealth services. Only instance
-         * {@link #INSTANCE_VENDOR} and in device / framework manifest are used. This function should
-         * only be called once.
-         *
-         * mCallback.onRegistration() is called synchronously (aka in init thread) before
-         * this method returns if callback is not null.
-         *
-         * @throws RemoteException transaction error when talking to IServiceManager
-         * @throws NoSuchElementException if one of the following cases:
-         *         - No service manager;
-         *         - {@link #INSTANCE_VENDOR} is not in manifests (i.e. not
-         *           available on this device), or none of these instances are available to current
-         *           process.
-         * @throws NullPointerException when supplier is null
-         */
-        void init(@Nullable Callback callback,
-                  IServiceManagerSupplier managerSupplier,
-                  IHealthSupplier healthSupplier)
-                throws RemoteException, NoSuchElementException, NullPointerException {
-            if (managerSupplier == null || healthSupplier == null) {
-                throw new NullPointerException();
-            }
-            IServiceManager manager;
-
-            mHealthSupplier = healthSupplier;
-
-            // Initialize mLastService and call callback for the first time (in init thread)
-            IHealth newService = null;
-            traceBegin("HealthInitGetService_" + INSTANCE_VENDOR);
-            try {
-                newService = healthSupplier.get(INSTANCE_VENDOR);
-            } catch (NoSuchElementException ex) {
-                /* ignored, handled below */
-            } finally {
-                traceEnd();
-            }
-            if (newService != null) {
-                mInstanceName = INSTANCE_VENDOR;
-                mLastService.set(newService);
-            }
-
-            if (mInstanceName == null || newService == null) {
-                throw new NoSuchElementException(String.format(
-                        "IHealth service instance %s isn't available. Perhaps no permission?",
-                        INSTANCE_VENDOR));
-            }
-
-            if (callback != null) {
-                mCallback = callback;
-                mCallback.onRegistration(null, newService, mInstanceName);
-            }
-
-            // Register for future service registrations
-            traceBegin("HealthInitRegisterNotification");
-            mHandlerThread.start();
-            try {
-                managerSupplier.get().registerForNotifications(
-                        IHealth.kInterfaceName, mInstanceName, mNotification);
-            } finally {
-                traceEnd();
-            }
-            Slog.i(TAG, "health: HealthServiceWrapper listening to instance " + mInstanceName);
-        }
-
-        @VisibleForTesting
-        HandlerThread getHandlerThread() {
-            return mHandlerThread;
-        }
-
-        interface Callback {
-            /**
-             * This function is invoked asynchronously when a new and related IServiceNotification
-             * is received.
-             * @param service the recently retrieved service from IServiceManager.
-             * Can be a dead service before service notification of a new service is delivered.
-             * Implementation must handle cases for {@link RemoteException}s when calling
-             * into service.
-             * @param instance instance name.
-             */
-            void onRegistration(IHealth oldService, IHealth newService, String instance);
-        }
-
-        /**
-         * Supplier of services.
-         * Must not return null; throw {@link NoSuchElementException} if a service is not available.
-         */
-        interface IServiceManagerSupplier {
-            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 {
-            default IHealth get(String name) throws NoSuchElementException, RemoteException {
-                return IHealth.getService(name, true /* retry */);
-            }
-        }
-
-        private class Notification extends IServiceNotification.Stub {
-            @Override
-            public final void onRegistration(String interfaceName, String instanceName,
-                    boolean preexisting) {
-                if (!IHealth.kInterfaceName.equals(interfaceName)) return;
-                if (!mInstanceName.equals(instanceName)) return;
-
-                // This runnable only runs on mHandlerThread and ordering is ensured, hence
-                // no locking is needed inside the runnable.
-                mHandlerThread.getThreadHandler().post(new Runnable() {
-                    @Override
-                    public void run() {
-                        try {
-                            IHealth newService = mHealthSupplier.get(mInstanceName);
-                            IHealth oldService = mLastService.getAndSet(newService);
-
-                            // preexisting may be inaccurate (race). Check for equality here.
-                            if (Objects.equals(newService, oldService)) return;
-
-                            Slog.i(TAG, "health: new instance registered " + mInstanceName);
-                            // #init() may be called with null callback. Skip null callbacks.
-                            if (mCallback == null) return;
-                            mCallback.onRegistration(oldService, newService, mInstanceName);
-                        } catch (NoSuchElementException | RemoteException ex) {
-                            Slog.e(TAG, "health: Cannot get instance '" + mInstanceName
-                                    + "': " + ex.getMessage() + ". Perhaps no permission?");
-                        }
-                    }
-                });
-            }
-        }
-    }
 }

services/core/java/com/android/server/health/HealthHalCallbackHidl.java

@@ -0,0 +1,115 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * 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.
+ */
+
+package com.android.server.health;
+
+import android.annotation.NonNull;
+import android.hardware.health.V2_0.IHealth;
+import android.hardware.health.V2_0.Result;
+import android.hardware.health.V2_1.BatteryCapacityLevel;
+import android.hardware.health.V2_1.Constants;
+import android.hardware.health.V2_1.IHealthInfoCallback;
+import android.os.RemoteException;
+import android.os.Trace;
+import android.util.Slog;
+
+/**
+ * On service registration, {@link HealthServiceWrapperHidl.Callback#onRegistration} is called,
+ * which registers {@code this}, a {@link IHealthInfoCallback}, to the health service.
+ *
+ * <p>When the health service has updates to health info, {@link HealthInfoCallback#update} is
+ * called.
+ *
+ * @hide
+ */
+class HealthHalCallbackHidl extends IHealthInfoCallback.Stub
+        implements HealthServiceWrapperHidl.Callback {
+
+    private static final String TAG = HealthHalCallbackHidl.class.getSimpleName();
+
+    private static void traceBegin(String name) {
+        Trace.traceBegin(Trace.TRACE_TAG_SYSTEM_SERVER, name);
+    }
+
+    private static void traceEnd() {
+        Trace.traceEnd(Trace.TRACE_TAG_SYSTEM_SERVER);
+    }
+
+    private HealthInfoCallback mCallback;
+
+    HealthHalCallbackHidl(@NonNull HealthInfoCallback callback) {
+        mCallback = callback;
+    }
+
+    @Override
+    public void healthInfoChanged(android.hardware.health.V2_0.HealthInfo props) {
+        android.hardware.health.V2_1.HealthInfo propsLatest =
+                new android.hardware.health.V2_1.HealthInfo();
+        propsLatest.legacy = props;
+
+        propsLatest.batteryCapacityLevel = BatteryCapacityLevel.UNSUPPORTED;
+        propsLatest.batteryChargeTimeToFullNowSeconds =
+                Constants.BATTERY_CHARGE_TIME_TO_FULL_NOW_SECONDS_UNSUPPORTED;
+
+        mCallback.update(propsLatest);
+    }
+
+    @Override
+    public void healthInfoChanged_2_1(android.hardware.health.V2_1.HealthInfo props) {
+        mCallback.update(props);
+    }
+
+    // on new service registered
+    @Override
+    public void onRegistration(IHealth oldService, IHealth newService, String instance) {
+        if (newService == null) return;
+
+        traceBegin("HealthUnregisterCallback");
+        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());
+        } finally {
+            traceEnd();
+        }
+
+        traceBegin("HealthRegisterCallback");
+        try {
+            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());
+        } finally {
+            traceEnd();
+        }
+    }
+}

services/core/java/com/android/server/health/HealthInfoCallback.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * 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.
+ */
+
+package com.android.server.health;
+
+/**
+ * A wrapper over HIDL / AIDL IHealthInfoCallback.
+ *
+ * @hide
+ */
+public interface HealthInfoCallback {
+    /**
+     * Signals to the client that health info is changed.
+     *
+     * @param props the new health info.
+     */
+    // TODO(b/177269435): AIDL
+    void update(android.hardware.health.V2_1.HealthInfo props);
+}

services/core/java/com/android/server/health/HealthServiceWrapper.java

@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * 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.
+ */
+
+package com.android.server.health;
+
+import android.annotation.NonNull;
+import android.annotation.Nullable;
+import android.os.BatteryProperty;
+import android.os.HandlerThread;
+import android.os.IBatteryPropertiesRegistrar;
+import android.os.RemoteException;
+
+import com.android.internal.annotations.VisibleForTesting;
+
+import java.util.NoSuchElementException;
+
+/**
+ * HealthServiceWrapper wraps the internal IHealth service and refreshes the service when necessary.
+ * This is essentially a wrapper over IHealth that is useful for BatteryService.
+ *
+ * <p>The implementation may be backed by a HIDL or AIDL HAL.
+ *
+ * <p>On new registration of IHealth service, the internal service is refreshed. On death of an
+ * existing IHealth service, the internal service is NOT cleared to avoid race condition between
+ * death notification and new service notification. Hence, a caller must check for transaction
+ * errors when calling into the service.
+ *
+ * @hide Should only be used internally.
+ */
+public abstract class HealthServiceWrapper {
+    /** @return the handler thread. Exposed for testing. */
+    @VisibleForTesting
+    abstract HandlerThread getHandlerThread();
+
+    /**
+     * Calls into get*() functions in the health HAL. This reads into the kernel interfaces
+     * directly.
+     *
+     * @see IBatteryPropertiesRegistrar#getProperty
+     */
+    public abstract int getProperty(int id, BatteryProperty prop) throws RemoteException;
+
+    /**
+     * Calls update() in the health HAL.
+     *
+     * @see IBatteryPropertiesRegistrar#scheduleUpdate
+     */
+    public abstract void scheduleUpdate() throws RemoteException;
+
+    /**
+     * Calls into getHealthInfo() in the health HAL. This returns a cached value in the health HAL
+     * implementation.
+     *
+     * @return health info. {@code null} if no health HAL service. {@code null} if any
+     *     service-specific error when calling {@code getHealthInfo}, e.g. it is unsupported.
+     * @throws RemoteException for any transaction-level errors
+     */
+    // TODO(b/177269435): AIDL
+    public abstract android.hardware.health.V1_0.HealthInfo getHealthInfo() throws RemoteException;
+
+    /**
+     * Create a new HealthServiceWrapper instance.
+     *
+     * @param healthInfoCallback the callback to call when health info changes
+     * @return the new HealthServiceWrapper instance, which may be backed by HIDL or AIDL service.
+     * @throws RemoteException transaction errors
+     * @throws NoSuchElementException no HIDL or AIDL service is available
+     */
+    public static HealthServiceWrapper create(@Nullable HealthInfoCallback healthInfoCallback)
+            throws RemoteException, NoSuchElementException {
+        return create(
+                healthInfoCallback == null ? null : new HealthHalCallbackHidl(healthInfoCallback),
+                new HealthServiceWrapperHidl.IServiceManagerSupplier() {},
+                new HealthServiceWrapperHidl.IHealthSupplier() {});
+    }
+
+    /**
+     * Create a new HealthServiceWrapper instance for testing.
+     *
+     * @param hidlRegCallback callback for HIDL service registration, or {@code null} if the client
+     *     does not care about HIDL service registration notifications
+     * @param hidlServiceManagerSupplier supplier of HIDL service manager
+     * @param hidlHealthSupplier supplier of HIDL health HAL
+     * @return the new HealthServiceWrapper instance, which may be backed by HIDL or AIDL service.
+     */
+    @VisibleForTesting
+    static @NonNull HealthServiceWrapper create(
+            @Nullable HealthServiceWrapperHidl.Callback hidlRegCallback,
+            @NonNull HealthServiceWrapperHidl.IServiceManagerSupplier hidlServiceManagerSupplier,
+            @NonNull HealthServiceWrapperHidl.IHealthSupplier hidlHealthSupplier)
+            throws RemoteException, NoSuchElementException {
+        return new HealthServiceWrapperHidl(
+                hidlRegCallback, hidlServiceManagerSupplier, hidlHealthSupplier);
+    }
+}

services/core/java/com/android/server/health/HealthServiceWrapperHidl.java

@@ -0,0 +1,311 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * 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.
+ */
+
+package com.android.server.health;
+
+import android.annotation.NonNull;
+import android.annotation.Nullable;
+import android.hardware.health.V1_0.HealthInfo;
+import android.hardware.health.V2_0.IHealth;
+import android.hardware.health.V2_0.Result;
+import android.hidl.manager.V1_0.IServiceManager;
+import android.hidl.manager.V1_0.IServiceNotification;
+import android.os.BatteryManager;
+import android.os.BatteryProperty;
+import android.os.HandlerThread;
+import android.os.RemoteException;
+import android.os.Trace;
+import android.util.MutableInt;
+import android.util.Slog;
+
+import com.android.internal.annotations.VisibleForTesting;
+
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicReference;
+
+/**
+ * Implement {@link HealthServiceWrapper} backed by the HIDL HAL.
+ *
+ * @hide
+ */
+final class HealthServiceWrapperHidl extends HealthServiceWrapper {
+    private static final String TAG = "HealthServiceWrapperHidl";
+    public static final String INSTANCE_VENDOR = "default";
+
+    private final IServiceNotification mNotification = new Notification();
+    private final HandlerThread mHandlerThread = new HandlerThread("HealthServiceHwbinder");
+    // These variables are fixed after init.
+    private Callback mCallback;
+    private IHealthSupplier mHealthSupplier;
+    private String mInstanceName;
+
+    // Last IHealth service received.
+    private final AtomicReference<IHealth> mLastService = new AtomicReference<>();
+
+    private static void traceBegin(String name) {
+        Trace.traceBegin(Trace.TRACE_TAG_SYSTEM_SERVER, name);
+    }
+
+    private static void traceEnd() {
+        Trace.traceEnd(Trace.TRACE_TAG_SYSTEM_SERVER);
+    }
+
+    @Override
+    public int getProperty(int id, final BatteryProperty prop) throws RemoteException {
+        traceBegin("HealthGetProperty");
+        try {
+            IHealth service = mLastService.get();
+            if (service == null) throw new RemoteException("no health service");
+            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;
+        } finally {
+            traceEnd();
+        }
+    }
+
+    @Override
+    public void scheduleUpdate() throws RemoteException {
+        getHandlerThread()
+                .getThreadHandler()
+                .post(
+                        () -> {
+                            traceBegin("HealthScheduleUpdate");
+                            try {
+                                IHealth service = mLastService.get();
+                                if (service == null) {
+                                    Slog.e(TAG, "no health service");
+                                    return;
+                                }
+                                service.update();
+                            } catch (RemoteException ex) {
+                                Slog.e(TAG, "Cannot call update on health HAL", ex);
+                            } finally {
+                                traceEnd();
+                            }
+                        });
+    }
+
+    private static class Mutable<T> {
+        public T value;
+    }
+
+    @Override
+    public HealthInfo getHealthInfo() throws RemoteException {
+        IHealth service = mLastService.get();
+        if (service == null) return null;
+        final Mutable<HealthInfo> ret = new Mutable<>();
+        service.getHealthInfo(
+                (result, value) -> {
+                    if (result == Result.SUCCESS) {
+                        ret.value = value.legacy;
+                    }
+                });
+        return ret.value;
+    }
+
+    /**
+     * Start monitoring registration of new IHealth services. Only instance {@link #INSTANCE_VENDOR}
+     * and in device / framework manifest are used. This function should only be called once.
+     *
+     * <p>mCallback.onRegistration() is called synchronously (aka in init thread) before this method
+     * returns if callback is not null.
+     *
+     * @throws RemoteException transaction error when talking to IServiceManager
+     * @throws NoSuchElementException if one of the following cases: - No service manager; - {@link
+     *     #INSTANCE_VENDOR} is not in manifests (i.e. not available on this device), or none of
+     *     these instances are available to current process.
+     * @throws NullPointerException when supplier is null
+     */
+    @VisibleForTesting
+    HealthServiceWrapperHidl(
+            @Nullable Callback callback,
+            @NonNull IServiceManagerSupplier managerSupplier,
+            @NonNull IHealthSupplier healthSupplier)
+            throws RemoteException, NoSuchElementException, NullPointerException {
+        if (managerSupplier == null || healthSupplier == null) {
+            throw new NullPointerException();
+        }
+        mHealthSupplier = healthSupplier;
+
+        // Initialize mLastService and call callback for the first time (in init thread)
+        IHealth newService = null;
+        traceBegin("HealthInitGetService_" + INSTANCE_VENDOR);
+        try {
+            newService = healthSupplier.get(INSTANCE_VENDOR);
+        } catch (NoSuchElementException ex) {
+            /* ignored, handled below */
+        } finally {
+            traceEnd();
+        }
+        if (newService != null) {
+            mInstanceName = INSTANCE_VENDOR;
+            mLastService.set(newService);
+        }
+
+        if (mInstanceName == null || newService == null) {
+            throw new NoSuchElementException(
+                    String.format(
+                            "IHealth service instance %s isn't available. Perhaps no permission?",
+                            INSTANCE_VENDOR));
+        }
+
+        if (callback != null) {
+            mCallback = callback;
+            mCallback.onRegistration(null, newService, mInstanceName);
+        }
+
+        // Register for future service registrations
+        traceBegin("HealthInitRegisterNotification");
+        mHandlerThread.start();
+        try {
+            managerSupplier
+                    .get()
+                    .registerForNotifications(IHealth.kInterfaceName, mInstanceName, mNotification);
+        } finally {
+            traceEnd();
+        }
+        Slog.i(TAG, "health: HealthServiceWrapper listening to instance " + mInstanceName);
+    }
+
+    @VisibleForTesting
+    public HandlerThread getHandlerThread() {
+        return mHandlerThread;
+    }
+
+    /** Service registration callback. */
+    interface Callback {
+        /**
+         * This function is invoked asynchronously when a new and related IServiceNotification is
+         * received.
+         *
+         * @param service the recently retrieved service from IServiceManager. Can be a dead service
+         *     before service notification of a new service is delivered. Implementation must handle
+         *     cases for {@link RemoteException}s when calling into service.
+         * @param instance instance name.
+         */
+        void onRegistration(IHealth oldService, IHealth newService, String instance);
+    }
+
+    /**
+     * Supplier of services. Must not return null; throw {@link NoSuchElementException} if a service
+     * is not available.
+     */
+    interface IServiceManagerSupplier {
+        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 {
+        default IHealth get(String name) throws NoSuchElementException, RemoteException {
+            return IHealth.getService(name, true /* retry */);
+        }
+    }
+
+    private class Notification extends IServiceNotification.Stub {
+        @Override
+        public final void onRegistration(
+                String interfaceName, String instanceName, boolean preexisting) {
+            if (!IHealth.kInterfaceName.equals(interfaceName)) return;
+            if (!mInstanceName.equals(instanceName)) return;
+
+            // This runnable only runs on mHandlerThread and ordering is ensured, hence
+            // no locking is needed inside the runnable.
+            mHandlerThread
+                    .getThreadHandler()
+                    .post(
+                            new Runnable() {
+                                @Override
+                                public void run() {
+                                    try {
+                                        IHealth newService = mHealthSupplier.get(mInstanceName);
+                                        IHealth oldService = mLastService.getAndSet(newService);
+
+                                        // preexisting may be inaccurate (race). Check for equality
+                                        // here.
+                                        if (Objects.equals(newService, oldService)) return;
+
+                                        Slog.i(
+                                                TAG,
+                                                "health: new instance registered " + mInstanceName);
+                                        // #init() may be called with null callback. Skip null
+                                        // callbacks.
+                                        if (mCallback == null) return;
+                                        mCallback.onRegistration(
+                                                oldService, newService, mInstanceName);
+                                    } catch (NoSuchElementException | RemoteException ex) {
+                                        Slog.e(
+                                                TAG,
+                                                "health: Cannot get instance '"
+                                                        + mInstanceName
+                                                        + "': "
+                                                        + ex.getMessage()
+                                                        + ". Perhaps no permission?");
+                                    }
+                                }
+                            });
+        }
+    }
+}

services/core/java/com/android/server/health/Utils.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2021 The Android Open Source Project
+ *
+ * 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.
+ */
+
+package com.android.server.health;
+
+/**
+ * Utils for {@link om.android.server.BatteryService} to deal with health info structs.
+ *
+ * @hide
+ */
+public class Utils {
+    private Utils() {}
+
+    /**
+     * Copy health info struct.
+     *
+     * @param dst destination
+     * @param src source
+     */
+    public static void copy(
+            android.hardware.health.V1_0.HealthInfo dst,
+            android.hardware.health.V1_0.HealthInfo src) {
+        dst.chargerAcOnline = src.chargerAcOnline;
+        dst.chargerUsbOnline = src.chargerUsbOnline;
+        dst.chargerWirelessOnline = src.chargerWirelessOnline;
+        dst.maxChargingCurrent = src.maxChargingCurrent;
+        dst.maxChargingVoltage = src.maxChargingVoltage;
+        dst.batteryStatus = src.batteryStatus;
+        dst.batteryHealth = src.batteryHealth;
+        dst.batteryPresent = src.batteryPresent;
+        dst.batteryLevel = src.batteryLevel;
+        dst.batteryVoltage = src.batteryVoltage;
+        dst.batteryTemperature = src.batteryTemperature;
+        dst.batteryCurrent = src.batteryCurrent;
+        dst.batteryCycleCount = src.batteryCycleCount;
+        dst.batteryFullCharge = src.batteryFullCharge;
+        dst.batteryChargeCounter = src.batteryChargeCounter;
+        dst.batteryTechnology = src.batteryTechnology;
+    }
+}

services/core/java/com/android/server/stats/pull/StatsPullAtomService.java

@@ -92,7 +92,6 @@ import android.content.pm.UserInfo;
 import android.hardware.biometrics.BiometricsProtoEnums;
 import android.hardware.face.FaceManager;
 import android.hardware.fingerprint.FingerprintManager;
-import android.hardware.health.V2_0.IHealth;
 import android.net.ConnectivityManager;
 import android.net.INetworkStatsService;
 import android.net.INetworkStatsSession;
@@ -190,13 +189,13 @@ import com.android.internal.os.SystemServerCpuThreadReader.SystemServiceCpuThrea
 import com.android.internal.util.CollectionUtils;
 import com.android.internal.util.FrameworkStatsLog;
 import com.android.role.RoleManagerLocal;
-import com.android.server.BatteryService;
 import com.android.server.BinderCallsStatsService;
 import com.android.server.LocalManagerRegistry;
 import com.android.server.LocalServices;
 import com.android.server.SystemService;
 import com.android.server.SystemServiceManager;
 import com.android.server.am.MemoryStatUtil.MemoryStat;
+import com.android.server.health.HealthServiceWrapper;
 import com.android.server.notification.NotificationManagerService;
 import com.android.server.stats.pull.IonMemoryUtil.IonAllocations;
 import com.android.server.stats.pull.ProcfsMemoryUtil.MemorySnapshot;
@@ -226,6 +225,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.MissingResourceException;
+import java.util.NoSuchElementException;
 import java.util.Random;
 import java.util.Set;
 import java.util.UUID;
@@ -354,7 +354,7 @@ public class StatsPullAtomService extends SystemService {
     private File mBaseDir;
 
     @GuardedBy("mHealthHalLock")
-    private BatteryService.HealthServiceWrapper mHealthService;
+    private HealthServiceWrapper mHealthService;
 
     @Nullable
     @GuardedBy("mCpuTimePerThreadFreqLock")
@@ -799,10 +799,9 @@ public class StatsPullAtomService extends SystemService {
                 KernelCpuThreadReaderSettingsObserver.getSettingsModifiedReader(mContext);
 
         // Initialize HealthService
-        mHealthService = new BatteryService.HealthServiceWrapper();
         try {
-            mHealthService.init();
+            mHealthService = HealthServiceWrapper.create(null);
-        } catch (RemoteException e) {
+        } catch (RemoteException | NoSuchElementException e) {
             Slog.e(TAG, "failed to initialize healthHalWrapper");
         }
 
@@ -3975,38 +3974,40 @@ public class StatsPullAtomService extends SystemService {
     }
 
     int pullHealthHalLocked(int atomTag, List<StatsEvent> pulledData) {
-        IHealth healthService = mHealthService.getLastService();
+        if (mHealthService == null) {
-        if (healthService == null) {
             return StatsManager.PULL_SKIP;
         }
+        android.hardware.health.V1_0.HealthInfo healthInfo;
         try {
-            healthService.getHealthInfo((result, value) -> {
+            healthInfo = mHealthService.getHealthInfo();
-                int pulledValue;
-                switch(atomTag) {
-                    case FrameworkStatsLog.BATTERY_LEVEL:
-                        pulledValue = value.legacy.batteryLevel;
-                        break;
-                    case FrameworkStatsLog.REMAINING_BATTERY_CAPACITY:
-                        pulledValue = value.legacy.batteryChargeCounter;
-                        break;
-                    case FrameworkStatsLog.FULL_BATTERY_CAPACITY:
-                        pulledValue = value.legacy.batteryFullCharge;
-                        break;
-                    case FrameworkStatsLog.BATTERY_VOLTAGE:
-                        pulledValue = value.legacy.batteryVoltage;
-                        break;
-                    case FrameworkStatsLog.BATTERY_CYCLE_COUNT:
-                        pulledValue = value.legacy.batteryCycleCount;
-                        break;
-                    default:
-                        throw new IllegalStateException("Invalid atomTag in healthHal puller: "
-                                + atomTag);
-                }
-                pulledData.add(FrameworkStatsLog.buildStatsEvent(atomTag, pulledValue));
-            });
         } catch (RemoteException | IllegalStateException e) {
             return StatsManager.PULL_SKIP;
         }
+        if (healthInfo == null) {
+            return StatsManager.PULL_SKIP;
+        }
+
+        int pulledValue;
+        switch (atomTag) {
+            case FrameworkStatsLog.BATTERY_LEVEL:
+                pulledValue = healthInfo.batteryLevel;
+                break;
+            case FrameworkStatsLog.REMAINING_BATTERY_CAPACITY:
+                pulledValue = healthInfo.batteryChargeCounter;
+                break;
+            case FrameworkStatsLog.FULL_BATTERY_CAPACITY:
+                pulledValue = healthInfo.batteryFullCharge;
+                break;
+            case FrameworkStatsLog.BATTERY_VOLTAGE:
+                pulledValue = healthInfo.batteryVoltage;
+                break;
+            case FrameworkStatsLog.BATTERY_CYCLE_COUNT:
+                pulledValue = healthInfo.batteryCycleCount;
+                break;
+            default:
+                return StatsManager.PULL_SKIP;
+        }
+        pulledData.add(FrameworkStatsLog.buildStatsEvent(atomTag, pulledValue));
         return StatsManager.PULL_SUCCESS;
     }
 

services/tests/servicestests/src/com/android/server/health/HealthServiceWrapperTest.java

@@ -14,19 +14,25 @@
  * limitations under the License.
  */
 
-package com.android.server;
+package com.android.server.health;
 
-import static junit.framework.Assert.*;
+import static junit.framework.Assert.assertFalse;
+import static junit.framework.Assert.fail;
 
 import static org.mockito.Mockito.*;
 
 import android.hardware.health.V2_0.IHealth;
 import android.hidl.manager.V1_0.IServiceManager;
 import android.hidl.manager.V1_0.IServiceNotification;
-import android.test.AndroidTestCase;
+import android.os.RemoteException;
 
 import androidx.test.filters.SmallTest;
+import androidx.test.runner.AndroidJUnit4;
 
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
 import org.mockito.ArgumentMatcher;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
@@ -36,36 +42,39 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.NoSuchElementException;
 
-public class BatteryServiceTest extends AndroidTestCase {
+@RunWith(AndroidJUnit4.class)
+public class HealthServiceWrapperTest {
 
     @Mock IServiceManager mMockedManager;
     @Mock IHealth mMockedHal;
     @Mock IHealth mMockedHal2;
 
-    @Mock BatteryService.HealthServiceWrapper.Callback mCallback;
+    @Mock HealthServiceWrapperHidl.Callback mCallback;
-    @Mock BatteryService.HealthServiceWrapper.IServiceManagerSupplier mManagerSupplier;
+    @Mock HealthServiceWrapperHidl.IServiceManagerSupplier mManagerSupplier;
-    @Mock BatteryService.HealthServiceWrapper.IHealthSupplier mHealthServiceSupplier;
+    @Mock HealthServiceWrapperHidl.IHealthSupplier mHealthServiceSupplier;
-    BatteryService.HealthServiceWrapper mWrapper;
+    HealthServiceWrapper mWrapper;
 
-    private static final String VENDOR = BatteryService.HealthServiceWrapper.INSTANCE_VENDOR;
+    private static final String VENDOR = HealthServiceWrapperHidl.INSTANCE_VENDOR;
 
-    @Override
+    @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
     }
 
-    @Override
+    @After
     public void tearDown() {
-        if (mWrapper != null)
+        if (mWrapper != null) mWrapper.getHandlerThread().quitSafely();
-            mWrapper.getHandlerThread().quitSafely();
     }
 
     public static <T> ArgumentMatcher<T> isOneOf(Collection<T> collection) {
         return new ArgumentMatcher<T>() {
-            @Override public boolean matches(T e) {
+            @Override
+            public boolean matches(T e) {
                 return collection.contains(e);
             }
-            @Override public String toString() {
+
+            @Override
+            public String toString() {
                 return collection.toString();
             }
         };
@@ -73,27 +82,29 @@ public class BatteryServiceTest extends AndroidTestCase {
 
     private void initForInstances(String... instanceNamesArr) throws Exception {
         final Collection<String> instanceNames = Arrays.asList(instanceNamesArr);
-        doAnswer((invocation) -> {
+        doAnswer(
-                // technically, preexisting is ignored by
+                (invocation) -> {
-                // BatteryService.HealthServiceWrapper.Notification, but still call it correctly.
+                    // technically, preexisting is ignored by
-                sendNotification(invocation, true);
+                    // HealthServiceWrapperHidl.Notification, but still call it correctly.
-                sendNotification(invocation, true);
+                    sendNotification(invocation, true);
-                sendNotification(invocation, false);
+                    sendNotification(invocation, true);
-                return null;
+                    sendNotification(invocation, false);
-            }).when(mMockedManager).registerForNotifications(
+                    return null;
-                eq(IHealth.kInterfaceName),
+                })
-                argThat(isOneOf(instanceNames)),
+                .when(mMockedManager)
-                any(IServiceNotification.class));
+                .registerForNotifications(
+                        eq(IHealth.kInterfaceName),
+                        argThat(isOneOf(instanceNames)),
+                        any(IServiceNotification.class));
 
         doReturn(mMockedManager).when(mManagerSupplier).get();
-        doReturn(mMockedHal)        // init calls this
+        doReturn(mMockedHal) // init calls this
-            .doReturn(mMockedHal)   // notification 1
+                .doReturn(mMockedHal) // notification 1
-            .doReturn(mMockedHal)   // notification 2
+                .doReturn(mMockedHal) // notification 2
-            .doReturn(mMockedHal2)  // notification 3
+                .doReturn(mMockedHal2) // notification 3
-            .doThrow(new RuntimeException("Should not call getService for more than 4 times"))
+                .doThrow(new RuntimeException("Should not call getService for more than 4 times"))
-            .when(mHealthServiceSupplier).get(argThat(isOneOf(instanceNames)));
+                .when(mHealthServiceSupplier)
-
+                .get(argThat(isOneOf(instanceNames)));
-        mWrapper = new BatteryService.HealthServiceWrapper();
     }
 
     private void waitHandlerThreadFinish() throws Exception {
@@ -108,16 +119,20 @@ public class BatteryServiceTest extends AndroidTestCase {
 
     private static void sendNotification(InvocationOnMock invocation, boolean preexisting)
             throws Exception {
-        ((IServiceNotification)invocation.getArguments()[2]).onRegistration(
+        ((IServiceNotification) invocation.getArguments()[2])
-                IHealth.kInterfaceName,
+                .onRegistration(
-                (String)invocation.getArguments()[1],
+                        IHealth.kInterfaceName, (String) invocation.getArguments()[1], preexisting);
-                preexisting);
+    }
+
+    private void createWrapper() throws RemoteException {
+        mWrapper = HealthServiceWrapper.create(mCallback, mManagerSupplier, mHealthServiceSupplier);
     }
 
     @SmallTest
+    @Test
     public void testWrapPreferVendor() throws Exception {
         initForInstances(VENDOR);
-        mWrapper.init(mCallback, mManagerSupplier, mHealthServiceSupplier);
+        createWrapper();
         waitHandlerThreadFinish();
         verify(mCallback, times(1)).onRegistration(same(null), same(mMockedHal), eq(VENDOR));
         verify(mCallback, never()).onRegistration(same(mMockedHal), same(mMockedHal), anyString());
@@ -125,10 +140,11 @@ public class BatteryServiceTest extends AndroidTestCase {
     }
 
     @SmallTest
+    @Test
     public void testNoService() throws Exception {
         initForInstances("unrelated");
         try {
-            mWrapper.init(mCallback, mManagerSupplier, mHealthServiceSupplier);
+            createWrapper();
             fail("Expect NoSuchElementException");
         } catch (NoSuchElementException ex) {
             // expected