Skip to content

Commit 40f9311

Browse files
authored
feat: migrate GPS/Event frame parsing from CLI to crate library (Issue #7) (#8)
* feat: migrate GPS/Event frame parsing from CLI to crate library (Issue #7) - Create GPS helper module (src/parser/gps.rs) with H-frame and G-frame parsing - parse_h_frame(): Parse GPS home position data - parse_g_frame(): Parse GPS position data with differential encoding - extract_home_coordinate() and extract_gps_coordinate() helpers - Create Event helper module (src/parser/event.rs) with E-frame parsing - parse_e_frame(): Full event type handling and description generation - Supports all event types: Sync beep, Autotune, Adjustment, Resume, Disarm, etc. - Integrate GPS/Event parsing into library frame parser (src/parser/frame.rs) - Replace G/H/E frame skipping with actual parsing using helpers - Populate gps_coordinates, home_coordinates, and event_frames vectors - Track last_main_frame_timestamp for proper GPS/Event timestamping - Update types to support both CLI and crate usage - Updated EventFrame struct with event_data field - Renamed event_description to event_name for consistency - CLI remains fully functional with no behavior changes - All existing parsing logic preserved - Export functions work identically for both CLI and crate Tests: - cargo clippy: ✓ No warnings - cargo fmt: ✓ Properly formatted - cargo test: ✓ All tests pass - cargo build --release: ✓ No errors Resolves: Issue #7 * fix: prevent potential panic in parse_g_frame with incorrect history length Address CodeRabbit review feedback: - Changed is_empty() check to len() != frame_def.count comparison - Use resize() instead of vec assignment to handle pre-populated history - Prevents panic in copy_from_slice when caller passes history with wrong length - Added clarifying comment explaining the safety guarantee * refactor: address CodeRabbit nitpick feedback Address all 6 nitpick comments from CodeRabbit review: 1. examples/test_crate_gps.rs: - Add CLI argument handling instead of hardcoded input path - Follow pattern from gpx_export.rs for better usability 2. src/parser/event.rs: - Extract parse_inflight_adjustment() helper function - Reduce code duplication between event types 4 and 13 3. src/parser/gps.rs: - Add debug logging for read errors in fallback encoding handler - Use debug parameter with logging for G-frame parsing - Simplify duplicate home coordinate lookup using map/unwrap_or * fix: propagate errors for unsupported H-frame encodings instead of silently swallowing Replace fallback encoding handler in parse_h_frame to return a descriptive error instead of silently continuing with default values. Changes: - Detect unsupported encoding and return Err with descriptive message - Include field name and encoding value in error message - Prevents stream desynchronization from being masked by default values - Allows caller to handle the error appropriately This ensures that encountering an unsupported H-frame encoding properly propagates an error through the Result type, rather than continuing to read from the stream with potentially corrupted data. * docs: improve comments and debug logging for event/GPS parsing Address CodeRabbit nitpick feedback: 1. src/parser/event.rs (types 5 and 14): - Add clarifying comment explaining why both event types have identical implementations and appear in different firmware versions 2. src/parser/gps.rs (missing home coordinates): - Add debug logging when home_coordinates are empty - Logs when defaulting to (0.0, 0.0) as home position - Helps users understand GPS coordinate behavior * fix: align read_neg_14bit implementation with established pattern The read_neg_14bit() method was reading two raw bytes directly, which differs from the established encoding pattern used elsewhere in the codebase. Fix: - Change to use read_unsigned_vb() to read the value (consistent with other encodings) - Apply sign-extension using the same 14-bit logic as sign_extend_14bit() - Negate the result to implement NEG_14BIT encoding - Matches the pattern used in src/main.rs for H-frame parsing This ensures consistency across the codebase while maintaining the same effective behavior and output (verified via MD5 hash matching master). * fix: correct read_neg_14bit implementation and add comprehensive tests - Fix truncation bug: operate on full u32 from read_unsigned_vb() instead of casting to u16 - Fix sign-extension: use correct 14-bit two's complement with mask 0x3FFF and bit 13 as sign bit - Remove double negation and confusing masking logic - Add sign_extend_14bit() helper function following same pattern as other sign extensions - Add 6 unit tests covering: - Positive values (0, 1, max 8191) - Negative values (-8192 to -1) - Boundary conditions - Value overflow (>14 bits masked correctly) All tests pass, no clippy warnings, release build successful. * fix: add explicit directory whitelisting to .gitignore for src/** paths The pattern 'src/**/*.rs' alone doesn't work because parent directories need explicit whitelisting. Added '!src/**' to properly include all subdirectories under src/, allowing all .rs files to be staged. * refactor: address CodeRabbit nitpick feedback in stream.rs - Simplify read_neg_14bit casting chain (u32 → i32 → u16 becomes u32 → u16) - Rename sign_extend_14bit to sign_extend_14bit_twos_complement to disambiguate from bbl_format.rs::sign_extend_14bit which uses sign-magnitude encoding - Add documentation clarifying the two's complement semantics - Fix comment hex value in test_read_neg_14bit_masks_14_bits (0xFFFFF → 0x1FFFFF)
1 parent 69e741b commit 40f9311

10 files changed

Lines changed: 641 additions & 39 deletions

File tree

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
# Whitelist src directory and all Rust files within it
1717
!src/
18-
!src/*.rs
18+
!src/**
1919
!src/**/*.rs
2020

2121
# Whitelist examples directory

examples/gpx_export.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
//! GPX Export Example
22
//!
33
//! Demonstrates how to export GPS data to GPX format for use with mapping applications.
4-
//! Note: GPS data collection requires the parser to populate gps_coordinates.
5-
//! Currently, the parser module returns empty GPS vectors.
6-
//! Use the CLI for GPS export: `bbl_parser --gps flight.BBL`
4+
//! The parser module now fully supports GPS frame parsing (G-frames and H-frames).
75
86
use bbl_parser::{export_to_gpx, parse_bbl_file, ExportOptions};
97
use std::path::Path;
@@ -54,8 +52,8 @@ fn main() -> anyhow::Result<()> {
5452
println!(" Exported {} GPS coordinates", log.gps_coordinates.len());
5553
} else {
5654
println!("\n⊘ No GPS coordinates available");
57-
println!("Note: GPS data collection in parser module not yet implemented.");
58-
println!("For GPS export, use the CLI: bbl_parser --gps flight.BBL");
55+
println!("Note: This BBL file may not contain GPS data (no G-frames).");
56+
println!("Check that your flight controller has GPS enabled and connected.");
5957
}
6058

6159
Ok(())

examples/test_crate_gps.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
use bbl_parser::{parse_bbl_file, ExportOptions};
2+
use std::path::Path;
3+
4+
fn main() -> anyhow::Result<()> {
5+
let input_file = std::env::args().nth(1).unwrap_or_else(|| {
6+
println!("Usage: test_crate_gps <input.BBL> [output_dir]");
7+
println!("Example: test_crate_gps flight.BBL ./output");
8+
println!("Note: Demonstrates GPS and Event frame parsing via the crate library");
9+
std::process::exit(1);
10+
});
11+
12+
let output_dir = std::env::args().nth(2);
13+
14+
let export_opts = ExportOptions {
15+
csv: false,
16+
gpx: true,
17+
event: true,
18+
output_dir,
19+
force_export: false,
20+
};
21+
22+
println!("Parsing: {}", input_file);
23+
let log = parse_bbl_file(Path::new(&input_file), export_opts, false)?;
24+
25+
println!("\nCrate Library Results:");
26+
println!(" Total frames: {}", log.stats.total_frames);
27+
println!(" I frames: {}", log.stats.i_frames);
28+
println!(" P frames: {}", log.stats.p_frames);
29+
println!(" G frames: {}", log.stats.g_frames);
30+
println!(" H frames: {}", log.stats.h_frames);
31+
println!(" E frames: {}", log.stats.e_frames);
32+
println!(" GPS coordinates collected: {}", log.gps_coordinates.len());
33+
println!(
34+
" GPS home coords collected: {}",
35+
log.home_coordinates.len()
36+
);
37+
println!(" Event frames collected: {}", log.event_frames.len());
38+
39+
Ok(())
40+
}

src/main.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ struct EventFrame {
357357
event_type: u8,
358358
#[allow(dead_code)]
359359
event_data: Vec<u8>,
360-
event_description: String,
360+
event_name: String,
361361
}
362362

363363
#[derive(Debug)]
@@ -2498,7 +2498,7 @@ fn parse_e_frame(stream: &mut bbl_format::BBLDataStream, debug: bool) -> Result<
24982498

24992499
// Read event data - the length depends on the event type
25002500
let mut event_data = Vec::new();
2501-
let event_description = match event_type {
2501+
let event_name = match event_type {
25022502
0 => {
25032503
// FLIGHT_LOG_EVENT_SYNC_BEEP
25042504
"Sync beep".to_string()
@@ -2642,15 +2642,15 @@ fn parse_e_frame(stream: &mut bbl_format::BBLDataStream, debug: bool) -> Result<
26422642
if debug {
26432643
println!(
26442644
"DEBUG: Event - Type: {}, Description: {}",
2645-
event_type, event_description
2645+
event_type, event_name
26462646
);
26472647
}
26482648

26492649
Ok(EventFrame {
26502650
timestamp_us: 0, // Will be set later from context
26512651
event_type,
26522652
event_data,
2653-
event_description,
2653+
event_name,
26542654
})
26552655
}
26562656

@@ -3163,7 +3163,7 @@ fn export_event_file(
31633163
writeln!(
31643164
event_file,
31653165
r#"{{"name":"{}", "time":{}}}"#,
3166-
event.event_description.replace('"', "\\\""),
3166+
event.event_name.replace('"', "\\\""),
31673167
event.timestamp_us
31683168
)?;
31693169
}

src/parser/event.rs

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
//! Event frame parsing helper module
2+
//!
3+
//! Contains functions for parsing E-frames (Event data) from blackbox log data.
4+
//! These helpers are used by both the library parser and CLI binary.
5+
6+
use crate::parser::stream::BBLDataStream;
7+
use crate::types::EventFrame;
8+
use crate::Result;
9+
10+
/// Helper function to parse inflight adjustment events (types 4 and 13)
11+
/// Returns the event description string
12+
fn parse_inflight_adjustment(
13+
stream: &mut BBLDataStream,
14+
event_data: &mut Vec<u8>,
15+
) -> Result<String> {
16+
let adjustment_function = stream.read_byte()?;
17+
event_data.extend_from_slice(&[adjustment_function]);
18+
if adjustment_function > 127 {
19+
let new_value = stream.read_unsigned_vb()? as f32;
20+
Ok(format!(
21+
"Inflight adjustment - Function: {}, New value: {:.3}",
22+
adjustment_function, new_value
23+
))
24+
} else {
25+
let new_value = stream.read_signed_vb()?;
26+
Ok(format!(
27+
"Inflight adjustment - Function: {}, New value: {}",
28+
adjustment_function, new_value
29+
))
30+
}
31+
}
32+
33+
/// Parse E-frame (Event frame) data from the stream
34+
///
35+
/// E-frames contain various event types such as sync beeps, autotune cycles,
36+
/// inflight adjustments, logging resume, disarm, flight mode changes, and log end.
37+
/// Each event type has its own data format that this function decodes.
38+
pub fn parse_e_frame(stream: &mut BBLDataStream, debug: bool) -> Result<EventFrame> {
39+
if debug {
40+
println!("Parsing E frame (Event frame)");
41+
}
42+
43+
// Read event type (1 byte)
44+
let event_type = stream.read_byte()?;
45+
46+
// Read event data - the length depends on the event type
47+
let mut event_data = Vec::new();
48+
let event_name = match event_type {
49+
0 => {
50+
// FLIGHT_LOG_EVENT_SYNC_BEEP
51+
"Sync beep".to_string()
52+
}
53+
1 => {
54+
// FLIGHT_LOG_EVENT_AUTOTUNE_CYCLE_START
55+
"Autotune cycle start".to_string()
56+
}
57+
2 => {
58+
// FLIGHT_LOG_EVENT_AUTOTUNE_CYCLE_RESULT
59+
let axis = stream.read_byte()?;
60+
let p_gain = stream.read_signed_vb()? as f32 / 1000.0;
61+
let i_gain = stream.read_signed_vb()? as f32 / 1000.0;
62+
let d_gain = stream.read_signed_vb()? as f32 / 1000.0;
63+
event_data.extend_from_slice(&[axis]);
64+
format!(
65+
"Autotune cycle result - Axis: {}, P: {:.3}, I: {:.3}, D: {:.3}",
66+
axis, p_gain, i_gain, d_gain
67+
)
68+
}
69+
3 => {
70+
// FLIGHT_LOG_EVENT_AUTOTUNE_TARGETS
71+
let current_angle = stream.read_signed_vb()?;
72+
let target_angle = stream.read_signed_vb()?;
73+
let target_angle_at_peak = stream.read_signed_vb()?;
74+
let first_peak_angle = stream.read_signed_vb()?;
75+
let second_peak_angle = stream.read_signed_vb()?;
76+
format!(
77+
"Autotune targets - Current: {}, Target: {}, Peak target: {}, First peak: {}, Second peak: {}",
78+
current_angle, target_angle, target_angle_at_peak, first_peak_angle, second_peak_angle
79+
)
80+
}
81+
4 => {
82+
// FLIGHT_LOG_EVENT_INFLIGHT_ADJUSTMENT
83+
parse_inflight_adjustment(stream, &mut event_data)?
84+
}
85+
5 => {
86+
// FLIGHT_LOG_EVENT_LOGGING_RESUME
87+
// Note: Event type 14 has identical implementation - both use this newer numbering
88+
let log_iteration = stream.read_unsigned_vb()?;
89+
let current_time = stream.read_unsigned_vb()?;
90+
format!(
91+
"Logging resume - Iteration: {}, Time: {}",
92+
log_iteration, current_time
93+
)
94+
}
95+
6 => {
96+
// FLIGHT_LOG_EVENT_LOG_END (old numbering)
97+
// Read end message bytes
98+
for _ in 0..4 {
99+
if !stream.eof {
100+
event_data.push(stream.read_byte()?);
101+
}
102+
}
103+
"Log end".to_string()
104+
}
105+
10 => {
106+
// FLIGHT_LOG_EVENT_AUTOTUNE_CYCLE_START (UNUSED)
107+
"Autotune cycle start (unused)".to_string()
108+
}
109+
11 => {
110+
// FLIGHT_LOG_EVENT_AUTOTUNE_CYCLE_RESULT (UNUSED)
111+
"Autotune cycle result (unused)".to_string()
112+
}
113+
12 => {
114+
// FLIGHT_LOG_EVENT_AUTOTUNE_TARGETS (UNUSED)
115+
"Autotune targets (unused)".to_string()
116+
}
117+
13 => {
118+
// FLIGHT_LOG_EVENT_INFLIGHT_ADJUSTMENT
119+
parse_inflight_adjustment(stream, &mut event_data)?
120+
}
121+
14 => {
122+
// FLIGHT_LOG_EVENT_LOGGING_RESUME (newer numbering, same as type 5)
123+
// Both event type numbers are used in different firmware versions
124+
let log_iteration = stream.read_unsigned_vb()?;
125+
let current_time = stream.read_unsigned_vb()?;
126+
format!(
127+
"Logging resume - Iteration: {}, Time: {}",
128+
log_iteration, current_time
129+
)
130+
}
131+
15 => {
132+
// FLIGHT_LOG_EVENT_DISARM
133+
"Disarm".to_string()
134+
}
135+
30 => {
136+
// FLIGHT_LOG_EVENT_FLIGHTMODE - flight mode status event
137+
// Read flight mode data
138+
for _ in 0..4 {
139+
if !stream.eof {
140+
event_data.push(stream.read_byte()?);
141+
}
142+
}
143+
"Flight mode change".to_string()
144+
}
145+
255 => {
146+
// FLIGHT_LOG_EVENT_LOG_END
147+
"Log end".to_string()
148+
}
149+
_ => {
150+
// Unknown event type - read a few bytes as data
151+
for _ in 0..8 {
152+
if stream.eof {
153+
break;
154+
}
155+
event_data.push(stream.read_byte()?);
156+
}
157+
format!("Unknown event type: {}", event_type)
158+
}
159+
};
160+
161+
if debug {
162+
println!(
163+
"DEBUG: Event - Type: {}, Description: {}",
164+
event_type, event_name
165+
);
166+
}
167+
168+
Ok(EventFrame {
169+
timestamp_us: 0, // Will be set later from context
170+
event_type,
171+
event_data,
172+
event_name,
173+
})
174+
}

0 commit comments

Comments
 (0)