Skip to content

Commit

Permalink
Fix digest continuation bug (FactbirdHQ#141)
Browse files Browse the repository at this point in the history
* Fix digest continuation bug

The digest should not continue to lower priority cases if the parser returns incomplete.

This fixes FactbirdHQ#140

* Make clippy happy

* Make clippy happy on nightly

* Make clippy even more happy
  • Loading branch information
rmja authored Feb 13, 2023
1 parent a4608e9 commit b2509f2
Show file tree
Hide file tree
Showing 3 changed files with 76 additions and 24 deletions.
83 changes: 67 additions & 16 deletions atat/src/digest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use core::marker::PhantomData;

use crate::InternalError;

#[derive(Debug, PartialEq)]
#[derive(Debug, PartialEq, Eq)]
pub enum DigestResult<'a> {
Urc(&'a [u8]),
Response(Result<&'a [u8], InternalError<'a>>),
Expand Down Expand Up @@ -115,25 +115,36 @@ impl<P: Parser> Digester for AtDigester<P> {
Err(_) => panic!("NOM ERROR - opt(echo)"),
};

// Incomplete. Eat the echo and do nothing else.
let incomplete = (DigestResult::None, echo_bytes);

// 2. Match for URC's
if let Ok((urc, len)) = P::parse(input) {
return (DigestResult::Urc(urc), len);
match P::parse(input) {
Ok((urc, len)) => return (DigestResult::Urc(urc), len),
Err(ParseError::Incomplete) => return incomplete,
_ => {}
}

// 3. Parse for success responses
// Custom successful replies first, if any
if let Ok((response, len)) = (self.custom_success)(buf) {
return (DigestResult::Response(Ok(response)), len + echo_bytes);
match (self.custom_success)(buf) {
Ok((response, len)) => return (DigestResult::Response(Ok(response)), len + echo_bytes),
Err(ParseError::Incomplete) => return incomplete,
_ => {}
}

// Generic success replies
if let Ok((_, (result, len))) = parser::success_response(buf) {
return (result, len + echo_bytes);
match parser::success_response(buf) {
Ok((_, (result, len))) => return (result, len + echo_bytes),
Err(nom::Err::Incomplete(_)) => return incomplete,
_ => {}
}

// Custom prompts for data replies first, if any
if let Ok((response, len)) = (self.custom_prompt)(buf) {
return (DigestResult::Prompt(response), len + echo_bytes);
match (self.custom_prompt)(buf) {
Ok((response, len)) => return (DigestResult::Prompt(response), len + echo_bytes),
Err(ParseError::Incomplete) => return incomplete,
_ => {}
}

// Generic prompts for data
Expand All @@ -143,20 +154,24 @@ impl<P: Parser> Digester for AtDigester<P> {

// 4. Parse for error responses
// Custom error matches first, if any
if let Ok((response, len)) = (self.custom_error)(buf) {
return (
DigestResult::Response(Err(InternalError::Custom(response))),
len + echo_bytes,
);
match (self.custom_error)(buf) {
Ok((response, len)) => {
return (
DigestResult::Response(Err(InternalError::Custom(response))),
len + echo_bytes,
)
}
Err(ParseError::Incomplete) => return incomplete,
_ => {}
}

// Generic error matches
if let Ok((_, (result, len))) = parser::error_response(buf) {
return (result, len + echo_bytes);
}

// No matches at all. Just eat the echo and do nothing else.
(DigestResult::None, echo_bytes)
// No matches at all.
incomplete
}
}

Expand Down Expand Up @@ -439,6 +454,8 @@ pub mod parser {
}
#[cfg(test)]
mod test {
use nom::{branch, bytes, character, combinator, sequence};

use super::parser::{echo, urc_helper};
use super::*;
use crate::{
Expand Down Expand Up @@ -1158,4 +1175,38 @@ mod test {

assert!(buf.is_empty());
}

#[test]
fn custom_success_with_prompt() {
let mut digester = AtDigester::<UrcTestParser>::new().with_custom_success(|buf| {
let (_reminder, (head, data, tail)) = branch::alt((sequence::tuple((
bytes::streaming::tag(b"\r\n"),
combinator::recognize(sequence::tuple((
bytes::streaming::tag(b"+CIPRXGET: 2,"),
character::streaming::u8,
bytes::streaming::tag(","),
combinator::flat_map(character::streaming::u16, |data_len| {
combinator::recognize(sequence::tuple((
bytes::streaming::tag(","),
character::streaming::u16,
bytes::streaming::tag("\r\n"),
bytes::streaming::take(data_len),
)))
}),
))),
bytes::streaming::tag(b"\r\nOK\r\n"),
)),))(buf)?;

Ok((data, head.len() + data.len() + tail.len()))
});

assert_eq!(
(DigestResult::None, 0),
digester.digest(b"\r\n+CIPRXGET: 2,0,2,0\r\n> ")
);
assert_eq!(
(DigestResult::Response(Ok(b"+CIPRXGET: 2,0,2,0\r\n> ")), 30),
digester.digest(b"\r\n+CIPRXGET: 2,0,2,0\r\n> \r\nOK\r\n")
);
}
}
4 changes: 2 additions & 2 deletions atat/src/error/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub use cms_error::CmsError;
pub use connection_error::ConnectionError;

/// Errors returned used internally within the crate
#[derive(Clone, Debug, PartialEq)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum InternalError<'a> {
/// Serial read error
Read,
Expand Down Expand Up @@ -147,7 +147,7 @@ impl<'a> From<&'a [u8]> for Response<'a> {
}

/// Errors returned by the crate
#[derive(Clone, Debug, PartialEq)]
#[derive(Clone, Debug, PartialEq, Eq)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
pub enum Error {
/// Serial read error
Expand Down
13 changes: 7 additions & 6 deletions serde_at/src/de/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -673,12 +673,13 @@ impl fmt::Display for Error {
}

fn trim_ascii_whitespace(x: &[u8]) -> &[u8] {
let from = match x.iter().position(|x| !x.is_ascii_whitespace()) {
Some(i) => i,
None => return &x[0..0],
};
let to = x.iter().rposition(|x| !x.is_ascii_whitespace()).unwrap();
&x[from..=to]
x.iter().position(|x| !x.is_ascii_whitespace()).map_or_else(
|| &x[0..0],
|from| {
let to = x.iter().rposition(|x| !x.is_ascii_whitespace()).unwrap();
&x[from..=to]
},
)
}

/// Deserializes an instance of type `T` from bytes of AT Response text
Expand Down

0 comments on commit b2509f2

Please sign in to comment.