-
Notifications
You must be signed in to change notification settings - Fork 5
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
[WIP] Deserialization #3
[WIP] Deserialization #3
Conversation
045bd81
to
e40fe6d
Compare
@joaoantoniocardoso can you rebase your work ? |
@@ -5,7 +5,7 @@ use ping_rs::{common, PingMessagePack}; | |||
fn test_simple_serialization() { | |||
let general_request = | |||
common_messages::GeneralRequest(common::GeneralRequestStruct { requested_id: 5 }); | |||
let message = PingMessagePack::from(general_request); | |||
let message = PingMessagePack::from(&general_request); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears unnecessary on the current version
|
||
fn try_from(buffer: &Vec<u8>) -> Result<Self, Self::Error> { | ||
// Parse start1 and start2 | ||
if !((buffer[0] == b'B') && (buffer[1] == b'R')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should have a function and variables to compare the header and save B and R.
|
||
fn try_from(buffer: &Vec<u8>) -> Result<Self, Self::Error> { | ||
// Parse start1 and start2 | ||
if !((buffer[0] == b'B') && (buffer[1] == b'R')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should have a generic deserializer that stores the state of the parser. If we receive B, than R, than 3 more bytes, until the message is complete.
let data_type = field.typ.to_rust(); | ||
let data_size = field.typ.to_size(); | ||
let field_token = quote! { | ||
#name: #data_type::from_le_bytes(payload[#b..#b + #data_size].try_into().expect("Wrong slice length")), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can check the initial size with payload.len()
, try_into and everything else will be unnecessary,
let data_type = field.typ.to_rust(); | ||
let data_size = field.typ.to_size(); | ||
let field_token = quote! { | ||
#name: #data_type::from_le_bytes(payload[#b..#b + #data_size].try_into().expect("Wrong slice length")), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should do #b + #data_size
before putting on the template for better readability of the generated code.
// Get the package data | ||
let payload_length = u16::from_le_bytes([buffer[2], buffer[3]]); | ||
let message_id = u16::from_le_bytes([buffer[4], buffer[5]]); | ||
let _src_device_id = buffer[6]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should think how to handle it in a better way, maybe returning two structures, one that encapsulate the message and another one for the header or everything else, maybe a complete message.
@joaoantoniocardoso any update on that ? |
e40fe6d
to
81a9a98
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll continue working over it
No description provided.