Skip to content

Commit 0d65311

Browse files
marknolanclaude
andcommitted
DEV-793 Address adversarial review: gyro sensitivity, block-ID guard, split-safe offsets, accel helpers
Review findings from PR #277 (all verified against code/datasheet before fixing): 1) K1: LSM6DSV gyro sensitivity now uses the ST datasheet angular-rate spec (4.375 mdps/LSB at +-125 dps, doubling per range - same as the gen-1 LSM6DS3) instead of a 32768/FS derivation, which was ~12.8% low. Verified: Test_029's rotation peak rescales 512 -> 586.9 dps, exactly the predicted x1.147. 2) CRITICAL: parseDataBlockMetaData now only accepts data-block IDs with parse support (<= LSM6DSV); the declared-but-unimplemented gen-2 IDs (LIGHT/ALGO_HUB/SKIN_TEMP/FLICKER) are rejected with the informative exception instead of corrupting the parse, and calculateFifoBlockSize throws instead of returning Integer.MIN_VALUE. 3) MAJOR: byte-offset arithmetic through the payload now uses a raw block size (getQtySensorDataBytesInDatablockRaw) that the midday/midnight CSV-split logic cannot recompute - the recomputed dataPacketSize*sampleCount is wrong for the variable-length LSM6DSV tagged FIFO. 4) MAJOR: isAnAccelEnabled/getPrimaryAccel/isSensorKeyAnAccel now include the LSM6DSV. 5) NIT: the LSM6DSV 'Sensor config:' line labels the mag rate 'Configured =' and appends the effective cap when it exceeds the IMU ODR. 6) Comment documenting why the gen-2 battery divider is gated on hardware revision rather than firmware version. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 04d884c commit 0d65311

5 files changed

Lines changed: 61 additions & 12 deletions

File tree

ShimmerDriver/src/main/java/com/shimmerresearch/verisense/VerisenseDevice.java

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -805,6 +805,8 @@ public SENSORS getPrimaryAccel() {
805805
return SENSORS.LIS2DW12;
806806
} else if(isSensorEnabled(Configuration.Verisense.SENSOR_ID.LSM6DS3_ACCEL)) {
807807
return SENSORS.LSM6DS3;
808+
} else if(isSensorEnabled(Configuration.Verisense.SENSOR_ID.LSM6DSV_ACCEL)) {
809+
return SENSORS.LSM6DSV;
808810
}
809811
return null;
810812
}
@@ -1032,9 +1034,20 @@ && isAnyLsm6dsvChannelEnabled()) {
10321034
sb.append("; Mag ");
10331035
sb.append(SENSOR_CONFIG_STRINGS.RATE);
10341036
if(isSensorEnabled(Configuration.Verisense.SENSOR_ID.LSM6DSV_MAG)) {
1037+
// Labelled "Configured" because the sensor hub reads the LIS2MDL once per
1038+
// accel/gyro sample, so the EFFECTIVE mag rate is capped at the IMU ODR.
1039+
sb.append(SENSOR_CONFIG_STRINGS.SAMPLING_RATE_CONFIGURED);
10351040
sb.append(sensorLsm6dsv.getMagRateFreq());
10361041
sb.append(" ");
10371042
sb.append(CHANNEL_UNITS.FREQUENCY);
1043+
double imuOdrHz = sensorLsm6dsv.getRateFreq();
1044+
if(imuOdrHz>0 && sensorLsm6dsv.getMagRateFreq()>imuOdrHz) {
1045+
sb.append(" (capped to ");
1046+
sb.append(imuOdrHz);
1047+
sb.append(" ");
1048+
sb.append(CHANNEL_UNITS.FREQUENCY);
1049+
sb.append(" by the IMU rate)");
1050+
}
10381051
} else {
10391052
sb.append("Off");
10401053
}
@@ -1237,14 +1250,15 @@ public boolean isAnAdcChEnabled() {
12371250
//TODO set derived algorithm enabled bit like as done for PPG rather then checking
12381251
public boolean isAnAccelEnabled() {
12391252
if(isSensorEnabled(Configuration.Verisense.SENSOR_ID.LIS2DW12_ACCEL)
1240-
|| isSensorEnabled(Configuration.Verisense.SENSOR_ID.LSM6DS3_ACCEL)) {
1253+
|| isSensorEnabled(Configuration.Verisense.SENSOR_ID.LSM6DS3_ACCEL)
1254+
|| isSensorEnabled(Configuration.Verisense.SENSOR_ID.LSM6DSV_ACCEL)) {
12411255
return true;
12421256
}
12431257
return false;
12441258
}
12451259

12461260
public static boolean isSensorKeyAnAccel(SENSORS sensorClassKey) {
1247-
if(sensorClassKey==SENSORS.LIS2DW12 || sensorClassKey==SENSORS.LSM6DS3) {
1261+
if(sensorClassKey==SENSORS.LIS2DW12 || sensorClassKey==SENSORS.LSM6DS3 || sensorClassKey==SENSORS.LSM6DSV) {
12481262
return true;
12491263
}
12501264
return false;
@@ -1623,6 +1637,11 @@ public int calculateFifoBlockSize(SENSORS sensorClassKey) {
16231637
default:
16241638
break;
16251639
}
1640+
if(dataBlockSize==Integer.MIN_VALUE) {
1641+
// Fail loudly rather than returning a sentinel that silently corrupts the
1642+
// data-block byte index further down the parse.
1643+
throw new IllegalStateException("calculateFifoBlockSize: no FIFO block size defined for sensor class " + sensorClassKey);
1644+
}
16261645
return dataBlockSize;
16271646
}
16281647

@@ -1861,8 +1880,13 @@ public DataBlockDetails parseDataBlockMetaData(byte[] byteBuffer, int dataBlockS
18611880

18621881
DataBlockDetails dataBlockDetails = null;
18631882
DATABLOCK_SENSOR_ID[] payloadSensorIdValues = DATABLOCK_SENSOR_ID.values();
1864-
// -1 because PPG_MAX86150 has been temporarily added to PAYLOAD_SENSOR_ID
1865-
if(sensorId>0 && sensorId<payloadSensorIdValues.length-1) {
1883+
// Only IDs with a parse implementation are accepted. The enum intentionally
1884+
// declares the remaining second-generation block IDs (LIGHT, ALGO_HUB,
1885+
// SKIN_TEMP, FLICKER) ahead of their parser support - accepting them here
1886+
// without a getSensorKeysForDatablockId()/calculateFifoBlockSize() handler
1887+
// would corrupt the parse, so they are rejected with the informative
1888+
// exception below until their decoders are implemented.
1889+
if(sensorId>0 && sensorId<=DATABLOCK_SENSOR_ID.LSM6DSV.ordinal()) {
18661890
DATABLOCK_SENSOR_ID datablockSensorId = payloadSensorIdValues[sensorId];
18671891

18681892
List<SENSORS> listOfSensorClassKeys = getOrCreateListOfSensorClassKeysForDataBlockId(datablockSensorId);

ShimmerDriver/src/main/java/com/shimmerresearch/verisense/payloaddesign/DataBlockDetails.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,15 @@ public enum DATABLOCK_SENSOR_ID {
5353
private VerisenseTimeDetails timeDetailsUcClock = new VerisenseTimeDetails();
5454

5555
public int qtySensorDataBytesInDatablock;
56+
/**
57+
* The raw on-disk sensor-data byte length as parsed from the payload. Unlike
58+
* {@link #qtySensorDataBytesInDatablock} this is never recomputed by the
59+
* midday/midnight CSV-split logic ({@link #setSampleCountAndUpdateDataBlockSize(int)})
60+
* - essential for variable-length blocks (e.g. the LSM6DSV tagged FIFO, whose raw
61+
* length includes timestamp-tag and magnetometer entries that don't map 1:1 to
62+
* aligned samples). Always use this for advancing byte offsets through the payload.
63+
*/
64+
private int qtySensorDataBytesInDatablockRaw;
5665
public int dataPacketSize;
5766
private double samplingRate;
5867

@@ -102,10 +111,15 @@ public DataBlockDetails(DATABLOCK_SENSOR_ID datablockSensorId, int payloadIndex,
102111

103112
public void setMetadata(int dataBlockSizeSensorData, int dataPacketSize, double samplingRate) {
104113
this.qtySensorDataBytesInDatablock = dataBlockSizeSensorData;
114+
this.qtySensorDataBytesInDatablockRaw = dataBlockSizeSensorData;
105115
this.dataPacketSize = dataPacketSize;
106116
setSamplingRate(samplingRate);
107117
calculateSampleCount();
108118
}
119+
120+
public int getQtySensorDataBytesInDatablockRaw() {
121+
return qtySensorDataBytesInDatablockRaw;
122+
}
109123

110124
public void setRwcEndTimeMinutesAndCalculateTimings(long rtcEndTimeMinutes) {
111125
setUcClockOrRwcClockEndTimeMinutesAndCalculateTimings(rtcEndTimeMinutes, false);

ShimmerDriver/src/main/java/com/shimmerresearch/verisense/payloaddesign/PayloadContentsDetailsV8orAbove.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,10 @@ public void parsePayloadContentsMetaData(int binFileByteIndex) throws IOExceptio
5757
setOfPayloadSensorIds.add(dataBlockDetails.datablockSensorId);
5858

5959
// Update byte offset as it's passed by value into "parseDataBlockMetaData"
60-
int dataBlockTotalSize = BYTE_COUNT.PAYLOAD_CONTENTS_GEN8_SENSOR_ID + BYTE_COUNT.PAYLOAD_CONTENTS_RTC_BYTES_TICKS + dataBlockDetails.qtySensorDataBytesInDatablock;
60+
// Use the RAW block size for byte-offset arithmetic - qtySensorDataBytesInDatablock
61+
// can be recomputed by the midday/midnight split logic (wrongly for
62+
// variable-length blocks such as the LSM6DSV tagged FIFO).
63+
int dataBlockTotalSize = BYTE_COUNT.PAYLOAD_CONTENTS_GEN8_SENSOR_ID + BYTE_COUNT.PAYLOAD_CONTENTS_RTC_BYTES_TICKS + dataBlockDetails.getQtySensorDataBytesInDatablockRaw();
6164
currentByteIndexInPayload += dataBlockTotalSize;
6265

6366
if(isParserAtEndOfBuffer(byteBuffer.length, currentByteIndexInPayload)) {
@@ -407,7 +410,8 @@ public void parsePayloadSensorData() {
407410
if(dataBlockDetails.listOfSensorClassKeys.contains(sensorClassKey)) {
408411
verisenseDevice.parseDataBlockData(dataBlockDetails, byteBuffer, currentByteIndex, COMMUNICATION_TYPE.SD);
409412
}
410-
currentByteIndex += dataBlockDetails.qtySensorDataBytesInDatablock;
413+
// RAW size: immune to midday/midnight split recomputation (see above)
414+
currentByteIndex += dataBlockDetails.getQtySensorDataBytesInDatablockRaw();
411415

412416
dataBlockIndex++;
413417

ShimmerDriver/src/main/java/com/shimmerresearch/verisense/sensors/SensorBattVoltageVerisense.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,10 @@ public ObjectCluster processDataCustom(SensorDetails sensorDetails, byte[] rawDa
262262
mShimmerDevice.getShimmerVerObject().getHardwareVersion(), mShimmerDevice.getExpansionBoardRev())) {
263263
// Second-generation hardware (SR61-5/6, SR68-9/10) battery-sense divider.
264264
// Matches the firmware's own conversion in hal_asm_battery.c (measureBatteryVoltage).
265+
// Deliberately gated on the HARDWARE revision (not isPayloadDesignV13orAbove):
266+
// the divider is a property of the board's battery-sense circuit, present
267+
// regardless of which firmware happens to be flashed - the firmware applies
268+
// the same hardware-keyed rule.
265269
calData *= BATTERY_VOLTAGE_DIVIDER_RATIO_GEN2;
266270
}
267271
objectCluster.addCalData(channelDetails, calData, objectCluster.getIndexKeeper()-1);

ShimmerDriver/src/main/java/com/shimmerresearch/verisense/sensors/SensorLSM6DSV.java

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -241,12 +241,15 @@ public static final class DatabaseConfigHandle {
241241
public static final double[][] SENS_ACCEL_8G = {{417.6759,0,0},{0,417.6759,0},{0,0,417.6759}};
242242
public static final double[][] SENS_ACCEL_16G = {{208.8379,0,0},{0,208.8379,0},{0,0,208.8379}};
243243

244-
// Gyro sensitivity (LSB per dps) = 32768/FS_dps
245-
public static final double[][] SENS_GYRO_125DPS = {{262.144,0,0},{0,262.144,0},{0,0,262.144}};
246-
public static final double[][] SENS_GYRO_250DPS = {{131.072,0,0},{0,131.072,0},{0,0,131.072}};
247-
public static final double[][] SENS_GYRO_500DPS = {{65.536,0,0},{0,65.536,0},{0,0,65.536}};
248-
public static final double[][] SENS_GYRO_1000DPS = {{32.768,0,0},{0,32.768,0},{0,0,32.768}};
249-
public static final double[][] SENS_GYRO_2000DPS = {{16.384,0,0},{0,16.384,0},{0,0,16.384}};
244+
// Gyro sensitivity (LSB per dps) from the ST datasheet angular-rate sensitivity
245+
// (4.375 mdps/LSB at +-125 dps, doubling per range) - the same spec/values as the
246+
// gen-1 LSM6DS3. NOTE: the gyro does NOT span the full 16-bit range at its nominal
247+
// full scale (unlike the accel), so a 32768/FS derivation is ~12.8% off.
248+
public static final double[][] SENS_GYRO_125DPS = {{228.571428571,0,0},{0,228.571428571,0},{0,0,228.571428571}};
249+
public static final double[][] SENS_GYRO_250DPS = {{114.285714286,0,0},{0,114.285714286,0},{0,0,114.285714286}};
250+
public static final double[][] SENS_GYRO_500DPS = {{57.142857143,0,0},{0,57.142857143,0},{0,0,57.142857143}};
251+
public static final double[][] SENS_GYRO_1000DPS = {{28.571428571,0,0},{0,28.571428571,0},{0,0,28.571428571}};
252+
public static final double[][] SENS_GYRO_2000DPS = {{14.285714286,0,0},{0,14.285714286,0},{0,0,14.285714286}};
250253

251254
// Mag sensitivity (LSB per uT) = 1/0.15
252255
public static final double[][] SENS_MAG = {{6.666667,0,0},{0,6.666667,0},{0,0,6.666667}};

0 commit comments

Comments
 (0)