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

protocol: parse MsgType using scroll #63

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

hdhoang
Copy link
Collaborator

@hdhoang hdhoang commented May 17, 2018

As a PoC it only parses one byte. Next up would be buffer.pread::<Message>(0), but I haven't cooked up a multiplexing try_from_ctx for the different MsgType + XOp yet.

Further, we could have buffer.pwrite::<LedTheme> for sending the 0xca set-LED messages too.

cc #20

@hdhoang
Copy link
Collaborator Author

hdhoang commented May 17, 2018

proc-macro2 will be compatible with the new nightlies soon dtolnay/quote#73

FwUp = 11,
CustomLed = 12,
CustomKey = 13,
Reserved,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think these needs to stay, so that send_buffer[pos] = message_type as u8; in Serial::send works correctly consistently? Especially for operation codes, those are dis-contiguous

Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we could use the write functionality in scroll for that as well? Agree that we need to be careful about preserving what actually goes on the wire, so codes don't change.

@ah-
Copy link
Owner

ah- commented May 17, 2018

Oh that's really nice, I had been looking for a lib that does exactly this but hadn't tried out scroll yet. This is looking really promising, especially that it does proper parsing rather than just casting and hoping it all works.

unknown => {
return Err(SError::BadInput {
size: unknown as usize,
msg: "MsgType",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I try to sneak the unknown byte into the error message here, it should be formatted as (fmt, "bad input {} ({})", msg, size) with semihosting

FwInfo => 10,
FwUp => 11,
CustomLed => 12,
CustomKey => 13,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This looks ripe for a macro. scroll_derive doesn't support deriving on enums, so we'll need to write one here.

@hdhoang
Copy link
Collaborator Author

hdhoang commented May 18, 2018

I'd like to make some round-trip tests to run on x86-64 too.

@hdhoang
Copy link
Collaborator Author

hdhoang commented May 18, 2018

The test target hard-codes linux-gnu though, I will try to extract rustup target list|grep default | cut -d ' ' -f 1 into make somehow.

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 this pull request may close these issues.

2 participants