Periodically check int lines instead of relying on interrupts

src/battery/battery.cpp

@@ -35,7 +35,7 @@ static const uint32_t kEmptyChargeMilliVolts = 3200;  // BMS limit is 3100.
 
 using ChargeStatus = drivers::Samd::ChargeStatus;
 
-void check_voltage_cb(TimerHandle_t timer) {
+static void check_voltage_cb(TimerHandle_t timer) {
   Battery* instance = reinterpret_cast<Battery*>(pvTimerGetTimerID(timer));
   instance->Update();
 }

src/drivers/gpios.cpp

@@ -6,12 +6,13 @@
 
 #include "gpios.hpp"
 
-#include <stdint.h>
-
 #include <cstdint>
 
+#include "assert.h"
 #include "driver/gpio.h"
 #include "esp_attr.h"
+#include "esp_err.h"
+#include "esp_intr_alloc.h"
 #include "hal/gpio_types.h"
 
 #include "i2c.hpp"
@@ -60,12 +61,7 @@ constexpr std::pair<uint8_t, uint8_t> unpack(uint16_t ba) {
   return std::pair((uint8_t)ba, (uint8_t)(ba >> 8));
 }
 
-SemaphoreHandle_t Gpios::sReadPending;
-
-IRAM_ATTR static void interrupt_isr(void* arg) {
-  SemaphoreHandle_t sem = reinterpret_cast<SemaphoreHandle_t>(arg);
-  xSemaphoreGive(sem);
-}
+static constexpr gpio_num_t kIntPin = GPIO_NUM_34;
 
 auto Gpios::Create() -> Gpios* {
   Gpios* instance = new Gpios();
@@ -78,22 +74,10 @@ auto Gpios::Create() -> Gpios* {
 }
 
 Gpios::Gpios() : ports_(pack(kPortADefault, kPortBDefault)), inputs_(0) {
-  gpio_config_t config{
-      .pin_bit_mask = static_cast<uint64_t>(1) << GPIO_NUM_34,
-      .mode = GPIO_MODE_INPUT,
-      .pull_up_en = GPIO_PULLUP_ENABLE,
-      .pull_down_en = GPIO_PULLDOWN_DISABLE,
-      .intr_type = GPIO_INTR_NEGEDGE,
-  };
-  gpio_config(&config);
-  gpio_install_isr_service(ESP_INTR_FLAG_LEVEL1);
-  gpio_isr_handler_add(GPIO_NUM_34, &interrupt_isr, sReadPending);
+  gpio_set_direction(kIntPin, GPIO_MODE_INPUT);
 }
 
-Gpios::~Gpios() {
-  gpio_isr_handler_remove(GPIO_NUM_34);
-  gpio_uninstall_isr_service();
-}
+Gpios::~Gpios() {}
 
 auto Gpios::WriteBuffered(Pin pin, bool value) -> void {
   if (value) {
@@ -142,9 +126,4 @@ auto Gpios::Read() -> bool {
   return true;
 }
 
-auto Gpios::CreateReadPending() -> SemaphoreHandle_t {
-  sReadPending = xSemaphoreCreateBinary();
-  return sReadPending;
-}
-
 }  // namespace drivers

src/drivers/include/gpios.hpp

@@ -10,6 +10,7 @@
 
 #include <atomic>
 #include <functional>
+#include <map>
 #include <memory>
 #include <mutex>
 #include <optional>
@@ -108,8 +109,6 @@ class Gpios : public IGpios {
    */
   auto Read(void) -> bool;
 
-  static auto CreateReadPending() -> SemaphoreHandle_t;
-
   // Not copyable or movable. There should usually only ever be once instance
   // of this class, and that instance will likely have a static lifetime.
   Gpios(const Gpios&) = delete;
@@ -120,8 +119,6 @@ class Gpios : public IGpios {
 
   std::atomic<uint16_t> ports_;
   std::atomic<uint16_t> inputs_;
-
-  static SemaphoreHandle_t sReadPending;
 };
 
 }  // namespace drivers

src/drivers/include/samd.hpp

@@ -54,8 +54,6 @@ class Samd {
   auto ResetToFlashSamd() -> void;
   auto PowerDown() -> void;
 
-  static auto CreateReadPending() -> SemaphoreHandle_t;
-
   // Not copyable or movable. There should usually only ever be once instance
   // of this class, and that instance will likely have a static lifetime.
   Samd(const Samd&) = delete;
@@ -64,8 +62,6 @@ class Samd {
  private:
   std::optional<ChargeStatus> charge_status_;
   UsbStatus usb_status_;
-
-  static SemaphoreHandle_t sReadPending;
 };
 
 }  // namespace drivers

src/drivers/samd.cpp

@@ -5,12 +5,15 @@
  */
 
 #include "samd.hpp"
-#include <stdint.h>
+
+#include <cstdint>
 #include <optional>
 
 #include "esp_err.h"
 #include "esp_log.h"
+#include "hal/gpio_types.h"
 #include "hal/i2c_types.h"
+
 #include "i2c.hpp"
 
 enum Registers : uint8_t {
@@ -26,23 +29,10 @@ static const uint8_t kAddress = 0x45;
 
 namespace drivers {
 
-SemaphoreHandle_t Samd::sReadPending;
-
-static void interrupt_isr(void* arg) {
-  SemaphoreHandle_t sem = reinterpret_cast<SemaphoreHandle_t>(arg);
-  xSemaphoreGive(sem);
-}
+static constexpr gpio_num_t kIntPin = GPIO_NUM_35;
 
 Samd::Samd() {
-  gpio_config_t config{
-      .pin_bit_mask = static_cast<uint64_t>(1) << GPIO_NUM_35,
-      .mode = GPIO_MODE_INPUT,
-      .pull_up_en = GPIO_PULLUP_ENABLE,
-      .pull_down_en = GPIO_PULLDOWN_DISABLE,
-      .intr_type = GPIO_INTR_NEGEDGE,
-  };
-  gpio_config(&config);
-  gpio_isr_handler_add(GPIO_NUM_35, &interrupt_isr, sReadPending);
+  gpio_set_direction(kIntPin, GPIO_MODE_INPUT);
 
   // Being able to interface with the SAMD properly is critical. To ensure we
   // will be able to, we begin by checking the I2C protocol version is
@@ -153,9 +143,4 @@ auto Samd::PowerDown() -> void {
   ESP_ERROR_CHECK(transaction.Execute(3));
 }
 
-auto Samd::CreateReadPending() -> SemaphoreHandle_t {
-  sReadPending = xSemaphoreCreateBinary();
-  return sReadPending;
-}
-
 }  // namespace drivers

src/main/main.cpp

@@ -6,7 +6,6 @@
 
 #include "freertos/portmacro.h"
 
-#include "gpios.hpp"
 #include "i2c.hpp"
 #include "system_events.hpp"
 #include "tinyfsm.hpp"
@@ -18,32 +17,12 @@
 
 extern "C" void app_main(void) {
   ESP_ERROR_CHECK(drivers::init_i2c());
-  SemaphoreHandle_t gpios_semphr = drivers::Gpios::CreateReadPending();
-  SemaphoreHandle_t samd_semphr = drivers::Samd::CreateReadPending();
-
-  // Semaphores must be empty before being added to a queue set. Hence all this
-  // weird early init stuff; by being explicit about initialisation order, we're
-  // able to handle GPIO ISR notifcations + system events from the same task,
-  // and a little mess with worth not needing to allocate a whole extra stack.
-  QueueSetHandle_t set = xQueueCreateSet(3);
-  auto* event_queue = events::queues::SystemAndAudio();
-  xQueueAddToSet(event_queue->has_events(), set);
-  xQueueAddToSet(gpios_semphr, set);
-  xQueueAddToSet(samd_semphr, set);
 
   tinyfsm::FsmList<system_fsm::SystemState, ui::UiState,
                    audio::AudioState>::start();
 
+  auto* event_queue = events::queues::SystemAndAudio();
   while (1) {
-    QueueSetMemberHandle_t member = xQueueSelectFromSet(set, portMAX_DELAY);
-    if (member == event_queue->has_events()) {
-      event_queue->Service(0);
-    } else if (member == gpios_semphr) {
-      xSemaphoreTake(member, 0);
-      events::System().Dispatch(system_fsm::internal::GpioInterrupt{});
-    } else if (member == samd_semphr) {
-      xSemaphoreTake(member, 0);
-      events::System().Dispatch(system_fsm::internal::SamdInterrupt{});
-    }
+    event_queue->Service(portMAX_DELAY);
   }
 }

src/system_fsm/booting.cpp

@@ -51,6 +51,8 @@ static auto bt_event_cb(drivers::bluetooth::Event ev) -> void {
   }
 }
 
+static const TickType_t kInterruptCheckPeriod = pdMS_TO_TICKS(100);
+
 auto Booting::entry() -> void {
   ESP_LOGI(kTag, "beginning tangara boot");
   sServices.reset(new ServiceLocator());
@@ -109,6 +111,10 @@ auto Booting::exit() -> void {
   sAppConsole = new console::AppConsole();
   sAppConsole->sServices = sServices;
   sAppConsole->Launch();
+
+  TimerHandle_t timer = xTimerCreate("INTERRUPTS", kInterruptCheckPeriod, true,
+                                     NULL, check_interrupts_cb);
+  xTimerStart(timer, portMAX_DELAY);
 }
 
 auto Booting::react(const BootComplete& ev) -> void {

src/system_fsm/include/system_fsm.hpp

@@ -32,6 +32,8 @@
 
 namespace system_fsm {
 
+void check_interrupts_cb(TimerHandle_t timer);
+
 /*
  * State machine for the overall system state. Responsible for managing
  * peripherals, and bringing the rest of the system up and down.

src/system_fsm/system_fsm.cpp

@@ -6,6 +6,7 @@
 
 #include "system_fsm.hpp"
 #include "audio_fsm.hpp"
+#include "driver/gpio.h"
 #include "event_queue.hpp"
 #include "gpios.hpp"
 #include "relative_wheel.hpp"
@@ -23,6 +24,15 @@ std::unique_ptr<drivers::SdStorage> SystemState::sStorage;
 
 console::AppConsole* SystemState::sAppConsole;
 
+void check_interrupts_cb(TimerHandle_t timer) {
+  if (!gpio_get_level(GPIO_NUM_34)) {
+    events::System().Dispatch(internal::GpioInterrupt{});
+  }
+  if (!gpio_get_level(GPIO_NUM_35)) {
+    events::System().Dispatch(internal::SamdInterrupt{});
+  }
+}
+
 void SystemState::react(const FatalError& err) {
   if (!is_in_state<states::Error>()) {
     transit<states::Error>();