Rng Initialization breaks ADC1 (at least on ESP32)

[bjoernQ]

Change the ADC example to look like this

//! Connect a potentiometer to an IO pin and see the read values change when
//! rotating the shaft.
//!
//! Alternatively, you could also connect the IO pin to GND or 3V3 to see the
//! maximum and minimum raw values read.

//% CHIPS: esp32 esp32c2 esp32c3 esp32c6 esp32h2 esp32s2 esp32s3

#![no_std]
#![no_main]

use esp_backtrace as _;
use esp_hal::{
    analog::adc::{AdcConfig, Attenuation, ADC},
    clock::ClockControl,
    delay::Delay,
    gpio::IO,
    peripherals::{Peripherals, ADC1},
    prelude::*,
};
use esp_println::println;

#[entry]
fn main() -> ! {
    let peripherals = Peripherals::take();
    let system = peripherals.SYSTEM.split();
    let clocks = ClockControl::boot_defaults(system.clock_control).freeze();

    let io = IO::new(peripherals.GPIO, peripherals.IO_MUX);
    cfg_if::cfg_if! {
        if #[cfg(feature = "esp32")] {
            let analog_pin = io.pins.gpio32.into_analog();
        } else if #[cfg(any(feature = "esp32s2", feature = "esp32s3"))] {
            let analog_pin = io.pins.gpio3.into_analog();
        } else {
            let analog_pin = io.pins.gpio2.into_analog();
        }
    }

    // Create ADC instances
    let mut adc1_config = AdcConfig::new();
    let mut adc1_pin = adc1_config.enable_pin(analog_pin, Attenuation::Attenuation11dB);
    let mut adc1 = ADC::<ADC1>::new(peripherals.ADC1, adc1_config);

    let mut delay = Delay::new(&clocks);

    let mut rng = esp_hal::rng::Rng::new(peripherals.RNG);
    loop {
        let pin_value: u16 = nb::block!(adc1.read(&mut adc1_pin)).unwrap();
        println!("ADC reading = {} - random = {}", pin_value, rng.random());
        delay.delay_ms(1500u32);
    }
}

The ADC reading will be 0, then.

cc @playfulFence

[playfulFence]

Will take a look on Monday, thanks for a ping!

[playfulFence]

Here we need the function I created while working on a previous problem with TRNG, but it wasn't needed. It "undos" the TRNG changes to registers (which I called revert_trng) revert_trng will make ADC work with TRNG initialized, BUT... The problem is TRNG constructor changes ADC registers and stuff and calling the ADC constructor after it will configure things in a way to make ADC work (but RNG is not TRNG anymore) Having TRNG is nice, but working ADC is way better, that is why we deprecated TRNG functionality and corresponding CryptoRng trait FOR NOW

[playfulFence]

Possible solutions: 1) have two separate constructors for RNG (normal) and TRNG (will also take ADC peripheral so user won't be able to use them together) 2) Just provide a revert_trng for user, which is not a good solution from the point of "user-friendliness" (and many other points I guess) 3) somehow ensure that revert_trng was called where and when needed after TRNG initialization (is it possible?)

First one sounds a bit "hacky", third one sounds impossible/difficult.

IDF seems to be using the second approach, which means that random_disable function is being provided to user "on his own risk" : https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/system/random.html

I like the IDF way in this case: We'll make "RNG" be "TRNG" by default, then provide function (revert_trng) to revert the "True" part and make it be just RNG again. However, it's not the most user-friendly approach, I'll think a bit more about other options.

On the other hand, having an explicit interface AND constructor for the TRNG is better then giving the "unsafe sword" to the user.

Feel free to post your thoughts here

[bjoernQ]

I don't think the first option is hacky - it's actually enabling Rng to operate in two different modes. Users could use it to seed a software rng from true random numbers and then drop it to use ADC if they really need to use it.

IMHO actually the cleanest option