fix: guard debug[0-2] gyroUnfilt fallback to GYRO_SCALED mode only#157
Conversation
- Add src/eso.rs: 2nd-order LESO bandwidth optimization via golden-section search - Add src/report.rs: Per-axis signal statistics and ESO results in markdown format - Add CLI flags: --eso, --eso-axis, --eso-b0, --report - Add ESO constants to src/constants.rs (omega_0 bounds, GSS tolerance) - Update lib.rs and main.rs to wire modules and implement ESO/report execution - Update OVERVIEW.md with ESO and report sections - Add INFORMATION/ESO_HOWTO.md user guide - Tested on Betaflight 2025.12 and EmuFlight 0.4.3 full flights
…-domain plot - Add argmin 0.11 dependency; implement CostFunction trait for ESO bandwidth search - Replace monotone 1-step MSE cost with N-step-ahead open-loop prediction cost (unimodal: low omega0 = stale f_hat; high omega0 = noise amplification) Constants: ESO_N_AHEAD_STEPS=5, ESO_WARMUP_FRACTION=0.20 - EsoResult now carries omega_meas_trace, omega_hat_trace, f_hat_trace, timestamps - Add plot_eso.rs: stacked time-domain ESO output plot per axis (omega_meas blue thin over omega_hat orange thick, f_hat green scaled) - Wire ESO output plot into main.rs after optimization; generated as *_ESO_output_stacked.png - Update report.rs: MSE label clarified to N-step-ahead prediction - Add ESO plot colors to constants.rs (COLOR_ESO_MEAS/HAT/FHAT) - ESO_GSS_MAX_ITER type corrected to u64 for argmin compatibility
- estimate_b0(): OLS closed-form b0 = sum(u*d_omega) / (Ts * sum(u^2)) on samples where |PID sum| >= ESO_B0_MIN_CONTROL_THRESHOLD (10.0) - b0 auto-estimated from data when user has not overridden --eso-b0; falls back to ESO_DEFAULT_B0 (1.0) when too few valid samples - EsoResult.b0_auto: true when estimated, false when user-supplied - main.rs: print shows b0=X.XXXX (estimated|user) - report.rs: b0 row labels (auto-estimated|user-supplied); updated guidance - Add ESO_B0_MIN_CONTROL_THRESHOLD = 10.0 to constants.rs - Fix pre-existing clippy collapsible_match in pid_metadata.rs
- OVERVIEW.md: move *_report.md bullet out of 'Generated PNG Plots'
into a new 'Generated Reports' subsection (was self-contradictory)
- src/eso.rs: add upfront validation in run_eso_optimization
* axis >= AXIS_COUNT → descriptive Err instead of panic
* non-finite / non-positive sample_rate → Err
* invalid EsoConfig fields (omega0_min/max, b0) → Err
* add control-input excitation guard: reject axes where pid_sum
is all-zero / sub-epsilon so flat-input axes are not 'optimised'
- src/report.rs: fix PID term collection in extract_axis_signals
* replace unwrap_or(0.0) with per-field Option guards so absent
P/I/D/F terms are not silently reported as real zero samples
* replace hardcoded 'axis < 4' magic literal with
row.setpoint.get(axis).copied().flatten() (bounds-safe)
- src/plot_functions/plot_eso.rs + src/constants.rs:
* add ESO_FHAT_Y_FRACTION = 0.5 constant (no more magic 0.5)
* store fhat_max_abs in AxisEsoData instead of pre-computed scale
* compute fhat_scale inside draw_stacked_plot after half_range is
known: (half_range * ESO_FHAT_Y_FRACTION) / fhat_max_abs
fixes f_hat being squashed when half_range > UNIFIED_Y_AXIS_MIN_SCALE
- src/main.rs: --eso-axis and --eso-b0 now imply --eso
(previously silently ignored when --eso was omitted)
- src/eso.rs: fix estimate_b0 doc/behavior mismatch and magic numbers
* replace 'count < 10' with ESO_B0_MIN_OLS_SAMPLES constant
* replace 'den.abs() < 1e-12' with VALUE_EPSILON (already imported)
* replace 'b0.abs() > 1e-9' with 'b0 > ESO_B0_ESTIMATE_MIN_POSITIVE'
to enforce strictly positive estimates (negative b0 = inverted sign
convention, was accepted silently despite docstring saying otherwise)
* update docstring to reflect strict positivity requirement
- src/eso.rs + src/main.rs: replace fragile float epsilon b0 detection
* add b0_user_override: bool to EsoConfig
* add eso_b0_user_override: bool to PlotConfig
* --eso-b0 CLI flag now sets eso_b0_user_override = true
* run_eso_optimization branches on config.b0_user_override instead of
(config.b0 - ESO_DEFAULT_B0).abs() < 1e-12 so an explicit
'--eso-b0 1.0' is respected rather than silently overridden by OLS
- src/constants.rs: add two new constants for estimate_b0 thresholds
* ESO_B0_MIN_OLS_SAMPLES: usize = 10
* ESO_B0_ESTIMATE_MIN_POSITIVE: f64 = 1e-9
* add explanatory comment on ESO_OMEGA0_MAX conservative ceiling
(links to min(sample_rate/3, config.omega0_max) clamping behavior)
- src/report.rs: replace AXIS_NAMES.len().min(3) with AXIS_COUNT
* imports AXIS_COUNT from axis_names alongside AXIS_NAMES
* eliminates redundant .min(3) magic number
- src/main.rs: fix --eso-axis help text and error message
* help now lists 'roll,pitch,yaw,all (or 0,1,2)'
* error message lists 'roll, pitch, yaw, all, or 0, 1, 2'
- src/constants.rs: add ESO_OMEGA0_STABILITY_RATIO = 3.0
* named constant for the LESO discrete-time stability divisor
(omega_0 < sample_rate / ESO_OMEGA0_STABILITY_RATIO)
- src/eso.rs: replace hardcoded 3.0 stability divisor with constant
* omega0_max_stable = (sample_rate / ESO_OMEGA0_STABILITY_RATIO).min(...)
* error threshold = config.omega0_min * ESO_OMEGA0_STABILITY_RATIO
* import ESO_OMEGA0_STABILITY_RATIO from constants
- src/main.rs: replace all [bool; 3] / [true; 3] / [false; 3] with AXIS_COUNT
* PlotConfig::eso_axes: [bool; AXIS_COUNT]
* Default / none() initializers: [true; AXIS_COUNT]
* --eso-axis parser: [false; AXIS_COUNT] and [true; AXIS_COUNT] for 'all'
* eso_results declaration: [Option<eso::EsoResult>; AXIS_COUNT]
* import crate::axis_names::AXIS_COUNT at top level
- src/main.rs: replace fragile save/restore with PlotConfig::disable_plots()
* add disable_plots(&mut self) method that zeroes all plot-type flags
but leaves run_eso, run_report, eso_b0, eso_b0_user_override, eso_axes
untouched — prevents future ESO field additions from silently regressing
* remove now-unused PlotConfig::none() (was only called in the
save/restore block)
* replace the 9-line save/restore block with plot_config.disable_plots()
- src/report.rs: change eso_results parameter type from [_; 3] to [_; AXIS_COUNT]
- src/report.rs: document population-variance convention on compute_signal_stats
* add doc comment stating N (not N-1) divisor is intentional for a
complete time-series and showing how to switch to sample variance
- --eso-axis error message now matches help text (roll,pitch,yaw,all or 0,1,2) - add duplicate-arg detection for --eso-b0 and --eso-axis (mirrors --dps pattern) - remove unreachable any_valid guard; assignment is now unconditional - rename p95_idx -> pctl_idx in plot_eso.rs (derives from UNIFIED_Y_AXIS_PERCENTILE) - expand ESO_OMEGA0_MAX comment explaining conservative ~80 Hz ceiling
Add at_ceiling: bool to EsoResult, set when omega0_opt reaches the stability-constrained search bound (omega0_max_stable - GSS_TOLERANCE). Propagate through AxisEsoData into the plot legend as ' [at ceiling]' so users can see at a glance when the optimizer hit its upper bound rather than finding a true interior optimum. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
) Previously, debug[0-2] was unconditionally used as a fallback for gyroUnfilt when dedicated gyroUnfilt columns were absent. This produced incorrect unfiltered gyro analysis for any debug_mode other than GYRO_SCALED (6), e.g. RC_SMOOTHING (7) or ACCELEROMETER (4), where debug[0-2] does not contain raw gyro data. Now the fallback is guarded: only proceeds when debug_mode=6, warns and skips otherwise. When debug_mode is absent from headers, the fallback is allowed with an assumption warning. Closes #154 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
OVERVIEW.md (2)
182-182:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDocumentation claims report generation that is not wired in code.
Line 182 states "Results are also included in the markdown report if
--reportis also given," butsrc/main.rsneither implements the--reportflag nor callsgenerate_markdown_reportin theprocess_filefunction.The report generation function exists in
src/report.rs(context snippet 3) but is not integrated into the CLI workflow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@OVERVIEW.md` at line 182, The docs claim a `--report` option and that `process_file` writes results to a markdown report, but CLI and workflow don't invoke report generation; add a boolean CLI flag (e.g., --report) in the argument parsing code and pass it into process_file, then update process_file to call the existing generate_markdown_report function (from report.rs) when that flag is true, ensuring you pass the computed results and desired output path; reference the process_file function and generate_markdown_report symbol to locate where to wire the call and the CLI parsing code to add the flag.
31-32:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDocumentation describes unimplemented CLI flags.
Lines 31-32 document
--eso-axis <roll,pitch,yaw>and--reportflags that are not implemented insrc/main.rs. The code only implements--esoand--eso-b0.This creates a user-facing documentation error where users will attempt to use flags that do not exist.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@OVERVIEW.md` around lines 31 - 32, The docs list CLI flags --eso-axis and --report but src/main.rs only implements --eso and --eso-b0; either remove those flags from OVERVIEW.md or implement them in the CLI parsing: add Arg definitions for "--eso-axis" (accepting comma-separated values or multiple occurrences for roll,pitch,yaw) and "--report" (boolean) in your CLI builder (the function that constructs clap args used by main()/parse_args/ArgMatches), map the parsed values into the program configuration (e.g., the Config/args struct you pass through main logic), and ensure downstream code respects the new config fields (enable axis selection in the ESO code path and enable writing the markdown report where plots are generated); update/validate help text accordingly.
🧹 Nitpick comments (1)
src/plot_functions/plot_eso.rs (1)
25-25: ⚡ Quick winCentralize axis cardinality and numeric thresholds into shared constants.
3and1e-12are hardcoded in function code here. Please switch to shared constants so this module stays aligned with cross-file axis contracts and repo conventions.♻️ Proposed patch
-use crate::axis_names::AXIS_NAMES; +use crate::axis_names::{AXIS_COUNT, AXIS_NAMES}; use crate::constants::{ - COLOR_ESO_FHAT, COLOR_ESO_HAT, COLOR_ESO_MEAS, ESO_FHAT_Y_FRACTION, LINE_WIDTH_PLOT, + COLOR_ESO_FHAT, COLOR_ESO_HAT, COLOR_ESO_MEAS, ESO_FHAT_Y_FRACTION, ESO_FHAT_EPS, + LINE_WIDTH_PLOT, UNIFIED_Y_AXIS_HEADROOM_SCALE, UNIFIED_Y_AXIS_MIN_SCALE, UNIFIED_Y_AXIS_PERCENTILE, }; pub fn plot_eso_output( - eso_results: &[Option<EsoResult>; 3], + eso_results: &[Option<EsoResult>; AXIS_COUNT], root_name: &str, ) -> Result<(), Box<dyn Error>> { @@ - let mut axis_data: [Option<AxisEsoData>; 3] = [None, None, None]; + let mut axis_data: [Option<AxisEsoData>; AXIS_COUNT] = std::array::from_fn(|_| None); @@ - if data.fhat_max_abs > 1e-12 { + if data.fhat_max_abs > ESO_FHAT_EPS {// src/constants.rs pub const ESO_FHAT_EPS: f64 = 1.0e-12;As per coding guidelines,
src/**/*.rs: “All constants go insrc/constants.rs— no hardcoded magic numbers in function code” and “Axis indices … must use thesrc/axis_names.rsmodule withAXIS_COUNT,AXIS_NAMES, andaxis_name()function instead of creating redundant axis index constants.”Also applies to: 37-37, 102-103
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/plot_functions/plot_eso.rs` at line 25, Replace hardcoded axis cardinality and numeric threshold in plot_eso.rs: change the fixed array type signature eso_results: &[Option<EsoResult>; 3] to use the shared axis cardinality constant (AXIS_COUNT) from src/axis_names.rs, and replace any literal 1e-12 with the shared ESO_FHAT_EPS constant from src/constants.rs; update all references in the function(s) in plot_eso.rs that rely on the length/indexing or epsilon comparison so they import and use AXIS_COUNT and ESO_FHAT_EPS (refer to the eso_results parameter and any local uses of 1e-12/EsoResult comparisons) to conform to the repo conventions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/data_input/log_parser.rs`:
- Around line 456-457: Replace the hardcoded magic number Some(6) in the
debug-mode match inside src/data_input/log_parser.rs with a named constant: add
a public constant DEBUG_MODE_GYRO_SCALED = 6 (or suitable type) to
src/constants.rs and then import and use that constant in the match arm (e.g.,
change Some(6) -> Some(DEBUG_MODE_GYRO_SCALED)); update any related pattern
matches/comments in the same function to reference DEBUG_MODE_GYRO_SCALED so
there are no raw numeric literals left in the match logic.
In `@src/main.rs`:
- Around line 1670-1696: The docs mention CLI flags --eso-axis and --report and
a generate_markdown_report function but main.rs only parses --eso and --eso-b0;
fix by adding parsing for --eso-axis and --report (introduce variables like
eso_axis_selection and report_requested alongside eso_requested, eso_b0_value,
eso_b0_user_override) in the args loop, validate --eso-axis accepts
roll/pitch/yaw (or comma list) and set a clear data structure to carry selected
axes; wire those values into process_file (or whichever function computes ESO
results) so ESO computation respects eso_axis_selection and uses eso_b0_value
when eso_b0_user_override is true; finally, when report_requested is set call
generate_markdown_report after processing a file (or after batch processing) and
pass the computed ESO results so the markdown includes ESO output;
alternatively, if you prefer removing features, update OVERVIEW.md to remove
--eso-axis/--report and delete or mark generate_markdown_report unused.
---
Duplicate comments:
In `@OVERVIEW.md`:
- Line 182: The docs claim a `--report` option and that `process_file` writes
results to a markdown report, but CLI and workflow don't invoke report
generation; add a boolean CLI flag (e.g., --report) in the argument parsing code
and pass it into process_file, then update process_file to call the existing
generate_markdown_report function (from report.rs) when that flag is true,
ensuring you pass the computed results and desired output path; reference the
process_file function and generate_markdown_report symbol to locate where to
wire the call and the CLI parsing code to add the flag.
- Around line 31-32: The docs list CLI flags --eso-axis and --report but
src/main.rs only implements --eso and --eso-b0; either remove those flags from
OVERVIEW.md or implement them in the CLI parsing: add Arg definitions for
"--eso-axis" (accepting comma-separated values or multiple occurrences for
roll,pitch,yaw) and "--report" (boolean) in your CLI builder (the function that
constructs clap args used by main()/parse_args/ArgMatches), map the parsed
values into the program configuration (e.g., the Config/args struct you pass
through main logic), and ensure downstream code respects the new config fields
(enable axis selection in the ESO code path and enable writing the markdown
report where plots are generated); update/validate help text accordingly.
---
Nitpick comments:
In `@src/plot_functions/plot_eso.rs`:
- Line 25: Replace hardcoded axis cardinality and numeric threshold in
plot_eso.rs: change the fixed array type signature eso_results:
&[Option<EsoResult>; 3] to use the shared axis cardinality constant (AXIS_COUNT)
from src/axis_names.rs, and replace any literal 1e-12 with the shared
ESO_FHAT_EPS constant from src/constants.rs; update all references in the
function(s) in plot_eso.rs that rely on the length/indexing or epsilon
comparison so they import and use AXIS_COUNT and ESO_FHAT_EPS (refer to the
eso_results parameter and any local uses of 1e-12/EsoResult comparisons) to
conform to the repo conventions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5ea0864c-d7f5-461d-8b6f-b2ca3fbfeaac
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
Cargo.tomlOVERVIEW.mdsrc/constants.rssrc/data_input/log_parser.rssrc/eso.rssrc/lib.rssrc/main.rssrc/plot_functions/mod.rssrc/plot_functions/plot_eso.rssrc/report.rs
| Some(6) => { | ||
| // GYRO_SCALED: debug[0-2] contains raw unfiltered gyro |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Extract GYRO_SCALED mode id into shared constants
Some(6) is a hardcoded magic number in function logic. Please move this to src/constants.rs (e.g., DEBUG_MODE_GYRO_SCALED) and reference it here.
As per coding guidelines, src/**/*.rs: “All constants go in src/constants.rs — no hardcoded magic numbers in function code”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/data_input/log_parser.rs` around lines 456 - 457, Replace the hardcoded
magic number Some(6) in the debug-mode match inside src/data_input/log_parser.rs
with a named constant: add a public constant DEBUG_MODE_GYRO_SCALED = 6 (or
suitable type) to src/constants.rs and then import and use that constant in the
match arm (e.g., change Some(6) -> Some(DEBUG_MODE_GYRO_SCALED)); update any
related pattern matches/comments in the same function to reference
DEBUG_MODE_GYRO_SCALED so there are no raw numeric literals left in the match
logic.
Source: Coding guidelines
| } else if arg == "--eso" { | ||
| eso_requested = true; | ||
| } else if arg == "--eso-b0" { | ||
| if eso_b0_flag_present { | ||
| eprintln!("Error: --eso-b0 argument specified more than once."); | ||
| print_usage_and_exit(program_name); | ||
| } | ||
| if i + 1 >= args.len() { | ||
| eprintln!("Error: --eso-b0 requires a numeric value."); | ||
| print_usage_and_exit(program_name); | ||
| } | ||
| match args[i + 1].parse::<f64>() { | ||
| Ok(val) if val > 0.0 && val.is_finite() => { | ||
| eso_b0_value = val; | ||
| eso_b0_user_override = true; | ||
| eso_requested = true; | ||
| eso_b0_flag_present = true; | ||
| i += 1; | ||
| } | ||
| _ => { | ||
| eprintln!( | ||
| "Error: --eso-b0 must be a positive finite number: {}", | ||
| args[i + 1] | ||
| ); | ||
| print_usage_and_exit(program_name); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for --eso-axis and --report flag parsing across the codebase
echo "=== Searching for --eso-axis flag parsing ==="
rg -n '\-\-eso-axis' src/
echo "=== Searching for --report flag parsing ==="
rg -n '\-\-report' src/
echo "=== Searching for generate_markdown_report calls ==="
rg -n 'generate_markdown_report' src/Repository: nerdCopter/BlackBox_CSV_Render
Length of output: 268
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Search for flags across entire repo ==="
rg -n '\-\-eso-axis' .
rg -n '\-\-report' .
echo
echo "=== Search for generate_markdown_report calls across entire repo ==="
rg -n 'generate_markdown_report' .
echo
echo "=== Read OVERVIEW.md around mentioned sections ==="
if [ -f OVERVIEW.md ]; then
sed -n '1,220p' OVERVIEW.md | nl -ba | sed -n '25,60p;170,210p'
else
echo "OVERVIEW.md not found"
fiRepository: nerdCopter/BlackBox_CSV_Render
Length of output: 118
🏁 Script executed:
#!/bin/bash
# Re-run searches without failing when no matches are found; capture stderr too.
set -u
echo "=== Search for --eso-axis across entire repo (ignore no matches) ==="
rg -n '\-\-eso-axis' . 2>&1 || true
echo
echo "=== Search for --report across entire repo (ignore no matches) ==="
rg -n '\-\-report' . 2>&1 || true
echo
echo "=== Search for generate_markdown_report across entire repo ==="
rg -n 'generate_markdown_report' . 2>&1 || true
echo
echo "=== Search for likely report wiring (process/report keywords) ==="
rg -n 'markdown report|markdown_report|process_file|report::|src/report\.rs|generate_.*report' . 2>&1 || true
echo
echo "=== Read OVERVIEW.md around mentioned line ranges ==="
if [ -f OVERVIEW.md ]; then
nl -ba OVERVIEW.md | sed -n '20,60p'
nl -ba OVERVIEW.md | sed -n '160,210p'
else
echo "OVERVIEW.md not found"
fiRepository: nerdCopter/BlackBox_CSV_Render
Length of output: 736
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== CLI arg parsing / ESO / report keywords in src/main.rs ==="
rg -n "eso-axis|eso_b0|--eso|--eso-b0|--report|report|markdown" src/main.rs
echo
echo "=== process_file implementation around src/main.rs:561 ==="
sed -n '520,720p' src/main.rs
echo
echo "=== main() / flag handling around src/main.rs:1600-1900 ==="
sed -n '1600,1900p' src/main.rs
echo
echo "=== generate_markdown_report signature/body around src/report.rs:110-200 ==="
sed -n '90,220p' src/report.rs
echo
echo "=== OVERVIEW.md lines ~31-40 and ~175-195 (with line numbers) ==="
if [ -f OVERVIEW.md ]; then
cat -n OVERVIEW.md | sed -n '25,45p;175,195p;180,190p'
else
echo "OVERVIEW.md not found"
fiRepository: nerdCopter/BlackBox_CSV_Render
Length of output: 29086
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Repo-wide: any report/markdown-report mentions ==="
rg -n "(_report|report\.md|markdown report|markdown_report|generate_.*report|report )" src .
echo
echo "=== Repo-wide: any ESO-axis mention (with or without --) ==="
rg -n "eso-axis|eso_axis|ESO[-_ ]axis" src .
echo
echo "=== Call sites for generate_markdown_report( ==="
rg -n "generate_markdown_report\s*\(" src .
echo
echo "=== Search for eso_results usage in main.rs ==="
rg -n "eso_results|EsoResult" src/main.rs src/report.rs .Repository: nerdCopter/BlackBox_CSV_Render
Length of output: 845
🏁 Script executed:
#!/bin/bash
set -u
echo "=== --report parsing in repo (ignore no matches) ==="
rg -n '\-\-report\b' . 2>&1 || true
echo
echo "=== _report.md / report output strings in repo (ignore no matches) ==="
rg -n '(_report\.md|report\.md|markdown statistical report|markdown_report|markdown report)' . 2>&1 || true
echo
echo "=== ESO-axis parsing strings in repo (ignore no matches) ==="
rg -n 'eso-axis|eso_axis|ESO[-_ ]axis' . 2>&1 || true
echo
echo "=== generate_markdown_report call sites (ignore no matches) ==="
rg -n 'generate_markdown_report\s*\(' . 2>&1 || trueRepository: nerdCopter/BlackBox_CSV_Render
Length of output: 526
Documentation-code mismatch: --eso-axis / --report are documented but not implemented or wired
OVERVIEW.md documents --eso-axis <roll,pitch,yaw> and --report, and states ESO results are included in the markdown report when --report is also given. Code in src/main.rs parses only --eso and --eso-b0, and repo-wide there are no matches for --eso-axis or --report flag parsing; additionally, generate_markdown_report exists in src/report.rs but has no call sites.
Either remove/update the docs for these flags, or implement the missing CLI flags and connect generate_markdown_report into process_file with proper axis selection/ESO plumbing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main.rs` around lines 1670 - 1696, The docs mention CLI flags --eso-axis
and --report and a generate_markdown_report function but main.rs only parses
--eso and --eso-b0; fix by adding parsing for --eso-axis and --report (introduce
variables like eso_axis_selection and report_requested alongside eso_requested,
eso_b0_value, eso_b0_user_override) in the args loop, validate --eso-axis
accepts roll/pitch/yaw (or comma list) and set a clear data structure to carry
selected axes; wire those values into process_file (or whichever function
computes ESO results) so ESO computation respects eso_axis_selection and uses
eso_b0_value when eso_b0_user_override is true; finally, when report_requested
is set call generate_markdown_report after processing a file (or after batch
processing) and pass the computed ESO results so the markdown includes ESO
output; alternatively, if you prefer removing features, update OVERVIEW.md to
remove --eso-axis/--report and delete or mark generate_markdown_report unused.
…is/--report docs - Add DEBUG_MODE_GYRO_SCALED = 6 to src/constants.rs; use it in log_parser.rs instead of hardcoded magic number 6 (CodeRabbit #157) - Remove --eso-axis and --report flag references from OVERVIEW.md; these flags were previously removed from the CLI but documentation was not updated (CodeRabbit #157) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@coderabbitai, AI generated comment — addressed both actionable items from review 4471229108:
Please re-review commit e88abeb. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Fetch the commit diff
gh api repos/nerdCopter/BlackBox_CSV_Render/commits/e88abeb --jq '.files[] | {filename: .filename, patch: .patch}'Length of output: 6356
Everything looks clean and complete. 🐇✨ If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
AI Generated pull-request
Summary
debug[0-2]was unconditionally used as agyroUnfiltfallback whenever dedicatedgyroUnfiltcolumns were absent, regardless ofdebug_mode.debug_mode == 6(GYRO_SCALED), which is the only mode wheredebug[0-2]contains raw unfiltered gyro data.debug_modeis known and!= 6(e.g. RC_SMOOTHING=7, ACCELEROMETER=4): skips fallback, emits a warning, and gyroUnfilt-dependent analysis degrades gracefully to N/A.debug_modeis absent from headers (old embedded-only logs): allows fallback with an assumption warning.Behavior change
debug_mode=6+ no gyroUnfilt headersdebug_mode≠6+ no gyroUnfilt headersdebug_modeunknown + no gyroUnfilt headersFiles changed
src/data_input/log_parser.rs: guardedusing_debug_fallbackcomputation and per-row fallback assignmentTest plan
debug_mode=6and nogyroUnfiltcolumns: fallback applied,"Using debug[0-2] as gyroUnfilt fallback (debug_mode=GYRO_SCALED)"printedgyroUnfiltcolumns present: no fallback attempted (unchanged)debug_mode≠6and nogyroUnfilt: fallback skipped, warning printed, GyroVsUnfilt/delay analysis shows N/A🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--esoand--eso-b0CLI flags--reportflag producing per-log markdown documentationBug Fixes
Documentation