imxrt-rs / imxrt-hal

Rust for NXP i.MX RT
Apache License 2.0
133 stars 33 forks source link

Add tempmon abstruction #115

Closed Alexander89 closed 2 years ago

Alexander89 commented 2 years ago

I got a little bit side trapped by the Temperature Monitor (TEMPMON)

I thought it is barely simple and a great start in getting my hands dirty with the imxrt-ral ecosystem.

The interrupts are not really handy.


Open to discuss

I played around with the UX a little bit and finally, it is OK.

In my test app, I implemented a static alarm flag to note, when I have to enable the interrupts again. Another method would be, to stop the measurement process, but as soon you read the temperature, the interrupt is triggered again.

// skip rest

let mut t = peripherals.tempmon.init_with_measure_freq(0x100);
t.set_alarm_values(40.0, 70.0, 85.0);
let (low_alarm, high_alarm, panic_alarm) = t.alarm_values();
log::debug!("low {}, high {}, panic {}", low_alarm, high_alarm, panic_alarm);

// v2: AtomicBool 
static mut ALARM: bool = false;
#[cortex_m_rt::interrupt]
fn TEMP_LOW_HIGH() {
  unsafe { ALARM = true; }
  TempMon::disable_interrupts(true, false);
  cortex_m::asm::dsb();
}

loop {
  let temp = t.get_temp();
  if temp > low_alarm && temp < high_alarm && unsafe { alarm } == true {
    unsafe { alarm = false; }
    t.enable_interrupts(true, true);
  }
}

If anybody has a great idea on how to improve it, please let me know :smiley:

Alexander89 commented 2 years ago

Consider someone calls get_temp(), or measure_temp on an unpowered tempmon, what happens if it's off?

Actually, get_temp() still works or it just returns old data. So, somehow it doesn't really work, though. But measure_temp() will just loop to the end of time.

I will add a power_on check and as you mention and I turn the values into a Result.

One additional question:

Should I change the messure_temp() into non-blocking and return WouldBlock in the blocking case. (nb::block!() compatible)

teburd commented 2 years ago

Should I change the messure_temp() into non-blocking and return WouldBlock in the blocking case. (nb::block!() compatible)

Seems reasonable, a Result return here makes sense as it is fallible if the device isn't on. Perhaps using nb here would be nice, I haven't used it in an async way myself though so I can't comment one way or the other here

Alexander89 commented 2 years ago

ich changed the math to °mC and adopted the docs to that.

In addition, I implemented an error handling to deal with the power_down state.

The temp_mon.measure_temp() is now compatible to nb::block!()

I also ran cargo clippy and cargo fmt, I hope the pipeline is now happy :-)

Alexander89 commented 2 years ago

@mciantyre, I created an example app to test this code, should I add it to teensy4-rs/examples?

Alexander89 commented 2 years ago

Thanks for all your patience! I learned a lot about the RAL layer and many other things. :pray: :rocket:

I think this is it now :tada:

teburd commented 2 years ago

Noted the places where I saw units for parameters and returns might be useful, I believe its mC -> C and vice versa, but would be nice to have it noted there.