From 2d068b2ae4450b6add39f38e9391c870820b4d94 Mon Sep 17 00:00:00 2001 From: Lexi Robinson Date: Sun, 7 Apr 2024 20:04:19 +0100 Subject: [PATCH] Use a custom error type I've got a PR open for Winnow (winnow-rs/winnow#500) that should hopefully render this unnecessary once it's merged & released, but until then this lets me use context with bit parsers (and simplifies invocation for `bits::bits`!) --- src/bin/test_parse.rs | 10 +-- src/parse/application_layer/dib.rs | 9 +-- src/parse/application_layer/vib.rs | 18 +++--- src/parse/error.rs | 74 +++++++++++++++++++++++ src/parse/link_layer.rs | 10 +-- src/parse/transport_layer/control_info.rs | 4 +- src/parse/transport_layer/header.rs | 16 +++-- src/parse/types.rs | 5 +- src/parse/types/number.rs | 20 +++--- src/parse/types/string.rs | 14 ++--- 10 files changed, 119 insertions(+), 61 deletions(-) diff --git a/src/bin/test_parse.rs b/src/bin/test_parse.rs index d1aee12..b3d65be 100644 --- a/src/bin/test_parse.rs +++ b/src/bin/test_parse.rs @@ -7,7 +7,6 @@ use libmbus::parse::link_layer::Packet; use libmbus::parse::transport_layer::CICode; use std::error; use winnow::binary::bits; -use winnow::error::InputError; use winnow::{Bytes, Parser}; fn do_file(fname: &str) -> Result<(), Box> { @@ -25,12 +24,9 @@ fn do_file(fname: &str) -> Result<(), Box> { println!("{ci:#?}"); // Parse the first DIB & VIB - let (dib, vib) = bits::bits::<_, _, _, InputError<_>, _>(( - DataInfoBlock::parse, - ValueInfoBlock::parse, - )) - .parse_next(&mut data) - .map_err(|e| e.to_string())?; + let (dib, vib) = bits::bits((DataInfoBlock::parse, ValueInfoBlock::parse)) + .parse_next(&mut data) + .map_err(|e| e.to_string())?; println!("{dib:?}\n{vib:?}"); } _ => todo!(), diff --git a/src/parse/application_layer/dib.rs b/src/parse/application_layer/dib.rs index b4dce76..675f973 100644 --- a/src/parse/application_layer/dib.rs +++ b/src/parse/application_layer/dib.rs @@ -2,7 +2,8 @@ // Licensed under the EUPL-1.2 #![allow(dead_code)] -use crate::parse::types::{BResult, BitsInput}; +use crate::parse::error::MBResult; +use crate::parse::types::BitsInput; use winnow::binary::bits; use winnow::error::{ErrMode, ParserError}; use winnow::Parser; @@ -17,7 +18,7 @@ pub enum RawDataType { } impl RawDataType { - fn parse<'a>(input: &mut BitsInput<'a>) -> BResult<'a, Self> { + fn parse(input: &mut BitsInput<'_>) -> MBResult { bits::take(4_usize) .verify_map(|value: u8| match value { 0b0000 => Some(Self::None), @@ -44,7 +45,7 @@ pub enum DataFunction { } impl DataFunction { - fn parse<'a>(input: &mut BitsInput<'a>) -> BResult<'a, Self> { + fn parse(input: &mut BitsInput<'_>) -> MBResult { bits::take(2_usize) .map(|value: u8| match value { 0b00 => Self::InstantaneousValue, @@ -67,7 +68,7 @@ pub struct DataInfoBlock { } impl DataInfoBlock { - pub fn parse<'a>(input: &mut BitsInput<'a>) -> BResult<'a, Self> { + pub fn parse(input: &mut BitsInput<'_>) -> MBResult { let (mut extension, mut storage, function, raw_type): (bool, u64, _, _) = ( bits::bool, bits::take(1_usize), diff --git a/src/parse/application_layer/vib.rs b/src/parse/application_layer/vib.rs index a7775fe..5da64ec 100644 --- a/src/parse/application_layer/vib.rs +++ b/src/parse/application_layer/vib.rs @@ -2,10 +2,11 @@ // Licensed under the EUPL-1.2 #![allow(dead_code)] +use crate::parse::error::MBResult; use crate::parse::types::string::parse_length_prefix_ascii; -use crate::parse::types::{BResult, BitsInput}; +use crate::parse::types::BitsInput; use winnow::binary::bits; -use winnow::error::{ErrMode, ErrorKind, InputError, ParserError}; +use winnow::error::{ErrMode, ErrorKind, ParserError}; use winnow::prelude::*; const VIF_EXTENSION_1: u8 = 0b0111_1011; @@ -25,11 +26,11 @@ pub struct ValueInfoBlock { extra_vifes: Option>, } -pub fn parse_vif_byte<'a>(input: &mut BitsInput<'a>) -> BResult<'a, (bool, u8)> { +pub fn parse_vif_byte(input: &mut BitsInput<'_>) -> MBResult<(bool, u8)> { (bits::bool, bits::take(7_usize)).parse_next(input) } -pub fn dump_remaining_vifes<'a>(input: &mut BitsInput<'a>) -> BResult<'a, Vec> { +pub fn dump_remaining_vifes(input: &mut BitsInput<'_>) -> MBResult> { let mut ret = Vec::new(); loop { let (extension, value) = parse_vif_byte.parse_next(input)?; @@ -42,7 +43,7 @@ pub fn dump_remaining_vifes<'a>(input: &mut BitsInput<'a>) -> BResult<'a, Vec(input: &mut BitsInput<'a>) -> BResult<'a, Self> { + pub fn parse(input: &mut BitsInput<'_>) -> MBResult { let (mut extension, raw_value) = parse_vif_byte.parse_next(input)?; let value_type = match raw_value { @@ -85,10 +86,9 @@ impl ValueInfoBlock { // Now we've parsed all the VIFEs we can get the ascii VIF if necessary let value_type = match value_type { - ValueType::PlainText(_) => ValueType::PlainText( - bits::bytes::<_, _, InputError<_>, _, _>(parse_length_prefix_ascii) - .parse_next(input)?, - ), + ValueType::PlainText(_) => { + ValueType::PlainText(bits::bytes(parse_length_prefix_ascii).parse_next(input)?) + } value_type => value_type, }; diff --git a/src/parse/error.rs b/src/parse/error.rs index a189a20..6de046f 100644 --- a/src/parse/error.rs +++ b/src/parse/error.rs @@ -2,6 +2,12 @@ // Licensed under the EUPL-1.2 use std::{error, fmt}; +use winnow::error::{ + AddContext, ContextError, ErrorConvert, ErrorKind, FromExternalError, InputError, ParserError, + StrContext, +}; +use winnow::stream::Stream; +use winnow::PResult; #[derive(Debug, PartialEq, Eq)] pub enum ParseError { @@ -39,3 +45,71 @@ impl fmt::Display for ParseError { impl error::Error for ParseError {} pub type Result = std::result::Result; + +/// Because the version of Winnow we're using doesn't let you use `ContextError` +/// with the bit-level parsers I've had to wrap it in a struct I control so I +/// can implement `ErrorConvert` and get it working again +#[derive(Debug, Clone, PartialEq, Default)] +pub struct MBusError(ContextError); + +pub type MBResult = PResult; + +impl MBusError { + pub fn new() -> Self { + Self(ContextError::new()) + } + + pub fn context(&self) -> impl Iterator { + self.0.context() + } + + pub fn cause(&self) -> Option<&(dyn std::error::Error + Send + Sync + 'static)> { + self.0.cause() + } +} + +impl ParserError for MBusError { + fn append(self, input: &I, token_start: &::Checkpoint, kind: ErrorKind) -> Self { + Self(self.0.append(input, token_start, kind)) + } + + fn from_error_kind(input: &I, kind: ErrorKind) -> Self { + Self(ContextError::from_error_kind(input, kind)) + } +} + +impl std::fmt::Display for MBusError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.0.fmt(f) + } +} + +impl AddContext for MBusError { + fn add_context( + self, + input: &I, + token_start: &::Checkpoint, + context: StrContext, + ) -> Self { + Self(self.0.add_context(input, token_start, context)) + } +} + +impl FromExternalError for MBusError { + fn from_external_error(input: &I, kind: ErrorKind, e: E) -> Self { + Self(ContextError::from_external_error(input, kind, e)) + } +} + +impl ErrorConvert for MBusError { + fn convert(self) -> MBusError { + self + } +} + +// impl ErrorConvert> for MBusError { +impl ErrorConvert for InputError { + fn convert(self) -> MBusError { + MBusError::from_error_kind(&self.input, self.kind) + } +} diff --git a/src/parse/link_layer.rs b/src/parse/link_layer.rs index 00f0a38..77b91a8 100644 --- a/src/parse/link_layer.rs +++ b/src/parse/link_layer.rs @@ -8,6 +8,8 @@ use winnow::prelude::*; use winnow::stream::Stream; use winnow::Bytes; +use super::error::MBResult; + const LONG_FRAME_HEADER: u8 = 0x68; const SHORT_FRAME_HEADER: u8 = 0x10; const FRAME_TAIL: u8 = 0x16; @@ -27,7 +29,7 @@ pub enum Packet<'a> { }, } -fn parse_variable<'a>(input: &mut &'a Bytes) -> PResult> { +fn parse_variable<'a>(input: &mut &'a Bytes) -> MBResult> { let length = binary::u8 .context(StrContext::Label("length")) .parse_next(input)?; @@ -88,7 +90,7 @@ fn parse_variable<'a>(input: &mut &'a Bytes) -> PResult> { }) } -fn parse_fixed<'a>(input: &mut &'a Bytes) -> PResult> { +fn parse_fixed<'a>(input: &mut &'a Bytes) -> MBResult> { // mbus's fixed length datagrams are 2 bytes long, only control & address let (control, address, checksum, _) = ( binary::u8.context(StrContext::Label("control byte")), @@ -112,12 +114,12 @@ fn parse_fixed<'a>(input: &mut &'a Bytes) -> PResult> { Ok(Packet::Short { control, address }) } -fn parse_ack<'a>(_input: &mut &'a Bytes) -> PResult> { +fn parse_ack<'a>(_input: &mut &'a Bytes) -> MBResult> { Ok(Packet::Ack) } impl<'a> Packet<'a> { - pub fn parse(input: &mut &'a Bytes) -> PResult> { + pub fn parse(input: &mut &'a Bytes) -> MBResult> { alt(( preceded( LONG_FRAME_HEADER.void(), diff --git a/src/parse/transport_layer/control_info.rs b/src/parse/transport_layer/control_info.rs index 44a9933..38910a5 100644 --- a/src/parse/transport_layer/control_info.rs +++ b/src/parse/transport_layer/control_info.rs @@ -7,6 +7,8 @@ use winnow::error::StrContext; use winnow::prelude::*; use winnow::Bytes; +use crate::parse::error::MBResult; + use super::header::LongHeader; use super::header::TPLHeader; @@ -47,7 +49,7 @@ pub enum CICode { } impl CICode { - pub fn parse(input: &mut &Bytes) -> PResult { + pub fn parse(input: &mut &Bytes) -> MBResult { let ci = binary::u8 .context(StrContext::Label("CI field")) .parse_next(input)?; diff --git a/src/parse/transport_layer/header.rs b/src/parse/transport_layer/header.rs index ef6f457..b35009b 100644 --- a/src/parse/transport_layer/header.rs +++ b/src/parse/transport_layer/header.rs @@ -2,10 +2,11 @@ // Licensed under the EUPL-1.2 #![allow(dead_code)] use winnow::binary; -use winnow::error::{ContextError, InputError, ParserError, StrContext}; +use winnow::error::{InputError, StrContext}; use winnow::prelude::*; use winnow::Bytes; +use crate::parse::error::MBResult; use crate::parse::types::number::parse_bcd; use super::manufacturer::{device_name, unpack_manufacturer_code}; @@ -50,7 +51,7 @@ pub struct MeterStatus { } impl MeterStatus { - fn parse(input: &mut &Bytes) -> PResult { + fn parse(input: &mut &Bytes) -> MBResult { binary::bits::bits::<_, _, InputError<_>, _, _>(( binary::bits::bool, binary::bits::bool, @@ -86,9 +87,6 @@ impl MeterStatus { }, ) .parse_next(input) - .map_err(|err| { - err.map(|err: InputError<_>| ContextError::from_error_kind(&err.input, err.kind)) - }) } } @@ -101,11 +99,11 @@ pub struct ShortHeader { } impl ShortHeader { - pub fn parse(input: &mut &Bytes) -> PResult { + pub fn parse(input: &mut &Bytes) -> MBResult { Self::parse_raw.map(TPLHeader::Short).parse_next(input) } - fn parse_raw(input: &mut &Bytes) -> PResult { + fn parse_raw(input: &mut &Bytes) -> MBResult { ( binary::u8.context(StrContext::Label("access number")), MeterStatus::parse.context(StrContext::Label("status")), @@ -194,7 +192,7 @@ pub enum DeviceType { } impl DeviceType { - fn parse(input: &mut &Bytes) -> PResult { + fn parse(input: &mut &Bytes) -> MBResult { binary::u8 .map(|v| match v { 0x00 => DeviceType::Other, @@ -228,7 +226,7 @@ pub struct LongHeader { } impl LongHeader { - pub fn parse(input: &mut &Bytes) -> PResult { + pub fn parse(input: &mut &Bytes) -> MBResult { ( parse_bcd(4) .context(StrContext::Label("device identifier")) diff --git a/src/parse/types.rs b/src/parse/types.rs index 791bbfa..cd9a658 100644 --- a/src/parse/types.rs +++ b/src/parse/types.rs @@ -1,8 +1,7 @@ // Copyright 2023 Lexi Robinson // Licensed under the EUPL-1.2 -use winnow::error::InputError; -use winnow::{Bytes, PResult}; +use winnow::Bytes; use super::error::Result; @@ -32,5 +31,3 @@ pub enum DataType { pub type ParseResult = Result; pub type BitsInput<'a> = (&'a Bytes, usize); -pub type BitsError<'a> = InputError>; -pub type BResult<'a, O> = PResult>; diff --git a/src/parse/types/number.rs b/src/parse/types/number.rs index 5f01411..54c9ccb 100644 --- a/src/parse/types/number.rs +++ b/src/parse/types/number.rs @@ -3,16 +3,16 @@ use winnow::binary; use winnow::combinator::repeat; -use winnow::error::{ContextError, ErrMode, InputError, ParserError}; +use winnow::error::{ErrMode, ParserError}; use winnow::prelude::*; use winnow::Bytes; use crate::parse::application_layer::dib::RawDataType; use crate::parse::application_layer::vib::ValueType; -use crate::parse::error::{ParseError, Result}; +use crate::parse::error::{MBResult, MBusError, ParseError, Result}; use crate::parse::Datagram; -use super::{BResult, BitsInput, DataType, ParseResult}; +use super::{BitsInput, DataType, ParseResult}; pub fn parse_number(dt: RawDataType, vt: ValueType, dg: &mut Datagram) -> ParseResult { match dt { @@ -31,15 +31,15 @@ pub fn parse_number(dt: RawDataType, vt: ValueType, dg: &mut Datagram) -> ParseR } } -fn parse_nibble<'a>(input: &mut BitsInput<'a>) -> BResult<'a, i64> { +fn parse_nibble(input: &mut BitsInput<'_>) -> MBResult { binary::bits::take(4_usize).parse_next(input) } -fn parse_bcd_nibble<'a>(input: &mut BitsInput<'a>) -> BResult<'a, i64> { +fn parse_bcd_nibble(input: &mut BitsInput<'_>) -> MBResult { parse_nibble.verify(|v| *v < 10).parse_next(input) } -pub fn parse_bcd<'a>(bytes: usize) -> impl Parser<&'a Bytes, i64, ContextError> { +pub fn parse_bcd<'a>(bytes: usize) -> impl Parser<&'a Bytes, i64, MBusError> { let parser = move |input: &mut BitsInput<'a>| { if bytes == 0 { return Err(ErrMode::assert(input, "cannot parse 0 bytes")); @@ -72,13 +72,7 @@ pub fn parse_bcd<'a>(bytes: usize) -> impl Parser<&'a Bytes, i64, ContextError> Ok(if neg { -result } else { result }) }; - move |input: &mut &'a Bytes| { - binary::bits::bits::<_, _, InputError<_>, _, _>(parser) - .parse_next(input) - .map_err(|err| { - err.map(|err: InputError<_>| ContextError::from_error_kind(&err.input, err.kind)) - }) - } + move |input: &mut &'a Bytes| binary::bits::bits(parser).parse_next(input) } fn decode_bcd(mut data: Vec) -> ParseResult { diff --git a/src/parse/types/string.rs b/src/parse/types/string.rs index 4cf7b7e..2d82993 100644 --- a/src/parse/types/string.rs +++ b/src/parse/types/string.rs @@ -4,18 +4,12 @@ use encoding_rs::WINDOWS_1252; use winnow::binary; use winnow::combinator::repeat; -use winnow::error::{ContextError, InputError}; use winnow::prelude::*; use winnow::stream::Bytes; -pub fn parse_length_prefix_ascii<'a>( - input: &mut &'a Bytes, -) -> PResult< - String, - // TODO: This (currently) *has* to be InputError because it's called from a - // bits::bytes parser, but hopefully we can remove that soon - InputError<&'a Bytes>, -> { +use crate::parse::error::{MBResult, MBusError}; + +pub fn parse_length_prefix_ascii(input: &mut &Bytes) -> MBResult { binary::length_take(binary::u8) .try_map(convert_ascii_string) .parse_next(input) @@ -25,7 +19,7 @@ fn convert_ascii_string(data: &[u8]) -> core::result::Result(num_bytes: usize) -> impl Parser<&'a Bytes, String, ContextError> { +pub fn parse_latin1<'a>(num_bytes: usize) -> impl Parser<&'a Bytes, String, MBusError> { move |input: &mut &'a Bytes| { repeat::<_, _, (), _, _>(num_bytes, binary::u8) .recognize()