Fix vibration finished before HAL callback
Update TurnOffVibratorStep so it will be considered part of the main
vibration steps when it's waiting for the HAL completion callback.
This keeps the ramp down steps outside the main vibration duration,
but it makes sure the vibration will be completed after a callback is
received, even if delayed compared to the expected duration.
Also fix the VibratorManagerService command line to wait for the
vibration ramp down and clean up before ending the command, to prevent
vibrations from being cancelled because of binder death.
Fix: 327608323
Test: atest VibrationThreadTest
Change-Id: I9cf0e1263f73b719de16782a85df87f740c9c5d0
diff --git a/services/core/java/com/android/server/vibrator/CompleteEffectVibratorStep.java b/services/core/java/com/android/server/vibrator/CompleteEffectVibratorStep.java
index fb5140d..48dd992 100644
--- a/services/core/java/com/android/server/vibrator/CompleteEffectVibratorStep.java
+++ b/services/core/java/com/android/server/vibrator/CompleteEffectVibratorStep.java
@@ -51,8 +51,8 @@
public List<Step> cancel() {
if (mCancelled) {
// Double cancelling will just turn off the vibrator right away.
- return Arrays.asList(
- new TurnOffVibratorStep(conductor, SystemClock.uptimeMillis(), controller));
+ return Arrays.asList(new TurnOffVibratorStep(conductor, SystemClock.uptimeMillis(),
+ controller, /* isCleanUp= */ true));
}
return super.cancel();
}
@@ -92,8 +92,8 @@
} else {
// Vibration is completing normally, turn off after the deadline in case we
// don't receive the callback in time (callback also triggers it right away).
- return Arrays.asList(new TurnOffVibratorStep(
- conductor, mPendingVibratorOffDeadline, controller));
+ return Arrays.asList(new TurnOffVibratorStep(conductor,
+ mPendingVibratorOffDeadline, controller, /* isCleanUp= */ false));
}
}
diff --git a/services/core/java/com/android/server/vibrator/RampOffVibratorStep.java b/services/core/java/com/android/server/vibrator/RampOffVibratorStep.java
index 84da9f2..f40c994 100644
--- a/services/core/java/com/android/server/vibrator/RampOffVibratorStep.java
+++ b/services/core/java/com/android/server/vibrator/RampOffVibratorStep.java
@@ -44,8 +44,8 @@
@Override
public List<Step> cancel() {
- return Arrays.asList(
- new TurnOffVibratorStep(conductor, SystemClock.uptimeMillis(), controller));
+ return Arrays.asList(new TurnOffVibratorStep(conductor, SystemClock.uptimeMillis(),
+ controller, /* isCleanUp= */ true));
}
@Override
@@ -71,8 +71,8 @@
// Vibrator amplitude cannot go further down, just turn it off with the configured
// deadline that has been adjusted for the scenario when this was triggered by a
// cancelled vibration.
- return Arrays.asList(new TurnOffVibratorStep(
- conductor, mPendingVibratorOffDeadline, controller));
+ return Arrays.asList(new TurnOffVibratorStep(conductor, mPendingVibratorOffDeadline,
+ controller, /* isCleanUp= */ true));
}
return Arrays.asList(new RampOffVibratorStep(
conductor,
diff --git a/services/core/java/com/android/server/vibrator/TurnOffVibratorStep.java b/services/core/java/com/android/server/vibrator/TurnOffVibratorStep.java
index 297ef56..065ce11 100644
--- a/services/core/java/com/android/server/vibrator/TurnOffVibratorStep.java
+++ b/services/core/java/com/android/server/vibrator/TurnOffVibratorStep.java
@@ -32,20 +32,23 @@
*/
final class TurnOffVibratorStep extends AbstractVibratorStep {
+ private final boolean mIsCleanUp;
+
TurnOffVibratorStep(VibrationStepConductor conductor, long startTime,
- VibratorController controller) {
+ VibratorController controller, boolean isCleanUp) {
super(conductor, startTime, controller, /* effect= */ null, /* index= */ -1, startTime);
+ mIsCleanUp = isCleanUp;
}
@Override
public boolean isCleanUp() {
- return true;
+ return mIsCleanUp;
}
@Override
public List<Step> cancel() {
- return Arrays.asList(
- new TurnOffVibratorStep(conductor, SystemClock.uptimeMillis(), controller));
+ return Arrays.asList(new TurnOffVibratorStep(conductor, SystemClock.uptimeMillis(),
+ controller, /* isCleanUp= */ true));
}
@Override
diff --git a/services/core/java/com/android/server/vibrator/VibratorManagerService.java b/services/core/java/com/android/server/vibrator/VibratorManagerService.java
index 78e0ebb..5652387 100644
--- a/services/core/java/com/android/server/vibrator/VibratorManagerService.java
+++ b/services/core/java/com/android/server/vibrator/VibratorManagerService.java
@@ -16,7 +16,6 @@
package com.android.server.vibrator;
-import static android.os.ExternalVibrationScale.ScaleLevel.SCALE_MUTE;
import static android.os.VibrationEffect.VibrationParameter.targetAmplitude;
import static android.os.VibrationEffect.VibrationParameter.targetFrequency;
@@ -2146,6 +2145,7 @@
/** Provide limited functionality from {@link VibratorManagerService} as shell commands. */
private final class VibratorManagerShellCommand extends ShellCommand {
public static final String SHELL_PACKAGE_NAME = "com.android.shell";
+ public static final long VIBRATION_END_TIMEOUT_MS = 500; // Clean up shouldn't be too long.
private final class CommonOptions {
public boolean force = false;
@@ -2521,6 +2521,9 @@
// Waits for the client vibration to finish, but the VibrationThread may still
// do cleanup after this.
vib.waitForEnd();
+ // Wait for vibration clean up and possible ramp down before ending.
+ mVibrationThread.waitForThreadIdle(
+ mVibrationSettings.getRampDownDuration() + VIBRATION_END_TIMEOUT_MS);
} catch (InterruptedException e) {
}
}
diff --git a/services/tests/vibrator/src/com/android/server/vibrator/VibrationThreadTest.java b/services/tests/vibrator/src/com/android/server/vibrator/VibrationThreadTest.java
index 6e478d8..79ca14e 100644
--- a/services/tests/vibrator/src/com/android/server/vibrator/VibrationThreadTest.java
+++ b/services/tests/vibrator/src/com/android/server/vibrator/VibrationThreadTest.java
@@ -87,6 +87,8 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
import java.util.function.BooleanSupplier;
import java.util.stream.Collectors;
@@ -1243,6 +1245,112 @@
assertEquals(expectedAmplitudes(6), mVibratorProviders.get(3).getAmplitudes());
}
+ @Test
+ public void vibrate_withRampDown_vibrationFinishedAfterDurationAndBeforeRampDown()
+ throws Exception {
+ int expectedDuration = 100;
+ int rampDownDuration = 200;
+
+ when(mVibrationConfigMock.getRampDownDurationMs()).thenReturn(rampDownDuration);
+ mVibratorProviders.get(VIBRATOR_ID).setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL);
+
+ HalVibration vibration = createVibration(
+ CombinedVibration.createParallel(
+ VibrationEffect.createOneShot(
+ expectedDuration, VibrationEffect.DEFAULT_AMPLITUDE)));
+ CountDownLatch vibrationCompleteLatch = new CountDownLatch(1);
+ doAnswer(unused -> {
+ vibrationCompleteLatch.countDown();
+ return null;
+ }).when(mManagerHooks).onVibrationCompleted(eq(vibration.id), any());
+
+ startThreadAndDispatcher(vibration);
+ long startTime = SystemClock.elapsedRealtime();
+
+ assertTrue(vibrationCompleteLatch.await(expectedDuration + TEST_TIMEOUT_MILLIS,
+ TimeUnit.MILLISECONDS));
+ long vibrationEndTime = SystemClock.elapsedRealtime();
+
+ waitForCompletion(rampDownDuration + TEST_TIMEOUT_MILLIS);
+ long completionTime = SystemClock.elapsedRealtime();
+
+ verify(mControllerCallbacks).onComplete(VIBRATOR_ID, vibration.id);
+ // Vibration ends after duration, thread completed after ramp down
+ assertThat(vibrationEndTime - startTime).isAtLeast(expectedDuration);
+ assertThat(vibrationEndTime - startTime).isLessThan(expectedDuration + rampDownDuration);
+ assertThat(completionTime - startTime).isAtLeast(expectedDuration + rampDownDuration);
+ }
+
+ @Test
+ public void vibrate_withVibratorCallbackDelayShorterThanTimeout_vibrationFinishedAfterDelay()
+ throws Exception {
+ long expectedDuration = 10;
+ long callbackDelay = VibrationStepConductor.CALLBACKS_EXTRA_TIMEOUT / 2;
+
+ mVibratorProviders.get(VIBRATOR_ID).setCompletionCallbackDelay(callbackDelay);
+ mVibratorProviders.get(VIBRATOR_ID).setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL);
+
+ HalVibration vibration = createVibration(
+ CombinedVibration.createParallel(
+ VibrationEffect.createOneShot(
+ expectedDuration, VibrationEffect.DEFAULT_AMPLITUDE)));
+ CountDownLatch vibrationCompleteLatch = new CountDownLatch(1);
+ doAnswer(unused -> {
+ vibrationCompleteLatch.countDown();
+ return null;
+ }).when(mManagerHooks).onVibrationCompleted(eq(vibration.id), any());
+
+ startThreadAndDispatcher(vibration);
+ long startTime = SystemClock.elapsedRealtime();
+
+ assertTrue(vibrationCompleteLatch.await(callbackDelay + TEST_TIMEOUT_MILLIS,
+ TimeUnit.MILLISECONDS));
+ long vibrationEndTime = SystemClock.elapsedRealtime();
+
+ waitForCompletion(TEST_TIMEOUT_MILLIS);
+
+ verify(mControllerCallbacks).onComplete(VIBRATOR_ID, vibration.id);
+ assertThat(vibrationEndTime - startTime).isAtLeast(expectedDuration + callbackDelay);
+ }
+
+ @LargeTest
+ @Test
+ public void vibrate_withVibratorCallbackDelayLongerThanTimeout_vibrationFinishedAfterTimeout()
+ throws Exception {
+ long expectedDuration = 10;
+ long callbackTimeout = VibrationStepConductor.CALLBACKS_EXTRA_TIMEOUT;
+ long callbackDelay = callbackTimeout * 2;
+
+ mVibratorProviders.get(VIBRATOR_ID).setCompletionCallbackDelay(callbackDelay);
+ mVibratorProviders.get(VIBRATOR_ID).setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL);
+
+ HalVibration vibration = createVibration(
+ CombinedVibration.createParallel(
+ VibrationEffect.createOneShot(
+ expectedDuration, VibrationEffect.DEFAULT_AMPLITUDE)));
+ CountDownLatch vibrationCompleteLatch = new CountDownLatch(1);
+ doAnswer(unused -> {
+ vibrationCompleteLatch.countDown();
+ return null;
+ }).when(mManagerHooks).onVibrationCompleted(eq(vibration.id), any());
+
+ startThreadAndDispatcher(vibration);
+ long startTime = SystemClock.elapsedRealtime();
+
+ assertTrue(vibrationCompleteLatch.await(callbackTimeout + TEST_TIMEOUT_MILLIS,
+ TimeUnit.MILLISECONDS));
+ long vibrationEndTime = SystemClock.elapsedRealtime();
+
+ waitForCompletion(callbackDelay + TEST_TIMEOUT_MILLIS);
+ long completionTime = SystemClock.elapsedRealtime();
+
+ verify(mControllerCallbacks, never()).onComplete(VIBRATOR_ID, vibration.id);
+ // Vibration ends and thread completes after timeout, before the HAL callback
+ assertThat(vibrationEndTime - startTime).isAtLeast(expectedDuration + callbackTimeout);
+ assertThat(vibrationEndTime - startTime).isLessThan(expectedDuration + callbackDelay);
+ assertThat(completionTime - startTime).isLessThan(expectedDuration + callbackDelay);
+ }
+
@LargeTest
@Test
public void vibrate_withWaveform_totalVibrationTimeRespected() {
diff --git a/services/tests/vibrator/utils/com/android/server/vibrator/FakeVibratorControllerProvider.java b/services/tests/vibrator/utils/com/android/server/vibrator/FakeVibratorControllerProvider.java
index 12815fa..2ddb47b 100644
--- a/services/tests/vibrator/utils/com/android/server/vibrator/FakeVibratorControllerProvider.java
+++ b/services/tests/vibrator/utils/com/android/server/vibrator/FakeVibratorControllerProvider.java
@@ -53,6 +53,7 @@
private boolean mIsAvailable = true;
private boolean mIsInfoLoadSuccessful = true;
+ private long mCompletionCallbackDelay;
private long mOnLatency;
private long mOffLatency;
private int mOffCount;
@@ -206,7 +207,7 @@
private void scheduleListener(long vibrationDuration, long vibrationId) {
mHandler.postDelayed(() -> listener.onComplete(vibratorId, vibrationId),
- vibrationDuration);
+ vibrationDuration + mCompletionCallbackDelay);
}
}
@@ -241,6 +242,13 @@
}
/**
+ * Sets the delay this controller should fake for triggering the vibration completed callback.
+ */
+ public void setCompletionCallbackDelay(long millis) {
+ mCompletionCallbackDelay = millis;
+ }
+
+ /**
* Sets the latency this controller should fake for turning the vibrator hardware on or setting
* the vibration amplitude.
*/