Skip to content

Conversation

@jessebraham
Copy link
Member

It's been recommended by various notable people from the embedded Rust community for quite some time now that delay sources be clone-able. This may also become a requirement of embedded-hal for the SPI driver in the future, see here:
rust-embedded/embedded-hal#462 (comment)

Since our delay providers do not own any peripheral structs and only perform reads, it's safe to clone/copy as many instances as you would like.

The RISC-V and Xtensa implementations of the delay functionality are both safe to access from multiple contexts, and the `Delay` struct does not take ownership of any peripheral struct, so `Delay` is safe to clone/copy
@jessebraham jessebraham requested review from MabezDev and bjoernQ May 31, 2023 13:30
Copy link
Contributor

@bjoernQ bjoernQ left a comment

Choose a reason for hiding this comment

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

LGTM

@jessebraham jessebraham merged commit 67e9c60 into esp-rs:main May 31, 2023
@jessebraham jessebraham deleted the feature/delay-clone-copy branch May 31, 2023 13:49
MabezDev pushed a commit to MabezDev/esp-hal that referenced this pull request Jun 1, 2023
* Simplify the `Delay` driver, derive `Clone` and `Copy`

The RISC-V and Xtensa implementations of the delay functionality are both safe to access from multiple contexts, and the `Delay` struct does not take ownership of any peripheral struct, so `Delay` is safe to clone/copy

* Update the CHANGELOG
SergioGasquez pushed a commit to SergioGasquez/esp-hal that referenced this pull request Jun 9, 2023
* Simplify the `Delay` driver, derive `Clone` and `Copy`

The RISC-V and Xtensa implementations of the delay functionality are both safe to access from multiple contexts, and the `Delay` struct does not take ownership of any peripheral struct, so `Delay` is safe to clone/copy

* Update the CHANGELOG
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