Report audio-coupled haptics in Vibrator.isVibrating

Update VibratorController to set the isVibrating flag to true when the
vibrator enters external control. Starting from that point the audio
framework is free to send data to the vibrator motor directly, so the
service should reflect this state.

This will affect both the Vibrator.isVibrating and
OnVibratorStateChangedListener APIs.

Fix: 348589406
Flag: EXEMPT small bug fix
Test: VibratorControllerTest
Change-Id: I376c6c5773a3d5303bebe0a6e1c48121bf4e8ce8

5 files changed, 88 insertions, 51 deletions

diff --git a/core/proto/android/server/vibrator/vibratormanagerservice.proto b/core/proto/android/server/vibrator/vibratormanagerservice.proto
index e7f0560612cc..258832e3e7ff 100644
--- a/core/proto/android/server/vibrator/vibratormanagerservice.proto
+++ b/core/proto/android/server/vibrator/vibratormanagerservice.proto
@@ -157,10 +157,8 @@ message VibratorManagerServiceDumpProto {
     option (.android.msg_privacy).dest = DEST_AUTOMATIC;
     repeated int32 vibrator_ids = 1;
     optional VibrationProto current_vibration = 2;
-    optional bool is_vibrating = 3;
     optional int32 is_vibrator_controller_registered = 27;
     optional VibrationProto current_external_vibration = 4;
-    optional bool vibrator_under_external_control = 5;
     optional bool low_power_mode = 6;
     optional bool vibrate_on = 24;
     reserved 25; // prev keyboard_vibration_on
@@ -183,4 +181,6 @@ message VibratorManagerServiceDumpProto {
     repeated VibrationProto previous_vibrations = 16;
     repeated VibrationParamProto previous_vibration_params = 28;
     reserved 17; // prev previous_external_vibrations
+    reserved 3; // prev is_vibrating, check current_vibration instead
+    reserved 5; // prev vibrator_under_external_control, check current_external_vibration instead
 }
\ No newline at end of file
diff --git a/services/core/java/com/android/server/vibrator/VibratorController.java b/services/core/java/com/android/server/vibrator/VibratorController.java
index 3c478500876f..13f5009f245d 100644
--- a/services/core/java/com/android/server/vibrator/VibratorController.java
+++ b/services/core/java/com/android/server/vibrator/VibratorController.java
@@ -55,8 +55,7 @@ final class VibratorController {
     // for a snippet of the current known vibrator state/info.
     private volatile VibratorInfo mVibratorInfo;
     private volatile boolean mVibratorInfoLoadSuccessful;
-    private volatile boolean mIsVibrating;
-    private volatile boolean mIsUnderExternalControl;
+    private volatile VibratorState mCurrentState;
     private volatile float mCurrentAmplitude;
 
     /**
@@ -73,6 +72,11 @@ final class VibratorController {
         void onComplete(int vibratorId, long vibrationId);
     }
 
+    /** Representation of the vibrator state based on the interactions through this controller. */
+    private enum VibratorState {
+        IDLE, VIBRATING, UNDER_EXTERNAL_CONTROL
+    }
+
     VibratorController(int vibratorId, OnVibrationCompleteListener listener) {
         this(vibratorId, listener, new NativeWrapper());
     }
@@ -85,6 +89,7 @@ final class VibratorController {
         VibratorInfo.Builder vibratorInfoBuilder = new VibratorInfo.Builder(vibratorId);
         mVibratorInfoLoadSuccessful = mNativeWrapper.getInfo(vibratorInfoBuilder);
         mVibratorInfo = vibratorInfoBuilder.build();
+        mCurrentState = VibratorState.IDLE;
 
         if (!mVibratorInfoLoadSuccessful) {
             Slog.e(TAG,
@@ -104,7 +109,7 @@ final class VibratorController {
                     return false;
                 }
                 // Notify its callback after new client registered.
-                notifyStateListener(listener, mIsVibrating);
+                notifyStateListener(listener, isVibrating(mCurrentState));
             }
             return true;
         } finally {
@@ -164,7 +169,7 @@ final class VibratorController {
      * automatically notified to any registered {@link IVibratorStateListener} on change.
      */
     public boolean isVibrating() {
-        return mIsVibrating;
+        return isVibrating(mCurrentState);
     }
 
     /**
@@ -182,11 +187,6 @@ final class VibratorController {
         return mCurrentAmplitude;
     }
 
-    /** Return {@code true} if this vibrator is under external control, false otherwise. */
-    public boolean isUnderExternalControl() {
-        return mIsUnderExternalControl;
-    }
-
     /**
      * Check against this vibrator capabilities.
      *
@@ -212,7 +212,7 @@ final class VibratorController {
     /**
      * Set the vibrator control to be external or not, based on given flag.
      *
-     * <p>This will affect the state of {@link #isUnderExternalControl()}.
+     * <p>This will affect the state of {@link #isVibrating()}.
      */
     public void setExternalControl(boolean externalControl) {
         Trace.traceBegin(Trace.TRACE_TAG_VIBRATOR, "setExternalControl(" + externalControl + ")");
@@ -220,9 +220,11 @@ final class VibratorController {
             if (!mVibratorInfo.hasCapability(IVibrator.CAP_EXTERNAL_CONTROL)) {
                 return;
             }
+            VibratorState newState =
+                    externalControl ? VibratorState.UNDER_EXTERNAL_CONTROL : VibratorState.IDLE;
             synchronized (mLock) {
-                mIsUnderExternalControl = externalControl;
                 mNativeWrapper.setExternalControl(externalControl);
+                updateStateAndNotifyListenersLocked(newState);
             }
         } finally {
             Trace.traceEnd(Trace.TRACE_TAG_VIBRATOR);
@@ -260,7 +262,7 @@ final class VibratorController {
                 if (mVibratorInfo.hasCapability(IVibrator.CAP_AMPLITUDE_CONTROL)) {
                     mNativeWrapper.setAmplitude(amplitude);
                 }
-                if (mIsVibrating) {
+                if (mCurrentState == VibratorState.VIBRATING) {
                     mCurrentAmplitude = amplitude;
                 }
             }
@@ -285,7 +287,7 @@ final class VibratorController {
                 long duration = mNativeWrapper.on(milliseconds, vibrationId);
                 if (duration > 0) {
                     mCurrentAmplitude = -1;
-                    notifyListenerOnVibrating(true);
+                    updateStateAndNotifyListenersLocked(VibratorState.VIBRATING);
                 }
                 return duration;
             }
@@ -315,7 +317,7 @@ final class VibratorController {
                         vendorEffect.getAdaptiveScale(), vibrationId);
                 if (duration > 0) {
                     mCurrentAmplitude = -1;
-                    notifyListenerOnVibrating(true);
+                    updateStateAndNotifyListenersLocked(VibratorState.VIBRATING);
                 }
                 return duration;
             } finally {
@@ -342,7 +344,7 @@ final class VibratorController {
                         prebaked.getEffectStrength(), vibrationId);
                 if (duration > 0) {
                     mCurrentAmplitude = -1;
-                    notifyListenerOnVibrating(true);
+                    updateStateAndNotifyListenersLocked(VibratorState.VIBRATING);
                 }
                 return duration;
             }
@@ -370,7 +372,7 @@ final class VibratorController {
                 long duration = mNativeWrapper.compose(primitives, vibrationId);
                 if (duration > 0) {
                     mCurrentAmplitude = -1;
-                    notifyListenerOnVibrating(true);
+                    updateStateAndNotifyListenersLocked(VibratorState.VIBRATING);
                 }
                 return duration;
             }
@@ -398,7 +400,7 @@ final class VibratorController {
                 long duration = mNativeWrapper.composePwle(primitives, braking, vibrationId);
                 if (duration > 0) {
                     mCurrentAmplitude = -1;
-                    notifyListenerOnVibrating(true);
+                    updateStateAndNotifyListenersLocked(VibratorState.VIBRATING);
                 }
                 return duration;
             }
@@ -418,7 +420,7 @@ final class VibratorController {
             synchronized (mLock) {
                 mNativeWrapper.off();
                 mCurrentAmplitude = 0;
-                notifyListenerOnVibrating(false);
+                updateStateAndNotifyListenersLocked(VibratorState.IDLE);
             }
         } finally {
             Trace.traceEnd(Trace.TRACE_TAG_VIBRATOR);
@@ -439,9 +441,8 @@ final class VibratorController {
         return "VibratorController{"
                 + "mVibratorInfo=" + mVibratorInfo
                 + ", mVibratorInfoLoadSuccessful=" + mVibratorInfoLoadSuccessful
-                + ", mIsVibrating=" + mIsVibrating
+                + ", mCurrentState=" + mCurrentState.name()
                 + ", mCurrentAmplitude=" + mCurrentAmplitude
-                + ", mIsUnderExternalControl=" + mIsUnderExternalControl
                 + ", mVibratorStateListeners count="
                 + mVibratorStateListeners.getRegisteredCallbackCount()
                 + '}';
@@ -450,8 +451,7 @@ final class VibratorController {
     void dump(IndentingPrintWriter pw) {
         pw.println("Vibrator (id=" + mVibratorInfo.getId() + "):");
         pw.increaseIndent();
-        pw.println("isVibrating = " + mIsVibrating);
-        pw.println("isUnderExternalControl = " + mIsUnderExternalControl);
+        pw.println("currentState = " + mCurrentState.name());
         pw.println("currentAmplitude = " + mCurrentAmplitude);
         pw.println("vibratorInfoLoadSuccessful = " + mVibratorInfoLoadSuccessful);
         pw.println("vibratorStateListener size = "
@@ -460,14 +460,19 @@ final class VibratorController {
         pw.decreaseIndent();
     }
 
+    /**
+     * Updates current vibrator state and notify listeners if {@link #isVibrating()} result changed.
+     */
     @GuardedBy("mLock")
-    private void notifyListenerOnVibrating(boolean isVibrating) {
-        if (mIsVibrating != isVibrating) {
-            mIsVibrating = isVibrating;
+    private void updateStateAndNotifyListenersLocked(VibratorState state) {
+        boolean previousIsVibrating = isVibrating(mCurrentState);
+        final boolean newIsVibrating = isVibrating(state);
+        mCurrentState = state;
+        if (previousIsVibrating != newIsVibrating) {
             // The broadcast method is safe w.r.t. register/unregister listener methods, but lock
             // is required here to guarantee delivery order.
             mVibratorStateListeners.broadcast(
-                    listener -> notifyStateListener(listener, isVibrating));
+                    listener -> notifyStateListener(listener, newIsVibrating));
         }
     }
 
@@ -479,6 +484,11 @@ final class VibratorController {
         }
     }
 
+    /** Returns true only if given state is not {@link VibratorState#IDLE}. */
+    private static boolean isVibrating(VibratorState state) {
+        return state != VibratorState.IDLE;
+    }
+
     /** Wrapper around the static-native methods of {@link VibratorController} for tests. */
     @VisibleForTesting
     public static class NativeWrapper {
diff --git a/services/core/java/com/android/server/vibrator/VibratorManagerService.java b/services/core/java/com/android/server/vibrator/VibratorManagerService.java
index bd17e5ddd49e..3bc9162a6c5f 100644
--- a/services/core/java/com/android/server/vibrator/VibratorManagerService.java
+++ b/services/core/java/com/android/server/vibrator/VibratorManagerService.java
@@ -813,17 +813,9 @@ public class VibratorManagerService extends IVibratorManagerService.Stub {
                 mCurrentExternalVibration.getDebugInfo().dump(proto,
                         VibratorManagerServiceDumpProto.CURRENT_EXTERNAL_VIBRATION);
             }
-
-            boolean isVibrating = false;
-            boolean isUnderExternalControl = false;
             for (int i = 0; i < mVibrators.size(); i++) {
                 proto.write(VibratorManagerServiceDumpProto.VIBRATOR_IDS, mVibrators.keyAt(i));
-                isVibrating |= mVibrators.valueAt(i).isVibrating();
-                isUnderExternalControl |= mVibrators.valueAt(i).isUnderExternalControl();
             }
-            proto.write(VibratorManagerServiceDumpProto.IS_VIBRATING, isVibrating);
-            proto.write(VibratorManagerServiceDumpProto.VIBRATOR_UNDER_EXTERNAL_CONTROL,
-                    isUnderExternalControl);
         }
         mVibratorManagerRecords.dump(proto);
         mVibratorControlService.dump(proto);
diff --git a/services/tests/vibrator/src/com/android/server/vibrator/VibratorControllerTest.java b/services/tests/vibrator/src/com/android/server/vibrator/VibratorControllerTest.java
index 0d13be6d5ab2..e8ca8bf8ec63 100644
--- a/services/tests/vibrator/src/com/android/server/vibrator/VibratorControllerTest.java
+++ b/services/tests/vibrator/src/com/android/server/vibrator/VibratorControllerTest.java
@@ -127,13 +127,13 @@ public class VibratorControllerTest {
     public void setExternalControl_withCapability_enablesExternalControl() {
         mockVibratorCapabilities(IVibrator.CAP_EXTERNAL_CONTROL);
         VibratorController controller = createController();
-        assertFalse(controller.isUnderExternalControl());
+        assertFalse(controller.isVibrating());
 
         controller.setExternalControl(true);
-        assertTrue(controller.isUnderExternalControl());
+        assertTrue(controller.isVibrating());
 
         controller.setExternalControl(false);
-        assertFalse(controller.isUnderExternalControl());
+        assertFalse(controller.isVibrating());
 
         InOrder inOrderVerifier = inOrder(mNativeWrapperMock);
         inOrderVerifier.verify(mNativeWrapperMock).setExternalControl(eq(true));
@@ -143,10 +143,10 @@ public class VibratorControllerTest {
     @Test
     public void setExternalControl_withNoCapability_ignoresExternalControl() {
         VibratorController controller = createController();
-        assertFalse(controller.isUnderExternalControl());
+        assertFalse(controller.isVibrating());
 
         controller.setExternalControl(true);
-        assertFalse(controller.isUnderExternalControl());
+        assertFalse(controller.isVibrating());
 
         verify(mNativeWrapperMock, never()).setExternalControl(anyBoolean());
     }
@@ -181,6 +181,38 @@ public class VibratorControllerTest {
     }
 
     @Test
+    public void setAmplitude_vibratorIdle_ignoresAmplitude() {
+        VibratorController controller = createController();
+        assertFalse(controller.isVibrating());
+
+        controller.setAmplitude(1);
+        assertEquals(0, controller.getCurrentAmplitude(), /* delta= */ 0);
+    }
+
+    @Test
+    public void setAmplitude_vibratorUnderExternalControl_ignoresAmplitude() {
+        mockVibratorCapabilities(IVibrator.CAP_EXTERNAL_CONTROL);
+        VibratorController controller = createController();
+        controller.setExternalControl(true);
+        assertTrue(controller.isVibrating());
+
+        controller.setAmplitude(1);
+        assertEquals(0, controller.getCurrentAmplitude(), /* delta= */ 0);
+    }
+
+    @Test
+    public void setAmplitude_vibratorVibrating_setsAmplitude() {
+        when(mNativeWrapperMock.on(anyLong(), anyLong())).thenAnswer(args -> args.getArgument(0));
+        VibratorController controller = createController();
+        controller.on(100, /* vibrationId= */ 1);
+        assertTrue(controller.isVibrating());
+        assertEquals(-1, controller.getCurrentAmplitude(), /* delta= */ 0);
+
+        controller.setAmplitude(1);
+        assertEquals(1, controller.getCurrentAmplitude(), /* delta= */ 0);
+    }
+
+    @Test
     public void on_withDuration_turnsVibratorOn() {
         when(mNativeWrapperMock.on(anyLong(), anyLong())).thenAnswer(args -> args.getArgument(0));
         VibratorController controller = createController();
diff --git a/services/tests/vibrator/src/com/android/server/vibrator/VibratorManagerServiceTest.java b/services/tests/vibrator/src/com/android/server/vibrator/VibratorManagerServiceTest.java
index d99b20c689dd..538c3fc2ddae 100644
--- a/services/tests/vibrator/src/com/android/server/vibrator/VibratorManagerServiceTest.java
+++ b/services/tests/vibrator/src/com/android/server/vibrator/VibratorManagerServiceTest.java
@@ -266,12 +266,13 @@ public class VibratorManagerServiceTest {
     @After
     public void tearDown() throws Exception {
         if (mService != null) {
-            if (!mPendingVibrations.stream().allMatch(HalVibration::hasEnded)) {
-                // Cancel any pending vibration from tests.
-                cancelVibrate(mService);
-                for (HalVibration vibration : mPendingVibrations) {
-                    vibration.waitForEnd();
-                }
+            // Make sure we have permission to cancel test vibrations, even if the test denied them.
+            grantPermission(android.Manifest.permission.VIBRATE);
+            // Cancel any pending vibration from tests, including external vibrations.
+            cancelVibrate(mService);
+            // Wait until pending vibrations end asynchronously.
+            for (HalVibration vibration : mPendingVibrations) {
+                vibration.waitForEnd();
             }
             // Wait until all vibrators have stopped vibrating, waiting for ramp-down.
             // Note: if a test is flaky here something is wrong with the vibration finalization.
@@ -2242,7 +2243,7 @@ public class VibratorManagerServiceTest {
         VibratorManagerService service = createSystemReadyService();
 
         VibrationEffect effect = VibrationEffect.createOneShot(10 * TEST_TIMEOUT_MILLIS, 100);
-        vibrate(service, effect, HAPTIC_FEEDBACK_ATTRS);
+        HalVibration vibration = vibrate(service, effect, HAPTIC_FEEDBACK_ATTRS);
 
         // VibrationThread will start this vibration async, so wait until vibration is triggered.
         assertTrue(waitUntil(s -> s.isVibrating(1), service, TEST_TIMEOUT_MILLIS));
@@ -2255,7 +2256,8 @@ public class VibratorManagerServiceTest {
         assertNotEquals(ExternalVibrationScale.ScaleLevel.SCALE_MUTE, scale.scaleLevel);
 
         // Vibration is cancelled.
-        assertTrue(waitUntil(s -> !s.isVibrating(1), service, TEST_TIMEOUT_MILLIS));
+        vibration.waitForEnd();
+        assertThat(vibration.getStatus()).isEqualTo(Status.CANCELLED_SUPERSEDED);
         assertEquals(Arrays.asList(false, true),
                 mVibratorProviders.get(1).getExternalControlStates());
     }
@@ -2296,7 +2298,7 @@ public class VibratorManagerServiceTest {
 
         VibrationEffect repeatingEffect = VibrationEffect.createWaveform(
                 new long[]{100, 200, 300}, new int[]{128, 255, 255}, 1);
-        vibrate(service, repeatingEffect, ALARM_ATTRS);
+        HalVibration repeatingVibration = vibrate(service, repeatingEffect, ALARM_ATTRS);
 
         // VibrationThread will start this vibration async, so wait until vibration is triggered.
         assertTrue(waitUntil(s -> s.isVibrating(1), service, TEST_TIMEOUT_MILLIS));
@@ -2308,7 +2310,8 @@ public class VibratorManagerServiceTest {
         assertNotEquals(ExternalVibrationScale.ScaleLevel.SCALE_MUTE, scale.scaleLevel);
 
         // Vibration is cancelled.
-        assertTrue(waitUntil(s -> !s.isVibrating(1), service, TEST_TIMEOUT_MILLIS));
+        repeatingVibration.waitForEnd();
+        assertThat(repeatingVibration.getStatus()).isEqualTo(Status.CANCELLED_SUPERSEDED);
         assertEquals(Arrays.asList(false, true),
                 mVibratorProviders.get(1).getExternalControlStates());
     }