auxoncorp/rust-lcm-codec

Rust support for reading and writing the LCM data format.

rust-lcm-codec's Issues

`read_into` : Richer return types from item-reading functions

Right now when you want to read content from a single member of a struct in an array, you provide a function that works with the starting "ready to read" state for that struct and return a Result containing the "done reading" state for that struct.

Concretely, the signature for the primary read method of an "item in an array" state-struct is:

        pub fn read<F>(self, f: F) -> Result<(), rust_lcm_codec::DecodeValueError<R::Error>>
        where
            F: FnOnce(
                member_read_ready<'a, R>,
            ) -> Result<member_read_done<'a, R>, rust_lcm_codec::DecodeValueError<R::Error>>;

In practice, this means that the outer read function doesn't return anything useful to the end-user, so any state mutation must by be reference to something captured by closure (e.g. writing to a mutable field on the stack). This makes the side effects of reading less visible. Furthermore , the only errors that can be piped are LCM-specific errors.

I am currently proposing a slightly more expanded option where the handler function can actually return some content and error-types of interest to the end user.

pub fn read_into<F, T, CustomDecodeError>(self, f: F) -> Result<T, rust_lcm_codec::DecodeValueError<R::Error, CustomDecodeError>>
        where
            F: FnOnce(
                member_read_ready<'a, R>,
            ) -> Result<(member_read_done<'a, R>, T), rust_lcm_codec::DecodeValueError<R::Error, CustomDecodeError>>;

In the end, I would rather have:

let outcome = item_reader.read_into(|ready| {
   // do stuff with `ready`, including some possibly-fallible operations
  let (v, r) = ready.read_thing()?;
  let outcome = possibly_fallible_interpretation(v).map_err(|e| DecodeValueError::Custom(e))?;
  let (_, done) = r.read_some_other_thing()?;
  Ok((done, outcome))
})?;

than what I have to do now,

let mut maybe_outcome_result = None;
item_reader.read(|ready| {
   // do stuff with `ready`, including some possibly-fallible operations
  let (v, r) = ready.read_thing()?;
  maybe_outcome_result = Some(possibly_fallible_interpretation(v));
  let (_, done) = r.read_some_other_thing()?;
  Ok(done)
})?;
if let Some(result) = maybe_outcome_result {
    let outcome = result?;
}

This will require modifying DecodeValueError to have an addition Custom variant accepting a user-selected type.

Code generation should detect duplicate field names.

Currently code generation will successfully produce Rust code for LCM definitions with duplicate field names, which will fail at Rust compile time. It would be better to report the error earlier, during attempted LCM definition parsing and code generation.

Fields should be able to have a primitive name as part of their prefix.

Currently, any LCM field with one of the primitive type names in the first half of its content is not parsed correctly.

I would expect the following to be successful using the field_type parser function in rust-lcm-codgen/src/parser.rs , but it is not.

assert_eq!(field_type("boolean_with_suffix"),
            Ok(("", Type::Struct(StructType { namespace: None, name: "boolean_with_suffix".to_string() }))));

Instead, we get:

Expected :Ok(("", Struct(StructType { namespace: None, name: "boolean_with_suffix" })))
Actual   :Ok(("_with_suffix", Primitive(Boolean)))