From c0c7505028b0d5246b50835723bd13df713611ba Mon Sep 17 00:00:00 2001 From: Mark Nolan Date: Wed, 16 Sep 2026 10:45:06 +0100 Subject: [PATCH 1/6] DEV-1023: do not read an invalid zero timestamp as a counter roll-over Same defect as the Java driver, same rule, in the C# API. The packet tick counter returns to zero every 512 seconds and CalibrateTimeStamp added a modulo back whenever a sample read lower than the last. That is wrong for one input: a record whose timestamp field is exactly zero, which firmware never publishes deliberately - LogAndStream v1.00.x-v1.01.003 could emit one under SD write back-pressure. Read as a roll-over it makes everything after it 512 seconds late. New TimestampUnwrap mirrors the Java class of the same name, deliberately: the two read the same recordings and have to agree. A backward step is a roll-over unless the counter is the 3-byte one, the new value is exactly zero, and the previous value was more than a second below the top of the range - then the sample is rejected, the caller gets the previous timestamp back and the wrap count is untouched. A genuine wrap onto zero, a 2-byte counter and any non-zero backward step all keep their existing behaviour, and rejection cannot cascade. CalibrateTimeStamp uses it and exposes LastTimestampRejected. Packet-loss estimation skips a rejected sample rather than being handed a difference of zero that never happened. ShimmerSDLog.ReadPacketMsg drops such records outright, as the Java importer does, and reports the count. TestSerialPort had its own copy of the rule and now calls the shared one. A copy of this rule is precisely how the same defect reached four host APIs. ShimmerCaptureXamarin has a third copy (ShimmerBluetooth.cs) that is NOT fixed here. It is a vendored fork of the API rather than a consumer of it - no project reference, Xamarin target framework, and its own stale CalibrateTimeStamp that does not know about Shimmer3R. It cannot use the shared class and cannot be built or tested on a machine without the Xamarin workload, and adding a third divergent copy of the rule would make the problem worse rather than better. It needs to consume ShimmerAPI instead; raised separately. Verified: ShimmerAPI builds with 10 warnings and 0 errors, unchanged from master; TestSerialPort builds. TimestampUnwrap's ten cases were run against the real source file locally - the repository's MSTest projects are packages.config and need nuget plus MSBuild, which CI has and this machine does not, so TimestampUnwrapTest will first run in ShimmerBluetoothAPIUnitTest.yml. Note for anyone reading this alongside the SD path: the fix is correct but currently unreachable when parsing a Shimmer3 SD file, because that path leaves TimeStampPacketRawMaxValue at its 2-byte default. Demonstrated on a synthetic file - as shipped it parses to a span of MINUS 500 seconds; with the modulo corrected the same file gives 2998 rows, 2 rejected and a correct 5.949 s. That, and the absence of any sync-block handling in the C# SD parser, are separate defects raised on their own ticket. Co-Authored-By: Claude Opus 5 --- ShimmerAPI/ShimmerAPI/ShimmerDevice.cs | 24 ++- ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs | 38 ++++ ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs | 93 ++++++++ .../ShimmerUnitTests/ShimmerUnitTests.csproj | 1 + .../ShimmerUnitTests/TimestampUnwrapTest.cs | 198 ++++++++++++++++++ ShimmerAPI/TestSerialPort/Program.cs | 13 +- 6 files changed, 355 insertions(+), 12 deletions(-) create mode 100644 ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs create mode 100644 ShimmerAPI/ShimmerUnitTests/TimestampUnwrapTest.cs diff --git a/ShimmerAPI/ShimmerAPI/ShimmerDevice.cs b/ShimmerAPI/ShimmerAPI/ShimmerDevice.cs index 47570b6c..8d215535 100644 --- a/ShimmerAPI/ShimmerAPI/ShimmerDevice.cs +++ b/ShimmerAPI/ShimmerAPI/ShimmerDevice.cs @@ -11,6 +11,13 @@ public abstract class ShimmerDevice { protected double LastReceivedTimeStamp = 0; protected double CurrentTimeStampCycle = 0; + /// + /// True when the sample most recently passed to CalibrateTimeStamp carried an + /// invalid zero timestamp and was rejected rather than unwrapped. Its sensor + /// data is fine; only its timestamp is missing. A caller reading a file should + /// drop the record - see ShimmerSDLog.ReadPacketMsg. + /// + public bool LastTimestampRejected { get; protected set; } protected double LastReceivedCalibratedTimeStamp = -1; protected double CalTimeStart; protected double SamplingRate; @@ -35,12 +42,15 @@ protected double CalibrateTimeStamp(double timeStamp) { //first convert to continuous time stamp double calibratedTimeStamp = 0; - if (LastReceivedTimeStamp > (timeStamp + (TimeStampPacketRawMaxValue * CurrentTimeStampCycle))) - { - CurrentTimeStampCycle = CurrentTimeStampCycle + 1; - } + TimestampUnwrap.Result unwrapped = TimestampUnwrap.Unwrap( + timeStamp, LastReceivedTimeStamp, CurrentTimeStampCycle, TimeStampPacketRawMaxValue); - LastReceivedTimeStamp = (timeStamp + (TimeStampPacketRawMaxValue * CurrentTimeStampCycle)); + LastTimestampRejected = unwrapped.Rejected; + CurrentTimeStampCycle = unwrapped.Cycle; + //On a rejected sample this puts back the value it already held, which is + //what keeps the rejection from cascading: the next sample reads above it + //and is accepted normally. + LastReceivedTimeStamp = unwrapped.Unwrapped; double clockConstant = 1024; if (HardwareVersion == (int)ShimmerVersion.SHIMMER2R || HardwareVersion == (int)ShimmerVersion.SHIMMER2) @@ -58,8 +68,10 @@ protected double CalibrateTimeStamp(double timeStamp) FirstTimeCalTime = false; CalTimeStart = calibratedTimeStamp; } - if (LastReceivedCalibratedTimeStamp != -1) + if (LastReceivedCalibratedTimeStamp != -1 && !LastTimestampRejected) { + //A rejected sample carries the previous timestamp, so the difference + //here would be zero - a gap that never happened. double timeDifference = calibratedTimeStamp - LastReceivedCalibratedTimeStamp; double expectedTimeDifference = (1 / SamplingRate) * 1000; //in ms double adjustedETD = expectedTimeDifference + (expectedTimeDifference * 0.1); diff --git a/ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs b/ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs index effc13b1..52d40085 100644 --- a/ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs +++ b/ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs @@ -542,7 +542,45 @@ public void ProcessSDLogHeader(byte[] byteArrayInfo) } + /// + /// Reads the next usable sample from the file. + /// + /// Records whose timestamp field is zero are dropped rather than returned. The + /// firmware stamps a packet when its sample tick starts it and does not publish + /// one it never stamped, so such a record is invalid - its sensor data is fine + /// but there is nothing to place it on the timeline with. LogAndStream + /// v1.00.x-v1.01.003 could write one under SD write back-pressure, and reading + /// it as a 24-bit roll-over adds 512 seconds to everything after it. + /// + /// reports how many were dropped. + /// + /// the next sample, or null at end of file public ObjectCluster ReadPacketMsg() + { + ObjectCluster ojc = ReadOnePacketMsg(); + + while (ojc != null && LastTimestampRejected) + { + RejectedTimestampRowCount++; + if (EndOfFile) + { + return null; + } + ojc = ReadOnePacketMsg(); + } + + return ojc; + } + + /// + /// How many records this pass dropped because they carried no timestamp. Zero + /// for a file written by firmware without the defect described on + /// . + /// + public int RejectedTimestampRowCount { get; private set; } + + /// One physical record, whether or not it is usable. + private ObjectCluster ReadOnePacketMsg() { int fullPacketSize; diff --git a/ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs b/ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs new file mode 100644 index 00000000..67fe8f81 --- /dev/null +++ b/ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs @@ -0,0 +1,93 @@ +using System; + +namespace ShimmerAPI +{ + /// + /// Turns a Shimmer's wrapping packet tick counter into a monotonic one. + /// + /// The counter is 3 bytes at 32768 Hz on current firmware, so it returns to zero + /// every 512 seconds exactly, and a host has to add a whole modulo back each time + /// it does. The obvious rule - if this sample reads lower than the last one, a + /// wrap happened - is what every Shimmer host API has implemented, and it is + /// wrong for one input: a record whose timestamp field is exactly zero. + /// + /// + /// Firmware stamps a packet when the sample tick starts it and does not publish a + /// packet it never stamped, so 0x000000 in that field means the record is invalid, + /// not that the counter reached its origin. LogAndStream v1.00.x-v1.01.003 could + /// emit one under SD write back-pressure. Read as a wrap, a single such record + /// makes every later sample in the recording 512 seconds late - which is how a + /// 9 minute 30 second trial came back as 43 minutes 38. + /// + /// + /// So: a backward step is a roll-over unless the counter is the 3-byte one, the + /// new value is exactly zero, and the previous value was more than + /// below the top of the range. In that case the + /// sample is rejected - the caller is handed the previous timestamp back and the + /// wrap count is left alone. + /// + /// + /// The exemption is deliberately narrow. A genuine wrap that happens to land on + /// zero still has a predecessor near the top of the range, so it is accepted. A + /// 2-byte counter (older firmware, 2 second range, where a stall really can + /// exceed a second) is never rejected. And rejection cannot cascade: the following + /// sample reads above the retained previous value, so it is accepted normally and + /// the timeline carries on at its true spacing. + /// + /// + /// This mirrors TimestampUnwrap in the Java driver deliberately. The two must + /// agree: the same recording is read by both. + /// + /// + public static class TimestampUnwrap + { + /// Maximum value of the 3-byte tick counter, exclusive: 2^24 ticks = 512 s. + public const int TicksMax3Byte = 1 << 24; + + /// + /// How close to the top of the range the previous sample must have been for a + /// drop to exactly zero to be believed as a wrap. One second at 32768 Hz. + /// + public const int WrapWindowTicks = 32768; + + /// Outcome of unwrapping one sample. + public struct Result + { + /// The unwrapped tick count to use. On a rejected sample, the previous one. + public double Unwrapped; + /// Wrap count after this sample. Unchanged when the sample was rejected. + public double Cycle; + /// True when the raw value was an invalid zero rather than a real wrap. + public bool Rejected; + } + + /// the packet's raw tick value + /// the unwrapped value returned for the previous sample + /// how many wraps have been counted so far + /// the counter's modulo - 2^24, or 2^16 on old firmware + public static Result Unwrap(double rawTicks, double lastUnwrapped, double cycle, int maxTicks) + { + double candidate = rawTicks + (maxTicks * cycle); + + if (lastUnwrapped > candidate) + { + // The counter went backwards. Either it wrapped, or this record is bad. + double lastRaw = lastUnwrapped - (maxTicks * cycle); + + if (maxTicks == TicksMax3Byte + && rawTicks == 0.0 + && lastRaw < (maxTicks - WrapWindowTicks)) + { + // Mid-range and then exactly zero: a record the firmware never + // stamped. Hold the timeline where it was and say so. + return new Result { Unwrapped = lastUnwrapped, Cycle = cycle, Rejected = true }; + } + + cycle += 1; + candidate = rawTicks + (maxTicks * cycle); + } + + return new Result { Unwrapped = candidate, Cycle = cycle, Rejected = false }; + } + } +} diff --git a/ShimmerAPI/ShimmerUnitTests/ShimmerUnitTests.csproj b/ShimmerAPI/ShimmerUnitTests/ShimmerUnitTests.csproj index 01d705bb..46798ada 100644 --- a/ShimmerAPI/ShimmerUnitTests/ShimmerUnitTests.csproj +++ b/ShimmerAPI/ShimmerUnitTests/ShimmerUnitTests.csproj @@ -61,6 +61,7 @@ + diff --git a/ShimmerAPI/ShimmerUnitTests/TimestampUnwrapTest.cs b/ShimmerAPI/ShimmerUnitTests/TimestampUnwrapTest.cs new file mode 100644 index 00000000..14a8b0e7 --- /dev/null +++ b/ShimmerAPI/ShimmerUnitTests/TimestampUnwrapTest.cs @@ -0,0 +1,198 @@ +using Microsoft.VisualStudio.TestTools.UnitTesting; +using ShimmerAPI; + +namespace ShimmerBluetoothTests +{ + /// + /// Covers , and in particular the one input that made + /// a customer's 9 minute 30 second recording import as 43 minutes 38: a record + /// whose timestamp field is exactly zero, read as a 24-bit roll-over. + /// + /// Mirrors API_00009_TimestampUnwrapTest in the Java driver. The two + /// implementations read the same recordings, so they have to agree. + /// + /// + [TestClass] + public class TimestampUnwrapTest + { + private const int Max3Byte = TimestampUnwrap.TicksMax3Byte; // 2^24 + private const int Max2Byte = 65536; + private const int PeriodTicks = 65; // 504.123 Hz, the rate the field evidence is in + + /// Feeds a series of raw values through the unwrapper, as a caller would. + private class Unwrapper + { + public double LastUnwrapped; + public double Cycle; + public bool LastRejected; + + public double Feed(double rawTicks, int maxTicks) + { + TimestampUnwrap.Result r = TimestampUnwrap.Unwrap(rawTicks, LastUnwrapped, Cycle, maxTicks); + LastRejected = r.Rejected; + Cycle = r.Cycle; + LastUnwrapped = r.Unwrapped; + return r.Unwrapped; + } + } + + [TestMethod] + public void TestMonotonicSamplesAreUnchanged() + { + var u = new Unwrapper(); + Assert.AreEqual(1000, u.Feed(1000, Max3Byte)); + Assert.AreEqual(1065, u.Feed(1065, Max3Byte)); + Assert.AreEqual(1130, u.Feed(1130, Max3Byte)); + Assert.IsFalse(u.LastRejected, "nothing here is invalid"); + Assert.AreEqual(0.0, u.Cycle, "no wrap was counted"); + } + + [TestMethod] + public void TestGenuineWrapIsCounted() + { + var u = new Unwrapper(); + u.Feed(Max3Byte - 16, Max3Byte); + double afterWrap = u.Feed(16, Max3Byte); + + Assert.IsFalse(u.LastRejected, "a real wrap is not a rejection"); + Assert.AreEqual(1.0, u.Cycle, "one wrap counted"); + Assert.AreEqual(Max3Byte + 16, afterWrap, "advanced by 32 ticks, not by a modulo"); + } + + /// + /// A wrap can legitimately land on zero - the counter simply reached its last + /// tick. What distinguishes it from a corrupt record is where it came from. + /// + [TestMethod] + public void TestWrapLandingExactlyOnZeroIsAccepted() + { + var u = new Unwrapper(); + u.Feed(Max3Byte - 100, Max3Byte); + double afterWrap = u.Feed(0, Max3Byte); + + Assert.IsFalse(u.LastRejected, "predecessor was at the top of the range, so this is a wrap"); + Assert.AreEqual(1.0, u.Cycle, "one wrap counted"); + Assert.AreEqual(Max3Byte, afterWrap); + } + + /// The defect: mid-range, then exactly zero. + [TestMethod] + public void TestIsolatedZeroMidRangeIsRejected() + { + var u = new Unwrapper(); + u.Feed(7406506, Max3Byte); + double afterZero = u.Feed(0, Max3Byte); + + Assert.IsTrue(u.LastRejected, "an exact zero from mid-range is an invalid record"); + Assert.AreEqual(0.0, u.Cycle, "no wrap may be counted for it"); + Assert.AreEqual(7406506, afterZero, "the timeline holds where it was"); + } + + [TestMethod] + public void TestRejectionDoesNotCascade() + { + var u = new Unwrapper(); + u.Feed(7406506, Max3Byte); + u.Feed(0, Max3Byte); + double next = u.Feed(7406571, Max3Byte); + + Assert.IsFalse(u.LastRejected, "the following sample is ordinary"); + Assert.AreEqual(7406571, next, "and lands one sample period on"); + Assert.AreEqual(0.0, u.Cycle, "still no wrap counted"); + } + + /// + /// The exact sequence recovered from the customer's file, either side of one of + /// its four bad records. Before the fix this spanned 512 seconds. + /// + [TestMethod] + public void TestCustomerSignatureSpansFourSamplePeriods() + { + double[] raw = { 7406116, 7406506, 0, 7406571 }; + var u = new Unwrapper(); + double first = u.Feed(raw[0], Max3Byte); + double last = 0; + int rejected = 0; + for (int i = 1; i < raw.Length; i++) + { + last = u.Feed(raw[i], Max3Byte); + if (u.LastRejected) + { + rejected++; + } + } + + Assert.AreEqual(1, rejected, "exactly one record was invalid"); + Assert.AreEqual(0.0, u.Cycle, "no modulo was added"); + Assert.AreEqual(455, last - first, "the four records span 455 ticks"); + } + + [TestMethod] + public void TestFirstSampleZeroIsAccepted() + { + var u = new Unwrapper(); + double first = u.Feed(0, Max3Byte); + + Assert.IsFalse(u.LastRejected, "nothing precedes it, so there is nothing to contradict"); + Assert.AreEqual(0.0, first); + Assert.AreEqual(0.0, u.Cycle); + } + + /// + /// Old firmware uses a 2-byte counter whose whole range is 2 seconds, so a + /// stall really can cross it. The exemption must not apply there. + /// + [TestMethod] + public void TestTwoByteCounterZeroIsStillAWrap() + { + var u = new Unwrapper(); + u.Feed(30000, Max2Byte); + double afterZero = u.Feed(0, Max2Byte); + + Assert.IsFalse(u.LastRejected, "2-byte counters keep their existing behaviour"); + Assert.AreEqual(1.0, u.Cycle, "one wrap counted"); + Assert.AreEqual(Max2Byte, afterZero); + } + + [TestMethod] + public void TestNonZeroBackwardStepIsStillAWrap() + { + var u = new Unwrapper(); + u.Feed(7406506, Max3Byte); + double after = u.Feed(1, Max3Byte); + + Assert.IsFalse(u.LastRejected, "a value of 1 is not the invalid marker"); + Assert.AreEqual(1.0, u.Cycle, "so it is read as a wrap, as before"); + Assert.AreEqual(Max3Byte + 1, after); + } + + /// + /// Four bad records is what the customer's 9m30s file contained; read as wraps + /// they added 2048 seconds and it imported as 43m38s. + /// + [TestMethod] + public void TestBadRecordsNoLongerInflateARecording() + { + var u = new Unwrapper(); + double startTicks = 1000000; + double t = startTicks; + int samples = 0; + + for (int i = 0; i < 800; i++) + { + if (i > 0 && i % 200 == 0) + { + u.Feed(0, Max3Byte); + Assert.IsTrue(u.LastRejected, "planted record is rejected"); + } + u.Feed(t, Max3Byte); + samples++; + t += PeriodTicks; + } + + Assert.AreEqual(0.0, u.Cycle, "no wraps counted across the whole recording"); + Assert.AreEqual((samples - 1) * PeriodTicks, u.LastUnwrapped - startTicks, + "span is exactly the samples that were taken"); + } + } +} diff --git a/ShimmerAPI/TestSerialPort/Program.cs b/ShimmerAPI/TestSerialPort/Program.cs index 82ebc2bf..878b41dc 100644 --- a/ShimmerAPI/TestSerialPort/Program.cs +++ b/ShimmerAPI/TestSerialPort/Program.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Text; using System.Threading.Tasks; +using ShimmerAPI; using static ShimmerAPI.ShimmerBluetooth; namespace TestSerialPort @@ -93,12 +94,12 @@ protected static double CalibrateTimeStamp(double timeStamp) { //first convert to continuous time stamp double calibratedTimeStamp = 0; - if (LastReceivedTimeStamp > (timeStamp + (TimeStampPacketRawMaxValue * CurrentTimeStampCycle))) - { - CurrentTimeStampCycle = CurrentTimeStampCycle + 1; - } - - LastReceivedTimeStamp = (timeStamp + (TimeStampPacketRawMaxValue * CurrentTimeStampCycle)); + //Shared with the API rather than copied: a copy of this rule is how the + //same defect ended up in four Shimmer host APIs at once. + TimestampUnwrap.Result unwrapped = TimestampUnwrap.Unwrap( + timeStamp, LastReceivedTimeStamp, CurrentTimeStampCycle, (int)TimeStampPacketRawMaxValue); + CurrentTimeStampCycle = unwrapped.Cycle; + LastReceivedTimeStamp = unwrapped.Unwrapped; calibratedTimeStamp = LastReceivedTimeStamp / 32768 * 1000; // to convert into mS if (FirstTimeCalTime) { From 608db0d4fa94fab510d6f3d0e1b37fe1d7e0a192 Mon Sep 17 00:00:00 2001 From: Mark Nolan Date: Wed, 16 Sep 2026 12:42:33 +0100 Subject: [PATCH 2/6] DEV-1023: report a rejected timestamp in the serial-port sample TestSerialPort carries the 3-byte modulo, so a rejection can fire there, but it took only the unwrapped value and the cycle from the result and dropped the Rejected flag. That made it the one copy of this rule that neither drops the record nor says anything about it - the sample program would print the previous packet's timestamp as though it were this one's. It has nowhere to drop a packet to, so it reports instead: the flag is kept and the packet is called out on the console when it happens. Co-Authored-By: Claude Opus 5 --- ShimmerAPI/TestSerialPort/Program.cs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/ShimmerAPI/TestSerialPort/Program.cs b/ShimmerAPI/TestSerialPort/Program.cs index 878b41dc..c0f25050 100644 --- a/ShimmerAPI/TestSerialPort/Program.cs +++ b/ShimmerAPI/TestSerialPort/Program.cs @@ -11,6 +11,12 @@ namespace TestSerialPort class Program { protected static double LastReceivedTimeStamp = 0; + /// + /// True when the last packet carried an invalid zero timestamp and was + /// rejected rather than unwrapped - see TimestampUnwrap. This sample program + /// has nowhere to drop a packet to, so it reports it instead. + /// + protected static bool LastTimestampRejected = false; protected static double CurrentTimeStampCycle = 0; protected static double LastReceivedCalibratedTimeStamp = -1; protected static double TimeStampPacketRawMaxValue = 16777216;// 16777216 or 65536 @@ -73,6 +79,13 @@ static void Main(string[] args) } double parsedts = parseTimeStamps(dataTS); double calibratedts = CalibrateTimeStamp(parsedts); + if (LastTimestampRejected) + { + //Said every time rather than once a second: these are rare, and + //the timestamp printed for this packet is the previous one. + System.Console.WriteLine(); + System.Console.WriteLine("packet with no timestamp - held the previous one"); + } if (i % SamplingRate == 0) System.Console.Write(calibratedts + "," + (calibratedts-lastKnownTS) +"," + SerialPort.BytesToRead); lastKnownTS = calibratedts; @@ -98,6 +111,7 @@ protected static double CalibrateTimeStamp(double timeStamp) //same defect ended up in four Shimmer host APIs at once. TimestampUnwrap.Result unwrapped = TimestampUnwrap.Unwrap( timeStamp, LastReceivedTimeStamp, CurrentTimeStampCycle, (int)TimeStampPacketRawMaxValue); + LastTimestampRejected = unwrapped.Rejected; CurrentTimeStampCycle = unwrapped.Cycle; LastReceivedTimeStamp = unwrapped.Unwrapped; calibratedTimeStamp = LastReceivedTimeStamp / 32768 * 1000; // to convert into mS From 641d9859b1765855e6d7c12447cf05050a9a42a0 Mon Sep 17 00:00:00 2001 From: Mark Nolan Date: Wed, 16 Sep 2026 12:59:13 +0100 Subject: [PATCH 3/6] DEV-1024: parse a Shimmer3 SD file correctly - modulo and sync blocks Two defects that between them made ShimmerSDLog unable to read a Shimmer3 recording at all. Folded in here because the DEV-1023 zero-timestamp fix is scoped to the 3-byte counter, so it could never fire on the SD path while the first of these stood: the drop loop added for it was dead code. 1. The timestamp modulo was left at ShimmerDevice's 2-byte default of 65536. It is only ever set by UpdateBasedOnCompatibilityCode(), which runs on the Bluetooth connect path; ShimmerSDLog hard-set the compatibility code and byte size inside its Shimmer3R branch only, so a Shimmer3 file got neither. A 3-byte counter unwrapped against a 2-byte modulo adds 65536 per roll-over instead of 2^24, and the recording reads as a NEGATIVE duration. Now derived from the firmware version for every hardware version, the same way the Bluetooth path derives it. The hard-coded pair is gone: nothing distinguishes compatibility code 8 from 9, only >= 6 and == 1 are tested. 2. Sync-when-logging blocks were not handled. The firmware heads each 512-byte write buffer with a 9-byte offset record; that was being decoded as sample data, so the record stream lost alignment at the first block and never recovered. The flag itself was not even read - both header lines that would have set it are inside comment blocks, and no such setter exists. The flag is now read from trial config 0 bit 2, ahead of the per-hardware branches because the byte means the same in both header layouts. The offset record is read out of the stream at each block boundary and kept, rather than being given channels of its own: the Java importer carries it as a TIMESTAMP_OFFSET channel because Consensys aligns multiple devices with it, and this API has nothing that consumes it. LastSyncOffset() exposes the raw nine bytes for a caller that wants them. Also exposes IsSyncWhenLogging(), SamplesPerBlock() and SampleRecordSize() so a caller can check what it is about to read, and drops a dead local that the compiler had been warning about. ShimmerSDLogParseTest builds a Shimmer3 LogAndStream file by hand - 256-byte header, 21-byte records, 3000 of them starting near 0xFFF000 so the file crosses a real roll-over, with and without sync blocks, two records planted with a zero timestamp. Mirrors ADV_API_00031 in the Java driver, which reads the same files. Both defects were mutation-checked: reverting the modulo derivation fails 4 of the 5 cases, reverting the block handling fails 2. Co-Authored-By: Claude Opus 5 --- ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs | 109 +++++- .../ShimmerUnitTests/ShimmerSDLogParseTest.cs | 330 ++++++++++++++++++ .../ShimmerUnitTests/ShimmerUnitTests.csproj | 1 + 3 files changed, 431 insertions(+), 9 deletions(-) create mode 100644 ShimmerAPI/ShimmerUnitTests/ShimmerSDLogParseTest.cs diff --git a/ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs b/ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs index 52d40085..4ecfaa2a 100644 --- a/ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs +++ b/ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs @@ -78,6 +78,25 @@ public static class SDLogHeader private string MacAddress = ""; private long mSampleCount = 0; public Boolean EndOfFile = false; + + /// + /// Bytes in the sync offset record that heads each SD write buffer when the + /// trial was logged with sync on. Nine on every firmware this class reads; + /// the Java importer calls the same field a "u72". + /// + private const int OffsetLength = 9; + + /// + /// The firmware's SD write buffer. It decides how many sample records sit + /// between one offset record and the next, so it has to match + /// SD_WRITE_BUF_SIZE in the firmware rather than any host-side buffer. + /// + private const int SdWriteBufferSize = 512; + + private bool mSyncWhenLogging = false; + private int mSamplesPerBlock = 0; + private int mSampleCountInBlock = 0; + private byte[] mLastSyncOffset = null; protected override bool ShouldAddSystemTimestamp => false; public ShimmerSDLog(string filePath) { @@ -218,6 +237,11 @@ public void ProcessSDLogHeader(byte[] byteArrayInfo) ExpansionBoardRev = byteArrayInfo[215]; ExpansionBoardRevSpecial = byteArrayInfo[216]; + // Trial config 0, bit 2. Read here, ahead of the per-hardware branches, + // because the byte means the same thing in both header layouts and the + // record reader needs it whichever board wrote the file. + mSyncWhenLogging = ((byteArrayInfo[16] >> 2) & 0x01) == 1; + if (HardwareVersion == (int) ShimmerVersion.SHIMMER3R) { @@ -528,10 +552,17 @@ public void ProcessSDLogHeader(byte[] byteArrayInfo) // NOTE: same memory caveat as Java initializeAlgorithms(); */ - if (signalIdArray != null && HardwareVersion == (int)ShimmerVersion.SHIMMER3R) + // Derive the timestamp width and its modulo from the firmware version, + // the same way the Bluetooth path does on connect. Without this a Shimmer3 + // file keeps ShimmerDevice's 2-byte default of 65536 while carrying a 3-byte + // counter, so every roll-over adds 65536 instead of 2^24 and the recording + // reads as a negative duration. Replaces a hard-coded compatibility code + // that only the Shimmer3R branch used to set. + SetCompatibilityCode(); + UpdateBasedOnCompatibilityCode(); + + if (signalIdArray != null && HardwareVersion == (int)ShimmerVersion.SHIMMER3R) { - CompatibilityCode = 9; - TimeStampPacketByteSize = 3; InterpretDataPacketFormat(NumberofChannels, signalIdArray); } else @@ -539,6 +570,14 @@ public void ProcessSDLogHeader(byte[] byteArrayInfo) InterpretDataPacketFormat(); } + // How many sample records follow each offset record. Derived from the + // firmware's write buffer rather than from the file, so it is known before + // the first record is read. + mSamplesPerBlock = (mSyncWhenLogging && PacketSize > 0) + ? (SdWriteBufferSize - OffsetLength) / PacketSize + : 0; + mSampleCountInBlock = 0; + } @@ -579,26 +618,78 @@ public ObjectCluster ReadPacketMsg() /// public int RejectedTimestampRowCount { get; private set; } + /// + /// Bytes in one sample record, as the enabled-sensor bitmap in the header + /// describes it. Excludes the offset record, which is not sample data. + /// + public int SampleRecordSize() + { + return PacketSize; + } + + /// True when the trial was logged with sync on, so the file is laid + /// out in blocks headed by an offset record. + public bool IsSyncWhenLogging() + { + return mSyncWhenLogging; + } + + /// + /// Sample records between one offset record and the next; zero when the trial + /// was not synced and the records are contiguous. + /// + public int SamplesPerBlock() + { + return mSamplesPerBlock; + } + + /// + /// The most recent offset record read, or null. All-0xFF means the node never + /// recorded an offset, which is what an unsynced node in a synced trial + /// writes. Returned raw: it is nine bytes and nothing here consumes it. + /// + public byte[] LastSyncOffset() + { + return mLastSyncOffset == null ? null : (byte[])mLastSyncOffset.Clone(); + } + /// One physical record, whether or not it is usable. private ObjectCluster ReadOnePacketMsg() { - int fullPacketSize; - - // indicates when there will be an offset value - bool timeSync = false; + // Sync-when-logging puts an offset record at the head of each write + // buffer. It is not sample data - read it out of the way so the records + // after it stay aligned. Skipping it shifts the stream nine bytes at + // every block boundary and nothing downstream decodes. + if (mSamplesPerBlock > 0 && mSampleCountInBlock == 0) + { + byte[] offsetBytes = new byte[OffsetLength]; + if (ReadFromLog(offsetBytes) != OffsetLength) + { + EndOfFile = true; + return null; + } + mLastSyncOffset = offsetBytes; + } - fullPacketSize = PacketSize; + int fullPacketSize = PacketSize; byte[] newPacket = new byte[fullPacketSize]; if (ReadFromLog(newPacket) != 0) { mSampleCount++; + if (mSamplesPerBlock > 0) + { + mSampleCountInBlock = (mSampleCountInBlock + 1) % mSamplesPerBlock; + } // Java: super.buildMsg(newPacket, COMMUNICATION_TYPE.SD, timeSync, -1); ObjectCluster ojc = base.BuildMsg(newPacket.ToList()); - if (mTrackBytesRead == FileSize) + // >= rather than ==: with offset records the byte count no longer + // lands on a whole number of sample records, and a file with trailing + // padding would otherwise never report the end. + if (mTrackBytesRead >= FileSize) { EndOfFile = true; } diff --git a/ShimmerAPI/ShimmerUnitTests/ShimmerSDLogParseTest.cs b/ShimmerAPI/ShimmerUnitTests/ShimmerSDLogParseTest.cs new file mode 100644 index 00000000..a671cef2 --- /dev/null +++ b/ShimmerAPI/ShimmerUnitTests/ShimmerSDLogParseTest.cs @@ -0,0 +1,330 @@ +using System; +using System.Collections.Generic; +using System.IO; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using ShimmerAPI; + +namespace ShimmerBluetoothTests +{ + /// + /// Builds a Shimmer3 LogAndStream SD file by hand and reads it back through + /// . + /// + /// Covers three things that were each independently broken, and which only show + /// up together on a real recording: + /// + /// + /// the 3-byte timestamp was unwrapped against a 2-byte modulo, so every + /// roll-over added 65536 instead of 2^24 and a recording read as a negative + /// duration; + /// sync-when-logging blocks were not handled, so the 9-byte offset record + /// at the head of each write buffer was consumed as sample data and the record + /// stream lost alignment from the first block onwards; + /// records the firmware never stamped were read as roll-overs, adding + /// 512 seconds each. + /// + /// + /// Mirrors ADV_API_00031_ShimmerSDLogZeroTimestampTest in the Java driver. Both + /// read the same files, so they have to agree. + /// + /// + [TestClass] + public class ShimmerSDLogParseTest + { + private const int HeaderSize = 256; + /// 3-byte timestamp + gyro(6) + wide-range accel(6) + mag(6). + private const int RowBytes = 21; + /// 32768 / 65 = 504.123 Hz, the rate the field evidence is in. + private const int PeriodTicks = 65; + private const int SampleRateDivider = 65; + private const int SyncHeaderBytes = 9; + private const int SdWriteBufSize = 512; + private const double TicksPerSecond = 32768.0; + + private const int RowCount = 3000; + /// Near the top of the 24-bit range, so the file contains a real wrap. + private const long FirstTick = 0xFFF000L; + + private static byte[] BuildHeader(bool syncWhenLogging) + { + byte[] h = new byte[HeaderSize]; + + // 0-1: sampling rate divider, little-endian + h[0] = (byte)(SampleRateDivider & 0xFF); + h[1] = (byte)((SampleRateDivider >> 8) & 0xFF); + h[2] = 1; // buffer size + + // 3-7: enabled sensors. 0x60 = gyro | mag, 0x10 (byte 4) = wide-range accel + h[3] = 0x60; + h[4] = 0x10; + + // 16: trial config 0, bit 2 = sync when logging + h[16] = (byte)(syncWhenLogging ? 0x04 : 0x00); + h[17] = 0; + h[18] = 54; // broadcast interval + + // 24-29: MAC + h[24] = 0x00; h[25] = 0x06; h[26] = 0x66; + h[27] = 0x80; h[28] = 0xE0; h[29] = 0xE1; + + // 30-31: hardware version - Shimmer3 + h[30] = 0x00; h[31] = 0x03; + h[32] = 1; // trial id + h[33] = 1; // number of shimmers + + // 34-39: LogAndStream v1.01.003 + h[34] = 0x00; h[35] = 0x03; + h[36] = 0x00; h[37] = 0x01; + h[38] = 0x01; + h[39] = 0x03; + + // 214-216: expansion board, SR31-6-0 + h[214] = 31; h[215] = 6; h[216] = 0; + + // 251, 252-255: initial timestamp, split as the firmware writes it + long initialTs = FirstTick; + h[251] = (byte)((initialTs >> 32) & 0xFF); + h[252] = (byte)(initialTs & 0xFF); + h[253] = (byte)((initialTs >> 8) & 0xFF); + h[254] = (byte)((initialTs >> 16) & 0xFF); + h[255] = (byte)((initialTs >> 24) & 0xFF); + + return h; + } + + private static void WriteRow(Stream outStream, long ticks, int seq) + { + byte[] row = new byte[RowBytes]; + row[0] = (byte)(ticks & 0xFF); + row[1] = (byte)((ticks >> 8) & 0xFF); + row[2] = (byte)((ticks >> 16) & 0xFF); + // The sensor payload is irrelevant here, but make it non-constant so a + // parser reading the wrong offset would not look plausible. + for (int i = 3; i < RowBytes; i++) + { + row[i] = (byte)((seq + i) & 0xFF); + } + outStream.Write(row, 0, row.Length); + } + + /// rows whose timestamp field is written as 00 00 00 + private static string BuildFile(bool syncWhenLogging, int[] badRowIndexes) + { + // The reader takes the last three characters of the path as the SD file + // number, so give it a name shaped like one the firmware writes. + string dir = Path.Combine(Path.GetTempPath(), "shimmer_sdlog_" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(dir); + string path = Path.Combine(dir, "000"); + + int rowsPerBlock = (SdWriteBufSize - (syncWhenLogging ? SyncHeaderBytes : 0)) / RowBytes; + + using (var outStream = new FileStream(path, FileMode.Create, FileAccess.Write)) + { + byte[] header = BuildHeader(syncWhenLogging); + outStream.Write(header, 0, header.Length); + + for (int i = 0; i < RowCount; i++) + { + if (syncWhenLogging && (i % rowsPerBlock) == 0) + { + // Each write buffer opens with the node's sync offset. 0xFF + // means "no offset recorded", which is what an unsynced node + // writes. + byte[] syncHeader = new byte[SyncHeaderBytes]; + for (int j = 0; j < SyncHeaderBytes; j++) + { + syncHeader[j] = 0xFF; + } + outStream.Write(syncHeader, 0, syncHeader.Length); + } + + bool bad = Array.IndexOf(badRowIndexes, i) >= 0; + long ticks = (FirstTick + ((long)i * PeriodTicks)) & 0xFFFFFFL; + WriteRow(outStream, bad ? 0L : ticks, i); + } + } + + return path; + } + + private static void DeleteFile(string path) + { + try + { + Directory.Delete(Path.GetDirectoryName(path), true); + } + catch (IOException) + { + // A temp file left behind must not fail a test. + } + } + + /// Calibrated timestamps, in milliseconds, for every record returned. + private static List ReadAllTimestamps(ShimmerSDLog sdLog) + { + var timestamps = new List(); + while (!sdLog.EndOfFile) + { + ObjectCluster ojc = sdLog.ReadPacketMsg(); + if (ojc == null) + { + break; + } + SensorData ts = ojc.GetData( + ShimmerConfiguration.SignalNames.TIMESTAMP, + ShimmerConfiguration.SignalFormats.CAL); + Assert.IsNotNull(ts, "every record carries a calibrated timestamp"); + timestamps.Add(ts.Data); + } + return timestamps; + } + + private static void AssertParsesCleanly(bool syncWhenLogging) + { + int[] bad = { 500, 1500 }; + string path = BuildFile(syncWhenLogging, bad); + try + { + var sdLog = new ShimmerSDLog(path); + + Assert.AreEqual(syncWhenLogging, sdLog.IsSyncWhenLogging(), + "the sync flag is read out of the header"); + Assert.AreEqual(RowBytes, sdLog.SampleRecordSize(), + "gyro + wide-range accel + mag at 3-byte timestamps is a 21-byte record"); + if (syncWhenLogging) + { + Assert.AreEqual((SdWriteBufSize - SyncHeaderBytes) / RowBytes, sdLog.SamplesPerBlock(), + "23 records fit between one offset record and the next"); + } + else + { + Assert.AreEqual(0, sdLog.SamplesPerBlock(), "no blocks without sync"); + } + + List timestamps = ReadAllTimestamps(sdLog); + + Assert.AreEqual(RowCount - bad.Length, timestamps.Count, + "every record except the bad ones is returned"); + Assert.AreEqual(bad.Length, sdLog.RejectedTimestampRowCount, + "and the drops are reported"); + + double periodMs = PeriodTicks / TicksPerSecond * 1000.0; + double maxStep = 0; + for (int i = 1; i < timestamps.Count; i++) + { + double step = timestamps[i] - timestamps[i - 1]; + Assert.IsTrue(step > 0, "timestamps are strictly increasing at record " + i); + maxStep = Math.Max(maxStep, step); + } + + // A missed wrap, or a wrap unwrapped against the wrong modulo, shows + // up here as a step of thousands of milliseconds. + Assert.IsTrue(maxStep < 4 * periodMs, + "no step anywhere near a modulo (largest was " + maxStep + " ms)"); + + // First to last: RowCount samples one period apart, and the two that + // were dropped were in the middle, so the span is unchanged. + double expectedSpanMs = (RowCount - 1) * periodMs; + Assert.AreEqual(expectedSpanMs, timestamps[timestamps.Count - 1] - timestamps[0], 0.001, + "the recording spans exactly the samples it took"); + } + finally + { + DeleteFile(path); + } + } + + [TestMethod] + public void TestZeroTimestampRowsAreDropped() + { + AssertParsesCleanly(false); + } + + /// Sync-when-logging puts a 9-byte offset record at the head of each block. + [TestMethod] + public void TestZeroTimestampRowsAreDroppedWithSyncBlocks() + { + AssertParsesCleanly(true); + } + + /// A file with no bad records must be completely unaffected. + [TestMethod] + public void TestCleanFileIsUnchanged() + { + string path = BuildFile(false, new int[0]); + try + { + var sdLog = new ShimmerSDLog(path); + List timestamps = ReadAllTimestamps(sdLog); + + Assert.AreEqual(RowCount, timestamps.Count, "every record is returned"); + Assert.AreEqual(0, sdLog.RejectedTimestampRowCount, "nothing was dropped"); + + double periodMs = PeriodTicks / TicksPerSecond * 1000.0; + for (int i = 1; i < timestamps.Count; i++) + { + Assert.AreEqual(periodMs, timestamps[i] - timestamps[i - 1], 0.001, + "one sample period between consecutive records"); + } + } + finally + { + DeleteFile(path); + } + } + + /// + /// The defect this file was built to catch: a 3-byte counter unwrapped against + /// a 2-byte modulo. The file crosses 0xFFFFFF once, so a wrong modulo shows up + /// as a duration that runs backwards. + /// + [TestMethod] + public void TestRecordingDurationIsPositiveAcrossARollOver() + { + string path = BuildFile(false, new int[0]); + try + { + var sdLog = new ShimmerSDLog(path); + List timestamps = ReadAllTimestamps(sdLog); + + double spanMs = timestamps[timestamps.Count - 1] - timestamps[0]; + Assert.IsTrue(spanMs > 0, "a recording cannot have a negative duration"); + Assert.AreEqual(RowCount * PeriodTicks / TicksPerSecond, spanMs / 1000.0, 0.01, + "and it lasts as long as the samples it holds"); + } + finally + { + DeleteFile(path); + } + } + + /// + /// The offset record is read out of the stream rather than decoded as sample + /// data, and is available to a caller that wants it. + /// + [TestMethod] + public void TestSyncOffsetRecordIsReadNotDecoded() + { + string path = BuildFile(true, new int[0]); + try + { + var sdLog = new ShimmerSDLog(path); + Assert.IsNull(sdLog.LastSyncOffset(), "nothing read yet"); + + sdLog.ReadPacketMsg(); + byte[] offset = sdLog.LastSyncOffset(); + + Assert.IsNotNull(offset, "the first record of a block is preceded by one"); + Assert.AreEqual(SyncHeaderBytes, offset.Length); + foreach (byte b in offset) + { + Assert.AreEqual(0xFF, b, "this file records no offset, so all bytes are 0xFF"); + } + } + finally + { + DeleteFile(path); + } + } + } +} diff --git a/ShimmerAPI/ShimmerUnitTests/ShimmerUnitTests.csproj b/ShimmerAPI/ShimmerUnitTests/ShimmerUnitTests.csproj index 46798ada..32b8a462 100644 --- a/ShimmerAPI/ShimmerUnitTests/ShimmerUnitTests.csproj +++ b/ShimmerAPI/ShimmerUnitTests/ShimmerUnitTests.csproj @@ -62,6 +62,7 @@ + From e13241aecf2620e42355d38712c4656cb02dbdd8 Mon Sep 17 00:00:00 2001 From: Mark Nolan Date: Thu, 17 Sep 2026 12:32:58 +0100 Subject: [PATCH 4/6] DEV-1023: classify reordered and duplicated packets, not just invalid zeros Mirrors the Java driver, which this API has to agree with - the two read the same recordings. The exact-zero rejection already here is right for the firmware fault it was written for and wrong for everything else that makes the counter read backwards: a corrupted non-zero value, a duplicated packet and a reordered one all still added 2^24 ticks. Each sample is now classified by its modular forward distance from the last - duplicate, reordered, invalid zero, or forward - with forward, which is a wrap when the raw value fell, as the default everything else falls through to. Three things in that are load-bearing, each verified by mutation: - The comparison is modular, not on unwrapped values. Asking whether the new candidate is below the last one misses a packet arriving late from BEFORE a wrap boundary: its candidate sits nearly a modulo ahead, so it is accepted, and the next real sample is read as a second wrap. - The window is eight sample periods, not a fraction of the modulo. A reorder swaps adjacent packets; a dropout spanning the wrap point is most of a modulo. At modulo/8 on the 2-byte counter every dropout between 1.75 s and 2.0 s reads as a reorder and the wrap is silently lost. - An unknown rate gives a window of zero, never infinity. 32768/0 is PositiveInfinity in C#, and an infinite window makes every backward step a reorder and loses every wrap - worse than the naive rule being replaced. SamplingRate defaults to 0, so that is the normal state until an inquiry or an SD header has run. Shimmer2 and Shimmer2R get a window of zero. Their tick domain is unsettled - this API divides their 16-bit counter by 1024 while the Java driver divides by 32768 - so a rate-derived window would be wrong in one of the two. The first sample of a stream is passed through rather than measured against the reset state, which would otherwise let a first raw value near the top of the range read as a packet reordered across a boundary. Two callers beyond ShimmerDevice: - TestSerialPort supplies its own hard-coded rate. - ShimmerCaptureXamarin is a vendored fork, not built alongside ShimmerAPI, so it cannot reference TimestampUnwrap and carries the rule inline. It is flagged as uncompiled and untested here; keeping the two in step is manual. Also fixes the SD header rate, which fed none of this before and feeds the window now: 32768 / rawSamplingRate had two integral operands, so it was integer division - a divider of 65 gave 504 Hz rather than 504.123, and 640 gave 51 rather than 51.2 - and a divider of zero threw DivideByZeroException out of the constructor, taking the whole import with it. Rule and vectors: log-and-stream-common, docs/SHIMMER3_STREAMING_DATA_FORMAT.md section 2.1 and Test/conformance/timestamp_unwrap.json. Co-Authored-By: Mas Azalya <43565312+MAzalya@users.noreply.github.com> Co-Authored-By: Claude Opus 5 --- ShimmerAPI/ShimmerAPI/ShimmerDevice.cs | 32 ++- ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs | 11 +- ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs | 192 ++++++++++++++---- ShimmerAPI/TestSerialPort/Program.cs | 3 +- .../ShimmerCaptureXamarin/ShimmerBluetooth.cs | 75 ++++++- 5 files changed, 267 insertions(+), 46 deletions(-) diff --git a/ShimmerAPI/ShimmerAPI/ShimmerDevice.cs b/ShimmerAPI/ShimmerAPI/ShimmerDevice.cs index 8d215535..594215e8 100644 --- a/ShimmerAPI/ShimmerAPI/ShimmerDevice.cs +++ b/ShimmerAPI/ShimmerAPI/ShimmerDevice.cs @@ -38,12 +38,42 @@ public enum ShimmerVersion SHIMMER3R = 10, SHIMMER4SDK = 58 } + /// + /// How far behind its predecessor a sample may sit and still be read as a + /// reordered packet rather than a counter roll-over. See + /// for why it is sized in + /// sample periods, and why an unknown rate must give zero rather than an + /// infinite window. + /// + /// Derived on every sample rather than cached, so a rate written mid-session is + /// picked up by the next one and there is no stale window to reset. + /// defaults to zero, which disables the branch until + /// an inquiry or an SD header has set it. + /// + /// + /// Zero - reorder detection off - on Shimmer2 and Shimmer2R. Their tick domain + /// is not settled: this class divides their 16-bit counter by 1024 while the + /// Java driver divides by 32768, so a window derived from the rate would be + /// wrong in one of the two. Those devices keep the behaviour they have always + /// had; the invalid-zero rule never applied to a 2-byte counter anyway. + /// + /// + protected virtual double GetReorderWindowTicks() + { + if (HardwareVersion == (int)ShimmerVersion.SHIMMER2 || HardwareVersion == (int)ShimmerVersion.SHIMMER2R) + { + return 0.0; + } + return TimestampUnwrap.ReorderWindowTicks(SamplingRate, TimeStampPacketRawMaxValue); + } + protected double CalibrateTimeStamp(double timeStamp) { //first convert to continuous time stamp double calibratedTimeStamp = 0; TimestampUnwrap.Result unwrapped = TimestampUnwrap.Unwrap( - timeStamp, LastReceivedTimeStamp, CurrentTimeStampCycle, TimeStampPacketRawMaxValue); + timeStamp, LastReceivedTimeStamp, CurrentTimeStampCycle, TimeStampPacketRawMaxValue, + GetReorderWindowTicks()); LastTimestampRejected = unwrapped.Rejected; CurrentTimeStampCycle = unwrapped.Cycle; diff --git a/ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs b/ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs index 4ecfaa2a..7bcf71bc 100644 --- a/ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs +++ b/ShimmerAPI/ShimmerAPI/ShimmerSDLog.cs @@ -227,9 +227,16 @@ public void ProcessSDLogHeader(byte[] byteArrayInfo) // 0-1 Byte = Sampling Rate (little-endian per original Java expression) long rawSamplingRate = byteArrayInfo[0] | (byteArrayInfo[1] << 8); - - SamplingRate = (32768 / rawSamplingRate); + // 32768.0, not 32768: both operands were integral, so this was integer + // division and the rate came out truncated - a divider of 65 gave 504 Hz + // rather than 504.123, and 640 gave 51 rather than 51.2. The error is small + // but it feeds the reorder window and any rate the caller reads back. + // + // The guard is for a blank or truncated header: a divider of zero threw + // DivideByZeroException here, taking out the whole import. Zero means "not + // known", which every rate consumer already handles. + SamplingRate = rawSamplingRate == 0 ? 0.0 : 32768.0 / rawSamplingRate; ParseEnabledDerivedSensorsForMaps(byteArrayInfo); diff --git a/ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs b/ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs index 67fe8f81..11a7c78b 100644 --- a/ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs +++ b/ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs @@ -8,35 +8,53 @@ namespace ShimmerAPI /// The counter is 3 bytes at 32768 Hz on current firmware, so it returns to zero /// every 512 seconds exactly, and a host has to add a whole modulo back each time /// it does. The obvious rule - if this sample reads lower than the last one, a - /// wrap happened - is what every Shimmer host API has implemented, and it is - /// wrong for one input: a record whose timestamp field is exactly zero. + /// wrap happened - is what every Shimmer host API implemented, and it is wrong + /// three times over: on an out-of-order packet, on a record the firmware never + /// stamped, and on a packet arriving late from before a wrap boundary. /// /// + /// Each sample is classified by its modular forward distance from the + /// previous one, with forward motion as the default: + /// + /// + /// Duplicate - the same raw value again. The timeline holds. + /// Reordered - no further back than + /// . Placed where it was actually taken, which is + /// below its predecessor. The output is deliberately not monotonic: a late packet + /// belongs at the time it was sampled, not at the time it arrived. + /// Invalid - the 3-byte counter, a raw value of exactly zero, and a + /// predecessor more than below the top of the range. /// Firmware stamps a packet when the sample tick starts it and does not publish a - /// packet it never stamped, so 0x000000 in that field means the record is invalid, - /// not that the counter reached its origin. LogAndStream v1.00.x-v1.01.003 could - /// emit one under SD write back-pressure. Read as a wrap, a single such record - /// makes every later sample in the recording 512 seconds late - which is how a - /// 9 minute 30 second trial came back as 43 minutes 38. + /// packet it never stamped, so 0x000000 means the record is invalid, not + /// that the counter reached its origin. LogAndStream v1.00.x-v1.01.003 could emit + /// one under SD write back-pressure. Read as a wrap, a single such record makes + /// every later sample in the recording 512 seconds late - which is how a 9 minute + /// 30 second trial came back as 43 minutes 38. The timeline holds, the wrap count + /// is left alone and is set. + /// Forward - everything else, which is a wrap when the raw value + /// fell. This is the default, and that matters: a wrap preceded by a long + /// dropout is still a wrap, however much was lost before it. + /// + /// + /// Two details are easy to get wrong and are load-bearing here. /// /// - /// So: a backward step is a roll-over unless the counter is the 3-byte one, the - /// new value is exactly zero, and the previous value was more than - /// below the top of the range. In that case the - /// sample is rejected - the caller is handed the previous timestamp back and the - /// wrap count is left alone. + /// The comparison is on modular distance, not on unwrapped values. Asking + /// whether the new unwrapped candidate is below the last one misses a packet that + /// arrives late from before a wrap boundary: its candidate sits nearly a + /// whole modulo ahead, so it is accepted, and the next real sample is then read as + /// a second wrap. The sequence 16777206, 5, 16777206, 70 is the smallest case, and + /// it costs 512 seconds twice over. /// /// - /// The exemption is deliberately narrow. A genuine wrap that happens to land on - /// zero still has a predecessor near the top of the range, so it is accepted. A - /// 2-byte counter (older firmware, 2 second range, where a stall really can - /// exceed a second) is never rejected. And rejection cannot cascade: the following - /// sample reads above the retained previous value, so it is accepted normally and - /// the timeline carries on at its true spacing. + /// The reorder window is sized in sample periods, not as a fraction of the + /// modulo - see . /// /// - /// This mirrors TimestampUnwrap in the Java driver deliberately. The two must - /// agree: the same recording is read by both. + /// This mirrors TimestampUnwrap in the Java driver deliberately, and both are + /// checked against the same conformance vectors. The rule is specified in + /// log-and-stream-common, docs/SHIMMER3_STREAMING_DATA_FORMAT.md section 2.1; the + /// vectors are run against this class by TimestampUnwrapVectorsTest. /// /// public static class TimestampUnwrap @@ -50,44 +68,142 @@ public static class TimestampUnwrap /// public const int WrapWindowTicks = 32768; + /// Sample periods a packet may lag its predecessor and still be a reorder. + public const int ReorderPeriods = 8; + + /// + /// The clock the packet tick counter runs on. Not the sampling clock, which is + /// 312500 or 255765.625 Hz on a TCXO board - see . + /// + public const double RtcTicksPerSecond = 32768.0; + + /// The reorder window is never allowed past this fraction of the modulo. + public const int MaxWindowDivisor = 8; + /// Outcome of unwrapping one sample. public struct Result { /// The unwrapped tick count to use. On a rejected sample, the previous one. public double Unwrapped; - /// Wrap count after this sample. Unchanged when the sample was rejected. + /// + /// Wrap count after this sample: floor(Unwrapped / maxTicks). Unchanged when + /// the sample was rejected, and one lower than its predecessor's for a packet + /// that arrived late from before a wrap boundary. + /// public double Cycle; /// True when the raw value was an invalid zero rather than a real wrap. public bool Rejected; } + /// + /// How far back a sample may be from its predecessor and still be treated as a + /// reordered packet rather than a wrap. + /// + /// Sized in sample periods, because that is what distinguishes the two cases: a + /// reorder swaps packets that are adjacent in time - a handful of periods - + /// whereas a dropout that spans the counter's wrap point is most of a modulo. + /// Sizing the window as a fraction of the modulo confuses them. At + /// modulo / 8 on the 2-byte counter, every dropout between 1.75 and 2.0 + /// seconds reads as a reorder and the wrap is silently lost - and a 1.75 second + /// Bluetooth gap is ordinary. Eight periods shrinks the band in which that can + /// happen to about 16 milliseconds. + /// + /// + /// The rate must be in the domain. On a TCXO + /// board the sampling clock is 312500 or 255765.625 Hz, and deriving the window + /// from that would widen it by roughly nine and a half times. + /// + /// + /// Returns zero - the branch disabled - when the rate is not known. Never + /// guess: 32768 / 0 is in C#, and an + /// infinite window classifies every backward step as a reorder and loses every + /// wrap, which is worse than the naive rule this replaces. Rejecting an unstamped + /// record needs no rate, so that still works with the window at zero. + /// + /// + /// the configured rate, in Hz; zero, negative, NaN + /// or infinite all mean "not known" + /// the counter's modulo + /// the window in ticks, clamped so that it can never reach the modulo + /// and leave no backward step large enough to be read as a wrap + public static double ReorderWindowTicks(double samplingRateHz, int maxTicks) + { + if (double.IsNaN(samplingRateHz) || double.IsInfinity(samplingRateHz) || samplingRateHz <= 0.0) + { + return 0.0; + } + double window = ReorderPeriods * RtcTicksPerSecond / samplingRateHz; + return Math.Min(window, (double)maxTicks / MaxWindowDivisor); + } + + /// + /// Unwraps one sample with the reorder branch disabled. Equivalent to passing a + /// window of zero; kept so that a caller with no rate to hand still compiles. + /// + public static Result Unwrap(double rawTicks, double lastUnwrapped, double cycle, int maxTicks) + { + return Unwrap(rawTicks, lastUnwrapped, cycle, maxTicks, 0.0); + } + /// the packet's raw tick value /// the unwrapped value returned for the previous sample /// how many wraps have been counted so far /// the counter's modulo - 2^24, or 2^16 on old firmware - public static Result Unwrap(double rawTicks, double lastUnwrapped, double cycle, int maxTicks) + /// from ; zero + /// disables reorder detection + public static Result Unwrap(double rawTicks, double lastUnwrapped, double cycle, int maxTicks, + double reorderWindowTicks) { - double candidate = rawTicks + (maxTicks * cycle); - - if (lastUnwrapped > candidate) + if (lastUnwrapped == 0.0 && cycle == 0.0) { - // The counter went backwards. Either it wrapped, or this record is bad. - double lastRaw = lastUnwrapped - (maxTicks * cycle); + // Nothing has been unwrapped yet, so there is no predecessor to measure + // against. Taking the reset state as a real sample at zero would let a + // first raw value near the top of the range read as a packet reordered + // across a boundary, placing a whole recording one modulo early. + return new Result { Unwrapped = rawTicks, Cycle = 0.0, Rejected = false }; + } - if (maxTicks == TicksMax3Byte - && rawTicks == 0.0 - && lastRaw < (maxTicks - WrapWindowTicks)) - { - // Mid-range and then exactly zero: a record the firmware never - // stamped. Hold the timeline where it was and say so. - return new Result { Unwrapped = lastUnwrapped, Cycle = cycle, Rejected = true }; - } + double lastRaw = lastUnwrapped - (maxTicks * cycle); + double forward = rawTicks - lastRaw; + if (forward < 0) + { + forward += maxTicks; + } + double backwards = maxTicks - forward; - cycle += 1; - candidate = rawTicks + (maxTicks * cycle); + double candidate; + if (forward == 0.0) + { + // The same value again: a duplicate. Hold the timeline where it is. + candidate = lastUnwrapped; + } + else if (backwards <= reorderWindowTicks) + { + // Reordered, on either side of a wrap boundary. + candidate = lastUnwrapped - backwards; + } + else if (maxTicks == TicksMax3Byte + && rawTicks == 0.0 + && lastRaw < (maxTicks - WrapWindowTicks)) + { + // Mid-range and then exactly zero: a record the firmware never stamped. + // Hold the timeline where it was and say so. Nothing about this sample + // moves the state, so the next real one is an ordinary step forward and + // the rejection cannot cascade. + return new Result { Unwrapped = lastUnwrapped, Cycle = cycle, Rejected = true }; + } + else + { + // Forward motion, which is a wrap when the raw value fell. + candidate = lastUnwrapped + forward; } - return new Result { Unwrapped = candidate, Cycle = cycle, Rejected = false }; + return new Result + { + Unwrapped = candidate, + Cycle = Math.Floor(candidate / maxTicks), + Rejected = false + }; } } } diff --git a/ShimmerAPI/TestSerialPort/Program.cs b/ShimmerAPI/TestSerialPort/Program.cs index c0f25050..76947a93 100644 --- a/ShimmerAPI/TestSerialPort/Program.cs +++ b/ShimmerAPI/TestSerialPort/Program.cs @@ -110,7 +110,8 @@ protected static double CalibrateTimeStamp(double timeStamp) //Shared with the API rather than copied: a copy of this rule is how the //same defect ended up in four Shimmer host APIs at once. TimestampUnwrap.Result unwrapped = TimestampUnwrap.Unwrap( - timeStamp, LastReceivedTimeStamp, CurrentTimeStampCycle, (int)TimeStampPacketRawMaxValue); + timeStamp, LastReceivedTimeStamp, CurrentTimeStampCycle, (int)TimeStampPacketRawMaxValue, + TimestampUnwrap.ReorderWindowTicks(SamplingRate, (int)TimeStampPacketRawMaxValue)); LastTimestampRejected = unwrapped.Rejected; CurrentTimeStampCycle = unwrapped.Cycle; LastReceivedTimeStamp = unwrapped.Unwrapped; diff --git a/ShimmerCaptureXamarin/ShimmerCaptureXamarin/ShimmerBluetooth.cs b/ShimmerCaptureXamarin/ShimmerCaptureXamarin/ShimmerBluetooth.cs index afe26f9e..b51f1487 100644 --- a/ShimmerCaptureXamarin/ShimmerCaptureXamarin/ShimmerBluetooth.cs +++ b/ShimmerCaptureXamarin/ShimmerCaptureXamarin/ShimmerBluetooth.cs @@ -5498,24 +5498,91 @@ public void WriteEXGRate(byte exgRate) } } + /// + /// NOT COMPILED IN THIS WORKSPACE. This project is a vendored fork and is not + /// built alongside ShimmerAPI, so it cannot reference ShimmerAPI's + /// TimestampUnwrap and carries the rule inline instead. Keep the two in step by + /// hand: ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs is the original, and the rule + /// is specified in log-and-stream-common, + /// docs/SHIMMER3_STREAMING_DATA_FORMAT.md section 2.1, with conformance vectors + /// the other host APIs run against. This copy has no test covering it. + /// protected double CalibrateTimeStamp(double timeStamp) { //first convert to continuous time stamp double calibratedTimeStamp = 0; - if (LastReceivedTimeStamp > (timeStamp + (TimeStampPacketRawMaxValue * CurrentTimeStampCycle))) + + // Classify the sample by its modular forward distance from the last one, + // with forward motion as the default. The naive rule this replaces - any + // backward step is a roll-over - is wrong three times over: on an + // out-of-order packet, on a record the firmware never stamped, and on a + // packet arriving late from before a wrap boundary. + double maxTicks = TimeStampPacketRawMaxValue; + bool isShimmer2 = HardwareVersion == (int)ShimmerBluetooth.ShimmerVersion.SHIMMER2 + || HardwareVersion == (int)ShimmerBluetooth.ShimmerVersion.SHIMMER2R; + // Eight sample periods, clamped. Zero when the rate is unknown, and zero on + // Shimmer2, whose tick domain the two APIs disagree about. Never derive an + // infinite window from a rate of zero: that would make every backward step a + // reorder and lose every wrap. + double reorderWindow = (isShimmer2 || !(SamplingRate > 0)) + ? 0.0 + : Math.Min(8 * 32768.0 / SamplingRate, maxTicks / 8.0); + bool rejected = false; + + if (LastReceivedTimeStamp == 0 && CurrentTimeStampCycle == 0) + { + // No predecessor yet. Treating the reset state as a real sample at zero + // would let a first raw value near the top of the range read as a packet + // reordered across a boundary, placing a whole recording a modulo early. + LastReceivedTimeStamp = timeStamp; + } + else { - CurrentTimeStampCycle = CurrentTimeStampCycle + 1; + double lastRaw = LastReceivedTimeStamp - (maxTicks * CurrentTimeStampCycle); + double forward = timeStamp - lastRaw; + if (forward < 0) + { + forward += maxTicks; + } + double backwards = maxTicks - forward; + + if (forward == 0) + { + // A duplicate: hold the timeline where it is. + } + else if (backwards <= reorderWindow) + { + // Reordered, on either side of a wrap boundary. Placed where it was + // taken, which is below its predecessor. + LastReceivedTimeStamp -= backwards; + CurrentTimeStampCycle = Math.Floor(LastReceivedTimeStamp / maxTicks); + } + else if (maxTicks == 16777216 && timeStamp == 0 && lastRaw < (maxTicks - 32768)) + { + // Mid-range and then exactly zero: a record the firmware never + // stamped. Hold the timeline and say so. LogAndStream + // v1.00.x-v1.01.003 could emit one under SD write back-pressure; + // read as a wrap it makes every later sample 512 seconds late. + rejected = true; + } + else + { + // Forward motion, which is a wrap when the raw value fell. + LastReceivedTimeStamp += forward; + CurrentTimeStampCycle = Math.Floor(LastReceivedTimeStamp / maxTicks); + } } - LastReceivedTimeStamp = (timeStamp + (TimeStampPacketRawMaxValue * CurrentTimeStampCycle)); calibratedTimeStamp = LastReceivedTimeStamp / 32768 * 1000; // to convert into mS if (FirstTimeCalTime) { FirstTimeCalTime = false; CalTimeStart = calibratedTimeStamp; } - if (LastReceivedCalibratedTimeStamp != -1) + if (LastReceivedCalibratedTimeStamp != -1 && !rejected) { + //A rejected sample carries the previous timestamp, so the difference + //here would be zero - a gap that never happened. double timeDifference = calibratedTimeStamp - LastReceivedCalibratedTimeStamp; double clockConstant = 1024; if (HardwareVersion == (int)ShimmerBluetooth.ShimmerVersion.SHIMMER2R || HardwareVersion == (int)ShimmerBluetooth.ShimmerVersion.SHIMMER2) From b6b8eea77e80643f2300b7bbdafa591c05eb3b4f Mon Sep 17 00:00:00 2001 From: Mark Nolan Date: Thu, 17 Sep 2026 12:33:14 +0100 Subject: [PATCH 5/6] DEV-1023: run the shared timestamp-unwrap conformance vectors Four host APIs unwrap this counter and all four had the same defect, because nothing checked them against each other. log-and-stream-common now specifies the rule once and ships machine-readable vectors for it; this runs them. This project has no way to load a data file - the csproj is the old style, with no resource pipeline - so the 26 vectors are transcribed into a static array by Test/host/crosscheck_timestamp_unwrap.py --emit csharp rather than by hand, with the source revision and commit named in the file. The Java driver, pyshimmer and the web SDK run the same cases from the file itself. If the four disagree, one of them is wrong, which is the whole point and is what nobody could see last time. TimestampUnwrapVectorsTest also covers the window derivations, and natively the values the shared file cannot spell: infinity, NaN and a negative rate all have to give a window of zero, and infinity is the one that matters because it is what a division by an unset rate produces. ShimmerSDLogParseTest gains the end-to-end half - the window is derived from the header rate, so these prove that rate actually reaches the unwrapper on the SD path. Its file builder takes per-row timestamp overrides now, so a test can write records that are out of order or duplicated: the firmware does not produce those, but a corrupt card or a future writer could, and the rule has to survive them either way. Swapped records must not add a modulo; a duplicate must hold the timeline, with the record after it stepping two periods so the recording still spans what it took. Plus the two header-rate cases: 504.123 Hz rather than 504, and a zero divider that no longer throws. 47 tests, 0 failures. Five mutations were checked rather than assumed: reading unwrapped values instead of modular distances, sizing the window as modulo/8, letting an unknown rate become an infinite window, dropping the first-sample sentinel, and restoring the integer division in the header rate. Each is caught. The modulo/8 one is caught by the SD drop tests too - an oversized window reclassifies the zero-stamped records as reorders and stops dropping them. Co-Authored-By: Claude Opus 5 --- .../ShimmerUnitTests/ShimmerSDLogParseTest.cs | 158 +++++++- .../ShimmerUnitTests/ShimmerUnitTests.csproj | 1 + .../TimestampUnwrapVectorsTest.cs | 373 ++++++++++++++++++ 3 files changed, 531 insertions(+), 1 deletion(-) create mode 100644 ShimmerAPI/ShimmerUnitTests/TimestampUnwrapVectorsTest.cs diff --git a/ShimmerAPI/ShimmerUnitTests/ShimmerSDLogParseTest.cs b/ShimmerAPI/ShimmerUnitTests/ShimmerSDLogParseTest.cs index a671cef2..e9cce559 100644 --- a/ShimmerAPI/ShimmerUnitTests/ShimmerSDLogParseTest.cs +++ b/ShimmerAPI/ShimmerUnitTests/ShimmerSDLogParseTest.cs @@ -109,6 +109,17 @@ private static void WriteRow(Stream outStream, long ticks, int seq) /// rows whose timestamp field is written as 00 00 00 private static string BuildFile(bool syncWhenLogging, int[] badRowIndexes) + { + return BuildFile(syncWhenLogging, badRowIndexes, null); + } + + /// rows whose timestamp field is written as 00 00 00 + /// per-row timestamp overrides, applied after the + /// zeroing above. Lets a test write a file whose records are out of order or + /// duplicated - which the firmware does not produce, but a corrupt card or a + /// future writer could, and which the unwrap rule has to survive either way. + private static string BuildFile(bool syncWhenLogging, int[] badRowIndexes, + System.Collections.Generic.Dictionary tickOverrides) { // The reader takes the last three characters of the path as the SD file // number, so give it a name shaped like one the firmware writes. @@ -140,7 +151,15 @@ private static string BuildFile(bool syncWhenLogging, int[] badRowIndexes) bool bad = Array.IndexOf(badRowIndexes, i) >= 0; long ticks = (FirstTick + ((long)i * PeriodTicks)) & 0xFFFFFFL; - WriteRow(outStream, bad ? 0L : ticks, i); + if (bad) + { + ticks = 0L; + } + if (tickOverrides != null && tickOverrides.ContainsKey(i)) + { + ticks = tickOverrides[i]; + } + WriteRow(outStream, ticks, i); } } @@ -326,5 +345,142 @@ public void TestSyncOffsetRecordIsReadNotDecoded() DeleteFile(path); } } + + /// + /// The header rate is the divider, and the conversion from it has to be + /// floating point. It was integer division, so a divider of 65 gave 504 Hz + /// rather than 504.123 and 640 gave 51 rather than 51.2 - small, but it feeds + /// the reorder window and anything a caller reads back. + /// + [TestMethod] + public void TestHeaderRateIsNotTruncatedToAnInteger() + { + string path = BuildFile(false, new int[0]); + try + { + var sdLog = new ShimmerSDLog(path); + Assert.AreEqual(32768.0 / SampleRateDivider, sdLog.GetSamplingRate(), 1e-9, + "504.123 Hz, not 504"); + } + finally + { + DeleteFile(path); + } + } + + /// + /// A blank or truncated header has a divider of zero. That used to throw + /// DivideByZeroException out of the constructor and take the whole import with + /// it; zero now means "rate not known", which every consumer already handles. + /// + [TestMethod] + public void TestHeaderRateOfZeroDoesNotThrow() + { + string path = BuildFile(false, new int[0]); + try + { + byte[] content = File.ReadAllBytes(path); + content[0] = 0; + content[1] = 0; + File.WriteAllBytes(path, content); + + var sdLog = new ShimmerSDLog(path); + Assert.AreEqual(0.0, sdLog.GetSamplingRate(), 0.0, "not known"); + } + finally + { + DeleteFile(path); + } + } + + /// + /// Two adjacent records the wrong way round. The reorder window is derived from + /// the header rate, so this also proves that rate reaches the unwrapper: with no + /// window the second record would be read as a roll-over and the file would gain + /// 512 seconds. + /// + [TestMethod] + public void TestSwappedRecordsDoNotAddAModulo() + { + var overrides = new System.Collections.Generic.Dictionary(); + long at1000 = (FirstTick + (1000L * PeriodTicks)) & 0xFFFFFFL; + long at1001 = (FirstTick + (1001L * PeriodTicks)) & 0xFFFFFFL; + overrides[1000] = at1001; + overrides[1001] = at1000; + + string path = BuildFile(false, new int[0], overrides); + try + { + var sdLog = new ShimmerSDLog(path); + System.Collections.Generic.List timestamps = ReadAllTimestamps(sdLog); + + Assert.AreEqual(RowCount, timestamps.Count, "every record is returned"); + Assert.AreEqual(0, sdLog.RejectedTimestampRowCount, "a reorder is not an invalid record"); + + double periodMs = PeriodTicks / TicksPerSecond * 1000.0; + int backwardSteps = 0; + double maxStep = 0; + for (int i = 1; i < timestamps.Count; i++) + { + double step = timestamps[i] - timestamps[i - 1]; + if (step < 0) + { + backwardSteps++; + Assert.AreEqual(-periodMs, step, 0.001, "the swap is one period backwards"); + } + maxStep = Math.Max(maxStep, step); + } + + Assert.AreEqual(1, backwardSteps, "exactly one record sits below its predecessor"); + Assert.IsTrue(maxStep < 4 * periodMs, + "and no step anywhere near a modulo (largest was " + maxStep + " ms)"); + Assert.AreEqual((RowCount - 1) * periodMs, timestamps[timestamps.Count - 1] - timestamps[0], 0.001, + "the recording still spans exactly the samples it took"); + } + finally + { + DeleteFile(path); + } + } + + /// A repeated record holds the timeline rather than counting a wrap. + [TestMethod] + public void TestDuplicateRecordHoldsTheTimeline() + { + var overrides = new System.Collections.Generic.Dictionary(); + overrides[2000] = (FirstTick + (1999L * PeriodTicks)) & 0xFFFFFFL; + + string path = BuildFile(false, new int[0], overrides); + try + { + var sdLog = new ShimmerSDLog(path); + System.Collections.Generic.List timestamps = ReadAllTimestamps(sdLog); + + Assert.AreEqual(RowCount, timestamps.Count); + Assert.AreEqual(0, sdLog.RejectedTimestampRowCount); + + double periodMs = PeriodTicks / TicksPerSecond * 1000.0; + int zeroSteps = 0; + for (int i = 1; i < timestamps.Count; i++) + { + double step = timestamps[i] - timestamps[i - 1]; + Assert.IsTrue(step >= 0, "a duplicate never goes backwards"); + if (step == 0) + { + zeroSteps++; + } + } + Assert.AreEqual(1, zeroSteps, "exactly one repeated timestamp"); + // The duplicate holds the timeline, and the record after it then steps + // two periods rather than one - so the recording still spans exactly + // what it took. Holding rather than advancing is what keeps that true. + Assert.AreEqual((RowCount - 1) * periodMs, timestamps[timestamps.Count - 1] - timestamps[0], 0.001, + "and the span is unchanged: the next record recovers the held period"); + } + finally + { + DeleteFile(path); + } + } } } diff --git a/ShimmerAPI/ShimmerUnitTests/ShimmerUnitTests.csproj b/ShimmerAPI/ShimmerUnitTests/ShimmerUnitTests.csproj index 32b8a462..19c33342 100644 --- a/ShimmerAPI/ShimmerUnitTests/ShimmerUnitTests.csproj +++ b/ShimmerAPI/ShimmerUnitTests/ShimmerUnitTests.csproj @@ -63,6 +63,7 @@ + diff --git a/ShimmerAPI/ShimmerUnitTests/TimestampUnwrapVectorsTest.cs b/ShimmerAPI/ShimmerUnitTests/TimestampUnwrapVectorsTest.cs new file mode 100644 index 00000000..3ad857fc --- /dev/null +++ b/ShimmerAPI/ShimmerUnitTests/TimestampUnwrapVectorsTest.cs @@ -0,0 +1,373 @@ +using System; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using ShimmerAPI; + +namespace ShimmerBluetoothTests +{ + /// + /// Runs against the conformance vectors shared by + /// every Shimmer host API. + /// + /// The vectors live in log-and-stream-common at + /// Test/conformance/timestamp_unwrap.json, alongside the rule they encode + /// (docs/SHIMMER3_STREAMING_DATA_FORMAT.md section 2.1) and the reference + /// implementation that generates them. This project has no way to load a data + /// file - the csproj is the old style, with no resource pipeline - so the array + /// below is transcribed mechanically by + /// Test/host/crosscheck_timestamp_unwrap.py --emit csharp rather than by + /// hand. Regenerate it; do not edit it. + /// + /// + /// The point of the shared file is that the Java driver, pyshimmer and the + /// TypeScript web SDK run the same cases. Four implementations of one wire format + /// drifted apart once already - the same unwrap defect sat in all four - and + /// reviewing them against each other by hand is what let that happen. If this test + /// and its counterparts disagree, one of the four is wrong. + /// + /// + /// covers the same rule in a form meant to be + /// read; this one covers it in a form meant to be shared. + /// + /// + [TestClass] + public class TimestampUnwrapVectorsTest + { + /// One shared vector: inputs, and what every host must produce. + internal sealed class UnwrapVector + { + public readonly string Id; + public readonly int Modulo; + public readonly double ReorderWindowTicks; + public readonly double[] Raw; + public readonly double[] ExpectedUnwrapped; + public readonly bool[] ExpectedRejected; + public readonly double ExpectedFinalCycle; + + public UnwrapVector(string id, int modulo, double reorderWindowTicks, + double[] raw, double[] expectedUnwrapped, bool[] expectedRejected, + double expectedFinalCycle) + { + Id = id; + Modulo = modulo; + ReorderWindowTicks = reorderWindowTicks; + Raw = raw; + ExpectedUnwrapped = expectedUnwrapped; + ExpectedRejected = expectedRejected; + ExpectedFinalCycle = expectedFinalCycle; + } + } + + /// The revision of the vector file this test was transcribed from. + private const int ExpectedRevision = 1; + + // Source: log-and-stream-common Test/conformance/timestamp_unwrap.json + // revision 1, at commit 8e76966. + // Generated by Test/host/crosscheck_timestamp_unwrap.py --emit csharp + // Source: Test/conformance/timestamp_unwrap.json revision 1 + // Do not hand-edit; regenerate when the vector file changes. + internal static readonly UnwrapVector[] Vectors = + { + new UnwrapVector( + "monotonic-24bit", + 16777216, 520.0, + new double[] { 1000, 1065, 1130 }, + new double[] { 1000, 1065, 1130 }, + new bool[] { false, false, false }, + 0), + new UnwrapVector( + "wrap-24bit", + 16777216, 520.0, + new double[] { 16777200, 16 }, + new double[] { 16777200, 16777232 }, + new bool[] { false, false }, + 1), + new UnwrapVector( + "wrap-lands-on-zero-24bit", + 16777216, 520.0, + new double[] { 16777116, 0 }, + new double[] { 16777116, 16777216 }, + new bool[] { false, false }, + 1), + new UnwrapVector( + "invalid-zero-signature-24bit", + 16777216, 520.0, + new double[] { 7406116, 7406506, 0, 7406571 }, + new double[] { 7406116, 7406506, 7406506, 7406571 }, + new bool[] { false, false, true, false }, + 0), + new UnwrapVector( + "invalid-zero-no-cascade-24bit", + 16777216, 520.0, + new double[] { 7406506, 0, 7406571, 7406636 }, + new double[] { 7406506, 7406506, 7406571, 7406636 }, + new bool[] { false, true, false, false }, + 0), + new UnwrapVector( + "first-sample-zero-24bit", + 16777216, 520.0, + new double[] { 0, 65, 130 }, + new double[] { 0, 65, 130 }, + new bool[] { false, false, false }, + 0), + new UnwrapVector( + "wrap-16bit", + 65536, 5120.0, + new double[] { 65436, 28 }, + new double[] { 65436, 65564 }, + new bool[] { false, false }, + 1), + new UnwrapVector( + "zero-on-16bit-is-a-wrap", + 65536, 5120.0, + new double[] { 30000, 0 }, + new double[] { 30000, 65536 }, + new bool[] { false, false }, + 1), + new UnwrapVector( + "backward-step-outside-window-is-a-wrap-24bit", + 16777216, 520.0, + new double[] { 7406506, 1 }, + new double[] { 7406506, 16777217 }, + new bool[] { false, false }, + 1), + new UnwrapVector( + "duplicate-24bit", + 16777216, 520.0, + new double[] { 1000, 1065, 1065, 1130 }, + new double[] { 1000, 1065, 1065, 1130 }, + new bool[] { false, false, false, false }, + 0), + new UnwrapVector( + "reorder-one-period-24bit", + 16777216, 520.0, + new double[] { 1000, 1130, 1065, 1195 }, + new double[] { 1000, 1130, 1065, 1195 }, + new bool[] { false, false, false, false }, + 0), + new UnwrapVector( + "reorder-one-period-16bit", + 65536, 5120.0, + new double[] { 40000, 41280, 40640, 41920 }, + new double[] { 40000, 41280, 40640, 41920 }, + new bool[] { false, false, false, false }, + 0), + new UnwrapVector( + "reorder-across-wrap-boundary-24bit", + 16777216, 520.0, + new double[] { 16777206, 5, 16777206, 70 }, + new double[] { 16777206, 16777221, 16777206, 16777286 }, + new bool[] { false, false, false, false }, + 1), + new UnwrapVector( + "wrap-after-heavy-loss-24bit", + 16777216, 520.0, + new double[] { 16000000, 100 }, + new double[] { 16000000, 16777316 }, + new bool[] { false, false }, + 1), + new UnwrapVector( + "wrap-after-heavy-loss-16bit", + 65536, 5120.0, + new double[] { 60000, 1000 }, + new double[] { 60000, 66536 }, + new bool[] { false, false }, + 1), + new UnwrapVector( + "wrap-spanning-dropout-1p8s-16bit", + 65536, 5120.0, + new double[] { 60000, 53446 }, + new double[] { 60000, 118982 }, + new bool[] { false, false }, + 1), + new UnwrapVector( + "wrap-spanning-dropout-152s-24bit", + 16777216, 520.0, + new double[] { 16000000, 4222784 }, + new double[] { 16000000, 21000000 }, + new bool[] { false, false }, + 1), + new UnwrapVector( + "rate-unknown-backward-step-is-a-wrap-24bit", + 16777216, 0.0, + new double[] { 1000, 1130, 1065, 1195 }, + new double[] { 1000, 1130, 16778281, 16778411 }, + new bool[] { false, false, false, false }, + 1), + new UnwrapVector( + "rate-unknown-zero-still-rejected-24bit", + 16777216, 0.0, + new double[] { 7406506, 0, 7406571 }, + new double[] { 7406506, 7406506, 7406571 }, + new bool[] { false, true, false }, + 0), + new UnwrapVector( + "zero-within-window-of-origin-24bit", + 16777216, 520.0, + new double[] { 300, 365, 0, 430 }, + new double[] { 300, 365, 0, 430 }, + new bool[] { false, false, false, false }, + 0), + new UnwrapVector( + "zero-within-window-after-wrap-24bit", + 16777216, 520.0, + new double[] { 16777100, 100, 165, 0, 230 }, + new double[] { 16777100, 16777316, 16777381, 16777216, 16777446 }, + new bool[] { false, false, false, false, false }, + 1), + new UnwrapVector( + "reorder-window-boundary-inclusive-24bit", + 16777216, 512.0, + new double[] { 10512, 10000 }, + new double[] { 10512, 10000 }, + new bool[] { false, false }, + 0), + new UnwrapVector( + "reorder-window-boundary-exclusive-24bit", + 16777216, 512.0, + new double[] { 10513, 10000 }, + new double[] { 10513, 16787216 }, + new bool[] { false, false }, + 1), + new UnwrapVector( + "low-rate-clamp-16bit", + 65536, 8192.0, + new double[] { 60000, 1000 }, + new double[] { 60000, 66536 }, + new bool[] { false, false }, + 1), + new UnwrapVector( + "high-rate-reorder-24bit", + 16777216, 256.0, + new double[] { 5000, 5032, 5000, 5064 }, + new double[] { 5000, 5032, 5000, 5064 }, + new bool[] { false, false, false, false }, + 0), + new UnwrapVector( + "reorder-beyond-eight-periods-is-a-wrap-24bit", + 16777216, 256.0, + new double[] { 5000, 5288, 5000 }, + new double[] { 5000, 5288, 16782216 }, + new bool[] { false, false, false }, + 1), + }; + + internal const int VectorCount = 26; + + /// Feeds a series of raw values through the unwrapper, as a caller would. + private class Unwrapper + { + public double LastUnwrapped; + public double Cycle; + public bool LastRejected; + private readonly double _window; + + public Unwrapper(double window) + { + _window = window; + } + + public double Feed(double rawTicks, int maxTicks) + { + TimestampUnwrap.Result r = TimestampUnwrap.Unwrap( + rawTicks, LastUnwrapped, Cycle, maxTicks, _window); + LastRejected = r.Rejected; + Cycle = r.Cycle; + LastUnwrapped = r.Unwrapped; + return r.Unwrapped; + } + } + + private static System.Collections.Generic.List VectorIds() + { + var ids = new System.Collections.Generic.List(); + foreach (UnwrapVector v in Vectors) + { + ids.Add(v.Id); + } + return ids; + } + + [TestMethod] + public void TestVectorSetIsComplete() + { + Assert.AreEqual(VectorCount, Vectors.Length, + "the transcribed array and its count disagree - regenerate both with --emit csharp"); + Assert.AreEqual(1, ExpectedRevision, + "vector file revision - update this test deliberately, not to make it pass"); + + // Three vectors have to be here by name: each is the case that one + // superseded rule gets wrong, so losing one would quietly stop covering it. + CollectionAssert.Contains(VectorIds(), "wrap-spanning-dropout-1p8s-16bit"); + CollectionAssert.Contains(VectorIds(), "reorder-across-wrap-boundary-24bit"); + CollectionAssert.Contains(VectorIds(), "wrap-after-heavy-loss-24bit"); + } + + [TestMethod] + public void TestAllSharedVectorsAgree() + { + foreach (UnwrapVector vector in Vectors) + { + Assert.AreEqual(vector.Raw.Length, vector.ExpectedUnwrapped.Length, + vector.Id + ": vector is self-consistent"); + Assert.AreEqual(vector.Raw.Length, vector.ExpectedRejected.Length, + vector.Id + ": vector is self-consistent"); + + var u = new Unwrapper(vector.ReorderWindowTicks); + for (int i = 0; i < vector.Raw.Length; i++) + { + double got = u.Feed(vector.Raw[i], vector.Modulo); + string where = vector.Id + " [" + i + "] raw=" + vector.Raw[i]; + Assert.AreEqual(vector.ExpectedUnwrapped[i], got, 0.0, where + ": unwrapped"); + Assert.AreEqual(vector.ExpectedRejected[i], u.LastRejected, where + ": rejected"); + } + Assert.AreEqual(vector.ExpectedFinalCycle, u.Cycle, 0.0, vector.Id + ": final cycle"); + } + } + + /// + /// The window derivations from the same file. Eight sample periods, in the + /// 32768 Hz tick domain, clamped at a modulo/8 so a very low rate cannot leave + /// every backward step looking like a reorder. + /// + [TestMethod] + public void TestWindowDerivationCases() + { + int mod24 = TimestampUnwrap.TicksMax3Byte; + const int mod16 = 65536; + + Assert.AreEqual(520.0, TimestampUnwrap.ReorderWindowTicks(32768.0 / 65, mod24), 1e-9, + "504.123 Hz on the 3-byte counter"); + Assert.AreEqual(5120.0, TimestampUnwrap.ReorderWindowTicks(51.2, mod16), 1e-9, + "51.2 Hz on the 2-byte counter"); + Assert.AreEqual(512.0, TimestampUnwrap.ReorderWindowTicks(512.0, mod24), 1e-9, + "512 Hz"); + Assert.AreEqual(256.0, TimestampUnwrap.ReorderWindowTicks(1024.0, mod24), 1e-9, + "1024 Hz"); + Assert.AreEqual(mod16 / 8.0, TimestampUnwrap.ReorderWindowTicks(1.0, mod16), 0.0, + "1 Hz on the 2-byte counter is clamped"); + Assert.AreEqual(262144.0, TimestampUnwrap.ReorderWindowTicks(1.0, mod24), 1e-9, + "1 Hz on the 3-byte counter is not"); + Assert.AreEqual(511.705088, TimestampUnwrap.ReorderWindowTicks(312500.0 / 610, mod24), 1e-4, + "a TCXO rate, derived in the RTC tick domain and not the sampling clock"); + } + + /// + /// The values the shared file cannot spell. An unknown rate arriving as a + /// division result rather than as a literal zero is the case that matters: a + /// host that lets it through produces an infinite window, which reads every + /// backward step as a reorder and loses every wrap. + /// + [TestMethod] + public void TestWindowIsZeroForEveryShapeOfUnknownRate() + { + int mod = TimestampUnwrap.TicksMax3Byte; + Assert.AreEqual(0.0, TimestampUnwrap.ReorderWindowTicks(double.PositiveInfinity, mod), 0.0, + "positive infinity"); + Assert.AreEqual(0.0, TimestampUnwrap.ReorderWindowTicks(double.NegativeInfinity, mod), 0.0, + "negative infinity"); + Assert.AreEqual(0.0, TimestampUnwrap.ReorderWindowTicks(double.NaN, mod), 0.0, "NaN"); + Assert.AreEqual(0.0, TimestampUnwrap.ReorderWindowTicks(0.0, mod), 0.0, "zero"); + Assert.AreEqual(0.0, TimestampUnwrap.ReorderWindowTicks(-5.0, mod), 0.0, "negative"); + } + } +} From 081bcdf8528c265ed4d85a60662c1cb7a1932be3 Mon Sep 17 00:00:00 2001 From: Mark Nolan Date: Thu, 17 Sep 2026 14:19:56 +0100 Subject: [PATCH 6/6] DEV-1023: tell the unwrap whether a previous sample exists This API keeps an unwrapped value and a cycle count rather than the previous raw value, so it has to encode "no sample yet" somehow, and (0, 0) was the encoding. That state is also reachable: a reordered packet landing exactly on the counter's origin leaves LastReceivedTimeStamp and CurrentTimeStampCycle both at zero in the middle of a stream. The next packet is then read as a first sample and passed through, so one arriving from just before the origin is placed a whole modulo late rather than sixteen ticks behind. Found by running the two formulations of the rule against each other rather than by reading them. [520, 0, 16777200] gives -16 where the previous raw value is kept - the web SDK and pyshimmer - and 16777200 here. So: a six-argument Unwrap that is told outright, and HasPreviousTimeStamp on ShimmerDevice, cleared at the four places a stream starts (ShimmerBluetooth twice, ShimmerLogAndStream, ShimmerSDBT). The five-argument overload stays, still inferring it from (0, 0), for callers written before the distinction existed. ShimmerCaptureXamarin carries the rule inline - it is a vendored fork and cannot reference this project - and has the same change. Still NOT COMPILED IN THIS WORKSPACE and still without a test, as its header says. The sequence is now the shared conformance vector reorder-onto-origin-then-earlier-packet-24bit, so the other four host APIs are held to the same answer. It is the first vector whose final cycle is negative, which is a real state here: the raw value is derived back out of it. The transcribed array is regenerated with --emit csharp, not hand-edited. 47 tests pass. Co-Authored-By: Mas Azalya <43565312+MAzalya@users.noreply.github.com> Co-Authored-By: Claude Opus 5 --- ShimmerAPI/ShimmerAPI/ShimmerBluetooth.cs | 6 ++++ ShimmerAPI/ShimmerAPI/ShimmerDevice.cs | 9 +++++- ShimmerAPI/ShimmerAPI/ShimmerLogAndStream.cs | 3 ++ ShimmerAPI/ShimmerAPI/ShimmerSDBT.cs | 3 ++ ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs | 30 ++++++++++++++++++- .../TimestampUnwrapVectorsTest.cs | 22 +++++++++++--- .../ShimmerCaptureXamarin/ShimmerBluetooth.cs | 20 ++++++++++++- 7 files changed, 86 insertions(+), 7 deletions(-) diff --git a/ShimmerAPI/ShimmerAPI/ShimmerBluetooth.cs b/ShimmerAPI/ShimmerAPI/ShimmerBluetooth.cs index 55e4bd26..aaede67c 100644 --- a/ShimmerAPI/ShimmerAPI/ShimmerBluetooth.cs +++ b/ShimmerAPI/ShimmerAPI/ShimmerBluetooth.cs @@ -5421,6 +5421,9 @@ public virtual void StartStreaming() StreamTimeOutCount = 0; LastReceivedTimeStamp = 0; CurrentTimeStampCycle = 0; + //A stream start: the next sample is the first one, and the pair + //above cannot say so on their own. + HasPreviousTimeStamp = false; LastReceivedCalibratedTimeStamp = -1; FirstTimeCalTime = true; FirstSystemTimestamp = true; @@ -7301,6 +7304,9 @@ public void StartStreamingEXGSawtoothTestSignal(int value) StreamTimeOutCount = 0; LastReceivedTimeStamp = 0; CurrentTimeStampCycle = 0; + //A stream start: the next sample is the first one, and the pair + //above cannot say so on their own. + HasPreviousTimeStamp = false; LastReceivedCalibratedTimeStamp = -1; FirstTimeCalTime = true; FirstSystemTimestamp = true; diff --git a/ShimmerAPI/ShimmerAPI/ShimmerDevice.cs b/ShimmerAPI/ShimmerAPI/ShimmerDevice.cs index 594215e8..70cfb83b 100644 --- a/ShimmerAPI/ShimmerAPI/ShimmerDevice.cs +++ b/ShimmerAPI/ShimmerAPI/ShimmerDevice.cs @@ -12,6 +12,12 @@ public abstract class ShimmerDevice protected double LastReceivedTimeStamp = 0; protected double CurrentTimeStampCycle = 0; /// + /// False only before the first sample of a stream. The pair above cannot say + /// it on their own: (0, 0) is the reset state and also a state the unwrap can + /// reach, when a reordered packet lands exactly on the counter's origin. + /// + protected bool HasPreviousTimeStamp = false; + /// /// True when the sample most recently passed to CalibrateTimeStamp carried an /// invalid zero timestamp and was rejected rather than unwrapped. Its sensor /// data is fine; only its timestamp is missing. A caller reading a file should @@ -73,7 +79,8 @@ protected double CalibrateTimeStamp(double timeStamp) double calibratedTimeStamp = 0; TimestampUnwrap.Result unwrapped = TimestampUnwrap.Unwrap( timeStamp, LastReceivedTimeStamp, CurrentTimeStampCycle, TimeStampPacketRawMaxValue, - GetReorderWindowTicks()); + GetReorderWindowTicks(), HasPreviousTimeStamp); + HasPreviousTimeStamp = true; LastTimestampRejected = unwrapped.Rejected; CurrentTimeStampCycle = unwrapped.Cycle; diff --git a/ShimmerAPI/ShimmerAPI/ShimmerLogAndStream.cs b/ShimmerAPI/ShimmerAPI/ShimmerLogAndStream.cs index 1ceb9bd2..1ad09f98 100644 --- a/ShimmerAPI/ShimmerAPI/ShimmerLogAndStream.cs +++ b/ShimmerAPI/ShimmerAPI/ShimmerLogAndStream.cs @@ -468,6 +468,9 @@ public override void StartStreamingandLog() StreamingACKReceived = false; LastReceivedTimeStamp = 0; CurrentTimeStampCycle = 0; + //A stream start: the next sample is the first one, and the pair + //above cannot say so on their own. + HasPreviousTimeStamp = false; LastReceivedCalibratedTimeStamp = -1; FirstTimeCalTime = true; PacketLossCount = 0; diff --git a/ShimmerAPI/ShimmerAPI/ShimmerSDBT.cs b/ShimmerAPI/ShimmerAPI/ShimmerSDBT.cs index 92c269f3..177e4136 100644 --- a/ShimmerAPI/ShimmerAPI/ShimmerSDBT.cs +++ b/ShimmerAPI/ShimmerAPI/ShimmerSDBT.cs @@ -423,6 +423,9 @@ public override void StartStreamingandLog() StreamingACKReceived = false; LastReceivedTimeStamp = 0; CurrentTimeStampCycle = 0; + //A stream start: the next sample is the first one, and the pair + //above cannot say so on their own. + HasPreviousTimeStamp = false; LastReceivedCalibratedTimeStamp = -1; FirstTimeCalTime = true; PacketLossCount = 0; diff --git a/ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs b/ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs index 11a7c78b..e1d7b9a9 100644 --- a/ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs +++ b/ShimmerAPI/ShimmerAPI/TimestampUnwrap.cs @@ -154,7 +154,35 @@ public static Result Unwrap(double rawTicks, double lastUnwrapped, double cycle, public static Result Unwrap(double rawTicks, double lastUnwrapped, double cycle, int maxTicks, double reorderWindowTicks) { - if (lastUnwrapped == 0.0 && cycle == 0.0) + // (0, 0) is the reset state AND a state the rule can reach, so this + // overload cannot always tell them apart - see the six-argument form. + // Kept for callers written before the distinction existed. + return Unwrap(rawTicks, lastUnwrapped, cycle, maxTicks, reorderWindowTicks, + !(lastUnwrapped == 0.0 && cycle == 0.0)); + } + + /// + /// As above, but told outright whether a previous sample exists. + /// + /// The five-argument form infers it from the state being (0, 0). That is the + /// reset state, and it is also a state the rule can produce: a reorder that + /// lands exactly on the counter's origin leaves lastUnwrapped = 0 and + /// cycle = 0 in the middle of a stream. The next packet is then read as a + /// first sample and passed through, so one arriving from just before the + /// origin is placed a whole modulo late rather than a few ticks behind. The + /// conformance vector reorder-onto-origin-then-earlier-packet-24bit is + /// exactly that sequence. + /// + /// Hosts that keep the previous RAW value instead of a cycle count - the web + /// SDK and pyshimmer - never had the ambiguity. + /// + /// + /// False only before the first sample of a stream. + /// + public static Result Unwrap(double rawTicks, double lastUnwrapped, double cycle, int maxTicks, + double reorderWindowTicks, bool hasPreviousSample) + { + if (!hasPreviousSample) { // Nothing has been unwrapped yet, so there is no predecessor to measure // against. Taking the reset state as a real sample at zero would let a diff --git a/ShimmerAPI/ShimmerUnitTests/TimestampUnwrapVectorsTest.cs b/ShimmerAPI/ShimmerUnitTests/TimestampUnwrapVectorsTest.cs index 3ad857fc..8078fc88 100644 --- a/ShimmerAPI/ShimmerUnitTests/TimestampUnwrapVectorsTest.cs +++ b/ShimmerAPI/ShimmerUnitTests/TimestampUnwrapVectorsTest.cs @@ -249,9 +249,16 @@ public UnwrapVector(string id, int modulo, double reorderWindowTicks, new double[] { 5000, 5288, 16782216 }, new bool[] { false, false, false }, 1), + new UnwrapVector( + "reorder-onto-origin-then-earlier-packet-24bit", + 16777216, 520.0, + new double[] { 520, 0, 16777200 }, + new double[] { 520, 0, -16 }, + new bool[] { false, false, false }, + -1), }; - internal const int VectorCount = 26; + internal const int VectorCount = 27; /// Feeds a series of raw values through the unwrapper, as a caller would. private class Unwrapper @@ -259,6 +266,7 @@ private class Unwrapper public double LastUnwrapped; public double Cycle; public bool LastRejected; + private bool _hasPrevious; private readonly double _window; public Unwrapper(double window) @@ -268,8 +276,11 @@ public Unwrapper(double window) public double Feed(double rawTicks, int maxTicks) { + // The six-argument form: a stream knows whether it has seen a sample, + // and (0, 0) cannot say so once a reorder can land on the origin. TimestampUnwrap.Result r = TimestampUnwrap.Unwrap( - rawTicks, LastUnwrapped, Cycle, maxTicks, _window); + rawTicks, LastUnwrapped, Cycle, maxTicks, _window, _hasPrevious); + _hasPrevious = true; LastRejected = r.Rejected; Cycle = r.Cycle; LastUnwrapped = r.Unwrapped; @@ -295,11 +306,14 @@ public void TestVectorSetIsComplete() Assert.AreEqual(1, ExpectedRevision, "vector file revision - update this test deliberately, not to make it pass"); - // Three vectors have to be here by name: each is the case that one - // superseded rule gets wrong, so losing one would quietly stop covering it. + // Four vectors have to be here by name: the first three are each the case + // that one superseded rule gets wrong, and the fourth is the one that + // separates a host storing (unwrapped, cycle) from one storing the raw + // value. Losing any of them would quietly stop covering it. CollectionAssert.Contains(VectorIds(), "wrap-spanning-dropout-1p8s-16bit"); CollectionAssert.Contains(VectorIds(), "reorder-across-wrap-boundary-24bit"); CollectionAssert.Contains(VectorIds(), "wrap-after-heavy-loss-24bit"); + CollectionAssert.Contains(VectorIds(), "reorder-onto-origin-then-earlier-packet-24bit"); } [TestMethod] diff --git a/ShimmerCaptureXamarin/ShimmerCaptureXamarin/ShimmerBluetooth.cs b/ShimmerCaptureXamarin/ShimmerCaptureXamarin/ShimmerBluetooth.cs index b51f1487..678112dc 100644 --- a/ShimmerCaptureXamarin/ShimmerCaptureXamarin/ShimmerBluetooth.cs +++ b/ShimmerCaptureXamarin/ShimmerCaptureXamarin/ShimmerBluetooth.cs @@ -205,6 +205,12 @@ public abstract class ShimmerBluetooth public double GainSGLow = 183.7; protected double LastReceivedTimeStamp = 0; protected double CurrentTimeStampCycle = 0; + /// + /// False only before the first sample of a stream. The pair above cannot say + /// it on their own: (0, 0) is the reset state and also a state the unwrap can + /// reach, when a reordered packet lands exactly on the counter's origin. + /// + protected bool HasPreviousTimeStamp = false; protected double LastReceivedCalibratedTimeStamp = -1; protected double CalTimeStart; public long PacketLossCount = 0; @@ -4029,6 +4035,9 @@ public virtual void StartStreaming() StreamTimeOutCount = 0; LastReceivedTimeStamp = 0; CurrentTimeStampCycle = 0; + //A stream start: the next sample is the first one, and the pair + //above cannot say so on their own. + HasPreviousTimeStamp = false; LastReceivedCalibratedTimeStamp = -1; FirstTimeCalTime = true; FirstSystemTimestamp = true; @@ -5529,12 +5538,19 @@ protected double CalibrateTimeStamp(double timeStamp) : Math.Min(8 * 32768.0 / SamplingRate, maxTicks / 8.0); bool rejected = false; - if (LastReceivedTimeStamp == 0 && CurrentTimeStampCycle == 0) + if (!HasPreviousTimeStamp) { // No predecessor yet. Treating the reset state as a real sample at zero // would let a first raw value near the top of the range read as a packet // reordered across a boundary, placing a whole recording a modulo early. + // + // Asked outright rather than inferred from (0, 0), which is the reset + // state AND a state this rule can reach: a reorder landing exactly on + // the counter's origin leaves both at zero mid stream, after which the + // next packet is read as a first sample. See the conformance vector + // reorder-onto-origin-then-earlier-packet-24bit. LastReceivedTimeStamp = timeStamp; + CurrentTimeStampCycle = 0; } else { @@ -5573,6 +5589,8 @@ protected double CalibrateTimeStamp(double timeStamp) } } + HasPreviousTimeStamp = true; + calibratedTimeStamp = LastReceivedTimeStamp / 32768 * 1000; // to convert into mS if (FirstTimeCalTime) {