Merge "Fix VibrationThread with repeating waveforms of positive amplitudes" into sc-dev

2 files changed, 111 insertions, 7 deletions

diff --git a/services/core/java/com/android/server/vibrator/VibrationThread.java b/services/core/java/com/android/server/vibrator/VibrationThread.java
index 03863723a5e9..dd4e260c6d91 100644
--- a/services/core/java/com/android/server/vibrator/VibrationThread.java
+++ b/services/core/java/com/android/server/vibrator/VibrationThread.java
@@ -1294,21 +1294,32 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
         @Override
         public boolean shouldPlayWhenVibratorComplete(int vibratorId) {
             if (controller.getVibratorInfo().getId() == vibratorId) {
+                mVibratorCallbackReceived = true;
                 mNextOffTime = SystemClock.uptimeMillis();
             }
-            // Timings are tightly controlled here, so never anticipate when vibrator is complete.
-            return false;
+            // Timings are tightly controlled here, so only anticipate if the vibrator was supposed
+            // to be ON but has completed prematurely, to turn it back on as soon as possible.
+            return mNextOffTime < startTime && controller.getCurrentAmplitude() > 0;
         }
 
         @Override
         public List<Step> play() {
             Trace.traceBegin(Trace.TRACE_TAG_VIBRATOR, "AmplitudeStep");
             try {
+                long now = SystemClock.uptimeMillis();
+                long latency = now - startTime;
                 if (DEBUG) {
-                    long latency = SystemClock.uptimeMillis() - startTime;
                     Slog.d(TAG, "Running amplitude step with " + latency + "ms latency.");
                 }
 
+                if (mVibratorCallbackReceived && latency < 0) {
+                    // This step was anticipated because the vibrator turned off prematurely.
+                    // Turn it back on and return this same step to run at the exact right time.
+                    mNextOffTime = turnVibratorBackOn(/* remainingDuration= */ -latency);
+                    return Arrays.asList(new AmplitudeStep(startTime, controller, effect,
+                            segmentIndex, mNextOffTime));
+                }
+
                 VibrationEffectSegment segment = effect.getSegments().get(segmentIndex);
                 if (!(segment instanceof StepSegment)) {
                     Slog.w(TAG, "Ignoring wrong segment for a AmplitudeStep: " + segment);
@@ -1321,17 +1332,16 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
                     return skipToNextSteps(/* segmentsSkipped= */ 1);
                 }
 
-                long now = SystemClock.uptimeMillis();
                 float amplitude = stepSegment.getAmplitude();
                 if (amplitude == 0) {
-                    if (mNextOffTime > now) {
+                    if (vibratorOffTimeout > now) {
                         // Amplitude cannot be set to zero, so stop the vibrator.
                         stopVibrating();
                         mNextOffTime = now;
                     }
                 } else {
                     if (startTime >= mNextOffTime) {
-                        // Vibrator has stopped. Turn vibrator back on for the duration of another
+                        // Vibrator is OFF. Turn vibrator back on for the duration of another
                         // cycle before setting the amplitude.
                         long onDuration = getVibratorOnDuration(effect, segmentIndex);
                         if (onDuration > 0) {
@@ -1350,6 +1360,22 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
             }
         }
 
+        private long turnVibratorBackOn(long remainingDuration) {
+            long onDuration = getVibratorOnDuration(effect, segmentIndex);
+            if (onDuration <= 0) {
+                // Vibrator is supposed to go back off when this step starts, so just leave it off.
+                return vibratorOffTimeout;
+            }
+            onDuration += remainingDuration;
+            float expectedAmplitude = controller.getCurrentAmplitude();
+            mVibratorOnResult = startVibrating(onDuration);
+            if (mVibratorOnResult > 0) {
+                // Set the amplitude back to the value it was supposed to be playing at.
+                changeAmplitude(expectedAmplitude);
+            }
+            return SystemClock.uptimeMillis() + onDuration + CALLBACKS_EXTRA_TIMEOUT;
+        }
+
         private long startVibrating(long duration) {
             if (DEBUG) {
                 Slog.d(TAG, "Turning on vibrator " + controller.getVibratorInfo().getId() + " for "
@@ -1383,7 +1409,10 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
                     repeatIndex = -1;
                 }
                 if (i == startIndex) {
-                    return 1000;
+                    // The repeating waveform keeps the vibrator ON all the time. Use a minimum
+                    // of 1s duration to prevent short patterns from turning the vibrator ON too
+                    // frequently.
+                    return Math.max(timing, 1000);
                 }
             }
             if (i == segmentCount && effect.getRepeatIndex() < 0) {
diff --git a/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java b/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java
index 1596483cdbe1..2e5c24c1a95c 100644
--- a/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java
+++ b/services/tests/servicestests/src/com/android/server/vibrator/VibrationThreadTest.java
@@ -246,6 +246,81 @@ public class VibrationThreadTest {
     }
 
     @Test
+    public void vibrate_singleVibratorRepeatingShortAlwaysOnWaveform_turnsVibratorOnForASecond()
+            throws Exception {
+        FakeVibratorControllerProvider fakeVibrator = mVibratorProviders.get(VIBRATOR_ID);
+        fakeVibrator.setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL);
+
+        long vibrationId = 1;
+        int[] amplitudes = new int[]{1, 2, 3};
+        VibrationEffect effect = VibrationEffect.createWaveform(
+                new long[]{1, 10, 100}, amplitudes, 0);
+        VibrationThread thread = startThreadAndDispatcher(vibrationId, effect);
+
+        assertTrue(waitUntil(t -> !fakeVibrator.getAmplitudes().isEmpty(), thread,
+                TEST_TIMEOUT_MILLIS));
+        thread.cancel();
+        waitForCompletion(thread);
+
+        verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED);
+        assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating());
+        assertEquals(Arrays.asList(expectedOneShot(1000)), fakeVibrator.getEffectSegments());
+    }
+
+    @Test
+    public void vibrate_singleVibratorRepeatingLongAlwaysOnWaveform_turnsVibratorOnForACycle()
+            throws Exception {
+        FakeVibratorControllerProvider fakeVibrator = mVibratorProviders.get(VIBRATOR_ID);
+        fakeVibrator.setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL);
+
+        long vibrationId = 1;
+        int[] amplitudes = new int[]{1, 2, 3};
+        VibrationEffect effect = VibrationEffect.createWaveform(
+                new long[]{5000, 500, 50}, amplitudes, 0);
+        VibrationThread thread = startThreadAndDispatcher(vibrationId, effect);
+
+        assertTrue(waitUntil(t -> !fakeVibrator.getAmplitudes().isEmpty(), thread,
+                TEST_TIMEOUT_MILLIS));
+        thread.cancel();
+        waitForCompletion(thread);
+
+        verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED);
+        assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating());
+        assertEquals(Arrays.asList(expectedOneShot(5550)), fakeVibrator.getEffectSegments());
+    }
+
+
+    @Test
+    public void vibrate_singleVibratorRepeatingAlwaysOnWaveform_turnsVibratorBackOn()
+            throws Exception {
+        FakeVibratorControllerProvider fakeVibrator = mVibratorProviders.get(VIBRATOR_ID);
+        fakeVibrator.setCapabilities(IVibrator.CAP_AMPLITUDE_CONTROL);
+
+        long vibrationId = 1;
+        int[] amplitudes = new int[]{1, 2};
+        VibrationEffect effect = VibrationEffect.createWaveform(
+                new long[]{900, 50}, amplitudes, 0);
+        VibrationThread thread = startThreadAndDispatcher(vibrationId, effect);
+
+        assertTrue(waitUntil(t -> fakeVibrator.getAmplitudes().size() > 2 * amplitudes.length,
+                thread, 1000 + TEST_TIMEOUT_MILLIS));
+        thread.cancel();
+        waitForCompletion(thread);
+
+        verifyCallbacksTriggered(vibrationId, Vibration.Status.CANCELLED);
+        assertFalse(thread.getVibrators().get(VIBRATOR_ID).isVibrating());
+        assertEquals(2, fakeVibrator.getEffectSegments().size());
+        // First time turn vibrator ON for minimum of 1s.
+        assertEquals(1000L, fakeVibrator.getEffectSegments().get(0).getDuration());
+        // Vibrator turns off in the middle of the second execution of first step, turn it back ON
+        // for another 1s + remaining of 850ms.
+        assertEquals(1850, fakeVibrator.getEffectSegments().get(1).getDuration(), /* delta= */ 20);
+        // Set amplitudes for a cycle {1, 2}, start second loop then turn it back on to same value.
+        assertEquals(expectedAmplitudes(1, 2, 1, 1),
+                mVibratorProviders.get(VIBRATOR_ID).getAmplitudes().subList(0, 4));
+    }
+
+    @Test
     public void vibrate_singleVibratorPredefinedCancel_cancelsVibrationImmediately()
             throws Exception {
         mVibratorProviders.get(VIBRATOR_ID).setCapabilities(IVibrator.CAP_COMPOSE_EFFECTS);