Add functions for going to sleep

[TheZoq2]

As discussed in #64, it would be nice to have functions for entering the various power save modes as that process requires quite a few steps.

[mjepronk]

I'm currently using some code in my project. If you are interested I can polish it up and create a pull request. I currently have this API:

#[derive(PartialEq)]
enum SleepMode {
    Standby,
    Stop(bool), // true: voltage regulators in low power mode
}

fn enable_sleep_mode(sleep_mode: SleepMode, scb: &mut SCB, pwr: &mut PWR) {
    ...
}

Using it is as easy as calling enable_sleep_mode in main or init (RTFM) and call wfi or wfe when the MCU needs to go in to standby or stop mode.

I will read up on RM0008 to see if there are more features that would be interesting to support.

[TheZoq2]

That would be great!

I like the look of your API, though I would probably suggest StopHighPower and StopLowPower rather than Stop(bool).

If you want more inspiration, I have code for going to sleep here https://github.com/TheZoq2/weather/blob/master/stmhardware/src/main.rs#L366 though I'm suspecting it has problems because it uses more current than I would expect.

[mjepronk]

I've some code for this in my clone of this repository. However, there are some problems:

• I can't wake up from Sleep and Stop mode when entered using WFI (when using WFE it works fine). WFI works fine to exit from Standby mode. Also, WKUP pin does not work. I think it has something to do with Auto-wakeup (AWU) described in section 5.6.3 of the RM0008 reference manual. However, I've no idea how to fix it. I've tried several things (see my code). Maybe some of you might have an idea?
• The API is quite convoluted, I needed quite a lot of registers in the arguments.
• I've changed the design a little bit from my initial sketch above, notably I call WFI or WFE from the function myself. This is because we need to run different code when we want to wake-up from WFI or WFE.

Comments and/or help are welcome!

[TheZoq2]

Sorry for the delayed reply.

It has been a while since I had a look at the sleep functions so I can't comment that much on implementation details.

The API is quite convoluted, I needed quite a lot of registers in the arguments.

That's true, and it also seems like some of the arguments are only required if others are present. Perhaps something like the builder pattern would be useful.

Something along the lines of

SleepMode::new(sleep_mode, entry, &mut scb, &mut pwr)
    .enable_wakeup_alarm(&mut nvic, &mut exti)
    // nvic and exti are now mut borrowed twice, so something smarter must be done
    .enable_wakeup_pin(&mut nvic, &mut exti, &mut apb1)
    .debug(&mut dbgmcu)
    .enter()

Clearly it's not fully thought out, but might be a good place to start

[mjepronk]

Sorry for the delayed reply.

No problem!

It has been a while since I had a look at the sleep functions so I can't comment that much on implementation details.

OK, I'll try to find it out...

Perhaps something like the builder pattern would be useful.

Yeah, this is a good idea. If I perform the configuration of the registers on the configuration methods (instead of doing everything in the enter method) it works out fine with the borrow checker. See the latest version in my repository.

[TheZoq2]

Yeah, this is a good idea. If I perform the configuration of the registers on the configuration methods (instead of doing everything in the enter method) it works out fine with the borrow checker. See the latest version in my repository.

That looks pretty good to me, but just like my example, it doesn't seem to allow wakeup from both RTC and pins simultaneously which would be a nice feature to have. Unfortunately, I can't really come up with a neat way of achieving that.

[mjepronk]

Euhm, I'm pretty sure it does. Please see examples/sleep.rs... I'm still wondering why STOP mode does not wake-up when entering using WFI though... still working on that.

[TheZoq2]

Oh right, I was thinking that it stores the reference to the register in the builder struct which wouldn't be allowed.

Though I think that leads to another issue. The following code would not wake up from an alarm, despite the builder being configured to do so.

let builder = SleepModeBuilder::new(
                SleepMode::Standby,
                SleepModeEntry::WFI,
                &mut scb,
                &mut pwr)
        .enable_wakeup_alarm(10, &mut rtc, &mut nvic, &mut exti);

nvic.do_something_that_disables_wakeup_alarm();

builder.enter();

[mjepronk]

Yes, that's true. It may be unexpected behavior, though at the same time I think it will be rare to actually cause a real problem. I don't see how we can keep an intuitive API based on the builder pattern and at the same time prevent the situation you describe... So we need to do some concession, or use an altogether different design.

[TheZoq2]

I think we should avoid unexpected behavoiur for as long as possible.

Perhaps we could have two builder structs, one basic version, and one which has a mut reference to the extra registers. Something like this:

struct SleepModeBuilder<'a> {
    mode: SleepMode,
    entry: SleepModeEntry,
    scp: &mut scb,
    pwr: &mut pwr,
}

impl SleepModeBuilder {
    // The normal build functions

    fn enable_wakeup_alarm(..., rtc, nvic, exti) -> SleepModeBuilderWithNvic {

    }
}

struct SleepModeBuilderWithNvic<'a> {
    core: SleepModeBuilder<'a>,
    nvic: &'a mut ..
}

impl SleepModeBuilder {
    // Normal build functions
}

Another option might be to use some generic type magic and do something like this

struct NeedsExtraRegisters;

// T is some sort of internal state of the type. If T is (), only needs
// access to the normal registers on entry. If it's NeedsExtraRegisters, it does.
struct SleepModeBuilder<T> {
    mode: SleepMode,
    entry: SleepModeEntry,
    wake_from_rtc: bool,
    wake_from_exti: bool,
}

impl<T> SleepModeBuilder<T> {
    // You can only construct the struct with no extra registers. This is represented
    // by the unit type
    fn new(..) -> SleepModeBuilder<()> {}

    // Normal builder functions. Don't change the internal state
    fn debug_mode() -> SleepModeBuilder<T> {}


    // If we want to configure wakeup stuff, we transform the struct into one that
    // needs extra registers
    fn wake_from_exti() -> SleepModeBuilder<NeedsExtraRegisters> {
        ...
    }

    // If we want to configure wakeup stuff, we transform the struct into one that
    // needs extra registers
    fn wake_from_rtc() -> SleepModeBuilder<NeedsExtraRegisters> {
        ...
    }
}

// The enter function does all the actual work of configuring registers.
// Its implementation depends on the state
impl SleepModeBuilder<()> {
    fn enter(&mut scb, &mut pwr) {}
}

impl SleepModeBuilder<NeedsExtraRegisters> {
    fn enter(&mut scb, &mut pwr, &mut nvic, &mut exti) {}
}

I think I prefer the second option as it would result in less code duplication. Though it might be a bit overengineered

[RCasatta]

any update on this?