Check for missed pulse_meter ISRs in the main loop (#6126)

esphome/components/pulse_meter/pulse_meter_sensor.cpp

@@ -18,6 +18,9 @@ void PulseMeterSensor::setup() {
   this->pin_->setup();
   this->isr_pin_ = pin_->to_isr();
 
+  // Set the pin value to the current value to avoid a false edge
+  this->last_pin_val_ = this->pin_->digital_read();
+
   // Set the last processed edge to now for the first timeout
   this->last_processed_edge_us_ = micros();
 
@@ -25,23 +28,37 @@ void PulseMeterSensor::setup() {
     this->pin_->attach_interrupt(PulseMeterSensor::edge_intr, this, gpio::INTERRUPT_RISING_EDGE);
   } else if (this->filter_mode_ == FILTER_PULSE) {
     // Set the pin value to the current value to avoid a false edge
-    this->pulse_state_.last_pin_val_ = this->isr_pin_.digital_read();
+    this->pulse_state_.latched_ = this->last_pin_val_;
-    this->pulse_state_.latched_ = this->pulse_state_.last_pin_val_;
     this->pin_->attach_interrupt(PulseMeterSensor::pulse_intr, this, gpio::INTERRUPT_ANY_EDGE);
   }
 }
 
 void PulseMeterSensor::loop() {
-  const uint32_t now = micros();
-
   // Reset the count in get before we pass it back to the ISR as set
   this->get_->count_ = 0;
 
+  {
+    // Lock the interrupt so the interrupt code doesn't interfere with itself
+    InterruptLock lock;
+
+    // Sometimes ESP devices miss interrupts if the edge rises or falls too slowly.
+    // See https://github.com/espressif/arduino-esp32/issues/4172
+    // If the edges are rising too slowly it also implies that the pulse rate is slow.
+    // Therefore the update rate of the loop is likely fast enough to detect the edges.
+    // When the main loop detects an edge that the ISR didn't it will run the ISR functions directly.
+    bool current = this->pin_->digital_read();
+    if (this->filter_mode_ == FILTER_EDGE && current && !this->last_pin_val_) {
+      PulseMeterSensor::edge_intr(this);
+    } else if (this->filter_mode_ == FILTER_PULSE && current != this->last_pin_val_) {
+      PulseMeterSensor::pulse_intr(this);
+    }
+    this->last_pin_val_ = current;
+
     // Swap out set and get to get the latest state from the ISR
-  // The ISR could interrupt on any of these lines and the results would be consistent
+    std::swap(this->set_, this->get_);
-  auto *temp = this->set_;
+  }
-  this->set_ = this->get_;
+
-  this->get_ = temp;
+  const uint32_t now = micros();
 
   // If an edge was peeked, repay the debt
   if (this->peeked_edge_ && this->get_->count_ > 0) {
@@ -131,6 +148,9 @@ void IRAM_ATTR PulseMeterSensor::edge_intr(PulseMeterSensor *sensor) {
     set.last_rising_edge_us_ = now;
     set.count_++;
   }
+
+  // This ISR is bound to rising edges, so the pin is high
+  sensor->last_pin_val_ = true;
 }
 
 void IRAM_ATTR PulseMeterSensor::pulse_intr(PulseMeterSensor *sensor) {
@@ -144,9 +164,9 @@ void IRAM_ATTR PulseMeterSensor::pulse_intr(PulseMeterSensor *sensor) {
   // Filter length has passed since the last interrupt
   const bool length = now - state.last_intr_ >= sensor->filter_us_;
 
-  if (length && state.latched_ && !state.last_pin_val_) {  // Long enough low edge
+  if (length && state.latched_ && !sensor->last_pin_val_) {  // Long enough low edge
     state.latched_ = false;
-  } else if (length && !state.latched_ && state.last_pin_val_) {  // Long enough high edge
+  } else if (length && !state.latched_ && sensor->last_pin_val_) {  // Long enough high edge
     state.latched_ = true;
     set.last_detected_edge_us_ = state.last_intr_;
     set.count_++;
@@ -158,7 +178,7 @@ void IRAM_ATTR PulseMeterSensor::pulse_intr(PulseMeterSensor *sensor) {
   set.last_rising_edge_us_ = !state.latched_ && pin_val ? now : set.last_detected_edge_us_;
 
   state.last_intr_ = now;
-  state.last_pin_val_ = pin_val;
+  sensor->last_pin_val_ = pin_val;
 }
 
 }  // namespace pulse_meter

esphome/components/pulse_meter/pulse_meter_sensor.h

@@ -49,9 +49,7 @@ class PulseMeterSensor : public sensor::Sensor, public Component {
 
   // This struct (and the two pointers) are used to pass data between the ISR and loop.
   // These two pointers are exchanged each loop.
-  // Therefore you can't use data in the pointer to loop receives to set values in the pointer to loop sends.
+  // Use these to send data from the ISR to the loop not the other way around (except for resetting the values).
-  // As a result it's easiest if you only use these pointers to send data from the ISR to the loop.
-  // (except for resetting the values)
   struct State {
     uint32_t last_detected_edge_us_ = 0;
     uint32_t last_rising_edge_us_ = 0;
@@ -61,9 +59,12 @@ class PulseMeterSensor : public sensor::Sensor, public Component {
   volatile State *set_ = state_;
   volatile State *get_ = state_ + 1;
 
-  // Only use these variables in the ISR
+  // Only use the following variables in the ISR or while guarded by an InterruptLock
   ISRInternalGPIOPin isr_pin_;
 
+  /// The last pin value seen
+  bool last_pin_val_ = false;
+
   /// Filter state for edge mode
   struct EdgeState {
     uint32_t last_sent_edge_us_ = 0;
@@ -74,7 +75,6 @@ class PulseMeterSensor : public sensor::Sensor, public Component {
   struct PulseState {
     uint32_t last_intr_ = 0;
     bool latched_ = false;
-    bool last_pin_val_ = false;
   };
   PulseState pulse_state_{};
 };