From 02daf5851cfd355fa5b74dcc1720c14adb14088c Mon Sep 17 00:00:00 2001 From: Mark Nolan Date: Mon, 21 Sep 2026 09:55:55 +0100 Subject: [PATCH] DEV-982: handle NACK from a Shimmer3 instead of hanging the caller The protocol has had a NACK (0xFE) reply for a long time; this API never learned about it. Every reply path in startProcessing() keys off ackCommand, so a refusal matched nothing, the pending CheckedContinuation was never resumed, and the caller's await never returned. Nothing rescues that. timeoutInSeconds is declared and never used, here and in VerisenseProtocol, so the suspended task stays suspended for the life of the process. That makes this the worst of the three host APIs that share the defect: Shimmer-Java-Android-API#293 tore the link down after two seconds, Shimmer-C-API times out and disconnects after about ten, and this one simply never comes back. Found by checking the sibling implementations of the wire format after the Java fix. The web SDK already handles NACK; C# is being fixed alongside this. The firmware refuses far more than it used to. Every command except SET_SD_SYNC_COMMAND and ACK is refused while SD sync is enabled (ShimBt_isCmdAllowedWhileSdSyncing), which covers the whole connect sequence, so a sync-enabled Shimmer could never finish connecting. Also any SET while sensing, a sync-mode mismatch, an out-of-range InfoMem or calibration write, and several commands the dispatcher accepts but that were never implemented. processNackFromCommand() resumes whichever continuation is outstanding with a failure value and clears it, so the caller gets a result instead of hanging. Resuming exactly once matters in both directions: a CheckedContinuation resumed twice traps at runtime, one never resumed hangs the task. The connection is left up, because a refusal means the device declined the command, not that the link is gone. The check sits ahead of the streaming/connected split rather than inside either, since neither branch looks for 0xFE, and it waits until the CRC bytes the firmware appends to the refusal have arrived so the whole packet is consumed at once - otherwise a CRC byte would be read as the next packet header. Not compiled locally: there is no Swift toolchain on the machine this was written on, so the CI run on this branch is the first time it is built. No unit test either - Shimmer3Protocol.init takes a concrete BleByteRadio that needs a live CBPeripheral, so no test can construct the protocol, which is a limitation worth lifting separately. Co-Authored-By: Claude Opus 5 --- .../ShimmerBluetooth/Shimmer3Protocol.swift | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/ShimmerBluetooth/ShimmerBluetooth/Shimmer3Protocol.swift b/ShimmerBluetooth/ShimmerBluetooth/Shimmer3Protocol.swift index 0e5481e..1f635c2 100644 --- a/ShimmerBluetooth/ShimmerBluetooth/Shimmer3Protocol.swift +++ b/ShimmerBluetooth/ShimmerBluetooth/Shimmer3Protocol.swift @@ -603,7 +603,22 @@ public class Shimmer3Protocol : NSObject, ShimmerProtocol { self.processing = true processingQueue.async { while self.processing { - if (self.BTState == Shimmer3BTState.STREAMING){ + /* The Shimmer refused the command with a NACK (0xFE) rather than + * acknowledging it. Checked ahead of the state branches below + * because neither of them looks for it: every reply path keys off + * ackCommand, so a refusal left the pending continuation suspended + * and the awaiting call never returned. Nothing rescues that - + * timeoutInSeconds is declared but never used - so the hang is + * permanent. + * + * Held until the CRC bytes that follow the refusal have arrived + * too, so the whole packet is consumed in one go and no CRC byte is + * left to be read as the next packet header. */ + if (self.receivedBytes.first==PacketTypeShimmer.nackCommand.rawValue + && self.receivedBytes.count >= 1+Int(self.CRCMode.rawValue)){ + self.processNackFromCommand() + } + else if (self.BTState == Shimmer3BTState.STREAMING){ if (self.receivedBytes.count>self.PacketSize){ var received = Array(self.receivedBytes.prefix(self.PacketSize+1)) //1 for the start of the packet @@ -1059,6 +1074,33 @@ public class Shimmer3Protocol : NSObject, ShimmerProtocol { } } + /// Unwinds the command in flight after the Shimmer answered it with a NACK + /// (0xFE) instead of an ACK. + /// + /// A refusal means the device declined this command, not that the link is + /// gone, so the connection is left alone. The pending continuation is resumed + /// with a failure value so that the caller's `await` returns. Resuming exactly + /// once and then clearing it matters: a CheckedContinuation resumed twice + /// traps at runtime, and one that is never resumed suspends the task for good. + /// + /// The firmware refuses far more than it used to. Every command except + /// setSDSyncCommand and ACK is refused while SD sync is enabled, which covers + /// the whole connect sequence; so is any SET while the device is sensing, a + /// sync-mode mismatch, an out-of-range InfoMem or calibration write, and + /// several commands the firmware accepts but never implemented. + private func processNackFromCommand() { + print("NACK Received - the Shimmer refused the command: \(String(describing: self.commandSent))") + + // The NACK is the whole response packet: drop it and its CRC bytes. + self.receivedBytes.removeFirst(1+Int(self.CRCMode.rawValue)) + self.commandSent = nil + + self.continuation?.resume(returning: false) + self.continuation = nil + self.continuationByteArray?.resume(returning: nil) + self.continuationByteArray = nil + } + func stopProcessing(){ self.processing = false } @@ -2073,6 +2115,7 @@ public class Shimmer3Protocol : NSObject, ShimmerProtocol { case setCRCCommand = 0x8b case calibDumpResponse = 0x99 case getCalibDumpCommand = 0x9a + case nackCommand = 0xFE case ackCommand = 0xFF case getBmp180CalibrationCoefficientsCommand = 0x59 case bmp180CalibrationCoefficientsResponse = 0x58