DEV-793 Widen + always apply the slow-sensor CSV gap window by marknolan · Pull Request #285 · ShimmerResearch/Shimmer-Java-Android-API

marknolan · 2026-08-11T07:42:07Z

Problem

The DEV-927 hardware-validation recording (25 min of MLX90632 skin temp at 16 Hz output — 1,506 blocks, tick-verified no samples lost) fragmented into 7 skin-temp CSVs when parsed. Healthy MLX conversion jitter (block spacing occasionally +12.5%, then catching up) was being treated as a data gap.

Two defects, both in the slow-sensor gap-window seeding

The measured window never engaged. It is seeded from the first payload carrying ≥ 2 slow-sensor blocks — but when a recording's first payload holds only one block, refineSlowSensorSamplingRateFromBlockTicks returns early and populateExpectedPayloadTsDiffLimitMapIfNeeded claims the global map key with a configured-rate ±10% band. The refine path's containsKey guard then skips forever. The put is now unconditional: the measured window wins as soon as it exists, and re-measures each payload.
Even when measured, the gap side was too tight. The window came from the observed per-payload period spread, which with 2–3 blocks per payload is often a single inter-block gap — no spread information at all. The gap side is now SLOW_SENSOR_MAX_INTER_BLOCK_GAP_RATIO (1.5×) of the achieved median spacing: healthy jitter stays continuous, while a genuinely dropped block (2× spacing) still splits with comfortable margin. The fast side keeps the observed-minimum-period basis with the standard ±10%.

Result

The recording parses into the correct CSV sets — splits only at genuine device events (a reset at recording start; an RWC re-sync during the download BLE connection, uptime vs wall-clock verified).

Validation

ASM_PC ASM_PC_00005_VerisenseFileParserPC 72/72 green (including the new Test_065 built from this recording) + ASM_PC_00032 7/7, run against this branch. All pre-existing tests byte-identical — the widened window changes no previously-passing output (it only removes spurious splits).

Sequencing

ASM_PC PR (Test_065 + UTF-8 writers) is blocked on a Jenkins shimmerdriver publish containing this — same flow as the recent 0.11.8_beta.

Jira: DEV-793 / DEV-927

🤖 Generated with Claude Code

The DEV-927 hardware-validation recording (25 min of MLX90632 skin temp at 16 Hz output, 1506 blocks, no samples lost) fragmented into 7 CSVs. Two defects in the gap-window seeding: 1. The measured window never engaged: it is seeded from the first payload carrying >= 2 slow-sensor blocks, but when the recording's first payload holds only one block (refine returns early), the configured-rate +/-10% band from populateExpectedPayloadTsDiffLimitMapIfNeeded claims the global map key first and the refine path's containsKey guard then skips forever. The put is now unconditional - the measured window wins as soon as it exists and re-measures on every payload. 2. The gap side of the window was too tight even when measured: it came from the observed per-payload period spread, which for 2-3 blocks per payload is often a single gap (no spread information), while MLX90632 conversions can slip by several refresh periods and catch up (observed +12.5% block spacing). The gap side is now SLOW_SENSOR_MAX_INTER_BLOCK_GAP_RATIO (1.5x) of the achieved median spacing - healthy jitter stays continuous, a genuinely dropped block (2x spacing) still splits. The fast side keeps the observed-minimum- period basis with the standard tolerance. With both fixes the recording parses into the correct CSV sets: splits only at the genuine device events (a reset at recording start and an RWC re-sync during the download BLE connection). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR adjusts how CSV-splitting gap windows are derived for slow sensors (VD6283 light, MLX90632 skin temp) so that normal conversion jitter and limited early payload information don’t cause false “data gap” splits.

Changes:

