Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 173774c9 authored by Lais Andrade's avatar Lais Andrade
Browse files

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
parent 19954aa1
Loading
Loading
Loading
Loading
+4 −4
Original line number Diff line number Diff line
@@ -51,8 +51,8 @@ final class CompleteEffectVibratorStep extends AbstractVibratorStep {
    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 @@ final class CompleteEffectVibratorStep extends AbstractVibratorStep {
                } 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));
                }
            }

+4 −4
Original line number Diff line number Diff line
@@ -44,8 +44,8 @@ final class RampOffVibratorStep extends AbstractVibratorStep {

    @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 @@ final class RampOffVibratorStep extends AbstractVibratorStep {
                // 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,
+7 −4
Original line number Diff line number Diff line
@@ -32,20 +32,23 @@ import java.util.List;
 */
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
+4 −1
Original line number Diff line number Diff line
@@ -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 @@ public class VibratorManagerService extends IVibratorManagerService.Stub {
    /** 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 @@ public class VibratorManagerService extends IVibratorManagerService.Stub {
                    // 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) {
                }
            }
+108 −0
Original line number Diff line number Diff line
@@ -87,6 +87,8 @@ import java.util.HashMap;
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 @@ public class VibrationThreadTest {
        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() {
Loading