Adding Auto Acknowledgements #24
Conversation
ProfFan
left a comment
ProfFan
left a comment
There was a problem hiding this comment.
Thank you for the PR!
Please use the script in example to generate a set of expected SPI commands and write some unit tests for the state transitions.
Some of the stuff here will conflict with #21 . Let's wait a few days on that PR to see if the author replies.
| /// Returns the current system time (accurate to the upper 32-bit) | ||
| #[maybe_async_attr] | ||
| pub async fn sys_time(&mut self) -> Result<u32, Error<SPI>> { | ||
| pub async fn sys_time(&mut self) -> Result<Instant, Error<SPI>> { |
There was a problem hiding this comment.
Please don't change this API. You can add a from_sys_time in Instant if you want.
There was a problem hiding this comment.
The sys_time is internally 40 bit of the DW3000, only 32 bits are exposed, why should we need to handle this outside of the driver? I think this is really confusing for new users. We can make a translation from one to the other, but not sure if that is the best approach.
There was a problem hiding this comment.
There is a bit of overhead as Instant is internally 64 bits. The other part is API stability, there already exists many users of this library.
| data: &[u8], | ||
| send_time: SendTime, | ||
| config: &Config, | ||
| continuation: TxContinuation, |
There was a problem hiding this comment.
I like that, but the number of arguments of this function is getting out of control.
Maybe wait until we get #21 's simplifications to the send function in?
|
Given #21 is merged, could you rebase on current main? Thank you very much! |
…r failed/filtered frames otherwise filtered frames will stop RX
3 participants
Instantfor sys_timeinspiration: jkelleyrtp/dw1000-rs#157