diff options
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 c120fc7d82f5..6aed00e0e32b 100644 --- a/services/core/java/com/android/server/vibrator/VibratorController.java +++ b/services/core/java/com/android/server/vibrator/VibratorController.java @@ -57,8 +57,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; /** @@ -75,6 +74,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()); } @@ -87,6 +91,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, @@ -106,7 +111,7 @@ final class VibratorController { return false; } // Notify its callback after new client registered. - notifyStateListener(listener, mIsVibrating); + notifyStateListener(listener, isVibrating(mCurrentState)); } return true; } finally { @@ -166,7 +171,7 @@ final class VibratorController { * automatically notified to any registered {@link IVibratorStateListener} on change. */ public boolean isVibrating() { - return mIsVibrating; + return isVibrating(mCurrentState); } /** @@ -184,11 +189,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. * @@ -214,7 +214,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_TAG_VIBRATOR, @@ -224,9 +224,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_TAG_VIBRATOR); @@ -264,7 +266,7 @@ final class VibratorController { if (mVibratorInfo.hasCapability(IVibrator.CAP_AMPLITUDE_CONTROL)) { mNativeWrapper.setAmplitude(amplitude); } - if (mIsVibrating) { + if (mCurrentState == VibratorState.VIBRATING) { mCurrentAmplitude = amplitude; } } @@ -289,7 +291,7 @@ final class VibratorController { long duration = mNativeWrapper.on(milliseconds, vibrationId); if (duration > 0) { mCurrentAmplitude = -1; - notifyListenerOnVibrating(true); + updateStateAndNotifyListenersLocked(VibratorState.VIBRATING); } return duration; } @@ -319,7 +321,7 @@ final class VibratorController { vendorEffect.getAdaptiveScale(), vibrationId); if (duration > 0) { mCurrentAmplitude = -1; - notifyListenerOnVibrating(true); + updateStateAndNotifyListenersLocked(VibratorState.VIBRATING); } return duration; } finally { @@ -346,7 +348,7 @@ final class VibratorController { prebaked.getEffectStrength(), vibrationId); if (duration > 0) { mCurrentAmplitude = -1; - notifyListenerOnVibrating(true); + updateStateAndNotifyListenersLocked(VibratorState.VIBRATING); } return duration; } @@ -374,7 +376,7 @@ final class VibratorController { long duration = mNativeWrapper.compose(primitives, vibrationId); if (duration > 0) { mCurrentAmplitude = -1; - notifyListenerOnVibrating(true); + updateStateAndNotifyListenersLocked(VibratorState.VIBRATING); } return duration; } @@ -402,7 +404,7 @@ final class VibratorController { long duration = mNativeWrapper.composePwle(primitives, braking, vibrationId); if (duration > 0) { mCurrentAmplitude = -1; - notifyListenerOnVibrating(true); + updateStateAndNotifyListenersLocked(VibratorState.VIBRATING); } return duration; } @@ -422,7 +424,7 @@ final class VibratorController { synchronized (mLock) { mNativeWrapper.off(); mCurrentAmplitude = 0; - notifyListenerOnVibrating(false); + updateStateAndNotifyListenersLocked(VibratorState.IDLE); } } finally { Trace.traceEnd(TRACE_TAG_VIBRATOR); @@ -443,9 +445,8 @@ final class VibratorController { return "VibratorController{" + "mVibratorInfo=" + mVibratorInfo + ", mVibratorInfoLoadSuccessful=" + mVibratorInfoLoadSuccessful - + ", mIsVibrating=" + mIsVibrating + + ", mCurrentState=" + mCurrentState.name() + ", mCurrentAmplitude=" + mCurrentAmplitude - + ", mIsUnderExternalControl=" + mIsUnderExternalControl + ", mVibratorStateListeners count=" + mVibratorStateListeners.getRegisteredCallbackCount() + '}'; @@ -454,8 +455,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 = " @@ -464,14 +464,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)); } } @@ -483,6 +488,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 95c648334327..07473d10b217 100644 --- a/services/core/java/com/android/server/vibrator/VibratorManagerService.java +++ b/services/core/java/com/android/server/vibrator/VibratorManagerService.java @@ -809,17 +809,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()); } |