From b24fa44e89536a5ebc8e22332f4423037ecb2d44 Mon Sep 17 00:00:00 2001 From: zunda pixel Date: Thu, 3 Sep 2026 10:18:10 +0900 Subject: [PATCH 1/2] putbytes: check the install response cookie again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check was commented out with "TODO this fired?" — it fired because the watch answers an install with cookie 0, not because the cookie was wrong. In prv_do_install the firmware reads s_pb_state.token for the response, and the commit that always precedes an install has already reset that state, so zero is what a phone gets back. Every other step here keeps its cookie check; only the install had to give one up. Zero is tolerated rather than assumed, so the check returns for firmware that answers with the install's own token, while a cookie belonging to some other transfer is still caught. Co-Authored-By: Claude Opus 5 --- .../endpointmanager/putbytes/PutBytesSession.kt | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/libpebble3/src/commonMain/kotlin/io/rebble/libpebblecommon/connection/endpointmanager/putbytes/PutBytesSession.kt b/libpebble3/src/commonMain/kotlin/io/rebble/libpebblecommon/connection/endpointmanager/putbytes/PutBytesSession.kt index 817f1f6f3..29352903f 100644 --- a/libpebble3/src/commonMain/kotlin/io/rebble/libpebblecommon/connection/endpointmanager/putbytes/PutBytesSession.kt +++ b/libpebble3/src/commonMain/kotlin/io/rebble/libpebblecommon/connection/endpointmanager/putbytes/PutBytesSession.kt @@ -123,7 +123,14 @@ class PutBytesSession( suspend fun sendInstall(cookie: UInt) { val installResponse = putBytesService.sendInstall(cookie) - // TODO this fired? -// check(installResponse.cookie.get() == cookie) { "Received response for wrong cookie" } + val responseCookie = installResponse.cookie.get() + // Firmware answers an install with cookie 0: prv_cleanup_and_send_response reads + // s_pb_state.token, and the commit that precedes an install has already reset the + // transfer state. That is why this check used to fire on every install. Zero is + // tolerated so the check can come back for the firmware that answers properly, while + // a cookie belonging to some other transfer is still caught. + check(responseCookie == cookie || responseCookie == 0u) { + "Received response for wrong cookie" + } } } \ No newline at end of file From d5ee8bd6e71d21234eaf5c701a540455ae577657 Mon Sep 17 00:00:00 2001 From: zunda pixel Date: Thu, 3 Sep 2026 22:40:12 +0900 Subject: [PATCH 2/2] putbytes: cover what an install may be answered with Its own cookie, zero, and somebody else's. The middle one is what the firmware sends today and the reason the check was commented out; the last is what the check is still there to catch. Co-Authored-By: Claude Opus 5 --- .../putbytes/PutBytesSessionTest.kt | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 libpebble3/src/jvmTest/kotlin/io/rebble/libpebblecommon/connection/endpointmanager/putbytes/PutBytesSessionTest.kt diff --git a/libpebble3/src/jvmTest/kotlin/io/rebble/libpebblecommon/connection/endpointmanager/putbytes/PutBytesSessionTest.kt b/libpebble3/src/jvmTest/kotlin/io/rebble/libpebblecommon/connection/endpointmanager/putbytes/PutBytesSessionTest.kt new file mode 100644 index 000000000..2effddc05 --- /dev/null +++ b/libpebble3/src/jvmTest/kotlin/io/rebble/libpebblecommon/connection/endpointmanager/putbytes/PutBytesSessionTest.kt @@ -0,0 +1,54 @@ +package io.rebble.libpebblecommon.connection.endpointmanager.putbytes + +import TestPebbleProtocolHandler +import io.rebble.libpebblecommon.di.ConnectionCoroutineScope +import io.rebble.libpebblecommon.packets.PutBytesInstall +import io.rebble.libpebblecommon.packets.PutBytesResponse +import io.rebble.libpebblecommon.packets.PutBytesResult +import io.rebble.libpebblecommon.services.PutBytesService +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runTest +import org.junit.Test +import kotlin.test.assertFailsWith + +class PutBytesSessionTest { + private fun ack(cookie: UInt) = PutBytesResponse().apply { + result.set(PutBytesResult.ACK.value) + this.cookie.set(cookie) + } + + /** A session whose watch answers every install with [answerWith]. */ + private suspend fun TestScope.sessionAnswering(answerWith: UInt): PutBytesSession { + val handler = TestPebbleProtocolHandler { packet -> + if (packet is PutBytesInstall) { + receivePacket(ack(answerWith)) + } + } + val service = PutBytesService( + handler, + ConnectionCoroutineScope(backgroundScope.coroutineContext), + ) + service.init() + testScheduler.runCurrent() + return PutBytesSession(service) + } + + @Test + fun anInstallAnsweredWithItsOwnCookieIsAccepted() = runTest { + sessionAnswering(answerWith = 7u).sendInstall(7u) + } + + @Test + fun anInstallAnsweredWithZeroIsAccepted() = runTest { + // What the firmware actually sends: prv_cleanup_and_send_response reads + // s_pb_state.token, which the preceding commit already cleared. + sessionAnswering(answerWith = 0u).sendInstall(7u) + } + + @Test + fun anInstallAnsweredForAnotherTransferIsNot() = runTest { + val session = sessionAnswering(answerWith = 8u) + + assertFailsWith { session.sendInstall(7u) } + } +}