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
diff --git a/services/core/java/com/android/server/vibrator/VibrationThread.java b/services/core/java/com/android/server/vibrator/VibrationThread.java
index 0386372..dd4e260 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 @@
         @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 @@
                     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 @@
             }
         }
 
+        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 @@
                     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 1596483c..2e5c24c 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 @@
     }
 
     @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);