From a925ddc0d7da183ea61f2c8202142ce2e3739a07 Mon Sep 17 00:00:00 2001 From: Gautham Date: Fri, 24 Jan 2025 14:20:38 -0500 Subject: [PATCH] address review comments --- Cargo.lock | 1 + Cargo.toml | 1 + micro-rdk-nmea-macros/src/attributes.rs | 60 ++++++++--------- micro-rdk-nmea-macros/src/composition.rs | 74 ++++++++++++++++----- micro-rdk-nmea-macros/src/lib.rs | 40 +---------- micro-rdk-nmea-macros/src/utils.rs | 8 +-- micro-rdk-nmea/Cargo.toml | 5 +- micro-rdk-nmea/src/lib.rs | 10 ++- micro-rdk-nmea/src/parse_helpers/errors.rs | 2 + micro-rdk-nmea/src/parse_helpers/parsers.rs | 58 ++++++++-------- 10 files changed, 140 insertions(+), 119 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bc6bcd915..fca0a1baa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2832,6 +2832,7 @@ name = "micro-rdk-nmea" version = "0.4.0" dependencies = [ "base64 0.22.1", + "chrono", "micro-rdk", "micro-rdk-nmea-macros", "thiserror 2.0.4", diff --git a/Cargo.toml b/Cargo.toml index 826bcd483..d337c9475 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -87,6 +87,7 @@ log = "0.4.22" mdns-sd = { version = "0.12", default-features = false, features = ["async"] } micro-rdk = { path = "./micro-rdk", default-features = false, features = [] } micro-rdk-macros = { path = "./micro-rdk-macros" } +micro-rdk-nmea-macros = { path = "./micro-rdk-nmea-macros" } micro-rdk-modular-driver-example = {path = "./examples/modular-drivers" } once_cell = "1.20.2" openssl = { version = "0.10.68" } diff --git a/micro-rdk-nmea-macros/src/attributes.rs b/micro-rdk-nmea-macros/src/attributes.rs index 98bd07e53..666e15395 100644 --- a/micro-rdk-nmea-macros/src/attributes.rs +++ b/micro-rdk-nmea-macros/src/attributes.rs @@ -5,35 +5,35 @@ use syn::{Expr, Field, Ident, Lit, Meta, Type}; use crate::utils::{error_tokens, UnitConversion}; -// `MacroAttributes` represents the important information derived from the macro attributes -// attached to the field of a struct using the PgnMessageDerive macro. -// -// bits -> size in bits of the field in the NMEA message byte slice -// offset -> the amount of bits to skip in the message data before parsing the field -// label -> name of the key for the field when serializing the message to a GenericReadingsResult -// scale_token -> scale factor to be applied to the raw value of the field -// unit -> unit that the field's value should be converted to before serialization -// is_lookup -> whether the field is a lookup field -// length_field -> if this field is a list of fieldsets, this is the field whose value represents -// the expected length of this list -// -// Ex. of usage (each attribute generates an instance of `MacroAttributes`) -// -// #[derive(PgnMessageDerive, Debug)] -// pub struct TemperatureExtendedRange { -// source_id: u8, -// instance: u8, -// #[lookup] -// #[label = "temp_src"] -// source: TemperatureSource, -// #[bits = 24] -// #[scale = 0.001] -// #[unit = "C"] -// temperature: u32, -// #[scale = 0.1] -// #[offset = 16] -// set_temperature: u16, -// } +/// `MacroAttributes` represents the important information derived from the macro attributes +/// attached to the field of a struct using the PgnMessageDerive macro. +/// +/// `bits`: size in bits of the field in the NMEA message byte slice +/// `offset`: the amount of bits to skip in the message data before parsing the field +/// `label`: name of the key for the field when serializing the message to a GenericReadingsResult +/// `scale_token`: scale factor to be applied to the raw value of the field +/// `unit`: unit that the field's value should be converted to before serialization +/// `is_lookup`: whether the field is a lookup field +/// `length_field`: if this field is a list of fieldsets, this is the field whose value represents +/// the expected length of this list +/// +/// Ex. of usage (each attribute generates an instance of `MacroAttributes`) +/// +/// #[derive(PgnMessageDerive, Debug)] +/// pub struct TemperatureExtendedRange { +/// source_id: u8, +/// instance: u8, +/// #[lookup] +/// #[label = "temp_src"] +/// source: TemperatureSource, +/// #[bits = 24] +/// #[scale = 0.001] +/// #[unit = "C"] +/// temperature: u32, +/// #[scale = 0.1] +/// #[offset = 16] +/// set_temperature: u16, +/// } #[derive(Debug)] pub(crate) struct MacroAttributes { @@ -230,7 +230,7 @@ impl MacroAttributes { let unit_token = unit_lit.token(); let unit_str = unit_token.to_string(); UnitConversion::try_from( - unit_str.as_str().trim_matches('"').to_string(), + unit_str.as_str().trim_matches('"'), )? } else { return Err(error_tokens("unit parameter must be str")); diff --git a/micro-rdk-nmea-macros/src/composition.rs b/micro-rdk-nmea-macros/src/composition.rs index acf138b8e..c7cc1e9c6 100644 --- a/micro-rdk-nmea-macros/src/composition.rs +++ b/micro-rdk-nmea-macros/src/composition.rs @@ -1,10 +1,10 @@ use proc_macro::TokenStream; -use proc_macro2::TokenStream as TokenStream2; +use proc_macro2::{Span, TokenStream as TokenStream2}; use quote::{format_ident, quote}; use syn::{DeriveInput, Field, Ident, Type}; use crate::attributes::MacroAttributes; -use crate::utils::{determine_supported_numeric, error_tokens, get_micro_nmea_crate_ident}; +use crate::utils::{error_tokens, get_micro_nmea_crate_ident, is_supported_integer_type}; /// Represents a subset of auto-generated code statements for the implementation of a particular /// NMEA message. Each field in a message struct contributes its own set of statements to the macro @@ -45,9 +45,7 @@ impl PgnComposition { statements.attribute_getters.push(quote! { pub fn #name(&self) -> #num_ty { self.#name } }); - statements - .struct_initialization - .push(quote! { source_id: source_id.unwrap(), }); + statements.struct_initialization.push(quote! { source_id, }); return Ok(statements); } @@ -59,7 +57,7 @@ impl PgnComposition { .push(quote! { let _ = cursor.read(#offset)?; }); } - let new_statements = if determine_supported_numeric(&field.ty) { + let new_statements = if is_supported_integer_type(&field.ty) { handle_number_field(name, field, ¯o_attrs)? } else if macro_attrs.is_lookup { handle_lookup_field(name, &field.ty, ¯o_attrs)? @@ -81,6 +79,38 @@ impl PgnComposition { } } + pub(crate) fn from_input(input: &DeriveInput) -> Result { + let src_fields = if let syn::Data::Struct(syn::DataStruct { ref fields, .. }) = input.data { + fields + } else { + return Err( + syn::Error::new(Span::call_site(), "PgnMessageDerive expected struct") + .to_compile_error() + .into(), + ); + }; + + let named_fields = if let syn::Fields::Named(f) = src_fields { + &f.named + } else { + return Err(crate::utils::error_tokens( + "PgnMessageDerive expected struct with named fields", + )); + }; + let mut statements = Self::new(); + for field in named_fields.iter() { + match PgnComposition::from_field(field) { + Ok(new_statements) => { + statements.merge(new_statements); + } + Err(err) => { + return Err(err); + } + }; + } + Ok(statements) + } + pub(crate) fn into_token_stream(self, input: &DeriveInput) -> TokenStream2 { let name = &input.ident; let parsing_logic = self.parsing_logic; @@ -93,7 +123,7 @@ impl PgnComposition { let mrdk_crate = crate::utils::get_micro_rdk_crate_ident(); quote! { impl #impl_generics #name #src_generics #src_where_clause { - pub fn from_bytes(data: Vec, source_id: Option) -> Result { + pub fn from_bytes(data: Vec, source_id: u8) -> Result { use #crate_ident::parse_helpers::parsers::{DataCursor, FieldReader}; let mut cursor = DataCursor::new(data); #(#parsing_logic)* @@ -159,15 +189,27 @@ fn handle_number_field( } } }; - scaling_logic = quote! { - #max_token - let result = match result { - x if x == max => { return Err(#error_ident::FieldNotPresent(#name_as_string_ident.to_string())); }, - x if x == (max - 1) => { return Err(#error_ident::FieldError(#name_as_string_ident.to_string())); }, - x => { - (x as f64) * #scale_token - } - }; + scaling_logic = match bits_size { + x if x > 4 => quote! { + #max_token + let result = match result { + x if x == max => { return Err(#error_ident::FieldNotPresent(#name_as_string_ident.to_string())); }, + x => { + (x as f64) * #scale_token + } + }; + }, + x if x >= 4 => quote! { + #max_token + let result = match result { + x if x == max => { return Err(#error_ident::FieldNotPresent(#name_as_string_ident.to_string())); }, + x if x == (max - 1) => { return Err(#error_ident::FieldError(#name_as_string_ident.to_string())); }, + x => { + (x as f64) * #scale_token + } + }; + }, + _ => quote! {}, }; return_type = quote! {f64}; } diff --git a/micro-rdk-nmea-macros/src/lib.rs b/micro-rdk-nmea-macros/src/lib.rs index 229c264d0..3d2eda15b 100644 --- a/micro-rdk-nmea-macros/src/lib.rs +++ b/micro-rdk-nmea-macros/src/lib.rs @@ -4,42 +4,6 @@ pub(crate) mod utils; use crate::composition::PgnComposition; use proc_macro::TokenStream; -use proc_macro2::Span; -use syn::DeriveInput; - -fn get_statements(input: &DeriveInput) -> Result { - let src_fields = if let syn::Data::Struct(syn::DataStruct { ref fields, .. }) = input.data { - fields - } else { - return Err( - syn::Error::new(Span::call_site(), "PgnMessageDerive expected struct") - .to_compile_error() - .into(), - ); - }; - - let named_fields = if let syn::Fields::Named(f) = src_fields { - &f.named - } else { - return Err(crate::utils::error_tokens( - "PgnMessageDerive expected struct with named fields", - )); - }; - - let mut statements = PgnComposition::new(); - for field in named_fields.iter() { - match PgnComposition::from_field(field) { - Ok(new_statements) => { - statements.merge(new_statements); - } - Err(err) => { - return Err(err); - } - }; - } - - Ok(statements) -} /// PgnMessageDerive is a macro that implements parsing logic for a struct in the form of a method /// `from_bytes(data: Vec, source_id: u8) -> Result`, attribute accessors, @@ -53,8 +17,8 @@ fn get_statements(input: &DeriveInput) -> Result { pub fn pgn_message_derive(item: TokenStream) -> TokenStream { let input = syn::parse_macro_input!(item as syn::DeriveInput); - match get_statements(&input) { - Ok(gen) => gen.into_token_stream(&input).into(), + match PgnComposition::from_input(&input) { + Ok(statements) => statements.into_token_stream(&input).into(), Err(tokens) => tokens, } } diff --git a/micro-rdk-nmea-macros/src/utils.rs b/micro-rdk-nmea-macros/src/utils.rs index bc2361a9a..804355403 100644 --- a/micro-rdk-nmea-macros/src/utils.rs +++ b/micro-rdk-nmea-macros/src/utils.rs @@ -17,10 +17,10 @@ pub(crate) enum UnitConversion { RadPerSecToDegPerSec, } -impl TryFrom for UnitConversion { +impl TryFrom<&str> for UnitConversion { type Error = TokenStream; - fn try_from(value: String) -> Result { - match value.as_str() { + fn try_from(value: &str) -> Result { + match value { "Ah" => Ok(Self::CoulombToAmpereHour), "bar" => Ok(Self::PascalToBar), "C" => Ok(Self::KelvinToCelsius), @@ -80,7 +80,7 @@ pub(crate) fn get_proto_import_prefix() -> TokenStream2 { quote! {#crate_ident::google::protobuf} } -pub(crate) fn determine_supported_numeric(field_type: &Type) -> bool { +pub(crate) fn is_supported_integer_type(field_type: &Type) -> bool { match field_type { Type::Path(type_path) => { type_path.path.is_ident("u32") diff --git a/micro-rdk-nmea/Cargo.toml b/micro-rdk-nmea/Cargo.toml index 29c9a1821..8bcea5bb4 100644 --- a/micro-rdk-nmea/Cargo.toml +++ b/micro-rdk-nmea/Cargo.toml @@ -12,6 +12,7 @@ rust-version.workspace = true [dependencies] base64 = { workspace = true } -micro-rdk = { path = "../micro-rdk", features = ["native"] } -micro-rdk-nmea-macros = { path = "../micro-rdk-nmea-macros" } +chrono = { workspace = true } +micro-rdk = { workspace = true, features = ["native"] } +micro-rdk-nmea-macros = { workspace = true } thiserror = { workspace = true } diff --git a/micro-rdk-nmea/src/lib.rs b/micro-rdk-nmea/src/lib.rs index 3e159d60d..e70951f91 100644 --- a/micro-rdk-nmea/src/lib.rs +++ b/micro-rdk-nmea/src/lib.rs @@ -10,13 +10,17 @@ mod tests { parse_helpers::{enums::TemperatureSource, errors::NumberFieldError}, }; + // The strings in the below test represent base64-encoded data examples taken from raw results + // posted by an active CAN sensor. The first 32 bytes involve a header that is represented in serialized + // form by parse_helpers::parsers::NmeaMessageMetadata. + #[test] fn water_depth_parse() { let water_depth_str = "C/UBAHg+gD/TL/RmAAAAAFZODAAAAAAACAD/ABMAAwD/1AAAAAAA/w=="; let mut data = Vec::::new(); let res = general_purpose::STANDARD.decode_vec(water_depth_str, &mut data); assert!(res.is_ok()); - let message = WaterDepth::from_bytes(data[33..].to_vec(), Some(13)); + let message = WaterDepth::from_bytes(data[33..].to_vec(), 13); assert!(message.is_ok()); let message = message.unwrap(); assert_eq!(message.source_id(), 13); @@ -38,7 +42,7 @@ mod tests { let mut data = Vec::::new(); let res = general_purpose::STANDARD.decode_vec(water_depth_str, &mut data); assert!(res.is_ok()); - let message = WaterDepth::from_bytes(data[33..].to_vec(), Some(13)); + let message = WaterDepth::from_bytes(data[33..].to_vec(), 13); assert!(message.is_ok()); let message = message.unwrap(); assert_eq!(message.source_id(), 13); @@ -61,7 +65,7 @@ mod tests { let res = general_purpose::STANDARD.decode_vec(temp_str, &mut data); assert!(res.is_ok()); - let message = TemperatureExtendedRange::from_bytes(data[33..].to_vec(), Some(23)); + let message = TemperatureExtendedRange::from_bytes(data[33..].to_vec(), 23); assert!(message.is_ok()); let message = message.unwrap(); assert_eq!(message.source_id(), 23); diff --git a/micro-rdk-nmea/src/parse_helpers/errors.rs b/micro-rdk-nmea/src/parse_helpers/errors.rs index 44b8393a9..488d6a503 100644 --- a/micro-rdk-nmea/src/parse_helpers/errors.rs +++ b/micro-rdk-nmea/src/parse_helpers/errors.rs @@ -18,6 +18,8 @@ pub enum NmeaParseError { TryFromSliceError(#[from] std::array::TryFromSliceError), #[error("not enough data to parse next field")] NotEnoughData, + #[error("could not parse timestamp")] + MalformedTimestamp, #[error("found unsupported PGN {0}")] UnsupportedPgn(u32), } diff --git a/micro-rdk-nmea/src/parse_helpers/parsers.rs b/micro-rdk-nmea/src/parse_helpers/parsers.rs index 60591ee27..4e1ce2f63 100644 --- a/micro-rdk-nmea/src/parse_helpers/parsers.rs +++ b/micro-rdk-nmea/src/parse_helpers/parsers.rs @@ -1,6 +1,7 @@ -use std::{array::TryFromSliceError, marker::PhantomData}; +use std::marker::PhantomData; -use micro_rdk::{common::sensor::GenericReadingsResult, google::protobuf::Timestamp}; +use chrono::{DateTime, Utc}; +use micro_rdk::common::sensor::GenericReadingsResult; use super::{ enums::NmeaEnumeratedField, @@ -215,35 +216,16 @@ where // The first 32 bytes of the unparsed message stores metadata appearing in the header // of an NMEA message (the library assumes that a previous process has correctly serialized -// this data from the CAN frame). For an explanation of the fields in this header, see -// the section labeled "ISO-11783 and NMEA2000 header" at https://canboat.github.io/canboat/canboat.html +// this data from the CAN frame). +#[derive(Debug, Clone)] pub struct NmeaMessageMetadata { - timestamp: Timestamp, + timestamp: DateTime, priority: u16, src: u16, dst: u16, } impl NmeaMessageMetadata { - pub fn from_bytes(data: [u8; 32]) -> Result { - let seconds = u64::from_le_bytes(data[8..16].try_into()?) as i64; - let millis = u64::from_le_bytes(data[16..24].try_into()?); - let timestamp = Timestamp { - seconds, - nanos: (millis * 1000) as i32, - }; - - let dst = u16::from_le_bytes(data[26..28].try_into()?); - let src = u16::from_le_bytes(data[28..30].try_into()?); - let priority = u16::from_le_bytes(data[30..32].try_into()?); - Ok(Self { - timestamp, - priority, - src, - dst, - }) - } - pub fn src(&self) -> u16 { self.src } @@ -256,11 +238,34 @@ impl NmeaMessageMetadata { self.priority } - pub fn timestamp(&self) -> Timestamp { + pub fn timestamp(&self) -> DateTime { self.timestamp.clone() } } +impl TryFrom> for NmeaMessageMetadata { + type Error = NmeaParseError; + fn try_from(value: Vec) -> Result { + if value.len() < 32 { + return Err(NmeaParseError::NotEnoughData); + } + let seconds = u64::from_le_bytes(value[8..16].try_into()?) as i64; + let millis = u64::from_le_bytes(value[16..24].try_into()?); + let timestamp = DateTime::from_timestamp(seconds, (millis * 1000) as u32) + .ok_or(NmeaParseError::MalformedTimestamp)?; + + let dst = u16::from_le_bytes(value[26..28].try_into()?); + let src = u16::from_le_bytes(value[28..30].try_into()?); + let priority = u16::from_le_bytes(value[30..32].try_into()?); + Ok(Self { + timestamp, + priority, + src, + dst, + }) + } +} + #[cfg(test)] mod tests { use base64::{engine::general_purpose, Engine}; @@ -280,7 +285,8 @@ mod tests { let res = general_purpose::STANDARD.decode_vec(data_str, &mut data); assert!(res.is_ok()); - let metadata = NmeaMessageMetadata::from_bytes(data[0..32].try_into().unwrap()); + let _ = data.split_off(32); + let metadata = NmeaMessageMetadata::try_from(data); assert!(metadata.is_ok()); let metadata = metadata.unwrap();