Introduces a slow-sensor-specific maximum inter-block gap ratio constant (SLOW_SENSOR_MAX_INTER_BLOCK_GAP_RATIO).
Updates slow-sensor gap-window seeding to use achieved median spacing for the “gap side” and to overwrite the global limits on every qualifying payload (instead of being locked by an early configured-rate seed).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
ShimmerDriver/src/main/java/com/shimmerresearch/verisense/payloaddesign/UtilCsvSplitting.java	Adds a slow-sensor-specific gap-ratio constant used when building splitting limits.
ShimmerDriver/src/main/java/com/shimmerresearch/verisense/payloaddesign/PayloadContentsDetailsV8orAbove.java	Changes slow-sensor rate refinement to widen the gap-side window and always apply measured limits per payload.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…review) The first paragraph still described the pre-fix containsKey-guarded / first-payload seeding, contradicting the unconditional-put explanation below it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

jyong15

Code review — DEV-793 slow-sensor CSV gap window

Recommended disposition: request changes. The diagnosis of the original bug is correct — I verified that populateExpectedPayloadTsDiffLimitMapIfNeeded (UtilCsvSplitting.java:85) runs after refineSlowSensorSamplingRateFromBlockTicks in parsePayloadContentsMetaData, so a first payload carrying <2 slow blocks did permanently lock in the configured-rate ±10% band. But dropping the containsKey guard trades that bug for a dropped-block blind spot.

Major

1. The gap window is now self-referential, so a dropped slow-sensor block goes undetected — and the split lands at the wrong boundary instead (PayloadContentsDetailsV8orAbove.java:399-407). With the guard removed, the window is re-derived every payload from the very block spacings it then judges — and the two quantities are identically the same expression: refine computes perSamplePeriodsS[i] = (deltaTicks/32768.0)/sampleCount, and the gap check computes calcSamplingRate(prevEnd, nextEnd, nextSampleCount) = sampleCount/Δt(s). So 1/measuredPeriod is exactly the rate the checker computes for that boundary, and it is always inside [rate/1.5, rate*1.1].

Failure scenario with true per-sample period P and a payload carrying exactly 2 skin-temp blocks (2–3 per payload is the norm per the PR body) with one block's worth of data dropped between them (spacing 2P):

	value
`perSamplePeriodsS`	`[2P]` → `medianPeriodS = 2P`, `achievedRateHz = 0.5/P`
new limits	`[0.333/P, 0.55/P]`
intra-payload boundary rate `0.5/P`	inside → real gap judged continuous, silently absorbed
both blocks	`setSamplingRate(0.5/P)` → that payload's CSV timestamps stretched 2×
previous-payload boundary (healthy `1/P`)	`> 0.55/P` → flagged → CSV splits one block early, at a boundary with no gap

Pre-PR, the seeded fixed window (e.g. [0.667/P, 1.1/P]) correctly split at the real gap. Suggested direction: keep re-measuring, but don't let one payload's measurement replace the window wholesale — e.g. only accept a measurement from a payload with ≥3 observed periods, or carry a running/first-robust estimate forward and only widen it, rather than puting the latest value unconditionally.

