Skip to content

Commit be171d8

Browse files
nerdCopterclaude
andcommitted
fix: address CodeRabbit inline findings from PR#156 review
Four issues resolved: 1. ESO PNG missing from report: moved report generation to after the ESO block so png_links is complete. Also adds the ESO stacked PNG to png_links on success. 2. Error propagation: replaced log-and-continue match on generate_markdown_report with ? operator so a write failure propagates out of process_file(). 3. Empty axis_delays in plot_setpoint_vs_gyro.rs: changed Vec::new() fallback to vec![None; AXIS_NAMES.len()] to satisfy the per-axis indexing contract expected by downstream consumers. 4. Hardcoded axis counts: replaced [Option<f64>; 3], [None, None, None], [Vec::new()...] and 0..2 introduced by the report wiring with AXIS_COUNT, std::array::from_fn, and ROLL_PITCH_AXIS_COUNT. Deferred: png_links as second source of truth (heavy refactor — tracked as follow-up). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent f77e29f commit be171d8

3 files changed

Lines changed: 42 additions & 38 deletions

File tree

src/main.rs

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -665,7 +665,7 @@ fn process_file(
665665
println!("Note: Optimal P:D ratio varies per aircraft. Check step response for overshoot/undershoot.");
666666
println!();
667667

668-
let pd_ratios_for_report: [Option<f64>; 3] = [
668+
let pd_ratios_for_report: [Option<f64>; AXIS_COUNT] = [
669669
pid_metadata.roll.calculate_pd_ratio(),
670670
pid_metadata.pitch.calculate_pd_ratio(),
671671
pid_metadata.yaw.calculate_pd_ratio(),
@@ -780,11 +780,12 @@ INFO ({input_file_str}): Skipping Step Response input data filtering: {reason}."
780780
let mut recommended_d_max_aggressive: [Option<u32>; 3] = [None, None, None];
781781

782782
// Setpoint authority per axis (captured for report)
783-
let mut setpoint_authority_names: [Option<&'static str>; 3] = [None, None, None];
784-
let mut setpoint_authority_means: [Option<f32>; 3] = [None, None, None];
783+
let mut setpoint_authority_names: [Option<&'static str>; AXIS_COUNT] =
784+
std::array::from_fn(|_| None);
785+
let mut setpoint_authority_means: [Option<f32>; AXIS_COUNT] = std::array::from_fn(|_| None);
785786

786787
// Step response warnings per axis (captured for report)
787-
let mut step_warnings: [Vec<String>; 3] = [Vec::new(), Vec::new(), Vec::new()];
788+
let mut step_warnings: [Vec<String>; AXIS_COUNT] = std::array::from_fn(|_| Vec::new());
788789

789790
if let Some(sr) = sample_rate {
790791
for axis_index in 0..crate::axis_names::AXIS_NAMES.len() {
@@ -843,7 +844,7 @@ INFO ({input_file_str}): Skipping Step Response input data filtering: {reason}."
843844
println!(" Always test in a safe environment. Conservative = safer first step.");
844845
println!(" Moderate = for experienced pilots (test carefully to avoid hot motors).");
845846
println!();
846-
for axis_index in 0..2 {
847+
for axis_index in 0..crate::axis_names::ROLL_PITCH_AXIS_COUNT {
847848
// Only Roll (0) and Pitch (1)
848849
let axis_name = crate::axis_names::AXIS_NAMES[axis_index];
849850

@@ -1198,7 +1199,7 @@ INFO ({input_file_str}): Skipping Step Response input data filtering: {reason}."
11981199
let mut step_reports: Vec<report::StepAxisReport> = Vec::new();
11991200
{
12001201
let dmax_enabled = pid_metadata.is_dmax_enabled();
1201-
for axis_index in 0..2 {
1202+
for axis_index in 0..crate::axis_names::ROLL_PITCH_AXIS_COUNT {
12021203
if let (Some(peak_value), Some(current_pd_ratio), Some(assessment)) = (
12031204
peak_values[axis_index],
12041205
current_pd_ratios[axis_index],
@@ -1694,35 +1695,6 @@ INFO ({input_file_str}): Skipping Step Response input data filtering: {reason}."
16941695
png_links.push(format!("{root_name_string}_PID_Activity_stacked.png"));
16951696
}
16961697

1697-
// --- Markdown Report ---
1698-
let report_filename = format!("{root_name_string}_report.md");
1699-
let report_path = std::path::Path::new(&report_filename);
1700-
println!("\n--- Generating Report: {report_filename} ---");
1701-
let flight_report = report::FlightReport {
1702-
root_name: root_name_string.clone(),
1703-
sample_rate,
1704-
header_metadata,
1705-
pd_ratios: pd_ratios_for_report,
1706-
step_reports,
1707-
optimal_p: optimal_p_for_report,
1708-
gyro_analysis,
1709-
dterm_results,
1710-
bode_results,
1711-
motor_results,
1712-
png_links,
1713-
filter_config,
1714-
dynamic_notch,
1715-
rpm_filter,
1716-
debug_fallback: using_debug_fallback,
1717-
debug_mode_name: debug_mode_label,
1718-
};
1719-
match report::generate_markdown_report(&flight_report, report_path) {
1720-
Ok(()) => println!(" [OK] Report written."),
1721-
Err(e) => eprintln!(" [ERROR] Report generation failed: {e}"),
1722-
}
1723-
1724-
// CWD restoration happens automatically when _cwd_guard goes out of scope
1725-
17261698
// --- ESO Gain Optimization ---
17271699
let mut eso_results: [Option<eso::EsoResult>; AXIS_COUNT] = [None, None, None];
17281700
if plot_config.run_eso {
@@ -1763,12 +1735,44 @@ INFO ({input_file_str}): Skipping Step Response input data filtering: {reason}."
17631735
if any_eso {
17641736
println!("\n--- Generating ESO Output Plot ---");
17651737
match plot_functions::plot_eso::plot_eso_output(&eso_results, &root_name_string) {
1766-
Ok(()) => println!(" [OK] ESO output plot written."),
1738+
Ok(()) => {
1739+
println!(" [OK] ESO output plot written.");
1740+
png_links.push(format!("{root_name_string}_ESO_output_stacked.png"));
1741+
}
17671742
Err(e) => eprintln!(" [ERROR] ESO output plot failed: {e}"),
17681743
}
17691744
}
17701745
}
17711746

1747+
// --- Markdown Report ---
1748+
// Must run after all plots (including ESO) so png_links is complete.
1749+
let report_filename = format!("{root_name_string}_report.md");
1750+
let report_path = std::path::Path::new(&report_filename);
1751+
println!("\n--- Generating Report: {report_filename} ---");
1752+
let flight_report = report::FlightReport {
1753+
root_name: root_name_string.clone(),
1754+
sample_rate,
1755+
header_metadata,
1756+
pd_ratios: pd_ratios_for_report,
1757+
step_reports,
1758+
optimal_p: optimal_p_for_report,
1759+
gyro_analysis,
1760+
dterm_results,
1761+
bode_results,
1762+
motor_results,
1763+
png_links,
1764+
filter_config,
1765+
dynamic_notch,
1766+
rpm_filter,
1767+
debug_fallback: using_debug_fallback,
1768+
debug_mode_name: debug_mode_label,
1769+
};
1770+
report::generate_markdown_report(&flight_report, report_path)
1771+
.map_err(|e| format!("Report generation failed: {e}"))?;
1772+
println!(" [OK] Report written.");
1773+
1774+
// CWD restoration happens automatically when _cwd_guard goes out of scope
1775+
17721776
println!("--- Finished processing file: {input_file_str} ---");
17731777
Ok(())
17741778
}

src/plot_functions/plot_setpoint_vs_gyro.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ pub fn plot_setpoint_vs_gyro(
3131
DelayAnalysisResult {
3232
average_delay: None,
3333
results: Vec::new(),
34-
axis_delays: Vec::new(),
34+
axis_delays: vec![None; AXIS_NAMES.len()],
3535
}
3636
};
3737

src/report.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ pub struct FlightReport {
4747
pub root_name: String,
4848
pub sample_rate: Option<f64>,
4949
pub header_metadata: Vec<(String, String)>,
50-
pub pd_ratios: [Option<f64>; 3],
50+
pub pd_ratios: [Option<f64>; AXIS_COUNT],
5151
pub step_reports: Vec<StepAxisReport>,
5252
pub optimal_p: [Option<OptimalPAnalysis>; AXIS_COUNT],
5353
pub gyro_analysis: Option<GyroAnalysisResult>,

0 commit comments

Comments
 (0)