-
Notifications
You must be signed in to change notification settings - Fork 2
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
It should have been like this from the beginning. Also: #18
Comments
@wucke13 I went over the decode code, and I think there is a problem. It assumes that the |
Well then have a look at how my decoder works. It knows upfront how many bytes are needed for the next fragment of a msp message, and only continues if this many bytes are present. It can easily be ported to be non-blocking code as well. Or is your comment aimed at my decoder? |
my comment aimed at https://github.com/wucke13/mavlink-msp-bridge/blob/master/src/msp.rs#L456 |
Ah, ok. The decoder is a state machine. Each state represents one part of the message. Almost all states require a fixed amount of bytes to be parsed, the only exception is the actual payload. However, before we come to parsing the payload we already parsed the fields, and thus know how big the payload is gonna be. This gives us one guarantee: At every point in time, we know exactly how many bytes we have to read to parse the next part/do the next state transition. Does that make sense so far? To your first question: Currently, the Are there questions left on your side? |
Ok, I got it now. thanks. do you think it would be better to refactor this library to use the |
In general yes. However, |
It should have been like this from the beginning. Also:
For serialization, have a look over here: https://github.com/wucke13/mavlink-msp-bridge/blob/master/src/msp.rs#L405
For deserialization, over there: https://github.com/wucke13/mavlink-msp-bridge/blob/master/src/msp.rs#L456 and here https://github.com/wucke13/mavlink-msp-bridge/blob/master/src/msp.rs#L230
The read macro could easily be adjusted to not use the Read trait but instead do some slicing operation, maybe with an offset?
Originally posted by @wucke13 in #16 (comment)
The text was updated successfully, but these errors were encountered: