Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
… 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
|
Follow-up commit: an adversarial review of this branch found that "no sample This API keeps an unwrapped value and a cycle count rather than the previous raw Found by running the two formulations of the rule against each other rather than
512 seconds apart on the same input, between APIs that are meant to be The unwrap is now told outright. The existing overload stays, still inferring it The sequence is now the shared conformance vector Probability in the field is negligible: it needs a reorder to land exactly on the |
Two tickets, because they turned out to be entangled — see Why both below. Depends on
log-and-stream-common#136,
which specifies the rule and ships the conformance vectors this runs. Mirrors
Shimmer-Java-Android-API#300,
which this API has to agree with — the two read the same recordings.
DEV-1023 — one rule for every way the counter can read backwards
Firmware stamps a packet when its sample tick starts it and does not publish a packet it
never stamped, so
0x000000means the record is invalid, not that the counter reached itsorigin. Read as a roll-over, one such record adds 2²⁴ ticks = 512.002 s to every later
sample, permanently.
Rejecting an exact zero is right for that fault and wrong for everything else that makes
the counter read backwards: a corrupted non-zero value, a duplicate and a reordered packet
all still added a modulo. Each sample is now classified by its modular forward distance
from the last — duplicate, reordered, invalid zero, or forward — with forward as the
default everything else falls through to.
Three things in it are load-bearing, all verified by mutation:
32768 / 0isPositiveInfinityin C#; an infinite window makes every backward step a reorder and loses every wrap.SamplingRatedefaults to 0, so that is the normal state until an inquiry or an SD header has runThe window is eight sample periods, not
modulo / 8: a reorder swaps adjacent packets, adropout spanning the wrap point is most of a modulo, and
modulo / 8confuses them — onthe 2-byte counter every dropout between 1.75 s and 2.0 s would read as a reorder and lose
the wrap, which is an ordinary Bluetooth gap.
Shimmer2/2R 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. Follow-up ticket to settle it.
All three copies in this repo are updated:
ShimmerDevice,TestSerialPort(ownhard-coded rate), and
ShimmerCaptureXamarin, which is a vendored fork not builtalongside
ShimmerAPI. It cannot referenceTimestampUnwrap, so it carries the ruleinline, flagged as uncompiled and untested here — keeping the two in step is manual.
DEV-1024 —
ShimmerSDLogcould not parse a Shimmer3 file at allThe timestamp modulo was left at the 2-byte default.
TimeStampPacketRawMaxValueisonly ever set by
UpdateBasedOnCompatibilityCode(), which runs on the Bluetoothconnect path;
ShimmerSDLognever called it. A 3-byte counter unwrapped against a 2-bytemodulo adds 65536 per roll-over instead of 2²⁴, and the recording reads as a negative
duration. Now derived from the firmware version for every hardware version.
Sync-when-logging blocks were not handled. The firmware heads each 512-byte write
buffer with a 9-byte offset record; it was decoded as sample data, so the record stream
lost alignment at the first block. The flag was not even read — both header lines that
would set it sit inside comment blocks. Now read from trial config 0 bit 2, with the offset
record consumed at each block boundary and exposed raw via
LastSyncOffset().And the header rate was integer division.
32768 / rawSamplingRatehad two integraloperands, so 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
DivideByZeroExceptionout of the constructor, takingthe whole import with it. That fed nothing before; it feeds the reorder window now.
Why both
The zero-timestamp rule is scoped to the 3-byte counter. While the modulo defect stood it
could never fire on the SD path — the drop loop was dead code there. Shipping it alone
would have claimed a fix it did not deliver. The Bluetooth path was unaffected throughout.
Tests
47 passed, 0 failed.
TimestampUnwrapVectorsTestruns the 26 shared vectors. This project has no way to load adata file, so they are transcribed into a static array by
crosscheck_timestamp_unwrap.py --emit csharprather than by hand, with the sourcerevision and commit named in the file. It also covers the window derivations and the values
JSON cannot spell — infinity being the one that matters, since it is what a division by an
unset rate produces.
ShimmerSDLogParseTestis the end-to-end half: the window comes from the header rate, sothese prove that rate reaches the unwrapper. Its file builder now takes per-row timestamp
overrides, so a test can write out-of-order or duplicated records — the firmware does not
produce those, but a corrupt card or a future writer could.
Five mutations checked, each caught:
TestAllSharedVectorsAgreemodulo / 8TestWindowIsZeroForEveryShapeOfUnknownRateTestAllSharedVectorsAgreeTestHeaderRateIsNotTruncatedToAnIntegerStill outstanding
ShimmerBLE/ShimmerBLEAPI/Sensors/Sensor.cshas a fourth naive unwrap for Verisense —different device family and counter semantics, so it is recorded as a follow-up rather
than changed here.
DEV-1023, DEV-1024.
🤖 Generated with Claude Code