Refactor: Consolidate geonav-sim into strapdown-sim with feature flag by jbrodovsky · Pull Request #212 · jbrodovsky/strapdown-rs

jbrodovsky · 2026-01-08T18:27:00Z

Summary

This PR consolidates the geonav-sim binary into strapdown-sim with an optional geonav feature flag, eliminating ~40-50% code duplication while keeping strapdown-geonav as a separate library crate.

Closes: #206, #207, #208, #209, #210, #211

Changes

Step 1: Extract shared utilities (#207)

Created sim/src/common.rs with shared CLI utilities (~400 lines)
Functions: init_logger, validate_input_path, get_csv_files, validate_output_path, user prompt utilities
Added tempfile as dev-dependency for tests

Step 2: Add geonav feature flag (#208)

Added geonav feature to sim/Cargo.toml with optional strapdown-geonav dependency
Added feature-gated GeophysicalArgs struct for CLI arguments
Integrated --geo flag and geophysical options into ClosedLoopSimArgs
Implemented run_geo_closed_loop_cli() for geophysical simulation

Step 3: Parallel processing

Prepared process_files() utility for future use (currently unused warning)

Step 4: Remove geonav-sim binary (#210)

Deleted geonav/src/main.rs (~840 lines)
Removed [[bin]] section from geonav/Cargo.toml
strapdown-geonav is now library-only

Step 5: Documentation (#211)

Updated README.md with geonav usage instructions
Rewrote geonav/README.md to reflect library-only status
Updated CLI examples for new syntax

Usage

# Standard GNSS-aided INS (no change)
strapdown-sim cl --filter ukf --input data.csv --output out/

# With geophysical navigation
cargo build --release --package strapdown-sim --features geonav
strapdown-sim cl --filter ukf --input data.csv --output out/ \
  --geo \
  --gravity-resolution one-minute

Breaking Changes

geonav-sim binary no longer exists
Users must use strapdown-sim --features geonav with --geo flag

Test plan

Build without geonav feature: cargo build --package strapdown-sim
Build with geonav feature: cargo build --package strapdown-sim --features geonav
All tests pass: cargo test --workspace
CLI help shows geo options when feature enabled
Geonav library still compiles as standalone

🤖 Generated with Claude Code

Closes #207 Extract duplicated utility functions from sim/src/main.rs and geonav/src/main.rs into a new shared module. This reduces code duplication by ~300 lines. Functions extracted: - init_logger() - Logger initialization - validate_input_path() - Path validation - get_csv_files() - CSV file discovery - validate_output_path() - Output directory creation - read_user_input() - User prompt utilities - prompt_config_name(), prompt_config_path() - prompt_input_path(), prompt_output_path() - prompt_f64_with_default() - Generic prompt helper - process_files() - Parallel/sequential file processing Also adds tempfile as a dev-dependency for unit tests. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Closes #208 Add optional `geonav` feature to strapdown-sim that enables geophysical navigation capabilities when compiled with `--features geonav`. Changes: - Add `geonav` feature flag in sim/Cargo.toml with optional strapdown-geonav dep - Add feature-gated `GeophysicalArgs` struct for CLI arguments - Integrate geo options into `ClosedLoopSimArgs` via `#[command(flatten)]` - Add `--geo` flag to enable geophysical navigation mode - Add geo-specific options: gravity/magnetic resolution, bias, noise, map files - Implement `run_geo_closed_loop_cli()` function for geophysical simulation - Add resolution conversion functions and map auto-detection helpers Usage: # Standard GNSS-aided INS strapdown-sim cl --filter ukf --input data.csv --output out/ # With geophysical navigation strapdown-sim cl --filter ukf --input data.csv --output out/ \ --geo --gravity-resolution one-minute Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Closes #210 Remove the standalone geonav-sim binary as it has been consolidated into strapdown-sim with the --geo flag. Changes: - Remove [[bin]] section from geonav/Cargo.toml - Delete geonav/src/main.rs (~840 lines) - Add usage comment in Cargo.toml pointing to new command Migration: Old: geonav-sim cl --input data.csv --output out.csv --gravity-resolution one-minute New: strapdown-sim cl --input data.csv --output out.csv --geo --gravity-resolution one-minute Build with: cargo build --package strapdown-sim --features geonav The strapdown-geonav crate is now library-only. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Closes #211 Update documentation to reflect the consolidation of geonav-sim into strapdown-sim. Changes: - Update README.md to remove references to standalone geonav-sim binary - Add geophysical navigation usage instructions to main README - Rewrite geonav/README.md to reflect library-only status - Update usage examples for new strapdown-sim --features geonav syntax Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR consolidates the geonav-sim binary into strapdown-sim with an optional geonav feature flag, successfully reducing code duplication by extracting shared utilities into a common module while maintaining the strapdown-geonav library as a standalone crate.

Key Changes:

Extracted ~400 lines of shared CLI utilities into sim/src/common.rs with comprehensive test coverage
Added geonav feature flag to sim/Cargo.toml with optional dependency on strapdown-geonav
Integrated geophysical navigation functionality into strapdown-sim with proper feature-gating
Removed the standalone geonav-sim binary (~840 lines) from geonav/src/main.rs
Updated documentation in README files to reflect the new usage pattern

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
sim/src/common.rs	New shared utility module with logger initialization, path validation, file discovery, user input prompts, and parallel processing utilities. Includes comprehensive unit tests.
sim/src/main.rs	Integrated geophysical navigation with feature-gated imports, structs, and functions. Added resolution conversion functions and `run_geo_closed_loop_cli` for geophysical simulations.
sim/Cargo.toml	Added `geonav` feature with optional `strapdown-geonav` dependency and `tempfile` dev-dependency for tests.
geonav/src/main.rs	Removed entire 836-line binary implementation, now consolidated into `strapdown-sim`.
geonav/Cargo.toml	Removed `[[bin]]` section and added helpful comments directing users to use `strapdown-sim --features geonav`.
geonav/README.md	Rewritten to reflect library-only status with clear instructions for using the consolidated simulation tool and API examples.
README.md	Updated with instructions for enabling geophysical navigation capabilities via feature flag.
Cargo.lock	Updated dependencies to reflect the new structure.

Copilot · 2026-01-08T18:35:47Z

+        GeoResolution::FourMinutes => MagneticResolution::FourMinutes,
+        GeoResolution::ThreeMinutes => MagneticResolution::ThreeMinutes,
+        GeoResolution::TwoMinutes => MagneticResolution::TwoMinutes,
+        _ => MagneticResolution::TwoMinutes,


The wildcard pattern in this match statement silently maps unsupported GeoResolution variants (OneMinute, ThirtySeconds, FifteenSeconds, ThreeSeconds, OneSecond) to TwoMinutes. This could lead to unexpected behavior where users specify a higher resolution than magnetic maps support, but get silently downgraded without warning. Consider explicitly matching all variants and returning an error for unsupported resolutions, or at least logging a warning.

Copilot · 2026-01-08T18:35:48Z

+pub fn process_files<F>(
+    csv_files: &[PathBuf],
+    parallel: bool,
+    processor: F,
+) -> Result<(), Box<dyn Error>>
+where
+    F: Fn(&Path) -> Result<(), Box<dyn Error>> + Send + Sync,
+{
+    let is_multiple = csv_files.len() > 1;
+
+    if parallel && is_multiple {
+        // Parallel processing
+        let errors = Mutex::new(Vec::new());
+
+        csv_files.par_iter().for_each(|input_file| {
+            if let Err(e) = processor(input_file) {
+                error!("Error processing {}: {}", input_file.display(), e);
+                errors
+                    .lock()
+                    .expect("Failed to acquire lock on error collection")
+                    .push((input_file.clone(), e.to_string()));
+            }
+        });
+
+        let errors = errors
+            .into_inner()
+            .expect("Failed to extract errors from mutex");
+        if !errors.is_empty() {
+            error!("{} file(s) failed to process", errors.len());
+            for (file, err) in &errors {
+                error!("  {}: {}", file.display(), err);
+            }
+            return Err(format!("{} file(s) failed to process", errors.len()).into());
+        }
+    } else {
+        // Sequential processing
+        let mut failures = 0usize;
+        for input_file in csv_files {
+            if let Err(e) = processor(input_file) {
+                if !is_multiple {
+                    return Err(e);
+                }
+                failures += 1;
+                error!("Error processing {}: {}", input_file.display(), e);
+            }
+        }
+        if failures > 0 {
+            error!("{} file(s) failed to process", failures);
+        }
+    }
+
+    Ok(())
+}


The process_files function is defined but never used in the codebase. According to the PR description, this is intentional for future use. However, this will generate compiler warnings. Consider either using the function now or marking it with the allow attribute to suppress warnings until it's needed.

Copilot · 2026-01-08T18:35:48Z

+        GeoResolution::ThreeMinutes => GravityResolution::ThreeMinutes,
+        GeoResolution::TwoMinutes => GravityResolution::TwoMinutes,
+        GeoResolution::OneMinute => GravityResolution::OneMinute,
+        _ => GravityResolution::OneMinute,


The wildcard pattern in this match statement silently maps unsupported GeoResolution variants (ThirtySeconds, FifteenSeconds, ThreeSeconds, OneSecond) to OneMinute. This could lead to unexpected behavior where users specify a higher resolution than gravity maps support, but get silently downgraded without warning. Consider explicitly matching all variants and returning an error for unsupported resolutions, or at least logging a warning.

jbrodovsky · 2026-01-08T18:54:26Z

@copilot open a new pull request to apply changes based on the comments in this thread

Copilot · 2026-01-08T18:54:35Z

@jbrodovsky I've opened a new pull request, #213, to work on those changes. Once the pull request is ready, I'll request review from you.

jbrodovsky and others added 4 commits January 8, 2026 13:10

Copilot AI review requested due to automatic review settings January 8, 2026 18:27

chore: update Cargo.lock

5b18faf

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot started reviewing on behalf of jbrodovsky January 8, 2026 18:27 View session

Copilot AI reviewed Jan 8, 2026

View reviewed changes

formatted and linted

859c3a4

Copilot AI mentioned this pull request Jan 8, 2026

[WIP] Refactor geonav-sim into strapdown-sim using feature flag #213

Merged

5 tasks

jbrodovsky merged commit 6bd4f3f into main Jan 8, 2026
1 check passed

jbrodovsky deleted the refactor branch January 8, 2026 23:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Refactor: Consolidate geonav-sim into strapdown-sim with feature flag#212

Refactor: Consolidate geonav-sim into strapdown-sim with feature flag#212
jbrodovsky merged 6 commits into
mainfrom
refactor

jbrodovsky commented Jan 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

Copilot AI Jan 8, 2026

Uh oh!

jbrodovsky commented Jan 8, 2026

Uh oh!

Copilot AI commented Jan 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

jbrodovsky commented Jan 8, 2026

Summary

Changes

Step 1: Extract shared utilities (#207)

Step 2: Add geonav feature flag (#208)

Step 3: Parallel processing

Step 4: Remove geonav-sim binary (#210)

Step 5: Documentation (#211)

Usage

Breaking Changes

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

jbrodovsky commented Jan 8, 2026

Uh oh!

Copilot AI commented Jan 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants