Fix VibrationThread with repeating waveforms of positive amplitudes

The VibrationThread is using a fixed duration of 1s to turn on the
vibrator for a repeating waveform that has all amplitudes positive.

This does not work if steps are longer than that, as the vibration will
stop prematurely and the thread will wait until the next step to turn
the vibrator back on, causing a noticible break.

Fix: 193206875
Test: VibrationThreadTest
Change-Id: I7ac3e397d588ac44dde5430f8fea852a283b4ae8

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);