Add geophysical measurement configuration to strapdown-sim#217
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds interactive configuration support for geophysical measurements (gravity and magnetic anomaly) to the strapdown-sim config wizard. Users can now configure geophysical navigation parameters through prompts rather than manually editing configuration files.
Key changes:
- Added geophysical configuration prompts to the CLI wizard with support for gravity and magnetic measurement settings
- Extended the core
SimulationConfigstruct with an optionalgeophysicalfield - Provided comprehensive documentation and example configurations
Reviewed changes
Copilot reviewed 8 out of 31 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| core/src/sim.rs | Added geophysical field to SimulationConfig struct to support geophysical measurement configuration |
| sim/src/main.rs | Implemented five new prompt functions for interactive geophysical configuration and integrated them into the config wizard |
| docs/GEOPHYSICAL_CONFIG.md | Added comprehensive documentation for using the geophysical configuration wizard |
| examples/configs/geonav_example.toml | Created example configuration file demonstrating geophysical measurement setup |
| conf/geonav_example.toml | Duplicate example configuration (identical to examples/configs version) |
| test_geo_config_wizard.sh | Added test script for validating the configuration wizard |
| .github/workflows/copilot-setup-steps.yml | Added GitHub Actions workflow for Copilot setup with system dependencies |
| GEOPHYSICAL_CONFIG_CHANGES.md | Added summary document describing all changes made in this PR |
| 11 | ||
| 10.0 | ||
| 150 | ||
| auto |
There was a problem hiding this comment.
The test script provides "auto" as input for the magnetic map file path, but the code checks for empty input to trigger auto-detection. The string "auto" will be treated as a file path rather than triggering auto-detection. This line should be empty to correctly test the auto-detection feature.
| 1. `/home/james/Code/strapdown-rs/core/src/sim.rs` | ||
| 2. `/home/james/Code/strapdown-rs/sim/src/main.rs` | ||
|
|
||
| ## Files Added | ||
|
|
||
| 1. `/home/james/Code/strapdown-rs/docs/GEOPHYSICAL_CONFIG.md` | ||
| 2. `/home/james/Code/strapdown-rs/examples/configs/geonav_example.toml` | ||
| 3. `/home/james/Code/strapdown-rs/test_geo_config_wizard.sh` |
There was a problem hiding this comment.
The file paths in this documentation contain hardcoded absolute paths specific to a user's local environment (e.g., /home/james/Code/strapdown-rs/). For documentation that may be committed to the repository, consider using relative paths or simply the filenames to make it more portable and professional.
| 1. `/home/james/Code/strapdown-rs/core/src/sim.rs` | |
| 2. `/home/james/Code/strapdown-rs/sim/src/main.rs` | |
| ## Files Added | |
| 1. `/home/james/Code/strapdown-rs/docs/GEOPHYSICAL_CONFIG.md` | |
| 2. `/home/james/Code/strapdown-rs/examples/configs/geonav_example.toml` | |
| 3. `/home/james/Code/strapdown-rs/test_geo_config_wizard.sh` | |
| 1. `core/src/sim.rs` | |
| 2. `sim/src/main.rs` | |
| ## Files Added | |
| 1. `docs/GEOPHYSICAL_CONFIG.md` | |
| 2. `examples/configs/geonav_example.toml` | |
| 3. `test_geo_config_wizard.sh` |
| # - Magnetic map file: (empty, auto-detect) | ||
| # - Geo frequency: 1.0 | ||
|
|
||
| cd /home/james/Code/strapdown-rs |
There was a problem hiding this comment.
This script contains a hardcoded absolute path specific to a user's local environment. Consider making this path-independent by removing this line and relying on the current working directory, or using a relative path. Users running this script from different locations will encounter errors.
|
|
||
| println!("\nGravity measurement bias (mGal) [0.0]: "); | ||
| let bias = match read_user_input() { | ||
| Some(input) if !input.is_empty() => match input.parse::<f64>() { | ||
| Ok(v) => Some(v), | ||
| Err(_) => { | ||
| println!("Invalid number. Using default (0.0)."); | ||
| None | ||
| } | ||
| }, | ||
| _ => None, | ||
| }; | ||
|
|
There was a problem hiding this comment.
The bias prompt lacks consistency with other numeric prompts in this configuration wizard. It doesn't support the 'q' to quit functionality that other prompts provide, and doesn't use the standard prompt_f64_with_default helper function. Consider using prompt_f64_with_default for the bias prompt to maintain consistency with the noise standard deviation prompt and provide range validation.
| println!("\nGravity measurement bias (mGal) [0.0]: "); | |
| let bias = match read_user_input() { | |
| Some(input) if !input.is_empty() => match input.parse::<f64>() { | |
| Ok(v) => Some(v), | |
| Err(_) => { | |
| println!("Invalid number. Using default (0.0)."); | |
| None | |
| } | |
| }, | |
| _ => None, | |
| }; | |
| let bias_value = prompt_f64_with_default( | |
| "Gravity measurement bias (mGal)", | |
| 0.0, | |
| f64::MIN, | |
| f64::MAX, | |
| ); | |
| let bias = Some(bias_value); |
|
|
||
| println!("\nMagnetic map file path (press Enter to auto-detect): "); | ||
| let map_file = match read_user_input() { | ||
| Some(input) if !input.is_empty() => Some(input), |
There was a problem hiding this comment.
The map file path prompt doesn't support the 'q' to quit functionality that other prompts in the configuration wizard provide. While the user can technically quit by using Ctrl+C, it would be more consistent to check for 'q' or 'quit' input and call std::process::exit(0) like other prompts do in this wizard.
| Some(input) if !input.is_empty() => Some(input), | |
| Some(input) if !input.is_empty() => { | |
| match input.to_lowercase().as_str() { | |
| "q" | "quit" => { | |
| println!("Configuration cancelled."); | |
| std::process::exit(0); | |
| } | |
| _ => Some(input), | |
| } | |
| } |
|
|
||
| println!("\nMagnetic measurement bias (nT) [0.0]: "); | ||
| let bias = match read_user_input() { | ||
| Some(input) if !input.is_empty() => match input.parse::<f64>() { | ||
| Ok(v) => Some(v), | ||
| Err(_) => { | ||
| println!("Invalid number. Using default (0.0)."); | ||
| None | ||
| } | ||
| }, | ||
| _ => None, | ||
| }; | ||
|
|
There was a problem hiding this comment.
The magnetic bias prompt lacks consistency with other numeric prompts in this configuration wizard. It doesn't support the 'q' to quit functionality that other prompts provide, and doesn't use the standard prompt_f64_with_default helper function. Consider using prompt_f64_with_default for the bias prompt to maintain consistency with the noise standard deviation prompt and provide range validation.
| println!("\nMagnetic measurement bias (nT) [0.0]: "); | |
| let bias = match read_user_input() { | |
| Some(input) if !input.is_empty() => match input.parse::<f64>() { | |
| Ok(v) => Some(v), | |
| Err(_) => { | |
| println!("Invalid number. Using default (0.0)."); | |
| None | |
| } | |
| }, | |
| _ => None, | |
| }; | |
| let bias_value = prompt_f64_with_default( | |
| "Magnetic measurement bias (nT)", | |
| 0.0, | |
| 0.0, | |
| f64::MAX, | |
| ); | |
| let bias = Some(bias_value); |
|
|
||
| println!("\nGravity map file path (press Enter to auto-detect): "); | ||
| let map_file = match read_user_input() { | ||
| Some(input) if !input.is_empty() => Some(input), |
There was a problem hiding this comment.
The map file path prompt doesn't support the 'q' to quit functionality that other prompts in the configuration wizard provide. While the user can technically quit by using Ctrl+C, it would be more consistent to check for 'q' or 'quit' input and call std::process::exit(0) like other prompts do in this wizard.
| Some(input) if !input.is_empty() => Some(input), | |
| Some(input) if !input.is_empty() => { | |
| let lower = input.to_lowercase(); | |
| if lower == "q" || lower == "quit" { | |
| println!("Configuration cancelled."); | |
| std::process::exit(0); | |
| } | |
| Some(input) | |
| } |
| println!("\nGeophysical measurement frequency (seconds) [auto]: "); | ||
|
|
||
| match read_user_input() { | ||
| Some(input) if !input.is_empty() => match input.parse::<f64>() { | ||
| Ok(freq) if freq > 0.0 => Some(freq), | ||
| Ok(_) => { | ||
| println!("Frequency must be positive. Using auto."); | ||
| None | ||
| } | ||
| Err(_) => { | ||
| println!("Invalid number. Using auto."); | ||
| None | ||
| } | ||
| }, | ||
| _ => None, |
There was a problem hiding this comment.
The geophysical measurement frequency prompt doesn't support the 'q' to quit functionality that other prompts in the configuration wizard provide. While the user can technically quit by using Ctrl+C, it would be more consistent to check for 'q' or 'quit' input and call std::process::exit(0) like other prompts do in this wizard.
| println!("\nGeophysical measurement frequency (seconds) [auto]: "); | |
| match read_user_input() { | |
| Some(input) if !input.is_empty() => match input.parse::<f64>() { | |
| Ok(freq) if freq > 0.0 => Some(freq), | |
| Ok(_) => { | |
| println!("Frequency must be positive. Using auto."); | |
| None | |
| } | |
| Err(_) => { | |
| println!("Invalid number. Using auto."); | |
| None | |
| } | |
| }, | |
| _ => None, | |
| loop { | |
| println!("\nGeophysical measurement frequency (seconds) [auto]: "); | |
| match read_user_input() { | |
| Some(input) => { | |
| let trimmed = input.trim(); | |
| if trimmed.is_empty() { | |
| // Use automatic behavior. | |
| return None; | |
| } | |
| match trimmed.to_lowercase().as_str() { | |
| "q" | "quit" => { | |
| println!("Configuration cancelled."); | |
| std::process::exit(0); | |
| } | |
| _ => match trimmed.parse::<f64>() { | |
| Ok(freq) if freq > 0.0 => { | |
| return Some(freq); | |
| } | |
| Ok(_) => { | |
| println!("Frequency must be positive. Please enter a positive number, press Enter for auto, or 'q' to quit."); | |
| continue; | |
| } | |
| Err(_) => { | |
| println!("Invalid number. Please enter a positive number, press Enter for auto, or 'q' to quit."); | |
| continue; | |
| } | |
| }, | |
| } | |
| } | |
| None => { | |
| println!("Invalid input. Please enter a positive number, press Enter for auto, or 'q' to quit."); | |
| continue; | |
| } | |
| } |
| # Example configuration with geophysical measurements | ||
| # Generated by strapdown-sim config wizard | ||
|
|
||
| input = "data/input/sample.csv" | ||
| output = "data/output/geonav_result.csv" | ||
| mode = "ClosedLoop" | ||
| seed = 42 | ||
| parallel = false | ||
| generate_plot = false | ||
|
|
||
| [logging] | ||
| level = "info" | ||
|
|
||
| [closed_loop] | ||
| filter = "Ukf" | ||
|
|
||
| [geophysical] | ||
| # Gravity anomaly measurements | ||
| gravity_resolution = "OneMinute" | ||
| gravity_bias = 0.0 | ||
| gravity_noise_std = 100.0 | ||
| # gravity_map_file will be auto-detected if not specified | ||
|
|
||
| # Magnetic anomaly measurements | ||
| magnetic_resolution = "OneMinute" | ||
| magnetic_bias = 0.0 | ||
| magnetic_noise_std = 150.0 | ||
| # magnetic_map_file will be auto-detected if not specified | ||
|
|
||
| # Measurement frequency (seconds) | ||
| geo_frequency_s = 1.0 | ||
|
|
||
| [gnss_degradation] | ||
| seed = 42 | ||
|
|
||
| [gnss_degradation.scheduler] | ||
| type = "Always" | ||
|
|
||
| [gnss_degradation.fault] | ||
| type = "None" |
There was a problem hiding this comment.
This configuration file appears to be duplicated in both conf/geonav_example.toml and examples/configs/geonav_example.toml. Having identical files in multiple locations creates a maintenance burden - if one is updated, the other must be kept in sync. Consider consolidating to a single location or documenting why both are needed.
| 11 | ||
| 5.0 | ||
| 100 | ||
| auto |
There was a problem hiding this comment.
The test script provides "auto" as input for the gravity map file path, but the code checks for empty input to trigger auto-detection. The string "auto" will be treated as a file path rather than triggering auto-detection. This line should be empty to correctly test the auto-detection feature. The same issue exists on line 58 for the magnetic map file.
Introduce configuration parameters for geophysical measurements, including gravity and magnetic anomaly settings, to the strapdown-sim config command. This enhancement simplifies the setup of simulations involving these measurements, allowing users to configure them interactively without manual file edits.
Fixes #214