nrf-rs / nrf-hal

A Rust HAL for the nRF family of devices
Apache License 2.0
499 stars 139 forks source link

Correct eb3b96b with a useful function signature for try_wait #413

Closed nospam2678 closed 1 year ago

nospam2678 commented 1 year ago

This is embarrassing. It appears my quality assurance for #412, while existent, was way flawed and insufficient.

Having a try_ method take ownership over self makes it impossible call it more than once. Clearly a mutable reference was what was indented and required to actually make the method meaningful.

It would be a good idea to either merge this PR, or revert the previous one, prior to making the next release.

jonas-schievink commented 1 year ago

This change would be unsound (due to the unreachable_unchecked; but the only alternative is a panic or a signature change in the other methods, which is also not great) if the user started another transfer while the old one is still around.

I think we might want to remove try_wait in favor of having the user check is_done - if is_done returns true, the user knows that the next call to wait won't block.

nospam2678 commented 1 year ago

Indeed. Makes sense! Branch has been updated to remove try_wait and only leave is_done.

Maybe the corresponding is_done method should be added to also to TransferFullDuplex? I lack a hardware setup to do any I²S in the other direction, so that would be completely written blindly if I did it.

jonas-schievink commented 1 year ago

bors r+

bors[bot] commented 1 year ago

Build succeeded: