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.

Sign up for GitHub

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

Jump to bottom

Parse error falls back to Invalid #1312

Merged
merged 43 commits into from
Jul 18, 2023
Merged

Parse error falls back to Invalid #1312

Changes from 1 commit
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
fa75ba1
parse error falls back to unknown
adrian-kong Mar 29, 2023
c17a922
something like this instead?
adrian-kong Apr 5, 2023
c2213ce
clippy
adrian-kong Apr 5, 2023
092d4bc
clippy
adrian-kong Apr 5, 2023
7613b89
Merge remote-tracking branch 'origin/master' into adrian/unknown-fall…
Apr 21, 2023
5fe6616
Handle missing data
Apr 21, 2023
893a6a6
Merge branch 'master' into adrian/unknown-fallback
pcrumley Jun 1, 2023
465a6a9
fix tests (#1339)
pcrumley Jun 15, 2023
2bc3417
Merge branch 'master' into adrian/unknown-fallback
pcrumley Jun 20, 2023
d4b9784
Merge branch 'adrian/unknown-fallback' of github.com:swift-nav/libsbp…
pcrumley Jun 20, 2023
5611749
wip
pcrumley Jun 22, 2023
b05b267
refactor parsing, add new error type with a sender ID and a msg_type ID
pcrumley Jun 22, 2023
443b1a6
unknown fallback
pcrumley Jun 22, 2023
383870c
add some docstrings where methods can panic
pcrumley Jun 22, 2023
0402244
fmt
pcrumley Jun 22, 2023
d2ee464
move diffs to generator. Mistakenly hand-edited auto-genarated code
pcrumley Jun 23, 2023
699036f
fix lint
pcrumley Jun 23, 2023
ef792bc
add invalid msg type
pcrumley Jun 23, 2023
688c17d
working on finishing this up
pcrumley Jun 23, 2023
602811f
MVP of feature
pcrumley Jun 29, 2023
0a81484
fix clippy
pcrumley Jun 29, 2023
c1e4cc9
Merge branch 'master' into adrian/unknown-fallback
pcrumley Jun 29, 2023
e89b957
update invalid message in code documentaiton
pcrumley Jun 29, 2023
b1e04b4
fix pointer to gen file
pcrumley Jun 30, 2023
1250509
Update rust/sbp/src/messages/invalid.rs
pcrumley Jul 5, 2023
fa8a272
Update rust/sbp/src/messages/invalid.rs
pcrumley Jul 5, 2023
3b73f6e
Update rust/sbp2json/src/bin/json2json.rs
pcrumley Jul 5, 2023
509ebbf
Update rust/sbp2json/src/bin/sbp2json.rs
pcrumley Jul 5, 2023
09cc839
Update rust/sbp2json/src/bin/json2sbp.rs
pcrumley Jul 5, 2023
d051faf
Merge branch 'master' into adrian/unknown-fallback
pcrumley Jul 5, 2023
58e9098
Merge branch 'adrian/unknown-fallback' of github.com:swift-nav/libsbp…
pcrumley Jul 5, 2023
bfbde5f
wip, address pr feedback
pcrumley Jul 11, 2023
c7d3da5
fix clippy
pcrumley Jul 11, 2023
e5125b5
maybe fix build
pcrumley Jul 11, 2023
e50072e
address feedback
pcrumley Jul 12, 2023
9c8a412
forgot to add generator files
pcrumley Jul 12, 2023
96ea494
remove let else
pcrumley Jul 12, 2023
c979177
do some work to support roundtrip, still wip
pcrumley Jul 12, 2023
5698712
remove crc from msg
pcrumley Jul 12, 2023
9a8bac0
still deserialize crc if possible
pcrumley Jul 12, 2023
c000e0a
add unit tests for recovering from parsing errors of sbp (#1347)
pcrumley Jul 13, 2023
8f6aebf
Require structs that impl SbpMessage also impl WireFormat and Clone
pcrumley Jul 13, 2023
0d97927
bump benchmark thresholds
silverjam Jul 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions rust/sbp/src/de.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ use dencode::FramedRead;
#[cfg(feature = "async")]
use futures::StreamExt;

use crate::messages::unknown::Unknown;
use crate::messages::SbpMessage;
use crate::wire_format::WireFormat;
use crate::{wire_format, Sbp, CRC_LEN, HEADER_LEN, MAX_FRAME_LEN, PAYLOAD_INDEX, PREAMBLE};

/// Deserialize the IO stream into an iterator of messages.
Expand Down Expand Up @@ -303,11 +306,14 @@ impl Frame {

pub fn to_sbp(&self) -> Result<Sbp, Error> {
adrian-kong marked this conversation as resolved.
Show resolved Hide resolved
self.check_crc()?;
Ok(Sbp::from_parts(
self.msg_type(),
self.sender_id(),
self.payload(),
)?)
let sender_id = self.sender_id();
let mut payload = self.payload();
let msg = Sbp::from_parts(self.msg_type(), sender_id, payload).or_else(|_| {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would mask malformed messages though? Like if the message type is a known message but the payload is too short. Then you'd have an Unknown message with a message type that corresponds to a known message which is a little odd. But maybe that's what we want, I guess it's a trade off between being lenient and surfacing errors better

Copy link
Contributor Author

@adrian-kong adrian-kong Mar 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea... think we're just treating Unknown as the Frame at this point to serialize as json :/

although that makes sense for the purpose of the Unknown message i guess.

maybe a better compromise would to just add a flag like strict to address dropping payload errors?

or maybe a 'from payload' like 'from fields' for json

issue with the raw decoder or persisting broken data is where do we actually trim the unnecessary data - which thought everything would propagate up to sbp converters and they would drop those - or maybe we should just never drop them 😱

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the Decoder should only output valid SBP (as in CRC is good and the payload is correct), if a user wants access to potentially invalid messages they can go with the Framer I think. So for example sbp2json should probably be doing something like

for frame in Framer::new(rdr) {
    if let Ok(msg) = frame.to_sbp() { 
        // valid crc/payload, possibly an Unknown message
        json_encoder.write(&msg)?;
    } else {
        // invalid crc or payload, could also be an Unknown message
        // need some json representation of a frame
        json_encoder.write(&frame)?;
    }
}

let mut unknown = Unknown::parse_unchecked(&mut payload);
unknown.set_sender_id(sender_id);
Ok::<Sbp, wire_format::PayloadParseError>(Sbp::Unknown(unknown))
})?;
Ok(msg)
}

pub fn as_bytes(&self) -> &[u8] {
Expand Down