fix(rssi): correct parse_esp32_frame offsets + carry RSSI through feature_state
Two server-side parsers (csi.rs::parse_esp32_frame and the duplicate in main.rs) read every field after `n_antennas` from offsets shifted by 2 bytes — n_subcarriers as u8 instead of u16, sequence at 10..14 instead of 12..16, rssi at 14 instead of 16. The saturating_neg() workaround hid the bug by always forcing a negative dBm value, so the trace looked plausible but was actually a slice of mid-sequence number. ADR-100 D3 documented this as an open item; this commit closes it. Adds two regression tests in csi.rs (header-offset round-trip with distinctive values per field, plus 20-byte boundary case) so the layout contract can't drift again without CI catching it. Even with both parsers correct, RSSI never reached the UI because the firmware now ships only rv_feature_state_t (0xC5110006) — raw CSI (0xC5110001) is no longer hot. rv_feature_state had no RSSI field; both parsers fell back to rssi: -50 hardcode. To fix without a protocol bump: repurpose the first byte of the trailing `reserved` field (offset 54) as `int8_t rssi_dbm`. Firmware fills it from radio_ops::get_health()::rssi_median_dbm in emit_feature_state. Server reads buf[54] as i8; 0 means "not measured yet" → keeps the historical -50 fallback for backward compat with pre-update nodes. Verified live on TP-Link WISP (192.168.0.100/101): node 1: -54 dBm node 2: -63 dBm (was plateau -50.0 fallback) Co-Authored-By: claude-flow <ruv@ruv.net>
This commit is contained in:
parent
c3126c39a3
commit
3393c1e839
|
|
@ -275,6 +275,11 @@ static void emit_feature_state(void)
|
|||
pkt.presence_score = obs.presence_score;
|
||||
pkt.anomaly_score = obs.anomaly_score;
|
||||
pkt.node_coherence = obs.node_coherence;
|
||||
/* ADR-100 D3: ship median RSSI through feature_state so the server
|
||||
* UI's RSSI trace has something other than the -50 fallback. The
|
||||
* value comes from radio_ops::get_health() which medians rx_ctrl.rssi
|
||||
* across the recent capture window. 0 means "not measured yet". */
|
||||
pkt.rssi_dbm = obs.rssi_median_dbm;
|
||||
}
|
||||
|
||||
/* Fill vitals from edge_processing's latest packet. */
|
||||
|
|
|
|||
|
|
@ -65,7 +65,11 @@ typedef struct __attribute__((packed)) {
|
|||
float env_shift_score; /**< 0..1, baseline drift. */
|
||||
float node_coherence; /**< 0..1, multi-link agreement. */
|
||||
uint16_t quality_flags; /**< RV_QFLAG_* bitmap. */
|
||||
uint16_t reserved;
|
||||
int8_t rssi_dbm; /**< Median RSSI over the emit window (i8, dBm). 0 = not measured.
|
||||
ADR-100 D3: previously the same byte was `reserved` — but downstream
|
||||
UI/classifier needs RSSI per node and the legacy raw-CSI parse path
|
||||
(0xC5110001) is no longer hot on this FW. Server reads buf[54] as i8. */
|
||||
uint8_t reserved; /**< Padding/aux byte; keep zero until next protocol bump. */
|
||||
uint32_t crc32; /**< IEEE CRC32 over bytes [0..end-4]. */
|
||||
} rv_feature_state_t;
|
||||
|
||||
|
|
|
|||
|
|
@ -40,6 +40,12 @@ pub fn parse_rv_feature_state(buf: &[u8]) -> Option<Esp32VitalsPacket> {
|
|||
let _env_shift_score = f32::from_le_bytes([buf[44], buf[45], buf[46], buf[47]]);
|
||||
let _node_coherence = f32::from_le_bytes([buf[48], buf[49], buf[50], buf[51]]);
|
||||
let quality_flags = u16::from_le_bytes([buf[52], buf[53]]);
|
||||
// ADR-100 D3: FW ships median RSSI in byte 54 (was `reserved`); 0 means
|
||||
// "not yet measured" → keep the historical -50 fallback so the UI's
|
||||
// RSSI trace isn't pinned at a misleading 0 dBm. Stays in sync with
|
||||
// the duplicate parser in main.rs (must remain identical).
|
||||
let rssi_byte = buf[54] as i8;
|
||||
let rssi: i8 = if rssi_byte == 0 { -50 } else { rssi_byte };
|
||||
|
||||
// Bit 0 of quality_flags = presence valid
|
||||
let presence_valid = (quality_flags & (1 << 0)) != 0;
|
||||
|
|
@ -58,7 +64,7 @@ pub fn parse_rv_feature_state(buf: &[u8]) -> Option<Esp32VitalsPacket> {
|
|||
motion,
|
||||
breathing_rate_bpm: respiration_bpm as f64,
|
||||
heartrate_bpm: heartbeat_bpm as f64,
|
||||
rssi: -50, // not carried; approximation so UI shows a value
|
||||
rssi,
|
||||
n_persons,
|
||||
motion_energy: motion_score,
|
||||
presence_score,
|
||||
|
|
@ -123,14 +129,32 @@ pub fn parse_esp32_frame(buf: &[u8]) -> Option<Esp32Frame> {
|
|||
let magic = u32::from_le_bytes([buf[0], buf[1], buf[2], buf[3]]);
|
||||
if magic != 0xC511_0001 { return None; }
|
||||
|
||||
let node_id = buf[4];
|
||||
let n_antennas = buf[5];
|
||||
let n_subcarriers = buf[6];
|
||||
let freq_mhz = u16::from_le_bytes([buf[8], buf[9]]);
|
||||
let sequence = u32::from_le_bytes([buf[10], buf[11], buf[12], buf[13]]);
|
||||
let rssi_raw = buf[14] as i8;
|
||||
let rssi = if rssi_raw > 0 { rssi_raw.saturating_neg() } else { rssi_raw };
|
||||
let noise_floor = buf[15] as i8;
|
||||
// On-wire layout — must stay in lockstep with
|
||||
// firmware/esp32-csi-node/main/csi_collector.c::serialize_csi_frame().
|
||||
// ADR-100 D3 fix: the previous version of this parser had every field
|
||||
// after `n_antennas` shifted by 2 bytes (n_subcarriers read as u8,
|
||||
// freq_mhz/sequence misaligned, rssi read from buf[14] instead of
|
||||
// buf[16]). That made `mean_rssi` random noise (a byte taken from
|
||||
// mid-sequence) which the saturating_neg() workaround then forced
|
||||
// negative — hiding the bug from cursory log inspection while keeping
|
||||
// RSSI traces useless. Layout below matches the FW byte-for-byte.
|
||||
// [0..4] magic (u32 LE)
|
||||
// [4] node_id (u8)
|
||||
// [5] n_antennas (u8)
|
||||
// [6..8] n_subcarriers(u16 LE)
|
||||
// [8..12] freq_mhz (u32 LE)
|
||||
// [12..16] sequence (u32 LE)
|
||||
// [16] rssi (i8)
|
||||
// [17] noise_floor (i8)
|
||||
// [18..20] reserved
|
||||
// [20..] I/Q payload
|
||||
let node_id = buf[4];
|
||||
let n_antennas = buf[5];
|
||||
let n_subcarriers = u16::from_le_bytes([buf[6], buf[7]]) as u8;
|
||||
let freq_mhz = u16::from_le_bytes([buf[8], buf[9]]); // upper bytes always 0 in practice
|
||||
let sequence = u32::from_le_bytes([buf[12], buf[13], buf[14], buf[15]]);
|
||||
let rssi = buf[16] as i8; // already in [-128..127]
|
||||
let noise_floor = buf[17] as i8;
|
||||
|
||||
let iq_start = 20;
|
||||
let n_pairs = n_antennas as usize * n_subcarriers as usize;
|
||||
|
|
@ -729,3 +753,63 @@ pub fn chrono_timestamp() -> u64 {
|
|||
.map(|d| d.as_secs())
|
||||
.unwrap_or(0)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
/// Regression test for ADR-100 D3: parse_esp32_frame must extract
|
||||
/// fields from the exact offsets the firmware writes in
|
||||
/// csi_collector.c::serialize_csi_frame(). A previous version
|
||||
/// shifted every field after `n_antennas` by 2 bytes, making RSSI
|
||||
/// random noise. This test builds a synthetic frame with distinctive
|
||||
/// values for every header field and asserts the parser recovers
|
||||
/// each one.
|
||||
#[test]
|
||||
fn parse_esp32_frame_header_offsets_match_firmware() {
|
||||
let n_sub: u16 = 64;
|
||||
let freq_mhz: u32 = 2462; // channel 11
|
||||
let sequence: u32 = 0x1122_3344;
|
||||
let rssi: i8 = -57;
|
||||
let noise_floor: i8 = -95;
|
||||
let n_pairs = 1 * n_sub as usize;
|
||||
let mut buf = vec![0u8; 20 + n_pairs * 2];
|
||||
|
||||
buf[0..4].copy_from_slice(&0xC511_0001u32.to_le_bytes());
|
||||
buf[4] = 7; // node_id
|
||||
buf[5] = 1; // n_antennas
|
||||
buf[6..8].copy_from_slice(&n_sub.to_le_bytes()); // u16
|
||||
buf[8..12].copy_from_slice(&freq_mhz.to_le_bytes()); // u32
|
||||
buf[12..16].copy_from_slice(&sequence.to_le_bytes()); // u32
|
||||
buf[16] = rssi as u8;
|
||||
buf[17] = noise_floor as u8;
|
||||
// [18..20] reserved zeros
|
||||
// I/Q: leave zeros — parser still needs them present
|
||||
|
||||
let f = parse_esp32_frame(&buf).expect("frame parses");
|
||||
assert_eq!(f.node_id, 7);
|
||||
assert_eq!(f.n_antennas, 1);
|
||||
assert_eq!(f.n_subcarriers as u16, n_sub);
|
||||
assert_eq!(f.freq_mhz, freq_mhz as u16); // parser narrows to u16 (upper bytes always 0 in WiFi)
|
||||
assert_eq!(f.sequence, sequence);
|
||||
assert_eq!(f.rssi, -57, "rssi must come from byte 16, not 14");
|
||||
assert_eq!(f.noise_floor, -95, "noise_floor must come from byte 17, not 15");
|
||||
assert_eq!(f.amplitudes.len(), n_pairs);
|
||||
}
|
||||
|
||||
/// Boundary case: minimum-size frame (20 B header, zero I/Q pairs)
|
||||
/// must not panic and must still expose RSSI correctly.
|
||||
#[test]
|
||||
fn parse_esp32_frame_min_size_rssi_only() {
|
||||
let mut buf = vec![0u8; 20];
|
||||
buf[0..4].copy_from_slice(&0xC511_0001u32.to_le_bytes());
|
||||
buf[5] = 0; // 0 antennas → 0 IQ pairs
|
||||
buf[6..8].copy_from_slice(&0u16.to_le_bytes());
|
||||
buf[16] = (-71i8) as u8;
|
||||
buf[17] = (-92i8) as u8;
|
||||
let f = parse_esp32_frame(&buf).expect("min frame parses");
|
||||
assert_eq!(f.rssi, -71);
|
||||
assert_eq!(f.noise_floor, -92);
|
||||
assert!(f.amplitudes.is_empty());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1106,6 +1106,11 @@ fn parse_rv_feature_state(buf: &[u8]) -> Option<Esp32VitalsPacket> {
|
|||
let respiration_bpm = f32::from_le_bytes([buf[24], buf[25], buf[26], buf[27]]);
|
||||
let heartbeat_bpm = f32::from_le_bytes([buf[32], buf[33], buf[34], buf[35]]);
|
||||
let quality_flags = u16::from_le_bytes([buf[52], buf[53]]);
|
||||
// ADR-100 D3: FW now ships median RSSI in byte 54 (was `reserved`). Zero
|
||||
// means "not yet measured" — keep the historical -50 fallback in that
|
||||
// case so the UI doesn't show a misleading 0 dBm.
|
||||
let rssi_byte = buf[54] as i8;
|
||||
let rssi: i8 = if rssi_byte == 0 { -50 } else { rssi_byte };
|
||||
|
||||
let presence_valid = (quality_flags & (1 << 0)) != 0;
|
||||
// Threshold lowered from 0.5 to 0.15 for low-SNR multi-meter deployments
|
||||
|
|
@ -1122,7 +1127,7 @@ fn parse_rv_feature_state(buf: &[u8]) -> Option<Esp32VitalsPacket> {
|
|||
motion,
|
||||
breathing_rate_bpm: respiration_bpm as f64,
|
||||
heartrate_bpm: heartbeat_bpm as f64,
|
||||
rssi: -50,
|
||||
rssi,
|
||||
n_persons,
|
||||
motion_energy: motion_score,
|
||||
presence_score,
|
||||
|
|
@ -1326,15 +1331,21 @@ fn parse_esp32_frame(buf: &[u8]) -> Option<Esp32Frame> {
|
|||
// [17] noise_floor (i8)
|
||||
// [18..19] reserved
|
||||
// [20..] I/Q data
|
||||
//
|
||||
// ADR-100 D3 fix: previous code read every field after `n_antennas`
|
||||
// from offsets shifted by 2 bytes (n_subcarriers as u8 instead of u16,
|
||||
// sequence at 10..14, rssi at 14, noise_floor at 15). That made the
|
||||
// RSSI byte a slice of mid-sequence number — random — and the
|
||||
// saturating_neg() workaround hid this by always producing a negative
|
||||
// value. Now matches FW byte-for-byte. The csi.rs duplicate of this
|
||||
// function had the same bug and is fixed in the same change.
|
||||
let node_id = buf[4];
|
||||
let n_antennas = buf[5];
|
||||
let n_subcarriers = buf[6];
|
||||
let n_subcarriers = u16::from_le_bytes([buf[6], buf[7]]) as u8;
|
||||
let freq_mhz = u16::from_le_bytes([buf[8], buf[9]]);
|
||||
let sequence = u32::from_le_bytes([buf[10], buf[11], buf[12], buf[13]]);
|
||||
let rssi_raw = buf[14] as i8;
|
||||
// Fix RSSI sign: ensure it's always negative (dBm convention).
|
||||
let rssi = if rssi_raw > 0 { rssi_raw.saturating_neg() } else { rssi_raw };
|
||||
let noise_floor = buf[15] as i8;
|
||||
let sequence = u32::from_le_bytes([buf[12], buf[13], buf[14], buf[15]]);
|
||||
let rssi = buf[16] as i8; // already signed in [-128..127]
|
||||
let noise_floor = buf[17] as i8;
|
||||
|
||||
let iq_start = 20;
|
||||
let n_pairs = n_antennas as usize * n_subcarriers as usize;
|
||||
|
|
|
|||
Loading…
Reference in New Issue