Skip to content

Commit

Permalink
Handle invalid BCDs
Browse files Browse the repository at this point in the history
Helpfully the libmbus test data has two files with error state BCDs so I
had to spend a while trying to understand the unhelpfully vague EN
13757-3:2018 Annex B and the ultimately entirely incorrect behaviour
introduced to `libmbus` in a series of commits several years ago.

The understanding I have come to is that it's still a normal BCD number,
but some elements have been replaced with hexadecimal symbols to
indicate particular parts are now errors. What do those errors mean?
Completely unspecified!

I have decided to return this as an uppercase hex string, taking
negatives into account. Converting this as per Table B.1 for use on a 7
segment display is left as an exercise to the reader.

Unfortunately, the way libmbus handles this is twofold.

1. If the data function is "value during error state" and you are
   generating the standard XML output, it parses it into a pure hex
   string, ignoring the rules regarding negative numbers
2. Otherwise, for each pair of nibbles the first one is discarded if it
   is over 0xA unless it's the last one, in which case it negates the
   entire number. This means that an input of 0xFFFF is interpreted as
   -0x0F0F, The second nibbles are then added without any clamping at
   all, resulting in -1616. 0x1F would be 17. This is suboptimal.

This is specifically unfortunate for me because I want to be start
testing my output against their test output and that means I am going to
have to *implement* both of these behaviours for my compatability layer.
  • Loading branch information
Lexicality committed Sep 14, 2024
1 parent 2e9d582 commit d32b780
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 3 deletions.
10 changes: 7 additions & 3 deletions src/parse/application_layer/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Licensed under the EUPL-1.2

use winnow::binary;
use winnow::combinator::repeat;
use winnow::combinator::{alt, repeat};
use winnow::error::{AddContext, ErrMode, ErrorKind, ParserError, StrContext};
use winnow::prelude::*;
use winnow::stream::Stream;
Expand All @@ -11,7 +11,7 @@ use winnow::Bytes;
use crate::parse::error::{MBResult, MBusError};
use crate::parse::types::date::{TypeFDateTime, TypeGDate, TypeIDateTime, TypeJTime, TypeKDST};
use crate::parse::types::number::{
parse_bcd, parse_binary_signed, parse_binary_unsigned, parse_real,
parse_bcd, parse_binary_signed, parse_binary_unsigned, parse_invalid_bcd, parse_real,
};
use crate::parse::types::string::parse_latin1;
use crate::parse::types::DataType;
Expand Down Expand Up @@ -55,7 +55,11 @@ impl Record {
// return Err(ErrMode::assert(input, "Type M dates not implemented yet"))
// }
_ => match dib.raw_type {
RawDataType::BCD(num) => parse_bcd(num).map(DataType::Signed).parse_next(input)?,
RawDataType::BCD(num) => alt((
parse_bcd(num).map(DataType::Signed),
parse_invalid_bcd(num).map(DataType::ErrorValue),
))
.parse_next(input)?,
RawDataType::Binary(num) => parse_binary(unsigned, num).parse_next(input)?,
RawDataType::Real => parse_real.map(DataType::Real).parse_next(input)?,
RawDataType::None => DataType::None,
Expand Down
1 change: 1 addition & 0 deletions src/parse/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub enum DataType {
Time(date::TypeJTime), // Type J
DST(date::TypeKDST), // Type K
String(String),
ErrorValue(String),
Invalid(Vec<u8>),
VariableLengthNumber(Vec<u8>),
ManufacturerSpecific(Vec<u8>),
Expand Down
127 changes: 127 additions & 0 deletions src/parse/types/number.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,133 @@ mod test_parse_bcd {
}
}

fn parse_hex_nibble(input: &mut BitsInput<'_>) -> MBResult<char> {
binary::bits::take(4_usize)
.verify_map(|i: u32| char::from_digit(i, 16))
.parse_next(input)
}

pub fn parse_invalid_bcd<'a>(bytes: usize) -> impl Parser<&'a Bytes, String, MBusError> {
let parser = move |input: &mut BitsInput<'a>| {
if bytes == 0 {
return Ok("".to_owned());
}
let mut initial_bytes: Vec<(char, char)> =
repeat(bytes - 1, (parse_hex_nibble, parse_hex_nibble))
.context(StrContext::Label("initial bytes"))
.parse_next(input)?;

// last byte is speical because of the `-` behaviour
initial_bytes.push(
(
parse_hex_nibble.map(|c| if c == 'f' { '-' } else { c }),
parse_hex_nibble,
)
.context(StrContext::Label("final byte"))
.parse_next(input)?,
);

let result: String = initial_bytes
.into_iter()
.rev()
.flat_map(|i| [i.0, i.1])
.collect();

Ok(result.to_uppercase())
};

binary::bits::bits(parser).context(StrContext::Label("signed BCD number"))
}

#[cfg(test)]
mod test_parse_invalid_bcd {
use winnow::error::ErrorKind;
use winnow::{Bytes, Parser};

use super::parse_invalid_bcd;

#[test]
fn test_basic_unsigned() {
let input = Bytes::new(&[0x12]);

let result = parse_invalid_bcd(1).parse(input).unwrap();

assert_eq!(result, "12");
}

#[test]
fn test_byte_order_unsigned() {
let input = Bytes::new(&[0x34, 0x12]);

let result = parse_invalid_bcd(2).parse(input).unwrap();

assert_eq!(result, "1234");
}

#[test]
fn test_basic_signed() {
let input = Bytes::new(&[0xF1]);

let result = parse_invalid_bcd(1).parse(input).unwrap();

assert_eq!(result, "-1");
}

#[test]
fn test_byte_order_signed() {
let input = Bytes::new(&[0x23, 0xF1]);

let result = parse_invalid_bcd(2).parse(input).unwrap();

assert_eq!(result, "-123");
}

#[test]
fn test_negative_zero() {
let input = Bytes::new(&[0xF0]);

let result = parse_invalid_bcd(1).parse(input).unwrap();

assert_eq!(result, "-0");
}

#[test]
fn test_parse_zero() {
let input = Bytes::new(&[]);

let result = parse_invalid_bcd(0).parse(input).unwrap();

assert_eq!(result, "");
}

#[test]
fn test_parse_not_enough_data() {
let input = Bytes::new(&[0x12]);

let result = parse_invalid_bcd(2).parse(input).unwrap_err();

assert_eq!(result.inner().kind(), ErrorKind::Eof);
}

#[test]
fn test_hex() {
let input = Bytes::new(&[0xEF, 0xCD, 0xAB]);

let result = parse_invalid_bcd(3).parse(input).unwrap();

assert_eq!(result, "ABCDEF");
}

#[test]
fn test_negative_hex() {
let input = Bytes::new(&[0xFF]);

let result = parse_invalid_bcd(1).parse(input).unwrap();

assert_eq!(result, "-F");
}
}

pub fn parse_binary_signed<'a>(bytes: usize) -> impl Parser<&'a Bytes, i64, MBusError> {
move |input: &mut &'a Bytes| {
match bytes {
Expand Down

0 comments on commit d32b780

Please sign in to comment.