From 1b21f262b7d9640ed4d7ad6830c25f94255a09ed Mon Sep 17 00:00:00 2001 From: Rik Bouwmeester Date: Fri, 12 Sep 2025 15:44:19 +0200 Subject: [PATCH 1/3] Fix parameter write response validation Parameter writes were incorrectly treating echoed values as error codes, causing all successful parameter sets to fail with 'Error setting the parameter: code X' where X was the actual value being set. Root cause: The firmware echoes back the successfully written parameter value, but the Rust implementation was checking byte 2 for an error code (expecting 0 for success). Fix: Compare the echoed bytes with the expected parameter value bytes to verify the parameter was set correctly. Tested: Resolves parameter setting failures in (upcoming) example where stabilizer.estimator could not be set to value 2. Can be tested by modifying test_params.rs --- src/subsystems/param.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/subsystems/param.rs b/src/subsystems/param.rs index 920bed0..fa478e9 100644 --- a/src/subsystems/param.rs +++ b/src/subsystems/param.rs @@ -250,15 +250,19 @@ impl Param { .wait_packet(PARAM_PORT, _WRITE_CHANNEL, ¶m_id.to_le_bytes()) .await?; - if answer.get_data()[2] == 0 { + // Verify the echoed value matches what we tried to set by comparing bytes + let expected_bytes: Vec = value.into(); + let echoed_bytes = &answer.get_data()[2..]; + if echoed_bytes == expected_bytes.as_slice() { + // Success: firmware echoed back the value we set // The param is tested as being in the TOC so this unwrap cannot fail *self.values.lock().await.get_mut(param).unwrap() = value; self.notify_watchers(param, value).await; Ok(()) } else { Err(Error::ParamError(format!( - "Error setting the parameter: code {}", - answer.get_data()[2] + "Parameter set failed: expected bytes {:?}, got echoed bytes {:?}", + expected_bytes, echoed_bytes ))) } } From d98f29e87e3bb5fc1854f834d510e647e8acabe1 Mon Sep 17 00:00:00 2001 From: Rik Bouwmeester Date: Fri, 12 Sep 2025 16:03:00 +0200 Subject: [PATCH 2/3] Improve parameter error messages Replace confusing byte array output with readable firmware error codes when parameter setting fails. Instead of showing expected and returned bytes. --- src/subsystems/param.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/subsystems/param.rs b/src/subsystems/param.rs index fa478e9..daa8318 100644 --- a/src/subsystems/param.rs +++ b/src/subsystems/param.rs @@ -250,19 +250,20 @@ impl Param { .wait_packet(PARAM_PORT, _WRITE_CHANNEL, ¶m_id.to_le_bytes()) .await?; - // Verify the echoed value matches what we tried to set by comparing bytes + // Success response: firmware echoes back the written value let expected_bytes: Vec = value.into(); let echoed_bytes = &answer.get_data()[2..]; if echoed_bytes == expected_bytes.as_slice() { - // Success: firmware echoed back the value we set // The param is tested as being in the TOC so this unwrap cannot fail *self.values.lock().await.get_mut(param).unwrap() = value; self.notify_watchers(param, value).await; Ok(()) } else { + // If echoed value doesn't match, it's likely a parameter error code + let error_code = echoed_bytes[0]; // For u8 params, single byte error code Err(Error::ParamError(format!( - "Parameter set failed: expected bytes {:?}, got echoed bytes {:?}", - expected_bytes, echoed_bytes + "Error setting parameter: parameter error code {}", + error_code ))) } } From e69a9aad935bea17fb8f9ba82f5eaa635c5e558d Mon Sep 17 00:00:00 2001 From: Rik Bouwmeester Date: Fri, 17 Oct 2025 15:24:05 +0200 Subject: [PATCH 3/3] Add bounds checks to prevent panics in param write response parsing Check response length before slicing to avoid potential panics if malformed response packets are received. Returns ProtocolError for too-short responses instead of panicking on slice access. --- src/subsystems/param.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/subsystems/param.rs b/src/subsystems/param.rs index daa8318..99bc6d1 100644 --- a/src/subsystems/param.rs +++ b/src/subsystems/param.rs @@ -252,7 +252,14 @@ impl Param { // Success response: firmware echoes back the written value let expected_bytes: Vec = value.into(); - let echoed_bytes = &answer.get_data()[2..]; + let data = answer.get_data(); + if data.len() < 2 { + return Err(Error::ProtocolError( + format!("Parameter write response too short: expected at least 2 bytes, got {}", data.len()) + )); + } + let echoed_bytes = &data[2..]; + if echoed_bytes == expected_bytes.as_slice() { // The param is tested as being in the TOC so this unwrap cannot fail *self.values.lock().await.get_mut(param).unwrap() = value; @@ -260,6 +267,11 @@ impl Param { Ok(()) } else { // If echoed value doesn't match, it's likely a parameter error code + if echoed_bytes.is_empty() { + return Err(Error::ProtocolError( + "Parameter write response invalid: no error code or echoed value".to_string() + )); + } let error_code = echoed_bytes[0]; // For u8 params, single byte error code Err(Error::ParamError(format!( "Error setting parameter: parameter error code {}",