Don't take the conductor lock for syncing vibrations.

The process of preparing and trigggering should be automatically
serialized and exclusive because no other step can be executing
on the VibrationThread, and only one VibrationThread is active
at any time.

Bug: 193792066
Test: presubmit
Change-Id: I93e6bdf5ea68c3602cda962fb32030c8695d5fa8
This commit is contained in:
Simon Bowden
2022-02-21 20:12:54 +00:00
parent 7ca22de115
commit f609ed9f60
2 changed files with 36 additions and 37 deletions

View File

@@ -189,9 +189,8 @@ final class StartSequentialEffectStep extends Step {
// vibrating different sets of vibrators in parallel. The manager can only prepareSynced // vibrating different sets of vibrators in parallel. The manager can only prepareSynced
// one set of vibrators at a time. // one set of vibrators at a time.
// This property is guaranteed by there only being one thread (VibrationThread) executing // This property is guaranteed by there only being one thread (VibrationThread) executing
// one Step at a time, so there's no need to hold the state lock. // one Step at a time, so there's no need to hold the state lock. Callbacks will be
// TODO: remove the large locked block in a dedicated change. // delivered asynchronously but enqueued until the step processing is finished.
synchronized (conductor.mLock) {
boolean hasPrepared = false; boolean hasPrepared = false;
boolean hasTriggered = false; boolean hasTriggered = false;
long maxDuration = 0; long maxDuration = 0;
@@ -230,7 +229,6 @@ final class StartSequentialEffectStep extends Step {
} }
} }
} }
}
private long startVibrating(AbstractVibratorStep step, List<Step> nextSteps) { private long startVibrating(AbstractVibratorStep step, List<Step> nextSteps) {
nextSteps.addAll(step.play()); nextSteps.addAll(step.play());

View File

@@ -242,8 +242,9 @@ final class VibrationThread extends Thread implements IBinder.DeathRecipient {
} }
} }
// Only run the next vibration step if we didn't have to wait in this loop. // Only run the next vibration step if we didn't have to wait in this loop.
// If we waited then the queue may have changed, so loop again to re-evaluate // If we waited then the queue may have changed or the wait could have been
// the scheduling of the queue top element. // interrupted by a cancel call, so loop again to re-evaluate the scheduling of
// the queue top element.
if (waitMillisBeforeNextStep <= 0) { if (waitMillisBeforeNextStep <= 0) {
if (DEBUG) { if (DEBUG) {
Slog.d(TAG, "Play vibration consuming next step..."); Slog.d(TAG, "Play vibration consuming next step...");