From 892a58f4f9313da1196a85fd09f1f4700ecd0ee4 Mon Sep 17 00:00:00 2001 From: Lais Andrade Date: Tue, 23 Feb 2021 13:29:13 +0000 Subject: [PATCH] Use compose duration returned from HAL Use the estimated duration for a composed effect returned by the vibrator HAL in VibratorController, instead of estimating with a fixed primitive duration. Fix: 177807015 Test: VibratorControllerTest Change-Id: Iedb757cbbc7b5167a7ddf4c020956a12722c367b --- .../server/vibrator/VibratorController.java | 18 +++++++----------- ...roid_server_vibrator_VibratorController.cpp | 11 ++++++----- .../FakeVibratorControllerProvider.java | 7 ++++--- .../vibrator/VibratorControllerTest.java | 6 ++++-- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/services/core/java/com/android/server/vibrator/VibratorController.java b/services/core/java/com/android/server/vibrator/VibratorController.java index 95f6059c482e3..eaba083c551cc 100644 --- a/services/core/java/com/android/server/vibrator/VibratorController.java +++ b/services/core/java/com/android/server/vibrator/VibratorController.java @@ -87,8 +87,8 @@ final class VibratorController { static native long vibratorPerformEffect( long nativePtr, long effect, long strength, long vibrationId); - static native void vibratorPerformComposedEffect(long nativePtr, - VibrationEffect.Composition.PrimitiveEffect[] effect, long vibrationId); + static native long vibratorPerformComposedEffect( + long nativePtr, VibrationEffect.Composition.PrimitiveEffect[] effect, long vibrationId); static native void vibratorSetExternalControl(long nativePtr, boolean enabled); @@ -269,13 +269,9 @@ final class VibratorController { VibrationEffect.Composition.PrimitiveEffect[] primitives = effect.getPrimitiveEffects().toArray( new VibrationEffect.Composition.PrimitiveEffect[0]); - mNativeWrapper.compose(primitives, vibrationId); - notifyVibratorOnLocked(); - // Compose don't actually give us an estimated duration, so we just guess here. - long duration = 0; - for (VibrationEffect.Composition.PrimitiveEffect primitive : primitives) { - // TODO(b/177807015): use exposed durations from IVibrator here instead - duration += 20 + primitive.delay; + long duration = mNativeWrapper.compose(primitives, vibrationId); + if (duration > 0) { + notifyVibratorOnLocked(); } return duration; } @@ -393,9 +389,9 @@ final class VibratorController { } /** Turns vibrator on to perform one of the supported composed effects. */ - public void compose( + public long compose( VibrationEffect.Composition.PrimitiveEffect[] effect, long vibrationId) { - VibratorController.vibratorPerformComposedEffect(mNativePtr, effect, + return VibratorController.vibratorPerformComposedEffect(mNativePtr, effect, vibrationId); } diff --git a/services/core/jni/com_android_server_vibrator_VibratorController.cpp b/services/core/jni/com_android_server_vibrator_VibratorController.cpp index 89b931d35c40e..ef2d0baff031d 100644 --- a/services/core/jni/com_android_server_vibrator_VibratorController.cpp +++ b/services/core/jni/com_android_server_vibrator_VibratorController.cpp @@ -238,12 +238,12 @@ static jlong vibratorPerformEffect(JNIEnv* env, jclass /* clazz */, jlong ptr, j return result.isOk() ? result.value().count() : -1; } -static void vibratorPerformComposedEffect(JNIEnv* env, jclass /* clazz */, jlong ptr, - jobjectArray composition, jlong vibrationId) { +static jlong vibratorPerformComposedEffect(JNIEnv* env, jclass /* clazz */, jlong ptr, + jobjectArray composition, jlong vibrationId) { VibratorControllerWrapper* wrapper = reinterpret_cast(ptr); if (wrapper == nullptr) { ALOGE("vibratorPerformComposedEffect failed because native wrapper was not initialized"); - return; + return -1; } size_t size = env->GetArrayLength(composition); std::vector effects; @@ -252,7 +252,8 @@ static void vibratorPerformComposedEffect(JNIEnv* env, jclass /* clazz */, jlong effects.push_back(effectFromJavaPrimitive(env, element)); } auto callback = wrapper->createCallback(vibrationId); - wrapper->hal()->performComposedEffect(effects, callback); + auto result = wrapper->hal()->performComposedEffect(effects, callback); + return result.isOk() ? result.value().count() : -1; } static jlong vibratorGetCapabilities(JNIEnv* env, jclass /* clazz */, jlong ptr) { @@ -296,7 +297,7 @@ static const JNINativeMethod method_table[] = { {"vibratorSetAmplitude", "(JI)V", (void*)vibratorSetAmplitude}, {"vibratorPerformEffect", "(JJJJ)J", (void*)vibratorPerformEffect}, {"vibratorPerformComposedEffect", - "(J[Landroid/os/VibrationEffect$Composition$PrimitiveEffect;J)V", + "(J[Landroid/os/VibrationEffect$Composition$PrimitiveEffect;J)J", (void*)vibratorPerformComposedEffect}, {"vibratorGetSupportedEffects", "(J)[I", (void*)vibratorGetSupportedEffects}, {"vibratorGetSupportedPrimitives", "(J)[I", (void*)vibratorGetSupportedPrimitives}, diff --git a/services/tests/servicestests/src/com/android/server/vibrator/FakeVibratorControllerProvider.java b/services/tests/servicestests/src/com/android/server/vibrator/FakeVibratorControllerProvider.java index 4634e12f1530c..2a3c2c46ce4ee 100644 --- a/services/tests/servicestests/src/com/android/server/vibrator/FakeVibratorControllerProvider.java +++ b/services/tests/servicestests/src/com/android/server/vibrator/FakeVibratorControllerProvider.java @@ -100,16 +100,17 @@ final class FakeVibratorControllerProvider { return EFFECT_DURATION; } - public void compose(VibrationEffect.Composition.PrimitiveEffect[] effect, + public long compose(VibrationEffect.Composition.PrimitiveEffect[] effect, long vibrationId) { VibrationEffect.Composed composed = new VibrationEffect.Composed(Arrays.asList(effect)); mEffects.add(composed); applyLatency(); - long duration = EFFECT_DURATION * effect.length; + long duration = 0; for (VibrationEffect.Composition.PrimitiveEffect e : effect) { - duration += e.delay; + duration += EFFECT_DURATION + e.delay; } scheduleListener(duration, vibrationId); + return duration; } public void setExternalControl(boolean enabled) { diff --git a/services/tests/servicestests/src/com/android/server/vibrator/VibratorControllerTest.java b/services/tests/servicestests/src/com/android/server/vibrator/VibratorControllerTest.java index 815aa8ee66aef..bad3e4c2ed923 100644 --- a/services/tests/servicestests/src/com/android/server/vibrator/VibratorControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/vibrator/VibratorControllerTest.java @@ -19,6 +19,7 @@ package com.android.server.vibrator; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.eq; @@ -202,7 +203,7 @@ public class VibratorControllerTest { VibrationEffect.Prebaked effect = (VibrationEffect.Prebaked) VibrationEffect.createPredefined(VibrationEffect.EFFECT_CLICK); - controller.on(effect, 11); + assertEquals(10L, controller.on(effect, 11)); assertTrue(controller.isVibrating()); verify(mNativeWrapperMock).perform(eq((long) VibrationEffect.EFFECT_CLICK), @@ -212,13 +213,14 @@ public class VibratorControllerTest { @Test public void on_withComposed_performsEffect() { mockVibratorCapabilities(IVibrator.CAP_COMPOSE_EFFECTS); + when(mNativeWrapperMock.compose(any(), anyLong())).thenReturn(15L); VibratorController controller = createController(); VibrationEffect.Composed effect = (VibrationEffect.Composed) VibrationEffect.startComposition() .addPrimitive(VibrationEffect.Composition.PRIMITIVE_CLICK, 0.5f, 10) .compose(); - controller.on(effect, 12); + assertEquals(15L, controller.on(effect, 12)); ArgumentCaptor primitivesCaptor = ArgumentCaptor.forClass(VibrationEffect.Composition.PrimitiveEffect[].class);