avr-rust / ruduino

Reusable components for the Arduino Uno.
Apache License 2.0
695 stars 49 forks source link

without_interrupts(): Restore previous interrupt state rather than always re-enabling interrupts #10

Open dylanmckay opened 5 years ago

dylanmckay commented 5 years ago

The without_interrupts() in PR #4 always enables interrupts when the closure finishes executing.

This could be problematic in libraries; a library using this function assumes that the program it is included in actually wants interrupts to be disabled, or even if interrupts were already disabled before the call to without_interrupts

We should make a copy of the the interrupt flag in the status register, and then restore the interrupt flag back to its previous state, not just re-enabled it.

shepmaster commented 5 years ago

store the interrupt flag in the status register

One thing that I think would be very interesting to explore would be to see if we could track the state of the interrupt flag via compile-time information. For example:

// This trait is not correct, but shows a general sketch of how it might look.

trait InterruptState {
    fn with_interrupts<R>(self, f: impl FnOnce() -> R) -> R;
    fn without_interrupts<R>(self, f: impl FnOnce() -> R) -> R;
}

struct InterruptsDisabled;

impl InterruptState for InterruptsDisabled {
    fn with_interrupts<R>(self, f: impl FnOnce() -> R) -> R {
        // set register
        let x = f();
        // unset register
        x
    }

    fn without_interrupts<R>(self, f: impl FnOnce() -> R) -> R{
        f()
    }
}

struct InterruptsEnabled;
// ditto

struct InterruptsUnknown;
// ditto

When we don't know the state, we use InterruptsUnknown which gets the current state. A obsessive program could thread this through everywhere, since IIRC we start the machine with interrupts in a known state.

dylanmckay commented 5 years ago

I think that's a good idea

stappersg commented 2 years ago

linking with #44