-
Notifications
You must be signed in to change notification settings - Fork 24
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: Add CAN mock #78
base: 1-alpha
Are you sure you want to change the base?
Conversation
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.
thanks for opening this! a few bits of feedback but overall seems good
|
||
impl Transaction { | ||
/// Create a Transmit transaction | ||
pub fn transmit(expected_frame: MockFrame) -> Transaction { |
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 think it probably makes sense to const
ify everywhere possible to make it easier to construct test vectors?
&t.expected_frame.unwrap(), | ||
frame, | ||
"can::transmit data does not match expectation" | ||
); |
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 looks pretty reasonable to me, tho i think is missing a path to return errors? (if expected_error
is set this value should be returned instead of Ok(())
(and the same for receive and the nb impls)
@@ -15,6 +15,12 @@ impl embedded_hal::digital::Error for MockError { | |||
} | |||
} | |||
|
|||
impl embedded_can::Error for MockError { |
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 suspect MockError
is going to need to reflect the variants in embedded_can::ErrorKind so mocks can deal with at least contention (Bit
) and CRC
errors... but we can probably expand on this later.
#[derive(Clone, Debug, PartialEq, Eq)] | ||
pub struct Transaction { | ||
expected_mode: Mode, | ||
expected_frame: Option<MockFrame>, |
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.
because it's either rx or tx there's only one frame (unlike SPI where you have both incoming and outgoing data) so it seems like expected_frame
and response_frame
could just be frame
?
This is a initial draft for adding a CAN mock. Any feedback would be greatly appreciated.