2. The fast side is now volatile too, though the PR describes it as unchanged (:402). The two sides have inconsistent bases: gap side = median (robust), fast side = 1.1/min(observedPeriods) — a single-sample extremum, now re-sampled every payload. Too tight: the scenario above gives limits[1] = 0.55/P, rejecting the true healthy rate. Too loose: one catch-up gap of 0.5P (the PR body's own "slip then catch up" mechanism) gives limits[1] = 2.2/P, effectively disabling overlap/fast detection for that payload's intra- and cross-payload checks. Consider achievedRateHz * UPPER or a symmetric ratio constant, so both sides derive from the median.

Minor

3. perSamplePeriodsS.get(size/2) is an upper-median (:366) — on a 2-element sorted list it returns the larger value, so for a 3-block payload with one dropped block ([P, 2P]), the "median" is 2P and achievedRateHz is half the truth, contradicting the comment on line 364. That value now feeds both setSamplingRate() and limits[0], and the resulting window [0.333/P, 1.1/P] contains both boundary rates, so the gap is missed entirely and all three blocks are stamped at half rate. Pre-existing line, but the new formula gives it a much larger role — worth fixing here (even size → average the two middles).

4. The new constant's javadoc is stale and self-contradictory (UtilCsvSplitting.java:24-28): "the window is seeded from the first payload that carries >= 2 blocks" is precisely the behavior this PR removes (commit 2 fixed the equivalent wording in PayloadContentsDetailsV8orAbove.java but not here), and "1.5x keeps comfortable margin on both sides" is wrong — the ratio applies only to the gap side; the fast side uses UPPER.

5. No driver-side unit test for the new arithmetic. Nothing in ShimmerDriver/src/test touches UtilCsvSplitting, SAMPLING_RATE_LIMITS_PER_SENSOR, or refineSlowSensorSamplingRateFromBlockTicks; validation is entirely ASM_PC integration tests in a separate, currently-blocked PR. The window computation is a pure function of a List<Double> of periods — the scenarios in findings 1 and 3 would each be a few-line unit test and would have caught them. Note the passing Test_065 recording is described as having no lost samples, so it never exercises the dropped-block path.

Nits

6. ShimmerDriver/build.gradle:52 stays 0.11.8_beta — if the Jenkins publish ASM_PC is blocked on reuses those coordinates, consumers can resolve a stale cached artifact of the same version. Flagging only so the uprev sequencing is explicit.

7. public class FILE_GAP_TOLERANCE_MULTIPLIER (UtilCsvSplitting.java:14) should be static — a non-static inner class holding only constants. Pre-existing; this PR adds a third constant to it.

Checked and cleared

No public/protected signature changes; the new constant is additive. SAMPLING_RATE_LIMITS_PER_SENSOR is only written internally, so the removed guard can only be overriding the configured-rate fallback, as claimed.
No cross-file leakage: ASM_PC calls clearMapOfSamplingRateLimitsPerSensor() on every CSV-set boundary.
No new threading race within this repo (no parallel payload parsing) — though the unsynchronised static HashMap remains one if a consumer ever parses two files concurrently in one JVM.
Unit consistency of achievedRateHz/1.5 holds whenever the median is measured correctly.
maxPeriodS removal leaves no dead code; FILE_GAP_TOLERANCE_MULTIPLIER.LOWER is still used by calculateSamplingRateLimits.

Findings 1 and 3 diminish if real payloads typically carry ≥4 slow-sensor blocks — the 2–3 figure is taken from the PR description, not measured.

Generated by Claude Code

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

…n + symmetric limits Review follow-up for #285. The window that decides whether a slow-sensor (VD6283 light / MLX90632 skin temp) block boundary is continuous was re-derived on every payload from the same handful of inter-block spacings it then had to judge, and its fast edge came from a single extremum. - Accumulate the measured per-sample periods per sensor across the payloads of a parse run (UtilCsvSplitting.SLOW_SENSOR_OBSERVED_PERIODS_PER_SENSOR) instead of replacing the estimate with the latest payload's values. A payload only carries 2-3 slow-sensor blocks, so a per-payload window absorbed a dropped block's 2x spacing into its own centre and never reported it - while flagging the healthy boundary back to the previous payload instead. Against a history spanning hundreds of payloads a single 2x outlier barely moves the median. The history is bounded (SLOW_SENSOR_PERIOD_HISTORY_MAX, oldest dropped first) so it still follows genuine long-term drift, and it shares its lifecycle with SAMPLING_RATE_LIMITS_PER_SENSOR - clearMapOfSamplingRateLimitsPerSensor() now clears both, so measurements cannot leak across a CSV-set boundary. - Derive BOTH sides of the window from that median: limits[0] = median / SLOW_SENSOR_MAX_INTER_BLOCK_GAP_RATIO, limits[1] = median * FILE_GAP_TOLERANCE_MULTIPLIER.UPPER. The fast side previously used 1/minObservedPeriod, i.e. a single-sample extremum. - Median helper now averages the two middle values for an even-sized input (the old get(size/2) was the upper median) and sorts a copy. The data blocks' sampling rate is set from the accumulated median too, so the CSV timestamps and the continuity check agree on the achieved cadence. - Rewrote the stale/incorrect SLOW_SENSOR_MAX_INTER_BLOCK_GAP_RATIO javadoc: it no longer claims first-payload seeding (removed by #285) and no longer claims the ratio applies to both sides. - FILE_GAP_TOLERANCE_MULTIPLIER is now a static nested class. - New driver-side JUnit tests (API_00009_UtilCsvSplittingSlowSensorGapWindow, 12 cases, no hardware data needed) covering the even-count median, +12.5% healthy jitter staying continuous, a dropped block being detected even though its own payload fed the estimate, the measured window taking over from a fallback-seeded band (and the fallback not clobbering it back), history bounding and the clear contract. The unconditional put is preserved: the measured window must still win over the configured-rate +/-10% band that populateExpectedPayloadTsDiffLimitMapIfNeeded seeds, as soon as a measurement exists. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014EmM3GD2F6zPqXmWHS94dh

remove(0) in a loop shifts the whole ArrayList once per removed element; clearing the excess head range via subList does it in a single pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014EmM3GD2F6zPqXmWHS94dh

…r-prs-bm3j94 DEV-793 Robust slow-sensor gap window: accumulated median + symmetric limits (review follow-up for #285)

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

+		int excess = accumulatedPeriodsS.size()-SLOW_SENSOR_PERIOD_HISTORY_MAX;
+		if(excess>0) {
+			accumulatedPeriodsS.subList(0, excess).clear();
+		}
+		return calculateMedian(accumulatedPeriodsS);


+		// The header-derived 10 Hz is an estimate; the achieved cadence is ~9.09 Hz,
+		// which the +/-10% band already calls a gap on every single boundary (this
+		// fragmented a 25-min DEV-927 recording into 7 CSVs).
+		assertTrue("baseline: the fallback band is too tight for the achieved cadence",
+				UtilCsvSplitting.isSamplingRateOutsideOfLimits(currentLimits(), boundaryRateHz(NOMINAL_PERIOD_S*1.125)));


…ymmetric limits (review follow-up for #285)"

…-asm-shimmer-prs-bm3j94 Revert "DEV-793 Robust slow-sensor gap window: accumulated median + symmetric limits (review follow-up for #285)"

Copilot

🟢 Approval recommended

The changes are localized, internally consistent, and align with the stated goal of preventing slow-sensor jitter from triggering erroneous CSV splits.

Review details

Files reviewed: 2/2 changed files
Comments generated: 1
Review effort level: Lite

 	public class FILE_GAP_TOLERANCE_MULTIPLIER {
 		// +/- 10%
 		public static final double UPPER = 1.1;
 		public static final double LOWER = 0.9;
+		/**


…oss payloads A Pulse+ recording (FW v2.01.001, 582 payloads, ~22 min) fragmented its VD6283 ambient-light stream into 129 CSVs of 10 samples each. The cause was the parser inventing the light sample spacing, not anything the device did. The VD6283 is not duty-cycled. The firmware samples it on a plain repeated app timer at one of 0.5/1/2/5/10/20 Hz (hal_slowSensorSampler.c slowSensorRateMs[]), buffers NUM_LIGHT_SAMPLES_PER_BLOCK = 10 samples and emits the block stamped with the time of its last sample. On this recording the rate is 1 Hz - the firmware's default when the sensor is enabled with rate index 0 - so a block spans 10 s and consecutive blocks are contiguous. The parser could not know that: the rate index is operational-config byte 75 and is NOT copied into the stored payload header (the header carries only the light enable, gain/dark and exposure index), so SensorVD6283.getRateFreq() fell back to 1e6/exposureUs. Exposure is purely an upper BOUND on the rate - the chip measures every max(inter-measurement, exposure) - and at the default 100 ms exposure that bound is 10 Hz, ten times the truth. Every block was therefore laid out over 0.9 s of the 10 s it actually spans, and the 9.1 s remainder looked like a gap: the continuity check measured 1.00 Hz against the [9.00, 11.00] Hz band and started a new CSV on every block. The occasional 11 s block is one failed I2C read, which does not increment lightSampleCount, so the block takes an extra period to fill. The fix refines the achieved per-sample period from the spacing of consecutive same-sensor block end ticks - the technique the storage format spec prescribes for the LSM6DSV - and applies it before the block timings are back-filled, as refineSlowSensorSamplingRateFromBlockTicks already did WITHIN a payload. The only reason it never engaged here is that it needed two blocks in one payload, and a 10 s block in a ~2 s payload is always alone, so the measurement now carries the previous payload's last block end ticks forward per slow-sensor id (UtilCsvSplitting.SLOW_SENSOR_LAST_BLOCK_END_TICKS, cleared with the rest of the splitting state whenever a CSV set is written out). Blocks then come out contiguous and the phantom gap is gone. Cross-payload measurement is only sound while the sensor's largest legitimate block span is under the one-minute wrap of the sub-minute tick counter, which isSlowSensorSpanUnambiguousAcrossPayloads checks: a 10-sample light block spans at most 20 s and is always safe, whereas a 16-sample skin-temp block at 0.25 Hz spans 64 s and would be ambiguous - so the MLX90632 keeps measuring within a payload only, which costs it nothing because its refresh code IS stored in the payload and its header-derived rate is already correct. Its output is unchanged. The CSV gap window then follows from the same measurements with the #285 formula (gap side median/1.5, fast side the fastest plausible boundary x1.1), since with the period right the achieved rate and the block-to-block rate are the same quantity. Robustness details: - The MEDIAN of the bounded history is applied, not the latest delta. A failed I2C read gives one 11 s block and a dropped block gives 20 s; the raw delta would stretch those blocks' samples by 10% or 100%, while the median keeps every block on the true period, which is where the samples actually are. - Every finite positive observation is learned from. Filtering at learn time is unsafe: the history is empty after every clear, so the first boundary would define what counts as plausible and a set opening on a dropped block would reject every healthy boundary thereafter. - Below SLOW_SENSOR_MIN_OBSERVATIONS_FOR_MEASURED_WINDOW = 3 the window is A-PRIORI, bounded by what the hardware can do rather than by an arbitrary factor: the firmware rate table for the VD6283 (MIN/MAX_SAMPLE_RATE_HZ = 0.5/20), the configured rate widened by the documented conversion slip for the MLX90632. Without it the boundary being judged is one of the one or two values a measured window would be built from, so that window would re-centre on it and the first boundary of every CSV set would be continuous however large its gap. - Only WHOLE blocks are measured; a block a midday/midnight transition cut in two is recombined exactly as a continuity check recombines it. - The unset block end time is DEFAULT_END_TIME_VALUE, not NaN. RESIDUAL, not fixed here: the first block of each CSV set is timed before anything has been observed, so it keeps the exposure-derived estimate and its reported start time is 8.1 s late at 1 Hz ((N-1) x (1.0 - 0.1) s). Only that block's start time and the CSV header start time it feeds are affected - the sample values and all block end times are correct. Re-timing it means revisiting the block after the next one arrives, by which point the file parser has deep-cloned it into the CSV dataset, so it belongs on the ASM_PC side. FOLLOW-UP for firmware: the light rate index is the only sensor rate not mirrored into the payload header. Adding it would remove the need for this inference and fix the first-block residual outright. Verified end to end on the recording: 129 light CSVs -> 1 CSV of 1290 rows whose sample data is byte-identical to the concatenation of the 129 it replaces; every block after the first now spans 9.000 s (10 samples, 1 s apart) instead of 0.900 s; zero VD6283 gap warnings, was 128. Accel/Gyro/Mag and SkinTemp CSVs byte-identical to master. The Payload_Metadata CSV changes in 123 of its 584 rows plus the VD6283 calculated-rate header line (11.111 -> 0.993 Hz): the light-bearing payloads' start times move ~8-9 s earlier, which is correct - the block really does carry samples taken over the preceding 10 s, exactly as skin-temp blocks already did. New driver-side unit tests (API_00009_VerisenseSlowSensorGapWindow, 17 cases, synthetic blocks only): 1 Hz light not splitting, the refined period making blocks contiguous, a configuration where the estimate equals the truth staying unchanged, a dropped block still splitting, the slowest firmware rate not splitting on a first boundary while a 60 s gap does, an I2C-dropped sample neither splitting nor stretching its neighbours, the DEV-927 skin-temp slip and catch-up, the ambiguity refusal, midday/midnight recombination, history bounding, the even-size median, the clear contract, fast sensors untouched, the measured window taking over, the overlap case and recovery from an anomalous start. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B9oz3Wc7og86aA7h2sn9ta

…oss payloads A Pulse+ recording (FW v2.01.001, 582 payloads, ~22 min) fragmented its VD6283 ambient-light stream into 129 CSVs of 10 samples each. The cause was the parser inventing the light sample spacing, not anything the device did. The VD6283 is not duty-cycled. The firmware samples it on a plain repeated app timer at one of 0.5/1/2/5/10/20 Hz (hal_slowSensorSampler.c slowSensorRateMs[]), buffers NUM_LIGHT_SAMPLES_PER_BLOCK = 10 samples and emits the block stamped with the time of its last sample. On this recording the rate is 1 Hz - the firmware's default when the sensor is enabled with rate index 0 - so a block spans 10 s and consecutive blocks are contiguous. The parser could not know that: the rate index is operational-config byte 75 and is NOT copied into the stored payload header (the header carries only the light enable, gain/dark and exposure index), so SensorVD6283.getRateFreq() fell back to 1e6/exposureUs. Exposure is purely an upper BOUND on the rate - the chip measures every max(inter-measurement, exposure) - and at the default 100 ms exposure that bound is 10 Hz, ten times the truth. Every block was therefore laid out over 0.9 s of the 10 s it actually spans, and the 9.1 s remainder looked like a gap: the continuity check measured 1.00 Hz against the [9.00, 11.00] Hz band and started a new CSV on every block. The occasional 11 s block is one failed I2C read, which does not increment lightSampleCount, so the block takes an extra period to fill. refineSlowSensorSamplingRateFromBlockTicks already recovered the period the right way - inter-block ticks / samples-per-block, the technique the storage-format spec prescribes for the LSM6DSV - but only WITHIN a payload, and a 10 s block in a ~2 s payload is always alone. It is now a dispatcher over two clearly separated paths: - refineSlowSensorSamplingRateAcrossPayloads carries the previous payload's last block end ticks forward per slow-sensor id (UtilCsvSplitting.SLOW_SENSOR_LAST_BLOCK_END_TICKS, cleared with the rest of the splitting state whenever a CSV set is written out), so a single block per payload is enough. Blocks then come out contiguous and the phantom gap is gone. - refineSlowSensorSamplingRatePerPayload is master 6d27fb2's body VERBATIM - per-payload block list, per-payload upper-middle (size()/2) median, early return below two blocks, old window formula and unconditional put, no cross-payload history. isSlowSensorSpanUnambiguousAcrossPayloads decides between them, and it is not a preference: the block end time is a counter that wraps every minute, so measuring across a payload boundary is only recoverable while the sensor's largest legitimate block span is under a minute. A 10-sample light block spans at most 20 s and qualifies; a 16-sample skin-temp block spans over a minute at its slowest output rate and does not. The MLX90632 therefore keeps master's path - which it also does not need to leave, because its refresh code IS stored in the payload so its header-derived rate is already correct. Retaining master's skin-temp logic is deliberate: the DEV-927 reference CSVs (ASM_PC Test_065) cannot be reached from this environment, so byte-identity for that sensor is made to hold BY CONSTRUCTION rather than on trust - an accumulated median or the new even-size median definition would move every skin-temp sample timestamp. FOLLOW-UP: with two blocks in a payload and a slip-then-catch-up boundary, master's per-payload window was measured to split at the catch-up; that false split is preserved here and is worth revisiting once Test_065's reference data is reachable. The CSV gap window then follows from the same measurements with the #285 formula (gap side median/1.5, fast side the fastest plausible boundary x1.1), since with the period right the achieved rate and the block-to-block rate are the same quantity. Robustness details: - The MEDIAN of the bounded history is applied, not the latest delta. A failed I2C read gives one 11 s block and a dropped block gives 20 s; the raw delta would stretch those blocks' samples by 10% or 100%, while the median keeps every block on the true period, which is where the samples actually are. - Every plausible observation is learned from. Filtering against the HISTORY at learn time is unsafe: it is empty after every clear, so the first boundary would define what counts as plausible and a set opening on a dropped block would reject every healthy boundary thereafter. - Neither recorded nor applied, however, is a period outside what the hardware can be configured to produce. A real gap of 60-70 s aliases through the sub-minute tick counter into an ordinary-looking ~1 s observation; detection is unaffected (isDataBlockContinuous works on absolute real-world-clock ms) but at a set start one aliased value can be the whole history and would re-time every block. - Below SLOW_SENSOR_MIN_OBSERVATIONS_FOR_MEASURED_WINDOW = 3 the window is A-PRIORI, bounded by the firmware rate table for the VD6283 and by the configured rate widened for conversion slip for the MLX90632. Without it the boundary being judged is one of the one or two values a measured window would be built from, so that window would re-centre on it and the first boundary of every CSV set would be continuous however large its gap. Its blind spot is quantified in the Javadoc: on those two boundaries a light spacing up to 30 s is accepted, so up to 20 s of lost data at 1 Hz goes unreported there permanently, and the fast side accepts a ~9.5 s backwards jump; 60 s still splits. - A block a midday/midnight transition cut in two is measured on its SECOND part - which keeps the original end ticks - with both parts' sample counts added back. Measuring a recombined block would lose the ticks, since recombineDataBlockDetailsForContinuityCheck only carries the millisecond times a continuity check needs. - The unset block end time is DEFAULT_END_TIME_VALUE, not NaN. RESIDUAL, not fixed here: the first block of each CSV set is timed before anything has been observed, so it keeps the exposure-derived estimate and its reported start time is 8.1 s late at 1 Hz ((N-1) x (1.0 - 0.1) s). Only that block's start time and the CSV header start time it feeds are affected - the sample values and all block end times are correct. Re-timing it means revisiting the block after the next one arrives, by which point the file parser has deep-cloned it into the CSV dataset, so it belongs on the ASM_PC side. FOLLOW-UP for firmware: the light rate index is the only sensor rate not mirrored into the payload header. Adding it would remove the need for this inference and fix the first-block residual outright. Verified end to end on the recording: 129 light CSVs -> 1 CSV of 1290 rows whose sample data is byte-identical to the concatenation of the 129 it replaces; every block after the first now spans 9.000 s (10 samples, 1 s apart) instead of 0.900 s; zero VD6283 gap warnings, was 128. The Accel/Gyro/Mag and SkinTemp CSVs are byte-identical to master's output. The Payload_Metadata CSV changes in 124 of its 584 rows plus the VD6283 calculated-rate header line (11.111 -> 0.993 Hz): the light-bearing payloads' start times move ~8-9 s earlier, which is correct - the block really does carry samples taken over the preceding 10 s, exactly as skin-temp blocks already did. New driver-side unit tests (API_00009_VerisenseSlowSensorGapWindow, 18 cases) drive the real package-private refinement methods on synthetic PayloadContentsDetailsV8orAbove payloads carrying end TICKS, and judge the split decisions through the real isDataBlockContinuous - no reflection and no re-implementation of the algorithm. Non-vacuity is demonstrated: with master's PayloadContentsDetailsV8orAbove swapped in, 12 of the 18 fail (including the 1 Hz light, minute-rebase, split-part and split-decision cases); the 6 that pass are the two that pin master's own skin-temp behaviour plus four that exercise UtilCsvSplitting alone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01B9oz3Wc7og86aA7h2sn9ta

marknolan requested review from jyong15 and a lite review from Copilot August 11, 2026 07:42

Copilot started reviewing on behalf of marknolan August 11, 2026 07:42 View session

Copilot AI reviewed Aug 11, 2026

View reviewed changes

Comment thread ...c/main/java/com/shimmerresearch/verisense/payloaddesign/PayloadContentsDetailsV8orAbove.java

Comment thread ...c/main/java/com/shimmerresearch/verisense/payloaddesign/PayloadContentsDetailsV8orAbove.java Outdated

marknolan self-assigned this Aug 11, 2026

jyong15 reviewed Aug 20, 2026

View reviewed changes

Merge branch 'master' into pr/285

b38425f

Copilot AI review requested due to automatic review settings August 20, 2026 01:46

Copilot started reviewing on behalf of jyong15 August 20, 2026 01:46 View session

Copilot AI reviewed Aug 20, 2026

View reviewed changes

jyong15 mentioned this pull request Aug 21, 2026

DEV-793 Robust slow-sensor gap window: accumulated median + symmetric limits (review follow-up for #285) #289

Merged

claude and others added 2 commits August 21, 2026 05:56

Merge pull request #289 from ShimmerResearch/claude/review-asm-shimme…

3bd5595

…r-prs-bm3j94 DEV-793 Robust slow-sensor gap window: accumulated median + symmetric limits (review follow-up for #285)

Copilot AI review requested due to automatic review settings August 21, 2026 06:48

Copilot started reviewing on behalf of jyong15 August 21, 2026 06:49 View session

Copilot AI reviewed Aug 21, 2026

View reviewed changes

jyong15 added 2 commits September 1, 2026 13:58

Revert "DEV-793 Robust slow-sensor gap window: accumulated median + s…

0ed5d27

…ymmetric limits (review follow-up for #285)"

Merge pull request #290 from ShimmerResearch/revert-289-claude/review…

7a29d3b

…-asm-shimmer-prs-bm3j94 Revert "DEV-793 Robust slow-sensor gap window: accumulated median + symmetric limits (review follow-up for #285)"

Copilot AI review requested due to automatic review settings September 1, 2026 05:59

Copilot started reviewing on behalf of jyong15 September 1, 2026 06:00 View session

Copilot AI reviewed Sep 1, 2026

View reviewed changes

Comment thread ShimmerDriver/src/main/java/com/shimmerresearch/verisense/payloaddesign/UtilCsvSplitting.java

Comment on lines 14 to +18

public class FILE_GAP_TOLERANCE_MULTIPLIER {

// +/- 10%

public static final double UPPER = 1.1;

public static final double LOWER = 0.9;

/**

jyong15 self-requested a review September 1, 2026 06:29

jyong15 approved these changes Sep 1, 2026

View reviewed changes

jyong15 merged commit 85593e7 into master Sep 1, 2026
2 checks passed

jyong15 deleted the DEV-793_slow_sensor_gap_window branch September 1, 2026 06:29

jyong15 mentioned this pull request Sep 3, 2026

DEV-979 Derive slow-sensor sample period from inter-block spacing across payloads #292

Merged

Navigation Menu

DEV-793 Widen + always apply the slow-sensor CSV gap window - #285

DEV-793 Widen + always apply the slow-sensor CSV gap window#285
jyong15 merged 8 commits into
masterfrom
DEV-793_slow_sensor_gap_window

marknolan commented Aug 11, 2026 •

edited by atlassian Bot

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

jyong15 left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

marknolan commented Aug 11, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Two defects, both in the slow-sensor gap-window seeding

Result

Validation

Sequencing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

jyong15 left a comment

Choose a reason for hiding this comment

Code review — DEV-793 slow-sensor CSV gap window

Major

Minor

Nits

Checked and cleared

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

marknolan commented Aug 11, 2026 •

edited by atlassian Bot

Loading