Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parsing issues #213

Open
t-moe opened this issue Oct 1, 2024 · 1 comment · May be fixed by #214
Open

Parsing issues #213

t-moe opened this issue Oct 1, 2024 · 1 comment · May be fixed by #214

Comments

@t-moe
Copy link
Contributor

t-moe commented Oct 1, 2024

I discovered that the parser cannot handle a newline followed by a valid URC, which is needed to be able to recover from garbage data being received.

Here is the failing (new) testcase (digest.rs)

    #[test]
    fn newline_garbage_followed_by_urc() {
        let mut digester = AtDigester::<UrcTestParser>::new();
        let mut buf = heapless::Vec::<u8, TEST_RX_BUF_LEN>::new();

        buf.extend_from_slice(
            b"\r\naa\r\n+UUSORD: 0,5\r\n", // 20 bytes
        )
            .unwrap();
        let (res, bytes) = digester.digest(&buf);
        assert_eq!((res, bytes), (DigestResult::Urc(b"+UUSORD: 0,5"), 20)); // fails with (None, 0)
        buf.rotate_left(bytes);
        buf.truncate(buf.len() - bytes);
        assert!(buf.is_empty());
    }

I'm happy to investigate this a bit further and possibly provide a fix, but one question first:
Shouldn't digest return the number of consumed bytes? Or what is the 2nd part of the return tuple? The trait has no documentation...
Here is a reproducer:

    #[test]
    fn consumed_bytes_are_correct_with_prefixed_garbage() {
        let mut digester = AtDigester::<UrcTestParser>::new();
        let mut buf = heapless::Vec::<u8, TEST_RX_BUF_LEN>::new(
        buf.extend_from_slice(
            b"aaaa\r\n+UUSORD: 0,5\r\n", // 20 bytes
        )
            .unwrap();
        let (res, bytes) = digester.digest(&buf);

        assert_eq!((res, bytes), (DigestResult::Urc(b"+UUSORD: 0,5"), 20)); // fails. only 16 are consumed
        buf.rotate_left(bytes);
        buf.truncate(buf.len() - bytes);
        assert!(buf.is_empty()); // followup error
    }

EDIT: Ok the 2nd issue is probably fixed by:

diff --git a/atat/src/digest.rs b/atat/src/digest.rs
--- a/atat/src/digest.rs	(revision 242628d2ad2add452c909a036af036d83ea2fb5a)
+++ b/atat/src/digest.rs	(date 1727770394036)
@@ -128,7 +128,7 @@
 
         // 2. Match for URC's
         match P::parse(buf) {
-            Ok((urc, len)) => return (DigestResult::Urc(urc), len),
+            Ok((urc, len)) => return (DigestResult::Urc(urc), len + space_and_echo_bytes),
             Err(ParseError::Incomplete) => return incomplete,
             _ => {}
         }

EDIT: the 1st issue seems to be more tricky though

@t-moe t-moe linked a pull request Oct 1, 2024 that will close this issue
@TomDeRybel
Copy link

TomDeRybel commented Nov 18, 2024

The #214 PR fixes a similar issue I've encountered with a u-blox NINA-W#214

This module sends the +STARTUP URC when it is done booting. This URC is preceded by a whole lot of start-up diagnostic messages, and the URC is not detected properly without the fix.

For completeness, here are the relevant log messages:
[2024-11-18T12:49:55.788462875Z DEBUG atat::ingress] Received echo or whitespace (24/32): "ets Jul 29 2019 12:21:46\r\n\r\nrst:"
[2024-11-18T12:49:56.735184632Z DEBUG atat::ingress] Received URC/128 (338/338): "+STARTUP"
[2024-11-18T12:49:56.735316114Z DEBUG atat::asynch::client] Sending command: "AT\r\n"
[2024-11-18T12:49:56.752052589Z DEBUG atat::ingress] Received OK (10/10)

Without the fix in the PR, the second line "Received URC/128 ... never occurs, and the URC is lost.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants