Skip to content

Commit dc53366

Browse files
committed
cleanup
1 parent 58e4bfa commit dc53366

File tree

3 files changed

+95
-46
lines changed

3 files changed

+95
-46
lines changed

crates/utils/re_mcap/src/layers/ros2_reflection.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,6 @@ impl Ros2ReflectionMessageParser {
4646
fn new(num_rows: usize, message_schema: MessageSchema) -> Self {
4747
let mut fields = Vec::new();
4848

49-
println!("{message_schema:#?}");
50-
5149
// Build Arrow builders for each field in the message, preserving order
5250
for field in &message_schema.spec.fields {
5351
let name = field.name.clone();
@@ -339,11 +337,6 @@ fn datatype_from_type(ty: &Type, dependencies: &[MessageSpec]) -> DataType {
339337
},
340338
Type::String(_) => DataType::Utf8,
341339
Type::Complex(complex_type) => {
342-
println!(
343-
"Resolving complex type: {:?}, resolved to: {:?}",
344-
complex_type,
345-
resolve_complex_type(complex_type, dependencies)
346-
);
347340
if let Some(spec) = resolve_complex_type(complex_type, dependencies) {
348341
let fields = spec
349342
.fields

crates/utils/re_mcap/src/parsers/ros2msg/idl/deserializer.rs

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use anyhow::{Context, bail};
1+
use anyhow::{Context as _, bail};
22
use cdr_encoding::CdrDeserializer;
33
use serde::de::{self, DeserializeSeed, Visitor};
44
use std::collections::BTreeMap;
@@ -12,7 +12,7 @@ use crate::parsers::{
1212
pub fn decode_bytes(top: &MessageSchema, buf: &[u8]) -> anyhow::Result<Value> {
1313
// 4-byte encapsulation header
1414
if buf.len() < 4 {
15-
return bail!("short encapsulation");
15+
bail!("short encapsulation");
1616
}
1717

1818
let representation_identifier = dds::RepresentationIdentifier::from_bytes([buf[0], buf[1]])
@@ -265,7 +265,6 @@ impl<'de, R: TypeResolver> DeserializeSeed<'de> for SchemaSeed<'_, R> {
265265
}
266266
},
267267
Type::Complex(complex_ty) => {
268-
println!("Resolving complex type: {complex_ty:?}");
269268
let msg = self.r.resolve(complex_ty).ok_or_else(|| {
270269
de::Error::custom(format!("unknown ComplexType: {complex_ty:?}"))
271270
})?;
@@ -340,18 +339,35 @@ impl<'de, R: TypeResolver> DeserializeSeed<'de> for SeqSeed<'_, R> {
340339
fn expecting(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
341340
write!(f, "cdr seq/array")
342341
}
342+
343343
fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
344344
where
345345
A: serde::de::SeqAccess<'de>,
346346
{
347-
let mut out = self.fixed_len.map(Vec::with_capacity).unwrap_or_default();
348-
while let Some(v) = seq.next_element_seed(SchemaSeed {
349-
ty: self.elem,
350-
r: self.type_resolver,
351-
})? {
352-
out.push(v);
347+
let cap = self.fixed_len.or_else(|| seq.size_hint()).unwrap_or(0);
348+
let mut out = Vec::with_capacity(cap);
349+
350+
if let Some(n) = self.fixed_len.or_else(|| seq.size_hint()) {
351+
for _ in 0..n {
352+
let v = seq
353+
.next_element_seed(SchemaSeed {
354+
ty: self.elem,
355+
r: self.type_resolver,
356+
})?
357+
.ok_or_else(|| serde::de::Error::custom("short sequence"))?;
358+
out.push(v);
359+
}
360+
Ok(out)
361+
} else {
362+
// Fallback for truly unbounded streams
363+
while let Some(v) = seq.next_element_seed(SchemaSeed {
364+
ty: self.elem,
365+
r: self.type_resolver,
366+
})? {
367+
out.push(v);
368+
}
369+
Ok(out)
353370
}
354-
Ok(out)
355371
}
356372
}
357373
match self.fixed_len {

crates/utils/re_mcap/src/parsers/ros2msg/idl/mod.rs

Lines changed: 69 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,23 @@ fn parse_schema_name(line: &str) -> Option<&str> {
6565
line.trim().strip_prefix("MSG: ").map(str::trim)
6666
}
6767

68-
/// A message field.
68+
/// A message field definition.
69+
/// Includes type, name, and optional default value.
70+
///
71+
/// Examples:
72+
/// ```text
73+
/// // Simple int32 field with no default value
74+
/// int32 field_name
75+
///
76+
/// // Bounded string with max length 10, default value "default"
77+
/// string<=10 name "default"
78+
///
79+
/// // Array of 3 float64s with default value [0.0, 0.0, 0.0]
80+
/// float64[3] position [0.0, 0.0, 0.0]
81+
///
82+
/// // Unbounded array of complex types
83+
/// pkg/Type[] items
84+
/// ```
6985
#[derive(Debug, Clone, PartialEq)]
7086
pub struct Field {
7187
pub ty: Type,
@@ -179,17 +195,30 @@ impl Type {
179195
let len_str = &s["string<=".len()..s.len() - 1];
180196
let len = len_str
181197
.parse::<usize>()
182-
.context("failed to parse bounded string length")?;
198+
.with_context(|| "failed to parse bounded string length")?;
183199
Ok(Self::String(Some(len)))
184200
} else {
185201
bail!("invalid string type specifier: `{s}`");
186202
}
187203
}
188204
}
189205

206+
/// A complex (non-primitive) type, possibly qualified with a package path.
207+
///
208+
/// Examples:
209+
/// ```text
210+
/// // Absolute type with package
211+
/// pkg/Type
212+
///
213+
/// // Relative type without package
214+
/// Type
215+
/// ```
190216
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
191217
pub enum ComplexType {
218+
/// An absolute type with package, e.g. `pkg/Type`
192219
Absolute { package: String, name: String },
220+
221+
/// A relative type without package, e.g. `Type`
193222
Relative { name: String },
194223
}
195224

@@ -211,12 +240,14 @@ impl ComplexType {
211240
"invalid complex type specifier: `{s}`, expected `some_package/SomeMessage` or `SomeMessage` format"
212241
);
213242
}
243+
214244
Ok(Self::Relative { name: s.to_owned() })
215245
}
216246
}
217247
}
218248

219-
#[derive(Debug, Clone, PartialEq)]
249+
/// Size specifier for array types.
250+
#[derive(Debug, Clone, PartialEq, Eq)]
220251
pub enum ArraySize {
221252
Fixed(usize),
222253
Bounded(usize),
@@ -227,7 +258,8 @@ impl ArraySize {
227258
fn parse(array_part: &str) -> Result<Self> {
228259
let array_part = array_part
229260
.strip_suffix(']')
230-
.context("missing closing ']' in array type")?;
261+
.with_context(|| "Missing closing ']' in array type")?;
262+
231263
let array_size = if array_part.is_empty() {
232264
Self::Unbounded
233265
} else if let Ok(size) = array_part.parse::<usize>() {
@@ -236,7 +268,7 @@ impl ArraySize {
236268
let size_str = &array_part[1..array_part.len() - 1];
237269
let size = size_str
238270
.parse::<usize>()
239-
.context("failed to parse bounded array size")?;
271+
.with_context(|| "Failed to parse bounded array size")?;
240272
Self::Bounded(size)
241273
} else {
242274
bail!("invalid array size specifier: `{array_part}`");
@@ -245,6 +277,8 @@ impl ArraySize {
245277
}
246278
}
247279

280+
/// A literal value, used for default values and constant definitions.
281+
/// Can be a primitive, string, or array of literals.
248282
#[derive(Debug, Clone, PartialEq)]
249283
pub enum Literal {
250284
Bool(bool),
@@ -272,25 +306,21 @@ impl Literal {
272306
| PrimitiveType::Int8
273307
| PrimitiveType::Int16
274308
| PrimitiveType::Int32
275-
| PrimitiveType::Int64 => {
276-
let v = s
277-
.parse::<i64>()
278-
.context("failed to parse integer literal")?;
279-
Ok(Self::Int(v))
280-
}
309+
| PrimitiveType::Int64 => s
310+
.parse::<i64>()
311+
.map(Self::Int)
312+
.with_context(|| "failed to parse integer literal"),
281313
PrimitiveType::UInt8
282314
| PrimitiveType::UInt16
283315
| PrimitiveType::UInt32
284-
| PrimitiveType::UInt64 => {
285-
let v = s
286-
.parse::<u64>()
287-
.context("failed to parse unsigned integer literal")?;
288-
Ok(Self::UInt(v))
289-
}
290-
PrimitiveType::Float32 | PrimitiveType::Float64 => {
291-
let v = s.parse::<f64>().context("failed to parse float literal")?;
292-
Ok(Self::Float(v))
293-
}
316+
| PrimitiveType::UInt64 => s
317+
.parse::<u64>()
318+
.map(Self::UInt)
319+
.with_context(|| "failed to parse unsigned integer literal"),
320+
PrimitiveType::Float32 | PrimitiveType::Float64 => s
321+
.parse::<f64>()
322+
.map(Self::Float)
323+
.with_context(|| "failed to parse float literal"),
294324
},
295325
Type::String(_) => {
296326
let s = s.trim_matches('"');
@@ -301,6 +331,7 @@ impl Literal {
301331
size: _,
302332
} => {
303333
let s = s.trim();
334+
304335
if !s.starts_with('[') || !s.ends_with(']') {
305336
bail!("array literal must start with '[' and end with ']': `{s}`");
306337
}
@@ -311,6 +342,7 @@ impl Literal {
311342
let elem = Self::parse(elem_str, elem_ty)?;
312343
elems.push(elem);
313344
}
345+
314346
Ok(Self::Array(elems))
315347
}
316348
Type::Complex(_) => bail!("cannot parse literal for named type"),
@@ -319,6 +351,19 @@ impl Literal {
319351
}
320352

321353
/// A compile-time constant defined alongside fields.
354+
/// Includes type, name, and value.
355+
///
356+
/// Examples:
357+
/// ```text
358+
/// // Integer constant
359+
/// int32 CONST_NAME=42
360+
///
361+
/// // String constant
362+
/// string CONST_STR="hello"
363+
///
364+
/// // Float constant
365+
/// float64 CONST_FLOAT=3.14
366+
/// ```
322367
#[derive(Debug, Clone, PartialEq)]
323368
pub struct Constant {
324369
pub ty: Type,
@@ -354,11 +399,11 @@ impl Constant {
354399
fn parse(line: &str) -> anyhow::Result<Self> {
355400
let (type_and_name, value_str) = line
356401
.split_once('=')
357-
.context("constant definition missing '='")?;
402+
.with_context(|| "constant definition missing '='")?;
358403
let (type_str, name) = type_and_name
359404
.trim()
360405
.rsplit_once(' ')
361-
.context("constant definition missing space between type and name")?;
406+
.with_context(|| "constant definition missing space between type and name")?;
362407

363408
let ty = Type::parse(type_str)?;
364409
let value = Literal::parse(value_str.trim(), &ty)?;
@@ -471,13 +516,8 @@ fn parse_section(lines: &[&str]) -> Option<(String, String)> {
471516

472517
#[cfg(test)]
473518
mod tests {
519+
use crate::parsers::dds;
474520
use cdr_encoding::CdrDeserializer;
475-
use serde::Deserializer;
476-
477-
use crate::{
478-
cdr,
479-
parsers::{dds, ros2msg::definitions::sensor_msgs},
480-
};
481521

482522
use super::*;
483523

0 commit comments

Comments
 (0)