Change VibratorInfo reload logic in VibratorManagerService

The VibratorController.getInfo method is reloading the VibratorInfo on
every call when at least one of the read calls to the vibrator HAL has
failed. This can force a device to enter a request loop when the HAL has
a broken getter implementation, and also has no control on how
frequently it requests HAL data.

This update will reload the VibratorInfo only once from
VibratorManagerService.systemReady, to give the controller a chance to
fetch the HAL data before the service becomes available to the system.
After this point that info (including defaults for failed fields) will
be considered the device vibrator profile and can be cached by client
calls and apps.

This is also adding more logs on failing to load primitive durations, to
help finding the root cause for the failed cts.

Bug: 195595741
Test: VibratorControllerTest and VibratorManagerServiceTest
Change-Id: Idd9457300c49d7e240f4d9a6326fdbe995e7089b
This commit is contained in:
Lais Andrade
2021-10-07 15:08:44 +01:00
parent 21223c9bb8
commit 39b2dab19d
8 changed files with 149 additions and 34 deletions

View File

@@ -51,6 +51,7 @@ public class SystemVibrator extends Vibrator {
mRegisteredListeners = new ArrayMap<>();
private final Object mLock = new Object();
@GuardedBy("mLock")
private AllVibratorsInfo mVibratorInfo;
@UnsupportedAppUsage
@@ -73,7 +74,15 @@ public class SystemVibrator extends Vibrator {
int[] vibratorIds = mVibratorManager.getVibratorIds();
VibratorInfo[] vibratorInfos = new VibratorInfo[vibratorIds.length];
for (int i = 0; i < vibratorIds.length; i++) {
vibratorInfos[i] = mVibratorManager.getVibrator(vibratorIds[i]).getInfo();
Vibrator vibrator = mVibratorManager.getVibrator(vibratorIds[i]);
if (vibrator instanceof NullVibrator) {
Log.w(TAG, "Vibrator manager service not ready; "
+ "Info not yet available for vibrator: " + vibratorIds[i]);
// This should never happen after the vibrator manager service is ready.
// Skip caching this vibrator until then.
return VibratorInfo.EMPTY_VIBRATOR_INFO;
}
vibratorInfos[i] = vibrator.getInfo();
}
return mVibratorInfo = new AllVibratorsInfo(vibratorInfos);
}

View File

@@ -165,7 +165,11 @@ public abstract class Vibrator {
return ctx != null ? ctx.getResources().getFloat(resId) : defaultValue;
}
/** @hide */
/**
* Get the info describing this vibrator.
*
* @hide
*/
protected VibratorInfo getInfo() {
return VibratorInfo.EMPTY_VIBRATOR_INFO;
}

View File

@@ -461,7 +461,8 @@ public class VibratorInfo implements Parcelable {
int supportedPrimitivesCount = mSupportedPrimitives.size();
String[] names = new String[supportedPrimitivesCount];
for (int i = 0; i < supportedPrimitivesCount; i++) {
names[i] = VibrationEffect.Composition.primitiveToString(mSupportedPrimitives.keyAt(i));
names[i] = VibrationEffect.Composition.primitiveToString(mSupportedPrimitives.keyAt(i))
+ "(" + mSupportedPrimitives.valueAt(i) + "ms)";
}
return names;
}

View File

@@ -41,12 +41,11 @@ final class VibratorController {
private final Object mLock = new Object();
private final NativeWrapper mNativeWrapper;
private final VibratorInfo.Builder mVibratorInfoBuilder;
@GuardedBy("mLock")
private VibratorInfo mVibratorInfo;
@GuardedBy("mLock")
private boolean mVibratorInfoLoaded;
private boolean mVibratorInfoLoadSuccessful;
@GuardedBy("mLock")
private final RemoteCallbackList<IVibratorStateListener> mVibratorStateListeners =
new RemoteCallbackList<>();
@@ -73,10 +72,16 @@ final class VibratorController {
NativeWrapper nativeWrapper) {
mNativeWrapper = nativeWrapper;
mNativeWrapper.init(vibratorId, listener);
mVibratorInfoBuilder = new VibratorInfo.Builder(vibratorId);
mVibratorInfoLoaded = mNativeWrapper.getInfo(SUGGESTED_FREQUENCY_SAFE_RANGE,
mVibratorInfoBuilder);
mVibratorInfo = mVibratorInfoBuilder.build();
VibratorInfo.Builder vibratorInfoBuilder = new VibratorInfo.Builder(vibratorId);
mVibratorInfoLoadSuccessful = mNativeWrapper.getInfo(SUGGESTED_FREQUENCY_SAFE_RANGE,
vibratorInfoBuilder);
mVibratorInfo = vibratorInfoBuilder.build();
if (!mVibratorInfoLoadSuccessful) {
Slog.e(TAG,
"Vibrator controller initialization failed to load some HAL info for vibrator "
+ vibratorId);
}
}
/** Register state listener for this vibrator. */
@@ -108,15 +113,33 @@ final class VibratorController {
}
}
/** Reruns the query to the vibrator to load the {@link VibratorInfo}, if not yet successful. */
public void reloadVibratorInfoIfNeeded() {
synchronized (mLock) {
if (mVibratorInfoLoadSuccessful) {
return;
}
int vibratorId = mVibratorInfo.getId();
VibratorInfo.Builder vibratorInfoBuilder = new VibratorInfo.Builder(vibratorId);
mVibratorInfoLoadSuccessful = mNativeWrapper.getInfo(SUGGESTED_FREQUENCY_SAFE_RANGE,
vibratorInfoBuilder);
mVibratorInfo = vibratorInfoBuilder.build();
if (!mVibratorInfoLoadSuccessful) {
Slog.e(TAG, "Failed retry of HAL getInfo for vibrator " + vibratorId);
}
}
}
/** Checks if the {@link VibratorInfo} was loaded from the vibrator hardware successfully. */
boolean isVibratorInfoLoadSuccessful() {
synchronized (mLock) {
return mVibratorInfoLoadSuccessful;
}
}
/** Return the {@link VibratorInfo} representing the vibrator controlled by this instance. */
public VibratorInfo getVibratorInfo() {
synchronized (mLock) {
if (!mVibratorInfoLoaded) {
// Try to load the vibrator metadata that has failed in the last attempt.
mVibratorInfoLoaded = mNativeWrapper.getInfo(SUGGESTED_FREQUENCY_SAFE_RANGE,
mVibratorInfoBuilder);
mVibratorInfo = mVibratorInfoBuilder.build();
}
return mVibratorInfo;
}
}
@@ -164,7 +187,9 @@ final class VibratorController {
* @return true if this vibrator has this capability, false otherwise
*/
public boolean hasCapability(long capability) {
return mVibratorInfo.hasCapability(capability);
synchronized (mLock) {
return mVibratorInfo.hasCapability(capability);
}
}
/** Return {@code true} if the underlying vibrator is currently available, false otherwise. */
@@ -178,10 +203,10 @@ final class VibratorController {
* <p>This will affect the state of {@link #isUnderExternalControl()}.
*/
public void setExternalControl(boolean externalControl) {
if (!mVibratorInfo.hasCapability(IVibrator.CAP_EXTERNAL_CONTROL)) {
return;
}
synchronized (mLock) {
if (!mVibratorInfo.hasCapability(IVibrator.CAP_EXTERNAL_CONTROL)) {
return;
}
mIsUnderExternalControl = externalControl;
mNativeWrapper.setExternalControl(externalControl);
}
@@ -192,10 +217,10 @@ final class VibratorController {
* if given {@code effect} is {@code null}.
*/
public void updateAlwaysOn(int id, @Nullable PrebakedSegment prebaked) {
if (!mVibratorInfo.hasCapability(IVibrator.CAP_ALWAYS_ON_CONTROL)) {
return;
}
synchronized (mLock) {
if (!mVibratorInfo.hasCapability(IVibrator.CAP_ALWAYS_ON_CONTROL)) {
return;
}
if (prebaked == null) {
mNativeWrapper.alwaysOnDisable(id);
} else {
@@ -268,10 +293,10 @@ final class VibratorController {
* do not support the input or a negative number if the operation failed.
*/
public long on(PrimitiveSegment[] primitives, long vibrationId) {
if (!mVibratorInfo.hasCapability(IVibrator.CAP_COMPOSE_EFFECTS)) {
return 0;
}
synchronized (mLock) {
if (!mVibratorInfo.hasCapability(IVibrator.CAP_COMPOSE_EFFECTS)) {
return 0;
}
long duration = mNativeWrapper.compose(primitives, vibrationId);
if (duration > 0) {
mCurrentAmplitude = -1;
@@ -290,10 +315,10 @@ final class VibratorController {
* @return The duration of the effect playing, or 0 if unsupported.
*/
public long on(RampSegment[] primitives, long vibrationId) {
if (!mVibratorInfo.hasCapability(IVibrator.CAP_COMPOSE_PWLE_EFFECTS)) {
return 0;
}
synchronized (mLock) {
if (!mVibratorInfo.hasCapability(IVibrator.CAP_COMPOSE_PWLE_EFFECTS)) {
return 0;
}
int braking = mVibratorInfo.getDefaultBraking();
long duration = mNativeWrapper.composePwle(primitives, braking, vibrationId);
if (duration > 0) {
@@ -327,6 +352,7 @@ final class VibratorController {
synchronized (mLock) {
return "VibratorController{"
+ "mVibratorInfo=" + mVibratorInfo
+ ", mVibratorInfoLoadSuccessful=" + mVibratorInfoLoadSuccessful
+ ", mIsVibrating=" + mIsVibrating
+ ", mCurrentAmplitude=" + mCurrentAmplitude
+ ", mIsUnderExternalControl=" + mIsUnderExternalControl
@@ -393,10 +419,15 @@ final class VibratorController {
* allocated and returned by {@link #nativeInit(int, OnVibrationCompleteListener)}.
*/
private static native long getNativeFinalizer();
private static native boolean isAvailable(long nativePtr);
private static native long on(long nativePtr, long milliseconds, long vibrationId);
private static native void off(long nativePtr);
private static native void setAmplitude(long nativePtr, float amplitude);
private static native long performEffect(long nativePtr, long effect, long strength,
long vibrationId);

View File

@@ -128,6 +128,8 @@ public class VibratorManagerService extends IVibratorManagerService.Stub {
private VibrationThread mNextVibration;
@GuardedBy("mLock")
private ExternalVibrationHolder mCurrentExternalVibration;
@GuardedBy("mLock")
private boolean mServiceReady;
private final VibrationSettings mVibrationSettings;
private final VibrationScaler mVibrationScaler;
@@ -201,6 +203,9 @@ public class VibratorManagerService extends IVibratorManagerService.Stub {
mWakeLock = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, "*vibrator*");
mWakeLock.setReferenceCounted(true);
// Load vibrator hardware info. The vibrator ids and manager capabilities are loaded only
// once and assumed unchanged for the lifecycle of this service. Each individual vibrator
// can still retry loading each individual vibrator hardware spec once more at systemReady.
mCapabilities = mNativeWrapper.getCapabilities();
int[] vibratorIds = mNativeWrapper.getVibratorIds();
if (vibratorIds == null) {
@@ -235,6 +240,11 @@ public class VibratorManagerService extends IVibratorManagerService.Stub {
Slog.v(TAG, "Initializing VibratorManager service...");
Trace.traceBegin(Trace.TRACE_TAG_VIBRATOR, "systemReady");
try {
// Will retry to load each vibrator's info, if any request have failed.
for (int i = 0; i < mVibrators.size(); i++) {
mVibrators.valueAt(i).reloadVibratorInfoIfNeeded();
}
mVibrationSettings.onSystemReady();
mInputDeviceDelegate.onSystemReady();
@@ -243,6 +253,9 @@ public class VibratorManagerService extends IVibratorManagerService.Stub {
// Will update settings and input devices.
updateServiceState();
} finally {
synchronized (mLock) {
mServiceReady = true;
}
Slog.v(TAG, "VibratorManager service initialized");
Trace.traceEnd(Trace.TRACE_TAG_VIBRATOR);
}
@@ -256,8 +269,19 @@ public class VibratorManagerService extends IVibratorManagerService.Stub {
@Override // Binder call
@Nullable
public VibratorInfo getVibratorInfo(int vibratorId) {
VibratorController controller = mVibrators.get(vibratorId);
return controller == null ? null : controller.getVibratorInfo();
final VibratorController controller = mVibrators.get(vibratorId);
if (controller == null) {
return null;
}
final VibratorInfo info = controller.getVibratorInfo();
synchronized (mLock) {
if (mServiceReady) {
return info;
}
}
// If the service is not ready and the load was unsuccessful then return null while waiting
// for the service to be ready. It will retry to load the complete info from the HAL.
return controller.isVibratorInfoLoadSuccessful() ? info : null;
}
@Override // Binder call

View File

@@ -443,7 +443,7 @@ static jboolean vibratorGetInfo(JNIEnv* env, jclass /* clazz */, jlong ptr,
env->CallObjectMethod(vibratorInfoBuilder, sVibratorInfoBuilderClassInfo.setFrequencyMapping,
frequencyMapping);
return info.checkAndLogFailure("vibratorGetInfo") ? JNI_FALSE : JNI_TRUE;
return info.isFailedLogged("vibratorGetInfo") ? JNI_FALSE : JNI_TRUE;
}
static const JNINativeMethod method_table[] = {

View File

@@ -51,6 +51,7 @@ final class FakeVibratorControllerProvider {
private final FakeNativeWrapper mNativeWrapper;
private boolean mIsAvailable = true;
private boolean mIsInfoLoadSuccessful = true;
private long mLatency;
private int mOffCount;
@@ -172,7 +173,7 @@ final class FakeVibratorControllerProvider {
infoBuilder.setFrequencyMapping(new VibratorInfo.FrequencyMapping(mMinFrequency,
mResonantFrequency, mFrequencyResolution, suggestedFrequencyRange,
mMaxAmplitudes));
return true;
return mIsInfoLoadSuccessful;
}
private void applyLatency() {
@@ -212,6 +213,14 @@ final class FakeVibratorControllerProvider {
mIsAvailable = false;
}
/**
* Sets the result for the method that loads the {@link VibratorInfo}, for faking a vibrator
* that fails to load some of the hardware data.
*/
public void setVibratorInfoLoadSuccessful(boolean successful) {
mIsInfoLoadSuccessful = successful;
}
/**
* Sets the latency this controller should fake for turning the vibrator hardware on or setting
* it's vibration amplitude.

View File

@@ -287,7 +287,26 @@ public class VibratorManagerServiceTest {
}
@Test
public void getVibratorInfo_withExistingVibratorId_returnsHalInfoForVibrator() {
public void getVibratorInfo_vibratorFailedLoadBeforeSystemReady_returnsNull() {
mockVibrators(1);
mVibratorProviders.get(1).setVibratorInfoLoadSuccessful(false);
assertNull(createService().getVibratorInfo(1));
}
@Test
public void getVibratorInfo_vibratorFailedLoadAfterSystemReady_returnsInfoForVibrator() {
mockVibrators(1);
mVibratorProviders.get(1).setVibratorInfoLoadSuccessful(false);
mVibratorProviders.get(1).setResonantFrequency(123.f);
VibratorInfo info = createSystemReadyService().getVibratorInfo(1);
assertNotNull(info);
assertEquals(1, info.getId());
assertEquals(123.f, info.getResonantFrequency(), 0.01 /*tolerance*/);
}
@Test
public void getVibratorInfo_vibratorSuccessfulLoadBeforeSystemReady_returnsInfoForVibrator() {
mockVibrators(1);
FakeVibratorControllerProvider vibrator = mVibratorProviders.get(1);
vibrator.setCapabilities(IVibrator.CAP_COMPOSE_EFFECTS, IVibrator.CAP_AMPLITUDE_CONTROL);
@@ -295,7 +314,7 @@ public class VibratorManagerServiceTest {
vibrator.setSupportedPrimitives(VibrationEffect.Composition.PRIMITIVE_CLICK);
vibrator.setResonantFrequency(123.f);
vibrator.setQFactor(Float.NaN);
VibratorInfo info = createSystemReadyService().getVibratorInfo(1);
VibratorInfo info = createService().getVibratorInfo(1);
assertNotNull(info);
assertEquals(1, info.getId());
@@ -312,6 +331,24 @@ public class VibratorManagerServiceTest {
assertTrue(Float.isNaN(info.getQFactor()));
}
@Test
public void getVibratorInfo_vibratorFailedThenSuccessfulLoad_returnsNullThenInfo() {
mockVibrators(1);
mVibratorProviders.get(1).setVibratorInfoLoadSuccessful(false);
VibratorManagerService service = createService();
assertNull(createService().getVibratorInfo(1));
mVibratorProviders.get(1).setVibratorInfoLoadSuccessful(true);
mVibratorProviders.get(1).setResonantFrequency(123.f);
service.systemReady();
VibratorInfo info = createService().getVibratorInfo(1);
assertNotNull(info);
assertEquals(1, info.getId());
assertEquals(123.f, info.getResonantFrequency(), 0.01 /*tolerance*/);
}
@Test
public void registerVibratorStateListener_callbacksAreTriggered() throws Exception {
mockVibrators(1);