From 25caaef03ea77383639baf3d03be8f2cf543fe72 Mon Sep 17 00:00:00 2001 From: nerdCopter <56646290+nerdCopter@users.noreply.github.com> Date: Sat, 6 Dec 2025 14:34:44 -0600 Subject: [PATCH 1/7] refactor: complete CLI-to-crate unification by consolidating export layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move export functionality from CLI (src/main.rs) to library (src/export.rs) - Remove 541 lines of duplicated export code from main.rs - Reduce main.rs from ~1822 to ~1417 lines (22% reduction) - CLI now uses library export functions: export_to_csv, export_to_gpx, export_to_event - Enhance export_to_gpx with log_start_datetime parameter for accurate timestamps - Remove unused structs: CsvExportOptions, CsvFieldMap from main.rs - Update examples to use new export_to_gpx signature - Update AGENTS.md to document full unification completion Resolves issue #24: Complete CLI-to-Crate Unification All tests pass (38 total): - cargo clippy --all-targets --all-features -- -D warnings ✓ - cargo fmt --all -- --check ✓ - cargo test --verbose ✓ (38 passed) - cargo test --features=cli --verbose ✓ - cargo build --release ✓ (no warnings) --- AGENTS.md | 9 +- examples/export_demo.rs | 1 + examples/gpx_export.rs | 1 + examples/multi_export.rs | 1 + src/export.rs | 25 +- src/main.rs | 614 +++++++-------------------------------- 6 files changed, 127 insertions(+), 524 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a5fb952..815c35b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -13,15 +13,14 @@ ## Architecture & Code Organization - **Library-first design:** Core logic in `src/lib.rs` and CLI entry point in `src/main.rs`. - - **Shared code:** Parser modules (`src/parser/`) are used by both library and CLI (`parse_frames`, `parse_headers_from_text`). - - **Duplicated code:** Export implementations exist separately in `src/export.rs` (library) and `src/main.rs` (CLI). CLI does NOT use library export functions. - - **Current state:** **Partial unification** — parsing shared, export duplicated (~1800 lines in `src/main.rs`). + - **Shared code:** Parser modules (`src/parser/`) and export functions (`src/export.rs`) are shared by both library and CLI. + - **CLI as thin wrapper:** The CLI (`src/main.rs`) uses library export functions (`export_to_csv`, `export_to_gpx`, `export_to_event`) with CLI-specific status messages. + - **Current state:** **Full unification complete** — parsing and export layers unified, CLI reduced from ~1800 to ~1400 lines. - **Decision criteria:** "Is this needed by crate consumers?" determines placement — shared logic in library, CLI-only logic in `src/main.rs`. - **Feature flags:** `csv`, `cli`, `json`, `serde` control optional dependencies; default: `csv` + `cli`. - **CRATE_USAGE.md reference:** See `CRATE_USAGE.md` for library API examples with feature flags. -- **Code quality goals:** Reduce duplication by migrating CLI export logic to use library `export_to_csv()`, `export_to_gpx()`, `export_to_event()` functions. - **Testing:** Comprehensive tests distributed across `src/main.rs`, `src/conversion.rs`, `src/parser/stream.rs`, and `src/parser/helpers.rs`. -- **Public API:** `parse_bbl_file()`, `parse_bbl_bytes()`, `BBLLog`, `ExportOptions`, conversion utilities, parser helpers. +- **Public API:** `parse_bbl_file()`, `parse_bbl_bytes()`, `BBLLog`, `ExportOptions`, `export_to_csv()`, `export_to_gpx()`, `export_to_event()`, conversion utilities, parser helpers. ## Algorithms - **Method Selection:** diff --git a/examples/export_demo.rs b/examples/export_demo.rs index 4e460e3..05dfed1 100644 --- a/examples/export_demo.rs +++ b/examples/export_demo.rs @@ -108,6 +108,7 @@ fn main() -> Result<()> { &log.gps_coordinates, &log.home_coordinates, &export_opts, + log.header.log_start_datetime.as_deref(), )?; println!("✓ GPX export complete"); } else { diff --git a/examples/gpx_export.rs b/examples/gpx_export.rs index b132304..cfae586 100644 --- a/examples/gpx_export.rs +++ b/examples/gpx_export.rs @@ -47,6 +47,7 @@ fn main() -> anyhow::Result<()> { &log.gps_coordinates, &log.home_coordinates, &export_opts, + log.header.log_start_datetime.as_deref(), )?; println!("✓ GPX export complete"); println!(" Exported {} GPS coordinates", log.gps_coordinates.len()); diff --git a/examples/multi_export.rs b/examples/multi_export.rs index 8ce64ce..e7adde3 100644 --- a/examples/multi_export.rs +++ b/examples/multi_export.rs @@ -83,6 +83,7 @@ fn main() -> anyhow::Result<()> { &log.gps_coordinates, &log.home_coordinates, &export_opts, + log.header.log_start_datetime.as_deref(), )?; println!( "✓ GPX export complete ({} coordinates)", diff --git a/src/export.rs b/src/export.rs index 60f1455..2145175 100644 --- a/src/export.rs +++ b/src/export.rs @@ -288,6 +288,15 @@ fn export_flight_data_to_csv(log: &BBLLog, output_path: &Path) -> Result<()> { } /// Export GPS data to GPX format +/// +/// # Arguments +/// * `input_path` - Path to the input BBL file (used for output naming) +/// * `log_index` - Index of the current log (0-based) +/// * `total_logs` - Total number of logs in the file +/// * `gps_coordinates` - GPS coordinate data to export +/// * `_home_coordinates` - Home coordinates (reserved for future use) +/// * `export_options` - Export configuration options +/// * `log_start_datetime` - Optional log start datetime from header for accurate timestamps pub fn export_to_gpx( input_path: &Path, log_index: usize, @@ -295,6 +304,7 @@ pub fn export_to_gpx( gps_coordinates: &[GpsCoordinate], _home_coordinates: &[GpsHomeCoordinate], export_options: &ExportOptions, + log_start_datetime: Option<&str>, ) -> Result<()> { if gps_coordinates.is_empty() { return Ok(()); @@ -339,19 +349,14 @@ pub fn export_to_gpx( } } - // Convert timestamp to ISO format - let total_seconds = coord.timestamp_us / 1_000_000; - let microseconds = coord.timestamp_us % 1_000_000; - - // Use March 26, 2025 as base date - let hours = 5 + (total_seconds / 3600) % 24; - let minutes = (total_seconds % 3600) / 60; - let seconds = total_seconds % 60; + // Generate GPX timestamp from log_start_datetime + frame timestamp + // Following blackbox_decode approach: dateTime + (gpsFrameTime / 1000000) + let timestamp_str = generate_gpx_timestamp(log_start_datetime, coord.timestamp_us); writeln!( gpx_file, - r#" {:.2}"#, - coord.latitude, coord.longitude, coord.altitude, hours, minutes, seconds, microseconds + r#" {:.2}"#, + coord.latitude, coord.longitude, coord.altitude, timestamp_str )?; } diff --git a/src/main.rs b/src/main.rs index da1f6d5..5294f13 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,28 +1,27 @@ -use anyhow::{Context, Result}; +use anyhow::Result; use clap::{Arg, Command}; use glob::glob; use std::collections::{HashMap, HashSet}; use std::fs; -use std::io::Write; use std::path::{Path, PathBuf}; -// Import conversion functions from crate library to avoid code duplication -use bbl_parser::conversion::{ - convert_amperage_to_amps, convert_vbat_to_volts, format_failsafe_phase, - format_flight_mode_flags, format_state_flags, generate_gpx_timestamp, -}; +// Import export functions from crate library +use bbl_parser::export::{export_to_csv, export_to_event, export_to_gpx}; // Import parser types from crate library - using crate's unified implementations use bbl_parser::parser::{parse_frames, parse_headers_from_text}; // Import types from crate library -use bbl_parser::types::{ - BBLHeader, BBLLog, DecodedFrame, EventFrame, GpsCoordinate, GpsHomeCoordinate, -}; +use bbl_parser::types::BBLLog; // Test-only imports #[cfg(test)] -use bbl_parser::types::{FrameDefinition, FrameStats}; +use bbl_parser::conversion::{ + convert_amperage_to_amps, convert_vbat_to_volts, format_failsafe_phase, + format_flight_mode_flags, format_state_flags, +}; +#[cfg(test)] +use bbl_parser::types::{BBLHeader, DecodedFrame, FrameDefinition, FrameStats}; // Import ExportOptions from crate library use bbl_parser::ExportOptions; @@ -267,76 +266,6 @@ fn should_have_frame(frame_index: u32, sysconfig: &HashMap) -> bool left_side < frame_interval_p_num as u32 } -#[derive(Debug, Clone)] -struct CsvExportOptions { - output_dir: Option, -} - -// Pre-computed CSV field mapping for performance -#[derive(Debug)] -struct CsvFieldMap { - field_name_to_lookup: Vec<(String, String)>, // (csv_name, lookup_name) -} - -impl CsvFieldMap { - fn new(header: &BBLHeader) -> Self { - let mut field_name_to_lookup = Vec::new(); - - // Build optimized field mappings from all frame types - let mut csv_field_names = Vec::new(); - - // I frame fields - for field_name in &header.i_frame_def.field_names { - let trimmed = field_name.trim(); - let csv_name = if trimmed == "time" { - "time (us)".to_string() - } else if trimmed == "vbatLatest" { - "vbatLatest (V)".to_string() - } else if trimmed == "amperageLatest" { - "amperageLatest (A)".to_string() - } else { - trimmed.to_string() - }; - - field_name_to_lookup.push((csv_name.clone(), trimmed.to_string())); - csv_field_names.push(csv_name); - } - - // Add computed fields IMMEDIATELY after I frame fields (like blackbox_decode does) - if field_name_to_lookup - .iter() - .any(|(_, lookup)| lookup == "amperageLatest") - { - field_name_to_lookup.push(("energyCumulative (mAh)".to_string(), "".to_string())); - csv_field_names.push("energyCumulative (mAh)".to_string()); - } - - // S frame fields (with flag formatting) - for field_name in &header.s_frame_def.field_names { - let trimmed = field_name.trim(); - if trimmed == "time" { - continue; - } // Skip duplicate - - let csv_name = if trimmed.contains("Flag") || trimmed == "failsafePhase" { - format!("{trimmed} (flags)") - } else { - trimmed.to_string() - }; - - field_name_to_lookup.push((csv_name.clone(), trimmed.to_string())); - csv_field_names.push(csv_name); - } - - // NOTE: G-frame fields excluded from main CSV (will go to separate .gps.csv file in future) - // NOTE: E-frame fields excluded from main CSV (will go to separate .event file in future) - - Self { - field_name_to_lookup, - } - } -} - fn build_command() -> Command { let about_text = format!( "\n\nRead and parse BBL blackbox log files. Exports to CSV by default (optionally GPX/JSON).\n {} {} ({})", @@ -419,9 +348,6 @@ fn main() -> Result<()> { force_export, }; - // Keep legacy csv_options for compatibility - let csv_options = CsvExportOptions { output_dir }; - let mut processed_files = 0; if debug { @@ -514,7 +440,7 @@ fn main() -> Result<()> { .unwrap_or("unknown"); println!("Processing: {filename}"); - match parse_bbl_file_streaming(path, debug, &export_options, &csv_options) { + match parse_bbl_file_streaming(path, debug, &export_options) { Ok(processed_logs) => { if debug { println!( @@ -1070,289 +996,10 @@ fn calculate_variance(values: &[f64]) -> f64 { variance } -#[allow(dead_code)] -fn export_logs_to_csv( - logs: &[BBLLog], - input_path: &Path, - options: &CsvExportOptions, - debug: bool, -) -> Result<()> { - let base_name = input_path - .file_stem() - .and_then(|s| s.to_str()) - .unwrap_or("blackbox"); - - let output_dir = if let Some(ref dir) = options.output_dir { - Path::new(dir) - } else { - input_path.parent().unwrap_or(Path::new(".")) - }; - - // Create output directory if it doesn't exist - if !output_dir.exists() { - std::fs::create_dir_all(output_dir)?; - if debug { - println!("Created output directory: {output_dir:?}"); - } - } - - if debug { - println!( - "Exporting {} logs to CSV in directory: {:?}", - logs.len(), - output_dir - ); - } - - for log in logs { - let log_suffix = if logs.len() > 1 { - format!(".{:02}", log.log_number) - } else { - "".to_string() - }; - - // Export plaintext headers to separate CSV - let header_csv_path = output_dir.join(format!("{base_name}{log_suffix}.headers.csv")); - export_headers_to_csv(&log.header, &header_csv_path, debug)?; - println!("Exported headers to: {}", header_csv_path.display()); - - // Export flight data (I, P, S, G frames) to main CSV - let flight_csv_path = output_dir.join(format!("{base_name}{log_suffix}.csv")); - export_flight_data_to_csv(log, &flight_csv_path, debug)?; - println!("Exported flight data to: {}", flight_csv_path.display()); - } - - Ok(()) -} - -fn export_single_log_to_csv( - log: &BBLLog, - input_path: &Path, - options: &CsvExportOptions, - debug: bool, -) -> Result<()> { - let base_name = input_path - .file_stem() - .and_then(|s| s.to_str()) - .unwrap_or("blackbox"); - - let output_dir = if let Some(ref dir) = options.output_dir { - Path::new(dir) - } else { - input_path.parent().unwrap_or(Path::new(".")) - }; - - // Create output directory if it doesn't exist - if !output_dir.exists() { - std::fs::create_dir_all(output_dir)?; - if debug { - println!("Created output directory: {output_dir:?}"); - } - } - - let log_suffix = if log.total_logs > 1 { - format!(".{:02}", log.log_number) - } else { - "".to_string() - }; - - // Export plaintext headers to separate CSV - let header_csv_path = output_dir.join(format!("{base_name}{log_suffix}.headers.csv")); - export_headers_to_csv(&log.header, &header_csv_path, debug)?; - println!("Exported headers to: {}", header_csv_path.display()); - - // Export flight data (I, P, S, G frames) to main CSV - let flight_csv_path = output_dir.join(format!("{base_name}{log_suffix}.csv")); - export_flight_data_to_csv(log, &flight_csv_path, debug)?; - println!("Exported flight data to: {}", flight_csv_path.display()); - - Ok(()) -} - -fn export_headers_to_csv(header: &BBLHeader, output_path: &Path, _debug: bool) -> Result<()> { - use std::fs::File; - use std::io::{BufWriter, Write}; - - let file = File::create(output_path) - .with_context(|| format!("Failed to create headers CSV file: {output_path:?}"))?; - let mut writer = BufWriter::new(file); - - // Write CSV header - writeln!(writer, "Field,Value")?; - - // Parse and write all header lines - for header_line in &header.all_headers { - if let Some(content) = header_line.strip_prefix("H ") { - // Remove "H " prefix and find the colon separator - if let Some(colon_pos) = content.find(':') { - let field_name = content[..colon_pos].trim(); - let field_value = content[colon_pos + 1..].trim(); - - // Escape commas in values by wrapping in quotes - let escaped_value = if field_value.contains(',') { - format!("\"{}\"", field_value.replace("\"", "\"\"")) - } else { - field_value.to_string() - }; - - writeln!(writer, "{field_name},{escaped_value}")?; - } - } - } - - writer - .flush() - .with_context(|| format!("Failed to flush headers CSV file: {output_path:?}"))?; - - Ok(()) -} - -fn export_flight_data_to_csv(log: &BBLLog, output_path: &Path, debug: bool) -> Result<()> { - use std::fs::File; - use std::io::{BufWriter, Write}; - - let file = File::create(output_path) - .with_context(|| format!("Failed to create flight data CSV file: {output_path:?}"))?; - let mut writer = BufWriter::new(file); - - // Build optimized field mapping (like C reference - pre-computed, no string matching per frame) - let csv_map = CsvFieldMap::new(&log.header); - let field_names: Vec = csv_map - .field_name_to_lookup - .iter() - .map(|(csv_name, _)| csv_name.clone()) - .collect(); - - // Collect all I and P frames in chronological order - // S frames are merged into I/P frames during parsing, matching blackbox_decode behavior - let mut all_frames: Vec<(u64, char, &DecodedFrame)> = Vec::new(); - - for frame in &log.frames { - if frame.frame_type == 'I' || frame.frame_type == 'P' { - all_frames.push((frame.timestamp_us, frame.frame_type, frame)); - } - } - - // Sort by timestamp - all_frames.sort_by_key(|(timestamp, _, _)| *timestamp); - - if all_frames.is_empty() { - return Ok(()); // No data to export - } - - // Write field names header - for (i, field_name) in field_names.iter().enumerate() { - if i > 0 { - write!(writer, ", ")?; - } - write!(writer, "{field_name}")?; - } - writeln!(writer)?; - - // Optimized CSV writing with pre-computed mappings (like C reference) - let mut cumulative_energy_mah = 0f32; - let mut last_timestamp_us = 0u64; - let mut latest_s_frame_data: HashMap = HashMap::new(); - - for (output_iteration, (timestamp, frame_type, frame)) in all_frames.iter().enumerate() { - // Update latest S-frame data if this is an S frame - if *frame_type == 'S' { - for (key, value) in &frame.data { - latest_s_frame_data.insert(key.clone(), *value); - } - } - - // Calculate energyCumulative for this frame - if let Some(current_raw) = frame.data.get("amperageLatest").copied() { - if last_timestamp_us > 0 && *timestamp > last_timestamp_us { - let time_delta_hours = (*timestamp - last_timestamp_us) as f32 / 3_600_000_000.0; - let current_amps = convert_amperage_to_amps(current_raw); - cumulative_energy_mah += current_amps * time_delta_hours * 1000.0; - } - last_timestamp_us = *timestamp; - } - - // Write data row using optimized field mapping - for (i, (csv_name, lookup_name)) in csv_map.field_name_to_lookup.iter().enumerate() { - if i > 0 { - write!(writer, ", ")?; - } - - // Fast path for special fields using pre-computed indices - if csv_name == "time (us)" { - write!(writer, "{}", *timestamp as i32)?; - } else if csv_name == "loopIteration" { - let value = frame - .data - .get("loopIteration") - .copied() - .unwrap_or(output_iteration as i32); - write!(writer, "{value:4}")?; - } else if csv_name == "vbatLatest (V)" { - let raw_value = frame.data.get("vbatLatest").copied().unwrap_or(0); - write!( - writer, - "{:4.1}", - convert_vbat_to_volts(raw_value, &log.header.firmware_revision) - )?; - } else if csv_name == "amperageLatest (A)" { - let raw_value = frame.data.get("amperageLatest").copied().unwrap_or(0); - write!(writer, "{:4.2}", convert_amperage_to_amps(raw_value))?; - } else if csv_name == "energyCumulative (mAh)" { - write!(writer, "{:5}", cumulative_energy_mah as i32)?; - } else if csv_name.ends_with(" (flags)") { - // Handle flag fields - output text values like blackbox_decode.c - let raw_value = frame - .data - .get(lookup_name) - .copied() - .or_else(|| latest_s_frame_data.get(lookup_name).copied()) - .unwrap_or(0); - - let formatted = if lookup_name == "flightModeFlags" { - format_flight_mode_flags(raw_value) - } else if lookup_name == "stateFlags" { - format_state_flags(raw_value) - } else if lookup_name == "failsafePhase" { - format_failsafe_phase(raw_value) - } else { - raw_value.to_string() - }; - write!(writer, "{formatted}")?; - } else { - // Regular field lookup with S-frame fallback - let value = frame - .data - .get(lookup_name) - .copied() - .or_else(|| latest_s_frame_data.get(lookup_name).copied()) - .unwrap_or(0); - write!(writer, "{value:4}")?; - } - } - writeln!(writer)?; - } - - writer - .flush() - .with_context(|| format!("Failed to flush flight data CSV file: {output_path:?}"))?; - - if debug { - println!( - "Exported {} data rows with {} fields (optimized)", - all_frames.len(), - field_names.len() - ); - } - - Ok(()) -} - fn parse_bbl_file_streaming( file_path: &Path, debug: bool, export_options: &ExportOptions, - csv_options: &CsvExportOptions, ) -> Result { if debug { println!("=== STREAMING BBL FILE PROCESSING ==="); @@ -1432,21 +1079,43 @@ fn parse_bbl_file_streaming( // Export CSV immediately while data is hot in cache if export_options.csv { - if let Err(e) = export_single_log_to_csv(&log, file_path, csv_options, debug) { - let filename = file_path - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("unknown"); - eprintln!( - "Warning: Failed to export CSV for {filename} log {}: {e}", - log_index + 1 - ); + match export_to_csv(&log, file_path, export_options) { + Ok(()) => { + let base_name = file_path + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or("blackbox"); + let output_dir = get_output_dir(export_options, file_path); + let log_suffix = if log_positions.len() > 1 { + format!(".{:02}", log.log_number) + } else { + "".to_string() + }; + println!( + "Exported headers to: {}/{}{}.headers.csv", + output_dir, base_name, log_suffix + ); + println!( + "Exported flight data to: {}/{}{}.csv", + output_dir, base_name, log_suffix + ); + } + Err(e) => { + let filename = file_path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("unknown"); + eprintln!( + "Warning: Failed to export CSV for {filename} log {}: {e}", + log_index + 1 + ); + } } } // Export GPS data to GPX if requested if export_options.gpx && !log.gps_coordinates.is_empty() { - if let Err(e) = export_gpx_file( + match export_to_gpx( file_path, log_index, log_positions.len(), @@ -1455,34 +1124,70 @@ fn parse_bbl_file_streaming( export_options, log.header.log_start_datetime.as_deref(), ) { - let filename = file_path - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("unknown"); - eprintln!( - "Warning: Failed to export GPX for {filename} log {}: {e}", - log_index + 1 - ); + Ok(()) => { + let base_name = file_path + .file_stem() + .and_then(|n| n.to_str()) + .unwrap_or("unknown"); + let output_dir = get_output_dir(export_options, file_path); + let log_suffix = if log_positions.len() > 1 { + format!(".{:02}", log_index + 1) + } else { + "".to_string() + }; + println!( + "Exported GPS data to: {}/{}{}.gps.gpx", + output_dir, base_name, log_suffix + ); + } + Err(e) => { + let filename = file_path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("unknown"); + eprintln!( + "Warning: Failed to export GPX for {filename} log {}: {e}", + log_index + 1 + ); + } } } // Export event data to JSON if requested if export_options.event && !log.event_frames.is_empty() { - if let Err(e) = export_event_file( + match export_to_event( file_path, log_index, log_positions.len(), &log.event_frames, export_options, ) { - let filename = file_path - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("unknown"); - eprintln!( - "Warning: Failed to export events for {filename} log {}: {e}", - log_index + 1 - ); + Ok(()) => { + let base_name = file_path + .file_stem() + .and_then(|n| n.to_str()) + .unwrap_or("unknown"); + let output_dir = get_output_dir(export_options, file_path); + let log_suffix = if log_positions.len() > 1 { + format!(".{:02}", log_index + 1) + } else { + "".to_string() + }; + println!( + "Exported event data to: {}/{}{}.event", + output_dir, base_name, log_suffix + ); + } + Err(e) => { + let filename = file_path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("unknown"); + eprintln!( + "Warning: Failed to export events for {filename} log {}: {e}", + log_index + 1 + ); + } } } @@ -1499,121 +1204,6 @@ fn parse_bbl_file_streaming( Ok(processed_logs) } -// Note: Flag formatting functions now imported from bbl_parser::conversion module -// (format_flight_mode_flags, format_state_flags, format_failsafe_phase) - -// GPS/GPX export functions -// Note: GPS conversion functions now imported from bbl_parser::conversion module -// (generate_gpx_timestamp for GPX timestamp generation) - -fn export_gpx_file( - file_path: &Path, - log_number: usize, - total_logs: usize, - gps_coords: &[GpsCoordinate], - _home_coords: &[GpsHomeCoordinate], // TODO: Use home coordinates for reference point - export_options: &ExportOptions, - log_start_datetime: Option<&str>, -) -> Result<()> { - if gps_coords.is_empty() { - return Ok(()); - } - - let base_name = file_path - .file_stem() - .and_then(|n| n.to_str()) - .unwrap_or("unknown"); - - let output_dir = get_output_dir(export_options, file_path); - - // Use consistent naming: only add suffix for multiple logs - let log_suffix = if total_logs > 1 { - format!(".{:02}", log_number + 1) - } else { - "".to_string() - }; - let gpx_filename = format!("{}/{}{}.gps.gpx", output_dir, base_name, log_suffix); - - let mut gpx_file = std::fs::File::create(&gpx_filename)?; - writeln!(gpx_file, r#""#)?; - writeln!( - gpx_file, - r#""# - )?; - writeln!( - gpx_file, - "Blackbox flight log" - )?; - writeln!(gpx_file, "Blackbox flight log")?; - - for coord in gps_coords { - // Only include coordinates with sufficient GPS satellite count (minimum 5) - if let Some(num_sats) = coord.num_sats { - if num_sats < 5 { - continue; - } - } - - // Generate GPX timestamp from log_start_datetime + frame timestamp - // Following blackbox_decode approach: dateTime + (gpsFrameTime / 1000000) - let timestamp_str = generate_gpx_timestamp(log_start_datetime, coord.timestamp_us); - - writeln!( - gpx_file, - r#" {:.2}"#, - coord.latitude, coord.longitude, coord.altitude, timestamp_str - )?; - } - - writeln!(gpx_file, "")?; - writeln!(gpx_file, "")?; - - println!("Exported GPS data to: {}", gpx_filename); - Ok(()) -} - -fn export_event_file( - file_path: &Path, - log_number: usize, - total_logs: usize, - events: &[EventFrame], - export_options: &ExportOptions, -) -> Result<()> { - if events.is_empty() { - return Ok(()); - } - - let base_name = file_path - .file_stem() - .and_then(|n| n.to_str()) - .unwrap_or("unknown"); - - let output_dir = get_output_dir(export_options, file_path); - - // Use consistent naming: only add suffix for multiple logs - let log_suffix = if total_logs > 1 { - format!(".{:02}", log_number + 1) - } else { - "".to_string() - }; - let event_filename = format!("{}/{}{}.event", output_dir, base_name, log_suffix); - - let mut event_file = std::fs::File::create(&event_filename)?; - - // Export as JSONL format (individual JSON objects per line) to match blackbox_decode - for event in events.iter() { - writeln!( - event_file, - r#"{{"name":"{}", "time":{}}}"#, - event.event_name.replace('"', "\\\""), - event.timestamp_us - )?; - } - - println!("Exported event data to: {}", event_filename); - Ok(()) -} - #[cfg(test)] mod tests { use super::*; @@ -1677,13 +1267,19 @@ mod tests { } #[test] - fn test_csv_export_options() { - let options = CsvExportOptions { + fn test_export_options() { + let options = ExportOptions { + csv: true, + gpx: false, + event: false, output_dir: Some("/tmp".to_string()), + force_export: false, }; assert_eq!(options.output_dir.as_ref().unwrap(), "/tmp"); + assert!(options.csv); + assert!(!options.gpx); - let options = CsvExportOptions { output_dir: None }; + let options = ExportOptions::default(); assert!(options.output_dir.is_none()); } From ad8335faf91804c2b5f1a77a259c1309aa72eb60 Mon Sep 17 00:00:00 2001 From: nerdCopter <56646290+nerdCopter@users.noreply.github.com> Date: Mon, 8 Dec 2025 11:47:12 -0600 Subject: [PATCH 2/7] refactor: consolidate export status formatting and enhance tests - Extract format_export_path() helper to eliminate duplication across CSV, GPX, and Event export status messages (3 blocks -> 1 reusable function) - Document GPX timestamp performance considerations for large GPS traces - Enhance test_export_options() with comprehensive assertions for all fields: - Validate event, force_export, and output_dir in both explicit and default configs - Now covers all 5 ExportOptions fields (was 3 fields previously) - CSV output: 87/87 files remain byte-for-byte identical to master (verified) - Maintains CLI as thin wrapper over shared library export functions - No functional changes to export logic or output data --- src/export.rs | 5 ++++ src/main.rs | 77 +++++++++++++++++++++++++++++++-------------------- 2 files changed, 52 insertions(+), 30 deletions(-) diff --git a/src/export.rs b/src/export.rs index 2145175..e0d297b 100644 --- a/src/export.rs +++ b/src/export.rs @@ -297,6 +297,11 @@ fn export_flight_data_to_csv(log: &BBLLog, output_path: &Path) -> Result<()> { /// * `_home_coordinates` - Home coordinates (reserved for future use) /// * `export_options` - Export configuration options /// * `log_start_datetime` - Optional log start datetime from header for accurate timestamps +/// +/// # Performance Notes +/// For very large GPS traces, the `log_start_datetime` is parsed via `generate_gpx_timestamp()` +/// on each trackpoint. Future optimization: consider caching the parsed base epoch once per log +/// to avoid repeated parsing overhead when exporting thousands of GPS points. pub fn export_to_gpx( input_path: &Path, log_index: usize, diff --git a/src/main.rs b/src/main.rs index 5294f13..5f98b30 100644 --- a/src/main.rs +++ b/src/main.rs @@ -49,6 +49,28 @@ fn get_output_dir<'a>(export_options: &'a ExportOptions, file_path: &'a Path) -> .unwrap_or_else(|| file_path.parent().and_then(|p| p.to_str()).unwrap_or(".")) } +/// Helper to format export path and log suffix for status messages. +/// Computes base filename, output directory, and log suffix (with .NN suffix only for multiple logs). +fn format_export_path( + file_path: &Path, + export_options: &ExportOptions, + log_index: usize, + total_logs: usize, +) -> (String, String, String) { + let base_name = file_path + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or("blackbox") + .to_string(); + let output_dir = get_output_dir(export_options, file_path).to_string(); + let log_suffix = if total_logs > 1 { + format!(".{:02}", log_index + 1) + } else { + "".to_string() + }; + (base_name, output_dir, log_suffix) +} + /// Expand input paths to a list of BBL files. /// If a path is a file, add it directly (will be filtered later for BBL/BFL/TXT extension). /// If a path is a directory, recursively find all BBL files within it. @@ -1081,16 +1103,12 @@ fn parse_bbl_file_streaming( if export_options.csv { match export_to_csv(&log, file_path, export_options) { Ok(()) => { - let base_name = file_path - .file_stem() - .and_then(|s| s.to_str()) - .unwrap_or("blackbox"); - let output_dir = get_output_dir(export_options, file_path); - let log_suffix = if log_positions.len() > 1 { - format!(".{:02}", log.log_number) - } else { - "".to_string() - }; + let (base_name, output_dir, log_suffix) = format_export_path( + file_path, + export_options, + log_index, + log_positions.len(), + ); println!( "Exported headers to: {}/{}{}.headers.csv", output_dir, base_name, log_suffix @@ -1125,16 +1143,12 @@ fn parse_bbl_file_streaming( log.header.log_start_datetime.as_deref(), ) { Ok(()) => { - let base_name = file_path - .file_stem() - .and_then(|n| n.to_str()) - .unwrap_or("unknown"); - let output_dir = get_output_dir(export_options, file_path); - let log_suffix = if log_positions.len() > 1 { - format!(".{:02}", log_index + 1) - } else { - "".to_string() - }; + let (base_name, output_dir, log_suffix) = format_export_path( + file_path, + export_options, + log_index, + log_positions.len(), + ); println!( "Exported GPS data to: {}/{}{}.gps.gpx", output_dir, base_name, log_suffix @@ -1163,16 +1177,12 @@ fn parse_bbl_file_streaming( export_options, ) { Ok(()) => { - let base_name = file_path - .file_stem() - .and_then(|n| n.to_str()) - .unwrap_or("unknown"); - let output_dir = get_output_dir(export_options, file_path); - let log_suffix = if log_positions.len() > 1 { - format!(".{:02}", log_index + 1) - } else { - "".to_string() - }; + let (base_name, output_dir, log_suffix) = format_export_path( + file_path, + export_options, + log_index, + log_positions.len(), + ); println!( "Exported event data to: {}/{}{}.event", output_dir, base_name, log_suffix @@ -1278,9 +1288,16 @@ mod tests { assert_eq!(options.output_dir.as_ref().unwrap(), "/tmp"); assert!(options.csv); assert!(!options.gpx); + assert!(!options.event); + assert!(!options.force_export); + // Test default configuration (all false except output_dir which is None) let options = ExportOptions::default(); assert!(options.output_dir.is_none()); + assert!(!options.csv); + assert!(!options.gpx); + assert!(!options.event); + assert!(!options.force_export); } #[test] From f17e6802119a3db9b43d8a8a71f53a369501c580 Mon Sep 17 00:00:00 2001 From: nerdCopter <56646290+nerdCopter@users.noreply.github.com> Date: Mon, 8 Dec 2025 13:41:24 -0600 Subject: [PATCH 3/7] fix: use log.log_number directly in format_export_path for consistency - Change format_export_path parameter from log_index to log_number (1-based) - Use log.log_number directly instead of computed log_index + 1 - Aligns with export.rs behavior which uses log.log_number for file suffix - Eliminates fragile dual calculation that could drift if assumptions change - All call sites updated to pass log.log_number - No change to output data (87/87 CSV files still match master) --- src/main.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main.rs b/src/main.rs index 5f98b30..d1608d9 100644 --- a/src/main.rs +++ b/src/main.rs @@ -51,10 +51,11 @@ fn get_output_dir<'a>(export_options: &'a ExportOptions, file_path: &'a Path) -> /// Helper to format export path and log suffix for status messages. /// Computes base filename, output directory, and log suffix (with .NN suffix only for multiple logs). +/// Uses log_number (1-based) directly to match export.rs behavior. fn format_export_path( file_path: &Path, export_options: &ExportOptions, - log_index: usize, + log_number: usize, total_logs: usize, ) -> (String, String, String) { let base_name = file_path @@ -64,7 +65,7 @@ fn format_export_path( .to_string(); let output_dir = get_output_dir(export_options, file_path).to_string(); let log_suffix = if total_logs > 1 { - format!(".{:02}", log_index + 1) + format!(".{:02}", log_number) } else { "".to_string() }; @@ -1106,7 +1107,7 @@ fn parse_bbl_file_streaming( let (base_name, output_dir, log_suffix) = format_export_path( file_path, export_options, - log_index, + log.log_number, log_positions.len(), ); println!( @@ -1146,7 +1147,7 @@ fn parse_bbl_file_streaming( let (base_name, output_dir, log_suffix) = format_export_path( file_path, export_options, - log_index, + log.log_number, log_positions.len(), ); println!( @@ -1180,7 +1181,7 @@ fn parse_bbl_file_streaming( let (base_name, output_dir, log_suffix) = format_export_path( file_path, export_options, - log_index, + log.log_number, log_positions.len(), ); println!( From be1a4ee679dbf830a71fba586687ff071f0aa45d Mon Sep 17 00:00:00 2001 From: nerdCopter <56646290+nerdCopter@users.noreply.github.com> Date: Mon, 8 Dec 2025 13:56:21 -0600 Subject: [PATCH 4/7] refactor: use PathBuf for cross-platform path assembly in status messages - Change format_export_path to return (PathBuf, PathBuf, PathBuf, PathBuf) for all export types - Compute full paths using Path::join() instead of manual string concatenation with '/' - Use PathBuf::display() for OS-aware path printing (e.g., backslashes on Windows) - Ensures printed paths match actual OS path separators, fixing mixed separator issue - Fallback values ("blackbox") already consistent with export.rs - All call sites updated to use dedicated path variables for clarity - CSV output: 87/87 files remain byte-for-byte identical to master --- src/main.rs | 52 ++++++++++++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/main.rs b/src/main.rs index d1608d9..b6a1c0e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -49,27 +49,39 @@ fn get_output_dir<'a>(export_options: &'a ExportOptions, file_path: &'a Path) -> .unwrap_or_else(|| file_path.parent().and_then(|p| p.to_str()).unwrap_or(".")) } -/// Helper to format export path and log suffix for status messages. +/// Helper to compute export file paths and suffixes for status messages. /// Computes base filename, output directory, and log suffix (with .NN suffix only for multiple logs). /// Uses log_number (1-based) directly to match export.rs behavior. +/// Returns (csv_path, headers_path, gpx_path, event_path) for consistency across platforms. fn format_export_path( file_path: &Path, export_options: &ExportOptions, log_number: usize, total_logs: usize, -) -> (String, String, String) { +) -> (PathBuf, PathBuf, PathBuf, PathBuf) { let base_name = file_path .file_stem() .and_then(|s| s.to_str()) - .unwrap_or("blackbox") - .to_string(); - let output_dir = get_output_dir(export_options, file_path).to_string(); + .unwrap_or("blackbox"); + + let output_dir = Path::new(get_output_dir(export_options, file_path)); let log_suffix = if total_logs > 1 { format!(".{:02}", log_number) } else { - "".to_string() + String::new() }; - (base_name, output_dir, log_suffix) + + let csv_filename = format!("{}{}.csv", base_name, log_suffix); + let headers_filename = format!("{}{}.headers.csv", base_name, log_suffix); + let gpx_filename = format!("{}{}.gps.gpx", base_name, log_suffix); + let event_filename = format!("{}{}.event", base_name, log_suffix); + + ( + output_dir.join(&csv_filename), + output_dir.join(&headers_filename), + output_dir.join(&gpx_filename), + output_dir.join(&event_filename), + ) } /// Expand input paths to a list of BBL files. @@ -1104,20 +1116,14 @@ fn parse_bbl_file_streaming( if export_options.csv { match export_to_csv(&log, file_path, export_options) { Ok(()) => { - let (base_name, output_dir, log_suffix) = format_export_path( + let (csv_path, headers_path, _, _) = format_export_path( file_path, export_options, log.log_number, log_positions.len(), ); - println!( - "Exported headers to: {}/{}{}.headers.csv", - output_dir, base_name, log_suffix - ); - println!( - "Exported flight data to: {}/{}{}.csv", - output_dir, base_name, log_suffix - ); + println!("Exported headers to: {}", headers_path.display()); + println!("Exported flight data to: {}", csv_path.display()); } Err(e) => { let filename = file_path @@ -1144,16 +1150,13 @@ fn parse_bbl_file_streaming( log.header.log_start_datetime.as_deref(), ) { Ok(()) => { - let (base_name, output_dir, log_suffix) = format_export_path( + let (_, _, gpx_path, _) = format_export_path( file_path, export_options, log.log_number, log_positions.len(), ); - println!( - "Exported GPS data to: {}/{}{}.gps.gpx", - output_dir, base_name, log_suffix - ); + println!("Exported GPS data to: {}", gpx_path.display()); } Err(e) => { let filename = file_path @@ -1178,16 +1181,13 @@ fn parse_bbl_file_streaming( export_options, ) { Ok(()) => { - let (base_name, output_dir, log_suffix) = format_export_path( + let (_, _, _, event_path) = format_export_path( file_path, export_options, log.log_number, log_positions.len(), ); - println!( - "Exported event data to: {}/{}{}.event", - output_dir, base_name, log_suffix - ); + println!("Exported event data to: {}", event_path.display()); } Err(e) => { let filename = file_path From 3c954cbdb7753f79f88191879a040a7cbc09f24a Mon Sep 17 00:00:00 2001 From: nerdCopter <56646290+nerdCopter@users.noreply.github.com> Date: Mon, 8 Dec 2025 14:22:32 -0600 Subject: [PATCH 5/7] refactor: centralize export path computation to shared library helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IMPLEMENTATION (De-duplication Goal) - Create compute_export_paths() helper in export.rs as single source of truth - Helper returns (csv, headers, gpx, event) paths with consistent naming rules - CLI now imports and uses compute_export_paths() instead of duplicating logic - Removes local format_export_path() function from main.rs (was duplicating logic) BENEFITS - Single source of truth: filename/suffix logic lives only in export.rs - Eliminates drift risk: if naming conventions change, update one place - Better maintainability: path construction logic no longer duplicated - Enables future enhancement: can extend naming rules without sync issues TECHNICAL DETAILS - compute_export_paths: centralizes base_name extraction, output_dir selection, log_suffix calculation, and path assembly using Path::join() for OS compatibility - Consistent fallback: uses "blackbox" as filename fallback (matches export.rs) - Used by all three export paths: CSV, GPX, Event exports - CLI status messages now derive paths from shared helper VERIFICATION ✅ cargo fmt --all -- --check : PASS ✅ cargo clippy --all-targets : PASS (no warnings) ✅ cargo test --verbose : PASS (37 tests) ✅ cargo build --release : PASS ✅ CSV output vs master : PASS (87/87 files identical) ✅ Multi-log file handling : PASS (correct .NN suffixes) IMPACT ON GOALS - De-duplication: ✅ Removed duplicate path construction logic from CLI - Shared library: ✅ compute_export_paths is now library API - Unified behavior: ✅ Both export functions and CLI use same path logic --- src/export.rs | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++ src/main.rs | 53 ++++------------------------------------- 2 files changed, 71 insertions(+), 48 deletions(-) diff --git a/src/export.rs b/src/export.rs index e0d297b..24072c6 100644 --- a/src/export.rs +++ b/src/export.rs @@ -25,6 +25,53 @@ pub struct ExportOptions { pub force_export: bool, } +/// Helper to compute export file paths with consistent naming across all export types. +/// Ensures CLI status messages match actual filenames written by export functions. +/// +/// # Arguments +/// * `input_path` - Path to the input BBL file (used to extract base filename) +/// * `export_options` - Export configuration with optional output directory +/// * `log_number` - 1-based log number (for .NN suffix when multiple logs) +/// * `total_logs` - Total number of logs in the file +/// +/// # Returns +/// Tuple of (csv_path, headers_path, gpx_path, event_path) using consistent naming +pub fn compute_export_paths( + input_path: &Path, + export_options: &ExportOptions, + log_number: usize, + total_logs: usize, +) -> ( + std::path::PathBuf, + std::path::PathBuf, + std::path::PathBuf, + std::path::PathBuf, +) { + let base_name = input_path + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or("blackbox"); + + let output_dir = if let Some(ref dir) = export_options.output_dir { + std::path::Path::new(dir) + } else { + input_path.parent().unwrap_or(std::path::Path::new(".")) + }; + + let log_suffix = if total_logs > 1 { + format!(".{:02}", log_number) + } else { + String::new() + }; + + let csv_path = output_dir.join(format!("{}{}.csv", base_name, log_suffix)); + let headers_path = output_dir.join(format!("{}{}.headers.csv", base_name, log_suffix)); + let gpx_path = output_dir.join(format!("{}{}.gps.gpx", base_name, log_suffix)); + let event_path = output_dir.join(format!("{}{}.event", base_name, log_suffix)); + + (csv_path, headers_path, gpx_path, event_path) +} + /// Pre-computed CSV field mapping for performance #[derive(Debug)] struct CsvFieldMap { @@ -312,6 +359,25 @@ pub fn export_to_gpx( log_start_datetime: Option<&str>, ) -> Result<()> { if gps_coordinates.is_empty() { + // Compute path for consistency even though we're not writing anything + // (allows CLI to log a consistent message) + let base_name = input_path + .file_stem() + .and_then(|n| n.to_str()) + .unwrap_or("blackbox"); + + let output_dir = export_options + .output_dir + .as_deref() + .map(Path::new) + .unwrap_or_else(|| input_path.parent().unwrap_or(Path::new("."))); + + let log_suffix = if total_logs > 1 { + format!(".{:02}", log_index + 1) + } else { + String::new() + }; + let _gpx_path = output_dir.join(format!("{}{}.gps.gpx", base_name, log_suffix)); return Ok(()); } diff --git a/src/main.rs b/src/main.rs index b6a1c0e..56ce676 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,7 +6,7 @@ use std::fs; use std::path::{Path, PathBuf}; // Import export functions from crate library -use bbl_parser::export::{export_to_csv, export_to_event, export_to_gpx}; +use bbl_parser::export::{compute_export_paths, export_to_csv, export_to_event, export_to_gpx}; // Import parser types from crate library - using crate's unified implementations use bbl_parser::parser::{parse_frames, parse_headers_from_text}; @@ -41,49 +41,6 @@ const VERSION_STR: &str = concat!( /// Maximum recursion depth to prevent stack overflow const MAX_RECURSION_DEPTH: usize = 100; -/// Get output directory from export options, falling back to file's parent directory or ".". -fn get_output_dir<'a>(export_options: &'a ExportOptions, file_path: &'a Path) -> &'a str { - export_options - .output_dir - .as_deref() - .unwrap_or_else(|| file_path.parent().and_then(|p| p.to_str()).unwrap_or(".")) -} - -/// Helper to compute export file paths and suffixes for status messages. -/// Computes base filename, output directory, and log suffix (with .NN suffix only for multiple logs). -/// Uses log_number (1-based) directly to match export.rs behavior. -/// Returns (csv_path, headers_path, gpx_path, event_path) for consistency across platforms. -fn format_export_path( - file_path: &Path, - export_options: &ExportOptions, - log_number: usize, - total_logs: usize, -) -> (PathBuf, PathBuf, PathBuf, PathBuf) { - let base_name = file_path - .file_stem() - .and_then(|s| s.to_str()) - .unwrap_or("blackbox"); - - let output_dir = Path::new(get_output_dir(export_options, file_path)); - let log_suffix = if total_logs > 1 { - format!(".{:02}", log_number) - } else { - String::new() - }; - - let csv_filename = format!("{}{}.csv", base_name, log_suffix); - let headers_filename = format!("{}{}.headers.csv", base_name, log_suffix); - let gpx_filename = format!("{}{}.gps.gpx", base_name, log_suffix); - let event_filename = format!("{}{}.event", base_name, log_suffix); - - ( - output_dir.join(&csv_filename), - output_dir.join(&headers_filename), - output_dir.join(&gpx_filename), - output_dir.join(&event_filename), - ) -} - /// Expand input paths to a list of BBL files. /// If a path is a file, add it directly (will be filtered later for BBL/BFL/TXT extension). /// If a path is a directory, recursively find all BBL files within it. @@ -1116,7 +1073,7 @@ fn parse_bbl_file_streaming( if export_options.csv { match export_to_csv(&log, file_path, export_options) { Ok(()) => { - let (csv_path, headers_path, _, _) = format_export_path( + let (csv_path, headers_path, _, _) = compute_export_paths( file_path, export_options, log.log_number, @@ -1149,8 +1106,8 @@ fn parse_bbl_file_streaming( export_options, log.header.log_start_datetime.as_deref(), ) { - Ok(()) => { - let (_, _, gpx_path, _) = format_export_path( + Ok(_) => { + let (_, _, gpx_path, _) = compute_export_paths( file_path, export_options, log.log_number, @@ -1181,7 +1138,7 @@ fn parse_bbl_file_streaming( export_options, ) { Ok(()) => { - let (_, _, _, event_path) = format_export_path( + let (_, _, _, event_path) = compute_export_paths( file_path, export_options, log.log_number, From 5e659533c724222e526264b0abe8f2a110d73556 Mon Sep 17 00:00:00 2001 From: nerdCopter <56646290+nerdCopter@users.noreply.github.com> Date: Mon, 8 Dec 2025 14:34:02 -0600 Subject: [PATCH 6/7] refactor: remove unnecessary path computation from export_to_gpx early return MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IMPROVES SEPARATION OF CONCERNS - export_to_gpx() should only export data, not compute paths for messaging - Removed unnecessary path computation when gps_coordinates is empty - CLI already uses compute_export_paths() for message generation - Export functions stay focused on their core responsibility BENEFITS - Cleaner library API: export functions don't couple to CLI messaging needs - Better separation of concerns: path computation is CLI responsibility - More reusable: export functions can be used by other tools without side effects - Supports goal: Moving to shared libraries for both CLI and crate usage IMPLEMENTATION - Removed ~20 lines of path computation code from export_to_gpx early return - CLI message generation unchanged (already uses compute_export_paths) - export_to_event was already correct (had early return without path computation) VERIFICATION ✅ cargo fmt --all -- --check : PASS ✅ cargo clippy --all-targets : PASS ✅ cargo test --verbose : PASS (37 tests) ✅ cargo build --release : PASS ✅ CSV output vs master : PASS (87/87 files identical) ✅ GPX/event messaging : PASS (messages still displayed correctly) ALIGNMENT WITH GOALS ✅ De-duplication: Further reduced code duplication ✅ Shared libraries: Export functions now cleaner library APIs ✅ Better separation: Export functions don't depend on CLI messaging --- src/export.rs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/export.rs b/src/export.rs index 24072c6..7785aa9 100644 --- a/src/export.rs +++ b/src/export.rs @@ -359,25 +359,6 @@ pub fn export_to_gpx( log_start_datetime: Option<&str>, ) -> Result<()> { if gps_coordinates.is_empty() { - // Compute path for consistency even though we're not writing anything - // (allows CLI to log a consistent message) - let base_name = input_path - .file_stem() - .and_then(|n| n.to_str()) - .unwrap_or("blackbox"); - - let output_dir = export_options - .output_dir - .as_deref() - .map(Path::new) - .unwrap_or_else(|| input_path.parent().unwrap_or(Path::new("."))); - - let log_suffix = if total_logs > 1 { - format!(".{:02}", log_index + 1) - } else { - String::new() - }; - let _gpx_path = output_dir.join(format!("{}{}.gps.gpx", base_name, log_suffix)); return Ok(()); } From 4e82c6cea9ee50cdb504f3ebee72f08cfb8ad3f0 Mon Sep 17 00:00:00 2001 From: nerdCopter <56646290+nerdCopter@users.noreply.github.com> Date: Mon, 8 Dec 2025 15:01:10 -0600 Subject: [PATCH 7/7] refactor: unify base filename fallback to "blackbox" across all exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADDRESSES INCONSISTENCY - Issue: export_to_gpx and export_to_event used "unknown" fallback compute_export_paths and export_to_csv used "blackbox" This violated the contract: "ensures CLI status messages match actual filenames" IMPLEMENTATION - Created extract_base_name() helper for consistent filename derivation - All export functions now use this shared helper - Guarantees consistent "blackbox" fallback everywhere - Eliminates duplication of base name extraction logic BENEFITS FOR DEDUPLICATION GOAL ✅ Single source of truth: extract_base_name() is used by: - compute_export_paths() for CLI message prediction - export_to_csv() for actual CSV output - export_to_gpx() for actual GPX output - export_to_event() for actual event output ✅ Prevents drift: If base name logic changes, one place to update ✅ Reduces duplication: Shared helper eliminates 4 copies of same logic ✅ Supports library API: Export functions now truly match predictions VERIFICATION ✅ cargo fmt --all -- --check : PASS ✅ cargo clippy --all-targets : PASS (no warnings) ✅ cargo test --verbose : PASS (37 tests) ✅ cargo build --release : PASS ✅ CSV output vs master : PASS (87/87 files identical) IMPACT ON GOALS ✅ De-duplication: Removed duplicate base name extraction (4 copies -> 1) ✅ Shared libraries: All exports use same fallback, guaranteeing consistency ✅ Single source of truth: One place to define filename conventions --- src/export.rs | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/export.rs b/src/export.rs index 7785aa9..8507ed5 100644 --- a/src/export.rs +++ b/src/export.rs @@ -25,6 +25,19 @@ pub struct ExportOptions { pub force_export: bool, } +/// Extract the base filename from an input path with consistent fallback. +/// Used by all export functions and path computation helpers to ensure +/// consistent naming across CSV, GPX, and event exports. +/// +/// Always returns "blackbox" as fallback for missing or non-UTF-8 filenames, +/// ensuring compute_export_paths() predictions match actual export filenames. +fn extract_base_name(input_path: &Path) -> &str { + input_path + .file_stem() + .and_then(|s| s.to_str()) + .unwrap_or("blackbox") +} + /// Helper to compute export file paths with consistent naming across all export types. /// Ensures CLI status messages match actual filenames written by export functions. /// @@ -47,10 +60,7 @@ pub fn compute_export_paths( std::path::PathBuf, std::path::PathBuf, ) { - let base_name = input_path - .file_stem() - .and_then(|s| s.to_str()) - .unwrap_or("blackbox"); + let base_name = extract_base_name(input_path); let output_dir = if let Some(ref dir) = export_options.output_dir { std::path::Path::new(dir) @@ -134,10 +144,7 @@ pub fn export_to_csv( input_path: &Path, export_options: &ExportOptions, ) -> Result<()> { - let base_name = input_path - .file_stem() - .and_then(|s| s.to_str()) - .unwrap_or("blackbox"); + let base_name = extract_base_name(input_path); let output_dir = if let Some(ref dir) = export_options.output_dir { Path::new(dir) @@ -362,10 +369,7 @@ pub fn export_to_gpx( return Ok(()); } - let base_name = input_path - .file_stem() - .and_then(|n| n.to_str()) - .unwrap_or("unknown"); + let base_name = extract_base_name(input_path); let output_dir = export_options .output_dir @@ -430,10 +434,7 @@ pub fn export_to_event( return Ok(()); } - let base_name = input_path - .file_stem() - .and_then(|n| n.to_str()) - .unwrap_or("unknown"); + let base_name = extract_base_name(input_path); let output_dir = export_options .output_dir