Skip to content

Rng API #38

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

Closed
therealprof opened this issue Feb 13, 2018 · 3 comments
Closed

Rng API #38

therealprof opened this issue Feb 13, 2018 · 3 comments
Labels

Comments

@therealprof
Copy link
Contributor

I'd like to suggest to suggest a new Rng API for a common way of getting random numbers from devices with some source of randomness.

I've already implemented this as a proof-of-concept in my nrf51-hal crate (https://github.com/therealprof/nrf51-hal) and an example use in my microbit crate (https://github.com/therealprof/microbit).

The suggested (blocking) API is similar to the I2c API in that a mutable u8 slice is passed as parameter which will be filled with the random data and a Result is returned:

//! Blocking hardware random number generator

/// Blocking read
pub trait Read {
    /// Error type
    type Error;

    /// Reads enough bytes from hardware random number generator to fill `buffer`
    fn read(&mut self, buffer: &mut [u8]) -> Result<(), Self::Error>;
}

Potentially this could also use an implementation for other (or even variable) array types, e.g. the rand crate requires a u32 slice as a seed.

I'd be happy to supply a PR for the the implementation.

@nickray
Copy link

nickray commented Dec 15, 2018

I'm implementing this for STM32L4(32): https://github.com/nickray/stm32l4-hal/blob/rng/src/rng.rs
It "works", but I'd like to make it rustic. Some questions:

  • should RngExt.enable be called .constrain in line with other traits? For me personally, that naming is a little weird (as the API is constrained, not the RNG), but maybe it just needs getting used to and having conventions is a good thing.
  • turning HSI48 on depends on other RCC settings, so it should probably not be done implicitly in .enable. On the other hand, I'm not happy with either a) asserting, or b) not checking settings at all
  • is there room for additional helper methods? I'm talking about things like wrapping self.rng.sr.read().cecs().bit() as .is_clock_error for people like me who don't know data sheets and register names by heart. Or does this go against HAL minimalism?
  • the Error enum:
    • should this have only standardized errors, or reveal MCU-specific errors also?
    • is it efficient to check clock settings here on every call (admittedly, calling the RNG is not the fastest operation)?

@eldruin
Copy link
Member

eldruin commented Sep 16, 2020

This trait was added. There is discussion about replacing/complementing this with rand_core at #128 which provide further methods.

@therealprof
Copy link
Contributor Author

Hah! I knew there were previous discussions about this. Let's close this then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants