esp-rs / esp-hal

no_std Hardware Abstraction Layers for ESP32 microcontrollers
https://docs.esp-rs.org/esp-hal/
Apache License 2.0
673 stars 190 forks source link

Document convention / guidelines / best practices on using peripherals in different modules, safely. #1780

Closed AnthonyGrondin closed 2 weeks ago

AnthonyGrondin commented 1 month ago

As the peripheral part of this library has come to stabilization, It could be relevant to write down a convention and guidelines on using different peripherals in different manners for common use cases.

Here are a few relevant questions that I can think of, that the document should answer in a way or another:

MabezDev commented 1 month ago

Some of these have a partial answer in the API-GUIDELINES document we created, I think this covers the first three.

What if we implement a trait that only takes &self, is there a way to safely access peripherals that might require a mutable access?

This probably needs to be accessed on a case-by-case basis, but generally if the trait is &self and the impl requires &mut self then worst case you'd need interior mutability via a mutex.

What about using optional peripherals Option<>?

Could you expand on this a little bit, does this mean options within constructor arguments or something else?

AnthonyGrondin commented 1 month ago

Some of these have a partial answer in the API-GUIDELINES document we created, I think this covers the first three.

Oops, I didn't see this document before opening the issue.

What about using optional peripherals Option<>?

Could you expand on this a little bit, does this mean options within constructor arguments or something else?

So I recently had to implement edge_nal::TcpAccept for esp-mbedtls for an experiment, and esp-mbedtls has support for hardware acceleration using RSA. The Session needs Rsa in a mutable access (owned or mutably borrowed), due to the requirement of the Rsa driver. Now, for the Option<> part, this is simply an implementation part; the peripheral can be omitted on a per-session basis, to instead use the software implementation. So Session takes the peripheral wrapped in an Option.

For implementing a trait that takes &self I ended up defining my struct with:

rsa: core::cell::RefCell<Option<esp_hal::peripherals::RSA>>,

and creating a session with:

let session = Session::new(
    socket,
    "",
    Mode::Server,
    self.version,
    self.certificates,
    if let Ok(ref mut rsa) = self.rsa.try_borrow_mut() {
        rsa.as_mut()
    } else {
        None
    },
)?;

to get a mutable access to an optional peripheral.

EDIT: According to the guidelines, Session in esp-mbedtls should use the builder pattern, and I agree that this would simplify things up, I might open a PR for that.

jessebraham commented 2 weeks ago

@MabezDev has there been any progress here? Do you still intend to do some work here for the 0.20.0 release, or should we punt this down the road a bit?