Add GPIO interrupt abstraction (esphome#535)

* Fix light mqtt_json warning

* Rewrite rotary encoder

* Rewrite interrupts

* Fix ESP8266 interrupt info

This Arduino API is just *horrible* to work with.

* Fix nextion initial ACK not needed

* Move nextion order

* Fixes

* Lint

src/esphome/api/basic_messages.cpp

@@ -9,8 +9,6 @@ ESPHOME_NAMESPACE_BEGIN
 
 namespace api {
 
-static const char *TAG = "api.message";
-
 // Hello
 bool HelloRequest::decode_length_delimited(uint32_t field_id, const uint8_t *value, size_t len) {
   switch (field_id) {

src/esphome/api/user_services.cpp

@@ -9,8 +9,6 @@ ESPHOME_NAMESPACE_BEGIN
 
 namespace api {
 
-static const char *TAG = "api.UserServices";
-
 template<> bool ExecuteServiceArgument::get_value<bool>() { return this->value_bool_; }
 template<> int ExecuteServiceArgument::get_value<int>() { return this->value_int_; }
 template<> float ExecuteServiceArgument::get_value<float>() { return this->value_float_; }

src/esphome/display/display.cpp

@@ -293,7 +293,7 @@ void DisplayBuffer::set_pages(std::vector<DisplayPage *> pages) {
   for (auto *page : pages)
     page->set_parent(this);
 
-  for (int i = 0; i < pages.size() - 1; i++) {
+  for (uint32_t i = 0; i < pages.size() - 1; i++) {
     pages[i]->set_next(pages[i + 1]);
     pages[i + 1]->set_prev(pages[i]);
   }

src/esphome/display/nextion.cpp

@@ -13,9 +13,8 @@ static const char *TAG = "display.nextion";
 
 void Nextion::setup() {
   this->send_command_no_ack("");
-  this->ack_();
-  this->goto_page("0");
   this->send_command_printf("bkcmd=3");
+  this->goto_page("0");
 }
 float Nextion::get_setup_priority() const { return setup_priority::POST_HARDWARE; }
 void Nextion::update() {

src/esphome/esphal.cpp

@@ -3,14 +3,20 @@
 #include "esphome/log.h"
 
 #ifdef ARDUINO_ARCH_ESP8266
-#include "FunctionalInterrupt.h"
-extern "C" void ICACHE_RAM_ATTR __attachInterruptArg(uint8_t pin, void (*)(void*), void* fp,  // NOLINT
-                                                     int mode);
+extern "C" {
+typedef struct {        // NOLINT
+  void *interruptInfo;  // NOLINT
+  void *functionInfo;   // NOLINT
+} ArgStructure;
+
+void ICACHE_RAM_ATTR __attachInterruptArg(uint8_t pin, void (*)(void *), void *fp,  // NOLINT
+                                          int mode);
+};
 #endif
 
 ESPHOME_NAMESPACE_BEGIN
 
-static const char* TAG = "esphal";
+static const char *TAG = "esphal";
 
 GPIOPin::GPIOPin(uint8_t pin, uint8_t mode, bool inverted)
     : pin_(pin),
@@ -29,8 +35,8 @@ GPIOPin::GPIOPin(uint8_t pin, uint8_t mode, bool inverted)
 {
 }
 
-const char* GPIOPin::get_pin_mode_name() const {
-  const char* mode_s;
+const char *GPIOPin::get_pin_mode_name() const {
+  const char *mode_s;
   switch (this->mode_) {
     case INPUT:
       mode_s = "INPUT";
@@ -114,6 +120,9 @@ void GPIOPin::setup() { this->pin_mode(this->mode_); }
 bool ICACHE_RAM_ATTR HOT GPIOPin::digital_read() {
   return bool((*this->gpio_read_) & this->gpio_mask_) != this->inverted_;
 }
+bool ICACHE_RAM_ATTR HOT ISRInternalGPIOPin::digital_read() {
+  return bool((*this->gpio_read_) & this->gpio_mask_) != this->inverted_;
+}
 void ICACHE_RAM_ATTR HOT GPIOPin::digital_write(bool value) {
 #ifdef ARDUINO_ARCH_ESP8266
   if (this->pin_ != 16) {
@@ -138,7 +147,59 @@ void ICACHE_RAM_ATTR HOT GPIOPin::digital_write(bool value) {
   }
 #endif
 }
-GPIOPin* GPIOPin::copy() const { return new GPIOPin(*this); }
+void ISRInternalGPIOPin::digital_write(bool value) {
+#ifdef ARDUINO_ARCH_ESP8266
+  if (this->pin_ != 16) {
+    if (value != this->inverted_) {
+      GPOS = this->gpio_mask_;
+    } else {
+      GPOC = this->gpio_mask_;
+    }
+  } else {
+    if (value != this->inverted_) {
+      GP16O |= 1;
+    } else {
+      GP16O &= ~1;
+    }
+  }
+#endif
+#ifdef ARDUINO_ARCH_ESP32
+  if (value != this->inverted_) {
+    (*this->gpio_set_) = this->gpio_mask_;
+  } else {
+    (*this->gpio_clear_) = this->gpio_mask_;
+  }
+#endif
+}
+ISRInternalGPIOPin::ISRInternalGPIOPin(uint8_t pin,
+#ifdef ARDUINO_ARCH_ESP32
+                                       volatile uint32_t *gpio_clear, volatile uint32_t *gpio_set,
+#endif
+                                       volatile uint32_t *gpio_read, uint32_t gpio_mask, bool inverted)
+    : pin_(pin),
+      gpio_read_(gpio_read),
+      gpio_mask_(gpio_mask),
+      inverted_(inverted)
+#ifdef ARDUINO_ARCH_ESP32
+      ,
+      gpio_clear_(gpio_clear),
+      gpio_set_(gpio_set)
+#endif
+{
+}
+void ICACHE_RAM_ATTR ISRInternalGPIOPin::clear_interrupt() {
+#ifdef ARDUINO_ARCH_ESP8266
+  GPIO_REG_WRITE(GPIO_STATUS_W1TC_ADDRESS, this->gpio_mask_);
+#endif
+#ifdef ARDUINO_ARCH_ESP32
+  if (this->pin_ < 32) {
+    GPIO.status_w1tc = this->gpio_mask_;
+  } else {
+    GPIO.status1_w1tc.intr_st = this->gpio_mask_;
+  }
+#endif
+}
+GPIOPin *GPIOPin::copy() const { return new GPIOPin(*this); }
 
 void ICACHE_RAM_ATTR HOT GPIOPin::pin_mode(uint8_t mode) { pinMode(this->pin_, mode); }
 
@@ -147,23 +208,51 @@ GPIOOutputPin::GPIOOutputPin(uint8_t pin, uint8_t mode, bool inverted) : GPIOPin
 GPIOInputPin::GPIOInputPin(uint8_t pin, uint8_t mode, bool inverted) : GPIOPin(pin, mode, inverted) {}
 
 #ifdef ARDUINO_ARCH_ESP8266
-void ICACHE_RAM_ATTR custom_interrupt_functional(void* arg) {
-  ArgStructure* local_arg = (ArgStructure*) arg;
-  if (local_arg->functionInfo->reqFunction) {
-    local_arg->functionInfo->reqFunction();
-  }
-}
+struct ESPHomeInterruptFuncInfo {
+  void (*func)(void *);
+  void *arg;
+};
 
-void ICACHE_RAM_ATTR attach_functional_interrupt(uint8_t pin, std::function<void()> func, int mode) {
-  FunctionInfo* fi = new FunctionInfo;
-  fi->reqFunction = func;
+void ICACHE_RAM_ATTR interrupt_handler(void *arg) {
+  ArgStructure *as = static_cast<ArgStructure *>(arg);
+  auto *info = static_cast<ESPHomeInterruptFuncInfo *>(as->functionInfo);
+  info->func(info->arg);
+}
+#endif
 
-  ArgStructure* as = new ArgStructure;
+void GPIOPin::attach_interrupt_(void (*func)(void *), void *arg, int mode) const {
+  if (this->inverted_) {
+    if (mode == RISING) {
+      mode = FALLING;
+    } else if (mode == FALLING) {
+      mode = RISING;
+    }
+  }
+#ifdef ARDUINO_ARCH_ESP8266
+  ArgStructure *as = new ArgStructure;
   as->interruptInfo = nullptr;
-  as->functionInfo = fi;
 
-  __attachInterruptArg(pin, custom_interrupt_functional, as, mode);
+  as->functionInfo = new ESPHomeInterruptFuncInfo{
+      .func = func,
+      .arg = arg,
+  };
+
+  __attachInterruptArg(this->pin_, interrupt_handler, as, mode);
+#endif
+#ifdef ARDUINO_ARCH_ESP32
+  // work around issue https://github.com/espressif/arduino-esp32/pull/1776 in arduino core
+  // yet again proves how horrible code is there :( - how could that have been accepted...
+  auto *attach = reinterpret_cast<void (*)(uint8_t, void (*)(void *), void *, int)>(attachInterruptArg);
+  attach(this->pin_, func, arg, mode);
+#endif
 }
+
+ISRInternalGPIOPin *GPIOPin::to_isr() const {
+  return new ISRInternalGPIOPin(this->pin_,
+#ifdef ARDUINO_ARCH_ESP32
+                                this->gpio_clear_, this->gpio_set_,
 #endif
+                                this->gpio_read_, this->gpio_mask_, this->inverted_);
+}
 
 ESPHOME_NAMESPACE_END

src/esphome/esphal.h

@@ -29,6 +29,29 @@ ESPHOME_NAMESPACE_BEGIN
 #define LOG_PIN_PATTERN "GPIO%u (Mode: %s%s)"
 #define LOG_PIN_ARGS(pin) (pin)->get_pin(), (pin)->get_pin_mode_name(), ((pin)->is_inverted() ? ", INVERTED" : "")
 
+/// Copy of GPIOPin that is safe to use from ISRs (with no virtual functions)
+class ISRInternalGPIOPin {
+ public:
+  ISRInternalGPIOPin(uint8_t pin,
+#ifdef ARDUINO_ARCH_ESP32
+                     volatile uint32_t *gpio_clear, volatile uint32_t *gpio_set,
+#endif
+                     volatile uint32_t *gpio_read, uint32_t gpio_mask, bool inverted);
+  bool digital_read();
+  void digital_write(bool value);
+  void clear_interrupt();
+
+ protected:
+  const uint8_t pin_;
+#ifdef ARDUINO_ARCH_ESP32
+  volatile uint32_t *const gpio_clear_;
+  volatile uint32_t *const gpio_set_;
+#endif
+  volatile uint32_t *const gpio_read_;
+  const uint32_t gpio_mask_;
+  const bool inverted_;
+};
+
 /** A high-level abstraction class that can expose a pin together with useful options like pinMode.
  *
  * Set the parameters for this at construction time and use setup() to apply them. The inverted parameter will
@@ -65,7 +88,13 @@ class GPIOPin {
   /// Return whether this pin shall be treated as inverted. (for example active-low)
   bool is_inverted() const;
 
+  template<typename T> void attach_interrupt(void (*func)(T *), T *arg, int mode) const;
+
+  ISRInternalGPIOPin *to_isr() const;
+
  protected:
+  void attach_interrupt_(void (*func)(void *), void *arg, int mode) const;
+
   const uint8_t pin_;
   const uint8_t mode_;
 #ifdef ARDUINO_ARCH_ESP32
@@ -77,10 +106,6 @@ class GPIOPin {
   const bool inverted_;
 };
 
-#ifdef ARDUINO_ARCH_ESP8266
-void attach_functional_interrupt(uint8_t pin, std::function<void()> func, int mode);
-#endif
-
 /**  Basically just a GPIOPin, but defaults to OUTPUT pinMode.
  *
  * Note that theoretically you can still assign an INPUT pinMode to this - we intentionally don't check this.
@@ -103,6 +128,10 @@ class GPIOInputPin : public GPIOPin {
   GPIOInputPin(uint8_t pin, uint8_t mode = INPUT, bool inverted = false);  // NOLINT
 };
 
+template<typename T> void GPIOPin::attach_interrupt(void (*func)(T *), T *arg, int mode) const {
+  this->attach_interrupt_(reinterpret_cast<void (*)(void *)>(func), arg, mode);
+}
+
 ESPHOME_NAMESPACE_END
 
 #endif  // ESPHOME_ESPHAL_H

src/esphome/light/mqtt_json_light_component.cpp

@@ -46,7 +46,6 @@ void MQTTJSONLightComponent::send_discovery(JsonObject &root, mqtt::SendDiscover
       effect_list.add(effect->get_name());
     effect_list.add("None");
   }
-  config.platform = "mqtt_json";
 }
 bool MQTTJSONLightComponent::send_initial_state() { return this->publish_state_(); }
 bool MQTTJSONLightComponent::is_internal() { return this->state_->is_internal(); }

src/esphome/mqtt/mqtt_component.cpp

@@ -73,14 +73,11 @@ bool MQTTComponent::send_discovery_() {
         SendDiscoveryConfig config;
         config.state_topic = true;
         config.command_topic = true;
-        config.platform = "mqtt";
 
         this->send_discovery(root, config);
 
         std::string name = this->friendly_name();
         root["name"] = name;
-        if (strcmp(config.platform, "mqtt") != 0)
-          root["platform"] = config.platform;
         if (config.state_topic)
           root["state_topic"] = this->get_state_topic_();
         if (config.command_topic)

src/esphome/mqtt/mqtt_component.h

@@ -14,9 +14,8 @@ namespace mqtt {
 
 /// Simple Helper struct used for Home Assistant MQTT send_discovery().
 struct SendDiscoveryConfig {
-  bool state_topic{true};        ///< If the state topic should be included. Defaults to true.
-  bool command_topic{true};      ///< If the command topic should be included. Default to true.
-  const char *platform{"mqtt"};  ///< The platform of this component. Defaults to "mqtt".
+  bool state_topic{true};    ///< If the state topic should be included. Defaults to true.
+  bool command_topic{true};  ///< If the command topic should be included. Default to true.
 };
 
 #define LOG_MQTT_COMPONENT(state_topic, command_topic) \