Skip to content

Commit

Permalink
Clarify restoration of interrupt flag as needed.
Browse files Browse the repository at this point in the history
  • Loading branch information
micromouseonline committed Apr 2, 2022
1 parent 4a8c08b commit 8c51bb7
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 12 deletions.
16 changes: 7 additions & 9 deletions BasicEncoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ class BasicEncoder {
}
~BasicEncoder() {}

void begin() {
reset();
}
void begin() { reset(); }

int8_t pin_state() {
int8_t state_now = 0;
Expand Down Expand Up @@ -90,7 +88,7 @@ class BasicEncoder {

// Read changes frequently enough that overflows cannot happen.
int8_t get_change() {
uint8_t sreg = SREG;
uint8_t sreg = SREG; // save the current interrupt enable flag
noInterrupts();
int8_t change = m_change;
// the switch statement can make better code because only optimised
Expand All @@ -108,24 +106,24 @@ class BasicEncoder {
m_change = 0;
break;
}
SREG = sreg;
SREG = sreg; // restore the previous interrupt enable flag state
return change;
}

int get_count() {
uint8_t sreg = SREG;
uint8_t sreg = SREG; // save the current interrupt enable flag
noInterrupts();
int count = m_steps / m_steps_per_count;
SREG = sreg;
SREG = sreg; // restore the previous interrupt enable flag state
return count;
}

void reset() {
uint8_t sreg = SREG;
uint8_t sreg = SREG; // save the current interrupt enable flag
noInterrupts();
m_steps = 0;
m_change = 0;
SREG = sreg;
SREG = sreg; // restore the previous interrupt enable flag state
}

void set_reverse() { m_reversed = true; }
Expand Down
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,8 @@ https://www.mikrocontroller.net/articles/Drehgeber

The library should work with multiple encoder instances. There is an example `two-encoders` that demonstrates this using a timer interrupt for polling. Pin-change interrupt implementations may also be fine if the encoders are in different groups though that has not yet been tested.

## A note about interrupts

In a few places, the code needs to disable interrupts to ensure that data values are not corrupted. Immediately before a call to `noInterrupts()`, the current value of the status register, `SREG`, is saved into a local variable. Once the critical code section is complete, the saved state of the interrupt enable flag is restored by simply copying the saved value of the status register back. The other flags are not critical in this context.

This method is used rather than simply enabling interrupts because interrupts may already have been disabled at the time the function is called and to re-enable interrupts when they should not be enabled might cause hard to track bugs in the user code.
4 changes: 2 additions & 2 deletions examples/using-pin-change/using-pin-change.ino
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ void pciSetup(byte pin) // Setup pin change interupt on pin
}

void setup_encoders(int a, int b) {
uint8_t old_sreg = SREG;
uint8_t old_sreg = SREG; // save the current interrupt enable flag
noInterrupts();
pciSetup(a);
pciSetup(b);
encoder.reset();
SREG = old_sreg;
SREG = old_sreg; // restore the previous interrupt enable flag state
}

ISR(PCINT2_vect) // pin change interrupt for D0 to D7
Expand Down
2 changes: 1 addition & 1 deletion library.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "BasicEncoder",
"version": "1.1.1",
"version": "1.1.2",
"description": "BasicEncoder counts pulses from one or more simple rotary encoder control knobs.",
"keywords": "encoder, rotary encoder, quadrature",
"repository":
Expand Down

0 comments on commit 8c51bb7

Please sign in to comment.