Refactor Encoder code, Encoder → AHEncoder

examples/1. MIDI Output/3. Rotary Encoders/RotaryEncoder/RotaryEncoder/RotaryEncoder.ino

@@ -3,7 +3,7 @@
  * can be used for changing effect parameters, volumes, pan and balance
  * controls, etc.
  *
- * @boards  AVR, AVR USB, Nano Every, Due, Nano 33 IoT, Nano 33 BLE, Pi Pico, Teensy 3.x, ESP32
+ * @boards  AVR, AVR USB, Nano Every, Due, Nano 33 IoT, Nano 33 BLE, Pi Pico, Teensy 3.x, ESP32, ESP8266
  * 
  * Connections
  * -----------

mock/Libraries/Encoder/Encoder.h

@@ -7,11 +7,12 @@ class EncoderMock {
     MOCK_CONST_METHOD0(read, long());
 };
 
-class Encoder {
+class AHEncoder {
   public:
-    Encoder(int, int) : mock(nullptr) {}
-    Encoder(const EncoderMock &mock) : mock(&mock) {}
-    Encoder(const Encoder &) = default;
+    AHEncoder(int, int) : mock(nullptr) {}
+    AHEncoder(const EncoderMock &mock) : mock(&mock) {}
+    AHEncoder(EncoderMock &&) = delete;
+    AHEncoder(const AHEncoder &) = default;
     long read() const { return mock ? mock->read() : 0; }
 
   private:

src/MIDI_Outputs/Abstract/MIDIAbsoluteEncoder.hpp

@@ -8,7 +8,7 @@
 #include <MIDI_Outputs/Abstract/MIDIOutputElement.hpp>
 
 #ifdef ARDUINO
-#include <Submodules/Encoder/Encoder.h>
+#include <Submodules/Encoder/AHEncoder.hpp>
 #else
 #include <Encoder.h> // Mock
 #endif
@@ -85,11 +85,11 @@ class GenericMIDIAbsoluteEncoder : public MIDIOutputElement {
 };
 
 template <class Sender>
-using MIDIAbsoluteEncoder = GenericMIDIAbsoluteEncoder<Encoder, Sender>;
+using MIDIAbsoluteEncoder = GenericMIDIAbsoluteEncoder<AHEncoder, Sender>;
 
 template <class Sender>
 using BorrowedMIDIAbsoluteEncoder =
-    GenericMIDIAbsoluteEncoder<Encoder &, Sender>;
+    GenericMIDIAbsoluteEncoder<AHEncoder &, Sender>;
 
 END_CS_NAMESPACE
 

src/MIDI_Outputs/Abstract/MIDIRotaryEncoder.hpp

@@ -7,7 +7,7 @@
 #include <MIDI_Outputs/Abstract/MIDIOutputElement.hpp>
 
 #ifdef ARDUINO
-#include <Submodules/Encoder/Encoder.h>
+#include <Submodules/Encoder/AHEncoder.hpp>
 #else
 #include <Encoder.h> // Mock
 #endif
@@ -62,10 +62,10 @@ class GenericMIDIRotaryEncoder : public MIDIOutputElement {
 };
 
 template <class Sender>
-using MIDIRotaryEncoder = GenericMIDIRotaryEncoder<Encoder, Sender>;
+using MIDIRotaryEncoder = GenericMIDIRotaryEncoder<AHEncoder, Sender>;
 
 template <class Sender>
-using BorrowedMIDIRotaryEncoder = GenericMIDIRotaryEncoder<Encoder &, Sender>;
+using BorrowedMIDIRotaryEncoder = GenericMIDIRotaryEncoder<AHEncoder &, Sender>;
 
 END_CS_NAMESPACE
 

src/MIDI_Outputs/Bankable/Abstract/MIDIAbsoluteEncoder.hpp

@@ -8,7 +8,7 @@
 #include <MIDI_Outputs/Abstract/MIDIOutputElement.hpp>
 
 #ifdef ARDUINO
-#include <Submodules/Encoder/Encoder.h>
+#include <Submodules/Encoder/AHEncoder.hpp>
 #else
 #include <Encoder.h> // Mock
 #endif
@@ -98,11 +98,11 @@ class GenericMIDIAbsoluteEncoder : public MIDIOutputElement {
 
 template <uint8_t NumBanks, class BankAddress, class Sender>
 using MIDIAbsoluteEncoder =
-    GenericMIDIAbsoluteEncoder<Encoder, NumBanks, BankAddress, Sender>;
+    GenericMIDIAbsoluteEncoder<AHEncoder, NumBanks, BankAddress, Sender>;
 
 template <uint8_t NumBanks, class BankAddress, class Sender>
 using BorrowedMIDIAbsoluteEncoder =
-    GenericMIDIAbsoluteEncoder<Encoder &, NumBanks, BankAddress, Sender>;
+    GenericMIDIAbsoluteEncoder<AHEncoder &, NumBanks, BankAddress, Sender>;
 
 } // namespace Bankable
 

src/MIDI_Outputs/Bankable/Abstract/MIDIRotaryEncoder.hpp

@@ -8,7 +8,7 @@
 #include <MIDI_Outputs/Abstract/MIDIOutputElement.hpp>
 
 #ifdef ARDUINO
-#include <Submodules/Encoder/Encoder.h>
+#include <Submodules/Encoder/AHEncoder.hpp>
 #else
 #include <Encoder.h> // Mock
 #endif
@@ -63,11 +63,11 @@ class GenericMIDIRotaryEncoder : public MIDIOutputElement {
 
 template <class BankAddress, class Sender>
 using MIDIRotaryEncoder =
-    GenericMIDIRotaryEncoder<Encoder, BankAddress, Sender>;
+    GenericMIDIRotaryEncoder<AHEncoder, BankAddress, Sender>;
 
 template <class BankAddress, class Sender>
 using BorrowedMIDIRotaryEncoder =
-    GenericMIDIRotaryEncoder<Encoder &, BankAddress, Sender>;
+    GenericMIDIRotaryEncoder<AHEncoder &, BankAddress, Sender>;
 
 } // namespace Bankable
 

src/MIDI_Outputs/Bankable/CCAbsoluteEncoder.hpp

@@ -55,7 +55,7 @@ class CCAbsoluteEncoder
      *          speed, increasing the number of pulsesPerStep will result in a 
      *          lower speed.
      */
-    CCAbsoluteEncoder(BankConfig<NumBanks> config, Encoder &&encoder,
+    CCAbsoluteEncoder(BankConfig<NumBanks> config, AHEncoder &&encoder,
                       MIDIAddress address, int16_t speedMultiply = 1,
                       uint8_t pulsesPerStep = 4)
         : MIDIAbsoluteEncoder<NumBanks, SingleAddress, ContinuousCCSender>(

src/MIDI_Outputs/Bankable/CCRotaryEncoder.hpp

@@ -51,7 +51,7 @@ class CCRotaryEncoder
      *          speed, increasing the number of pulsesPerStep will result in a 
      *          lower speed.
      */
-    CCRotaryEncoder(OutputBankConfig<> config, Encoder &&encoder,
+    CCRotaryEncoder(OutputBankConfig<> config, AHEncoder &&encoder,
                     MIDIAddress address, int16_t speedMultiply = 1,
                     uint8_t pulsesPerStep = 4)
         : MIDIRotaryEncoder<SingleAddress, RelativeCCSender>(

src/MIDI_Outputs/Bankable/PBAbsoluteEncoder.hpp

@@ -57,7 +57,7 @@ class PBAbsoluteEncoder
      *          speed, increasing the number of pulsesPerStep will result in a 
      *          lower speed.
      */
-    PBAbsoluteEncoder(const BankConfig<NumBanks> &config, Encoder &&encoder,
+    PBAbsoluteEncoder(const BankConfig<NumBanks> &config, AHEncoder &&encoder,
                       MIDIChannelCable address, int16_t speedMultiply = 1,
                       uint8_t pulsesPerStep = 4)
         : MIDIAbsoluteEncoder<NumBanks, SingleAddress, PitchBendSender<14>>(

src/MIDI_Outputs/CCAbsoluteEncoder.hpp

@@ -42,7 +42,7 @@ class CCAbsoluteEncoder : public MIDIAbsoluteEncoder<ContinuousCCSender> {
      *          speed, increasing the number of pulsesPerStep will result in a 
      *          lower speed.
      */
-    CCAbsoluteEncoder(Encoder &&encoder, MIDIAddress address,
+    CCAbsoluteEncoder(AHEncoder &&encoder, MIDIAddress address,
                       int16_t multiplier = 1, uint8_t pulsesPerStep = 4)
         : MIDIAbsoluteEncoder<ContinuousCCSender>(
               std::move(encoder), address, multiplier, pulsesPerStep, {}) {}

src/MIDI_Outputs/CCRotaryEncoder.hpp

@@ -43,7 +43,7 @@ class CCRotaryEncoder : public MIDIRotaryEncoder<RelativeCCSender> {
      *          speed, increasing the number of pulsesPerStep will result in a 
      *          lower speed.
      */
-    CCRotaryEncoder(Encoder &&encoder, MIDIAddress address,
+    CCRotaryEncoder(AHEncoder &&encoder, MIDIAddress address,
                     int16_t speedMultiply = 1, uint8_t pulsesPerStep = 4)
         : MIDIRotaryEncoder<RelativeCCSender>(
               std::move(encoder), address, speedMultiply, pulsesPerStep, {}) {}
@@ -86,7 +86,7 @@ class BorrowedCCRotaryEncoder
      *          speed, increasing the number of pulsesPerStep will result in a 
      *          lower speed.
      */
-    BorrowedCCRotaryEncoder(Encoder &encoder, MIDIAddress address,
+    BorrowedCCRotaryEncoder(AHEncoder &encoder, MIDIAddress address,
                             int16_t speedMultiply = 1, uint8_t pulsesPerStep = 4)
         : BorrowedMIDIRotaryEncoder<RelativeCCSender>(
               encoder, address, speedMultiply, pulsesPerStep, {}) {}

src/MIDI_Outputs/ManyAddresses/CCAbsoluteEncoder.hpp

@@ -57,7 +57,7 @@ class CCAbsoluteEncoder
      *          speed, increasing the number of pulsesPerStep will result in a 
      *          lower speed.
      */
-    CCAbsoluteEncoder(const Bank<NumBanks> &bank, Encoder &&encoder,
+    CCAbsoluteEncoder(const Bank<NumBanks> &bank, AHEncoder &&encoder,
                       const Array<MIDIAddress, NumBanks> &addresses,
                       int16_t speedMultiply = 1, uint8_t pulsesPerStep = 4)
         : MIDIAbsoluteEncoder<NumBanks, ManyAddresses<NumBanks>,

src/MIDI_Outputs/ManyAddresses/CCRotaryEncoder.hpp

@@ -52,7 +52,7 @@ class CCRotaryEncoder
      *          speed, increasing the number of pulsesPerStep will result in a 
      *          lower speed.
      */
-    CCRotaryEncoder(const Bank<NumBanks> &bank, Encoder &&encoder,
+    CCRotaryEncoder(const Bank<NumBanks> &bank, AHEncoder &&encoder,
                     const Array<MIDIAddress, NumBanks> &addresses,
                     int16_t speedMultiply = 1, uint8_t pulsesPerStep = 4)
         : MIDIRotaryEncoder<ManyAddresses<NumBanks>, RelativeCCSender>(

src/MIDI_Outputs/ManyAddresses/PBAbsoluteEncoder.hpp

@@ -58,7 +58,7 @@ class PBAbsoluteEncoder
      *          speed, increasing the number of pulsesPerStep will result in a 
      *          lower speed.
      */
-    PBAbsoluteEncoder(const Bank<NumBanks> &bank, Encoder &&encoder,
+    PBAbsoluteEncoder(const Bank<NumBanks> &bank, AHEncoder &&encoder,
                       const Array<MIDIChannelCable, NumBanks> &addresses,
                       int16_t speedMultiply = 1, uint8_t pulsesPerStep = 4)
         : MIDIAbsoluteEncoder<NumBanks, ManyAddresses<NumBanks>,

src/MIDI_Outputs/PBAbsoluteEncoder.hpp

@@ -43,7 +43,7 @@ class PBAbsoluteEncoder : public MIDIAbsoluteEncoder<PitchBendSender<14>> {
      *          speed, increasing the number of pulsesPerStep will result in a 
      *          lower speed.
      */
-    PBAbsoluteEncoder(Encoder &&encoder, MIDIChannelCable address,
+    PBAbsoluteEncoder(AHEncoder &&encoder, MIDIChannelCable address,
                       int16_t multiplier = 1, uint8_t pulsesPerStep = 4)
         : MIDIAbsoluteEncoder<PitchBendSender<14>>(
               std::move(encoder), address, multiplier, pulsesPerStep, {}) {}

src/Selectors/EncoderSelector.hpp

@@ -6,7 +6,7 @@
 #include <Def/TypeTraits.hpp>
 
 #ifdef ARDUINO
-#include <Submodules/Encoder/Encoder.h>
+#include <Submodules/Encoder/AHEncoder.hpp>
 #else
 #include <Encoder.h> // Mock
 #endif
@@ -58,7 +58,7 @@ class GenericEncoderSelector : public GenericSelector<N, Callback> {
     }
 
   private:
-    Encoder encoder;
+    AHEncoder encoder;
     pin_t switchPin;
     int8_t pulsesPerStep;
     Wrap wrap;

src/Submodules/Encoder/AHEncoder.cpp

@@ -0,0 +1,136 @@
+#ifdef ARDUINO
+
+#include "AHEncoder.hpp"
+
+#include <AH/STL/utility> // std::swap
+
+BEGIN_CS_NAMESPACE
+
+#include "codegen/ISRs-def.ipp"
+
+AHEncoder::AHEncoder(uint8_t pinA, uint8_t pinB)
+    : pins {pinA, pinB}, direct_pins {direct_pin_read(pinA),
+                                      direct_pin_read(pinB)} {
+    // It's much faster to use the GPIO registers directly, rather than
+    // calling digitalRead every time we need to read a pin.
+    // digitalRead looks up the register and bitmasks every time it's called
+    // but here, we look them up once in the constructor.
+}
+
+AHEncoder::AHEncoder(AHEncoder &&other) { swap(*this, other); }
+
+AHEncoder &AHEncoder::operator=(AHEncoder &&other) {
+    swap(*this, other);
+    return *this;
+}
+
+void swap(AHEncoder &a, AHEncoder &b) {
+    // First swap the normal member variables:
+    std::swap(a.interrupts_in_use, b.interrupts_in_use);
+    std::swap(a.pins, b.pins);
+
+    // Next, update the pointers in interruptArgs:
+    // When interrupts are in use, there is a global interrupt context
+    // variable that holds a pointer to the encoders that are being swapped
+    // or moved.
+    // After moving, this pointer would no longer be valid, so it has to be
+    // changed to point to the new encoder object.
+    // Calling attachInterrupt is not necessary, because this should already
+    // have happened in the begin method if interrupts_in_use is nonzero.
+    // Before messing with the state variables that can be changed or
+    // accessed from within an ISR, we have to disable interrupts.
+    noInterrupts();
+    std::swap(a.state, b.state);
+    std::swap(a.direct_pins, b.direct_pins);
+    std::swap(a.position, b.position);
+    if (a.interrupts_in_use > 0) {
+        int int1 = digitalPinToInterrupt(a.pins[0]);
+        if (int1 != NOT_AN_INTERRUPT)
+            AHEncoder::interruptArgs[int1] = &a;
+        int int2 = digitalPinToInterrupt(a.pins[1]);
+        if (int2 != NOT_AN_INTERRUPT)
+            AHEncoder::interruptArgs[int2] = &a;
+    }
+    if (b.interrupts_in_use > 0) {
+        int int1 = digitalPinToInterrupt(b.pins[0]);
+        if (int1 != NOT_AN_INTERRUPT)
+            AHEncoder::interruptArgs[int1] = &b;
+        int int2 = digitalPinToInterrupt(b.pins[1]);
+        if (int2 != NOT_AN_INTERRUPT)
+            AHEncoder::interruptArgs[int2] = &b;
+    }
+    interrupts();
+}
+
+AHEncoder::~AHEncoder() {
+    // If interrupts are in use, there are dangling pointers to the encoder
+    // state in the global interrupt contexts. These have to be detached
+    // when the encoder object is removed.
+    end();
+}
+
+void AHEncoder::begin() {
+    pinMode(pins[0], INPUT_PULLUP);
+    pinMode(pins[1], INPUT_PULLUP);
+    // allow time for a passive R-C filter to charge
+    // through the pullup resistors, before reading
+    // the initial state
+    delayMicroseconds(2000);
+    uint8_t s = 0;
+    if (direct_pins[0].read())
+        s |= 1;
+    if (direct_pins[1].read())
+        s |= 2;
+    state = s;
+
+    attachInterruptCtx(digitalPinToInterrupt(pins[0]));
+    attachInterruptCtx(digitalPinToInterrupt(pins[1]));
+#if defined(ARDUINO_ARCH_MBED) && !defined(ARDUINO_ARCH_RP2040)
+    // https://github.com/arduino/ArduinoCore-mbed/issues/253
+    pinMode(pins[0], INPUT_PULLUP);
+    pinMode(pins[1], INPUT_PULLUP);
+#endif
+}
+
+void AHEncoder::end() {
+    if (interrupts_in_use > 0) {
+        detachInterruptCtx(digitalPinToInterrupt(pins[0]));
+        detachInterruptCtx(digitalPinToInterrupt(pins[1]));
+    }
+}
+
+void AHEncoder::attachInterruptCtx(int interrupt) {
+    if (interrupt != NOT_AN_INTERRUPT) {
+        interruptArgs[interrupt] = this;
+        ++interrupts_in_use;
+#ifdef ARDUINO_ARCH_RP2040
+        gpio_set_irq_enabled_with_callback(
+            interrupt, GPIO_IRQ_EDGE_FALL | GPIO_IRQ_EDGE_RISE, true,
+            [](uint gpio, uint32_t) {
+                if (auto arg = interruptArgs[gpio])
+                    arg->update();
+            });
+#else
+        attachInterrupt(interrupt, EncoderISRs::getISR(interrupt), CHANGE);
+#endif
+    }
+}
+
+void AHEncoder::detachInterruptCtx(int interrupt) {
+    if (interrupt != NOT_AN_INTERRUPT) {
+#ifdef ARDUINO_ARCH_RP2040
+        gpio_set_irq_enabled(interrupt, GPIO_IRQ_EDGE_FALL | GPIO_IRQ_EDGE_RISE,
+                             false);
+#else
+        detachInterrupt(interrupt);
+#endif
+        --interrupts_in_use;
+        interruptArgs[interrupt] = nullptr;
+    }
+}
+
+AHEncoder *AHEncoder::interruptArgs[CS_ENCODER_ARGLIST_SIZE] {};
+
+END_CS_NAMESPACE
+
+#endif