address review comments

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" }

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"));

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, &macro_attrs)?
             } else if macro_attrs.is_lookup {
                 handle_lookup_field(name, &field.ty, &macro_attrs)?
@@ -81,6 +79,38 @@ impl PgnComposition {
         }
     }
 
+    pub(crate) fn from_input(input: &DeriveInput) -> Result<Self, TokenStream> {
+        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<u8>, source_id: Option<u8>) -> Result<Self, #error_ident> {
+                pub fn from_bytes(data: Vec<u8>, source_id: u8) -> Result<Self, #error_ident> {
                     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};
     }

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<PgnComposition, TokenStream> {
-    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<u8>, source_id: u8) -> Result<Self, NmeaParseError>`, attribute accessors,
@@ -53,8 +17,8 @@ fn get_statements(input: &DeriveInput) -> Result<PgnComposition, TokenStream> {
 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,
     }
 }

micro-rdk-nmea-macros/src/utils.rs

@@ -17,10 +17,10 @@ pub(crate) enum UnitConversion {
     RadPerSecToDegPerSec,
 }
 
-impl TryFrom<String> for UnitConversion {
+impl TryFrom<&str> for UnitConversion {
     type Error = TokenStream;
-    fn try_from(value: String) -> Result<Self, Self::Error> {
-        match value.as_str() {
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
+        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")

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 }

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::<u8>::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::<u8>::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);

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),
 }