Merge "Fix vibration finished before HAL callback" into main
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 af494a3..9e9025e 100644
--- a/services/core/java/com/android/server/vibrator/VibratorManagerService.java
+++ b/services/core/java/com/android/server/vibrator/VibratorManagerService.java
@@ -2097,6 +2097,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;
@@ -2472,6 +2473,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 55fc03e..0b76154 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;
@@ -1246,6 +1248,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.
*/