Clarify restoration of interrupt flag as needed.

Author: micromouseonline

BasicEncoder.h

@@ -38,9 +38,7 @@ class BasicEncoder {
   }
   ~BasicEncoder() {}
 
-  void begin() {
-    reset();
-  }
+  void begin() { reset(); }
 
   int8_t pin_state() {
     int8_t state_now = 0;
@@ -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
@@ -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; }

README.md

@@ -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.

examples/using-pin-change/using-pin-change.ino

@@ -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

library.json

@@ -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":