diff --git a/jindong-core/src/commonMain/kotlin/io/github/compose/jindong/core/HapticManager.kt b/jindong-core/src/commonMain/kotlin/io/github/compose/jindong/core/HapticManager.kt index ec9084f..8734529 100644 --- a/jindong-core/src/commonMain/kotlin/io/github/compose/jindong/core/HapticManager.kt +++ b/jindong-core/src/commonMain/kotlin/io/github/compose/jindong/core/HapticManager.kt @@ -21,9 +21,12 @@ import io.github.compose.jindong.core.executor.HapticExecutor import io.github.compose.jindong.core.executor.HapticHandle import io.github.compose.jindong.core.executor.createHapticExecutor import io.github.compose.jindong.core.model.HapticPattern +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock +import kotlinx.coroutines.withContext /** * Singleton manager for haptic pattern execution. @@ -124,12 +127,27 @@ object HapticManager { */ suspend fun execute(pattern: HapticPattern) { executionMutex.withLock { - val executor = withStateLock { + // Drive playback through the handle path (not the suspend executor.execute) so the in-flight + // vibration stays reachable via currentHandle: only HapticHandle.cancel() stops the motor, + // while a coroutine-cancelled executor.execute() would abort its delay but leave the actuator + // buzzing. Publishing the handle lets a concurrent executeAsync/execute cancel-and-restart it + // instead of overlapping. + val handle = withStateLock { currentHandle?.cancel() - currentHandle = null - getOrCreateExecutorLocked() + val newHandle = getOrCreateExecutorLocked().executeAsync(pattern) + currentHandle = newHandle + newHandle + } + try { + awaitCompletion(handle) + } finally { + // Stop the motor and clear the slot on normal end, cancellation, or a takeover by another + // caller. clearHandleIfCurrent guards against wiping a handle a newer call already installed. + withContext(NonCancellable) { + handle.cancel() + clearHandleIfCurrent(handle) + } } - executor.execute(pattern) } } @@ -142,12 +160,27 @@ object HapticManager { * @return A [HapticHandle] for cancelling the execution */ fun executeAsync(pattern: HapticPattern): HapticHandle = withStateLockBlocking { + // Shares currentHandle with execute(), so an executeAsync landing mid-execute cancels the + // in-flight handle here before starting its own, so the two paths never overlap. currentHandle?.cancel() val handle = getOrCreateExecutorLocked().executeAsync(pattern) currentHandle = handle handle } + /** + * Suspends until [handle] is no longer active — natural (duration-estimate) completion, or an + * external [HapticHandle.cancel] from a concurrent takeover. Polls because neither Android's + * `Vibrator` nor iOS' `CHHapticPatternPlayerProtocol` reports per-effect completion; the whole + * library already treats completion as a best-effort estimate (see [HandleExpiry]). The delay is a + * suspension point, so coroutine cancellation of execute() unwinds here immediately. + */ + private suspend fun awaitCompletion(handle: HapticHandle) { + while (handle.isActive) { + delay(COMPLETION_POLL_INTERVAL_MS) + } + } + /** * Cancels any ongoing haptic execution. */ @@ -178,6 +211,14 @@ object HapticManager { } } + // Clears currentHandle only when it still points at [handle]. execute()'s cleanup must not wipe a + // handle that a newer execute/executeAsync already installed after taking over. + private fun clearHandleIfCurrent(handle: HapticHandle) { + withStateLockBlocking { + if (currentHandle === handle) currentHandle = null + } + } + private suspend fun withStateLock(action: () -> T): T = stateMutex.withLock { action() } private fun withStateLockBlocking(action: () -> T): T = runBlocking { @@ -194,6 +235,11 @@ object HapticManager { executor = newExecutor return newExecutor } + + // Playback has no OS completion callback, so execute() polls the handle. 4ms keeps the caller's + // resume within one frame of natural end while cancellation (a takeover) still unwinds instantly + // at the next suspension point. + private const val COMPLETION_POLL_INTERVAL_MS = 4L } /** diff --git a/jindong-core/src/commonTest/kotlin/io/github/compose/jindong/core/HapticManagerTest.kt b/jindong-core/src/commonTest/kotlin/io/github/compose/jindong/core/HapticManagerTest.kt index e84c748..735a7ef 100644 --- a/jindong-core/src/commonTest/kotlin/io/github/compose/jindong/core/HapticManagerTest.kt +++ b/jindong-core/src/commonTest/kotlin/io/github/compose/jindong/core/HapticManagerTest.kt @@ -24,7 +24,14 @@ import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.collections.shouldBeEmpty import io.kotest.matchers.collections.shouldHaveSize import io.kotest.matchers.shouldBe - +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.testTimeSource +import kotlin.time.Duration.Companion.milliseconds + +@OptIn(ExperimentalCoroutinesApi::class) class HapticManagerTest : FunSpec({ lateinit var fakeExecutor: FakeHapticExecutor @@ -48,27 +55,48 @@ class HapticManagerTest : HapticManager.isSupported shouldBe false } - test("execute should execute pattern synchronously") { - val pattern = buildHapticPattern { - haptic(100.ms) - } - - HapticManager.execute(pattern) - - assertSoftly { - fakeExecutor.executedPatterns shouldHaveSize 1 - fakeExecutor.executedPatterns[0] shouldBe pattern + test("execute should play pattern through the handle path and wait for completion") { + runTest { + // execute() drives playback via executeAsync so the in-flight vibration stays reachable via + // currentHandle; it still suspends until the handle's playback window elapses. + val playbackMs = 10L + val timedExecutor = FakeHapticExecutor( + playbackDuration = playbackMs.milliseconds, + timeSource = testTimeSource, + ) + HapticManager.initializeExecutor(timedExecutor) + val pattern = buildHapticPattern { haptic(100.ms) } + + val before = testScheduler.currentTime + HapticManager.execute(pattern) + val elapsed = testScheduler.currentTime - before + + assertSoftly { + timedExecutor.asyncExecutedPatterns shouldHaveSize 1 + timedExecutor.asyncExecutedPatterns[0] shouldBe pattern + // Suspended for the whole playback window (poll granularity may round up the final step). + (elapsed >= playbackMs) shouldBe true + } } } test("execute should handle empty pattern") { - val emptyPattern = buildHapticPattern { } - - HapticManager.execute(emptyPattern) - - assertSoftly { - fakeExecutor.executedPatterns shouldHaveSize 1 - fakeExecutor.executedPatterns[0].events.shouldBeEmpty() + runTest { + val timedExecutor = FakeHapticExecutor( + playbackDuration = 0.milliseconds, + timeSource = testTimeSource, + ) + HapticManager.initializeExecutor(timedExecutor) + val emptyPattern = buildHapticPattern { } + + shouldNotThrowAny { + HapticManager.execute(emptyPattern) + } + + assertSoftly { + timedExecutor.asyncExecutedPatterns shouldHaveSize 1 + timedExecutor.asyncExecutedPatterns[0].events.shouldBeEmpty() + } } } @@ -99,6 +127,46 @@ class HapticManagerTest : } } + test("executeAsync should cancel an execute() playback that is still in flight") { + // Regression: while execute() is mid-playback, an executeAsync must find and cancel the + // in-flight handle (the two share currentHandle) instead of firing a second overlapping + // vibration. Reverting execute() to clear currentHandle before playback makes this RED. + runTest { + // Long window so the execute() playback is still active when executeAsync lands. + val timedExecutor = FakeHapticExecutor( + playbackDuration = 1_000.milliseconds, + timeSource = testTimeSource, + ) + HapticManager.initializeExecutor(timedExecutor) + + val longPattern = buildHapticPattern { haptic(500.ms) } + val otherPattern = buildHapticPattern { haptic(100.ms) } + + // Start the suspending execute() in the background; let it install its handle and begin its + // await, then interleave the async call while it is provably still in flight. + val executeJob = launch { HapticManager.execute(longPattern) } + advanceTimeBy(10) + val executeHandle = timedExecutor.issuedHandles.single() + executeHandle.isActive shouldBe true + + val asyncHandle = HapticManager.executeAsync(otherPattern) + + assertSoftly { + // The in-flight execute() playback was cancelled, not left overlapping. + executeHandle.cancelled shouldBe true + executeHandle.isActive shouldBe false + // Both playbacks reached the executor; only the async one is still live. + timedExecutor.asyncExecutedPatterns shouldBe listOf(longPattern, otherPattern) + asyncHandle.isActive shouldBe true + } + + // execute()'s cleanup unwinds once its handle is cancelled; it must not cancel the newer + // async handle that took over the currentHandle slot. + executeJob.join() + asyncHandle.isActive shouldBe true + } + } + test("cancel should cancel current execution") { val pattern = buildHapticPattern { haptic(100.ms) } val handle = HapticManager.executeAsync(pattern) diff --git a/jindong-core/src/commonTest/kotlin/io/github/compose/jindong/core/fake/FakeHapticExecutor.kt b/jindong-core/src/commonTest/kotlin/io/github/compose/jindong/core/fake/FakeHapticExecutor.kt index 957f3b3..ea8017d 100644 --- a/jindong-core/src/commonTest/kotlin/io/github/compose/jindong/core/fake/FakeHapticExecutor.kt +++ b/jindong-core/src/commonTest/kotlin/io/github/compose/jindong/core/fake/FakeHapticExecutor.kt @@ -19,23 +19,40 @@ import io.github.compose.jindong.core.executor.HapticExecutor import io.github.compose.jindong.core.executor.HapticHandle import io.github.compose.jindong.core.model.HapticPattern import kotlinx.coroutines.delay +import kotlin.time.Duration +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.TimeSource /** * Fake implementation of [HapticExecutor] for testing. * - * Records all executed patterns and simulates execution delays. + * Records all executed patterns and hands out handles whose natural completion is driven by an + * injectable [timeSource], so tests exercising [io.github.compose.jindong.core.HapticManager.execute] + * (which now awaits the handle) can advance virtual time deterministically via `runTest`'s + * `testTimeSource` instead of relying on a real monotonic clock. + * + * @param playbackDuration How long each [executeAsync] handle reports itself active. Defaults to a + * short window so tests only need to advance the virtual clock past it to observe completion. + * @param timeSource Clock the handles measure elapsed playback against. Pass `runTest`'s + * `testTimeSource` so `delay(...)` advances it; defaults to real monotonic for non-timed tests. */ class FakeHapticExecutor( override val isSupported: Boolean = true, override val hasAmplitudeControl: Boolean = true, + private val playbackDuration: Duration = 10.milliseconds, + private val timeSource: TimeSource = TimeSource.Monotonic, ) : HapticExecutor { private val _executedPatterns = mutableListOf() private val _asyncExecutedPatterns = mutableListOf() + private val _issuedHandles = mutableListOf() private var _releaseCalled = false val executedPatterns: List get() = _executedPatterns.toList() val asyncExecutedPatterns: List get() = _asyncExecutedPatterns.toList() + + /** Handles handed out by [executeAsync], in order, so tests can inspect in-flight cancellation. */ + val issuedHandles: List get() = _issuedHandles.toList() val releaseCalled: Boolean get() = _releaseCalled override suspend fun execute(pattern: HapticPattern) { @@ -46,7 +63,7 @@ class FakeHapticExecutor( override fun executeAsync(pattern: HapticPattern): HapticHandle { _asyncExecutedPatterns.add(pattern) - return FakeHapticHandle() + return FakeHapticHandle(playbackDuration, timeSource).also { _issuedHandles.add(it) } } override fun release() { @@ -56,22 +73,28 @@ class FakeHapticExecutor( fun reset() { _executedPatterns.clear() _asyncExecutedPatterns.clear() + _issuedHandles.clear() _releaseCalled = false } } /** * Fake implementation of [HapticHandle] for testing. + * + * Stays active until [cancel] is called or [playbackDuration] elapses on [timeSource], mirroring the + * real platform handles' duration-estimate completion (there is no OS completion callback). */ -class FakeHapticHandle : HapticHandle { +class FakeHapticHandle( + private val playbackDuration: Duration = 10.milliseconds, + timeSource: TimeSource = TimeSource.Monotonic, +) : HapticHandle { private var _cancelled = false - private var _isActive = true + private val start = timeSource.markNow() val cancelled: Boolean get() = _cancelled - override val isActive: Boolean get() = _isActive && !_cancelled + override val isActive: Boolean get() = !_cancelled && start.elapsedNow() < playbackDuration override fun cancel() { _cancelled = true - _isActive = false } }