Make pulse_meter PULSE filter report the pulse as soon as it can (#6014)

esphome/components/pulse_meter/pulse_filter.md

@@ -0,0 +1,61 @@
+# PULSE Filter
+
+The PULSE filter filters noisy pulses by ensuring that the pulse is in a steady state for at least `filter_length` before allowing the state change to occur.
+It counts the pulse time from the rising edge that stayed high for at least `filter_length`, so noise before this won't be considered the start of a pulse.
+It then must see a low pulse that is at least `filter_length` long before a subsequent rising edge is considered a new pulse start.
+
+It's operation should be the same as delayed_on_off from the Binary Sensor component.
+
+There are three moving parts in the algorithm that are used to determine the state of the filter.
+
+1. The time between interrupts, measured from the last interrupt, this is compared to filter_length to determine if the pulse has been in a steady state for long enough.
+2. A latch variable that is set true when a high pulse is long enough to be considered a valid pulse and is reset when a low pulse is long enough to allow for another pulse to begin.
+3. The previous and current state of the pin used to determine if the pulse is rising or falling.
+
+## Ghost interrupts
+
+Observations from the devices show that even though the interrupt should trigger on every rising or falling edge, sometimes interrupts show the same state twice in a row.
+The current theory is an interprets occurs, but then the pin changing back faster than the ISR can be called and read the value, meaning it sees the same state twice in a row.
+The algorithm interprets these when it sees them as two edges in a row, so will potentially reset a pulse if
+
+## Pulse Filter Truth table
+
+The following is all of the possible states of the filter along with the new inputs.
+It also shows a diagram of a possible series of interrupts that would cause the filter to enter that state.
+It then has the action that the filter should take and a description of what is happening.
+
+Diagram legend
+
+- `/` rising edge
+- `\` falling edge
+- `‾` high steady state of at least `filter_length`
+- `_` low steady state of at least `filter_length`
+- `¦` ghost interrupt
+
+| Length | Latch | From | To  | Diagram | Action             | Description                                                                                          |
+| ------ | ----- | ---- | --- | ------- | ------------------ | ---------------------------------------------------------------------------------------------------- |
+| T      | 1     | 0    | 0   | `‾\_¦ ` | Reset              | `filter_length` low, reset the latch                                                                 |
+| T      | 1     | 0    | 1   | `‾\_/ ` | Reset, Rising Edge | `filter_length` low, reset the latch, rising edge could be a new pulse                               |
+| T      | 1     | 1    | 0   | `‾\/‾\` | -                  | Already latched from a previous `filter_length` high                                                 |
+| T      | 1     | 1    | 1   | `‾\/‾¦` | -                  | Already latched from a previous `filter_length` high                                                 |
+| T      | 0     | 1    | 1   | `_/‾¦`  | Set and Publish    | `filter_length` high, set the latch and publish the pulse                                            |
+| T      | 0     | 1    | 0   | `_/‾\ ` | Set and Publish    | `filter_length` high, set the latch and publish the pulse                                            |
+| T      | 0     | 0    | 1   | `_/\_/` | Rising Edge        | Already unlatched from a previous `filter_length` low                                                |
+| T      | 0     | 0    | 0   | `_/\_¦` | -                  | Already unlatched from a previous `filter_length` low                                                |
+| F      | 1     | 0    | 0   | `‾\¦  ` | -                  | Low was not long enough to reset the latch                                                           |
+| F      | 1     | 0    | 1   | `‾\/  ` | -                  | Low was not long enough to reset the latch                                                           |
+| F      | 1     | 1    | 0   | `‾\/\ ` | -                  | Low was not long enough to reset the latch                                                           |
+| F      | 1     | 1    | 1   | `‾¦   ` | -                  | Ghost of 0 length definitely was not long was not long enough to reset the latch                     |
+| F      | 0     | 1    | 1   | `_/¦  ` | Rising Edge        | High was not long enough to set the latch, but the second half of the ghost can be a new rising edge |
+| F      | 0     | 1    | 0   | `_/\  ` | -                  | High was not long enough to set the latch                                                            |
+| F      | 0     | 0    | 1   | `_/\/ ` | Rising Edge        | High was not long enough to set the latch, but this may be a rising edge                             |
+| F      | 0     | 0    | 0   | `_¦ `   | -                  | Ghost of 0 length definitely was not long was not long enough to set the latch                       |
+
+## Startup
+
+On startup the filter should not consider whatever state it is in as valid so it does not count a strange pulse.
+There are two possible starting configurations, either the pin is high or the pin is low.
+If the pin is high, the subsequent falling edge should not count as a pulse as we never saw the rising edge.
+Therefore we start in the latched state.
+If the pin is low, the subsequent rising edge can be counted as the first pulse.
+Therefore we start in the unlatched state.

esphome/components/pulse_meter/pulse_meter_sensor.cpp

@@ -24,11 +24,16 @@ void PulseMeterSensor::setup() {
   if (this->filter_mode_ == FILTER_EDGE) {
     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->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;
 
@@ -38,6 +43,20 @@ void PulseMeterSensor::loop() {
   this->set_ = this->get_;
   this->get_ = temp;
 
+  // If an edge was peeked, repay the debt
+  if (this->peeked_edge_ && this->get_->count_ > 0) {
+    this->peeked_edge_ = false;
+    this->get_->count_--;
+  }
+
+  // If there is an unprocessed edge, and filter_us_ has passed since, count this edge early
+  if (this->get_->last_rising_edge_us_ != this->get_->last_detected_edge_us_ &&
+      now - this->get_->last_rising_edge_us_ >= this->filter_us_) {
+    this->peeked_edge_ = true;
+    this->get_->last_detected_edge_us_ = this->get_->last_rising_edge_us_;
+    this->get_->count_++;
+  }
+
   // Check if we detected a pulse this loop
   if (this->get_->count_ > 0) {
     // Keep a running total of pulses if a total sensor is configured
@@ -64,7 +83,6 @@ void PulseMeterSensor::loop() {
   }
   // No detected edges this loop
   else {
-    const uint32_t now = micros();
     const uint32_t time_since_valid_edge_us = now - this->last_processed_edge_us_;
 
     switch (this->meter_state_) {
@@ -102,11 +120,14 @@ void IRAM_ATTR PulseMeterSensor::edge_intr(PulseMeterSensor *sensor) {
   // This is an interrupt handler - we can't call any virtual method from this method
   // Get the current time before we do anything else so the measurements are consistent
   const uint32_t now = micros();
+  auto &state = sensor->edge_state_;
+  auto &set = *sensor->set_;
 
-  if ((now - sensor->last_edge_candidate_us_) >= sensor->filter_us_) {
+  if ((now - state.last_sent_edge_us_) >= sensor->filter_us_) {
-    sensor->last_edge_candidate_us_ = now;
+    state.last_sent_edge_us_ = now;
-    sensor->set_->last_detected_edge_us_ = now;
+    set.last_detected_edge_us_ = now;
-    sensor->set_->count_++;
+    set.last_rising_edge_us_ = now;
+    set.count_++;
   }
 }
 
@@ -115,33 +136,27 @@ void IRAM_ATTR PulseMeterSensor::pulse_intr(PulseMeterSensor *sensor) {
   // Get the current time before we do anything else so the measurements are consistent
   const uint32_t now = micros();
   const bool pin_val = sensor->isr_pin_.digital_read();
+  auto &state = sensor->pulse_state_;
+  auto &set = *sensor->set_;
 
-  // A pulse occurred faster than we can detect
+  // Filter length has passed since the last interrupt
-  if (sensor->last_pin_val_ == pin_val) {
+  const bool length = now - state.last_intr_ >= sensor->filter_us_;
-    // If we haven't reached the filter length yet we need to reset our last_intr_ to now
-    // otherwise we can consider this noise as the "pulse" was certainly less than filter_us_
-    if (now - sensor->last_intr_ < sensor->filter_us_) {
-      sensor->last_intr_ = now;
-    }
-  } else {
-    // Check if the last interrupt was long enough in the past
-    if (now - sensor->last_intr_ > sensor->filter_us_) {
-      // High pulse of filter length now falling (therefore last_intr_ was the rising edge)
-      if (!sensor->in_pulse_ && sensor->last_pin_val_) {
-        sensor->last_edge_candidate_us_ = sensor->last_intr_;
-        sensor->in_pulse_ = true;
-      }
-      // Low pulse of filter length now rising (therefore last_intr_ was the falling edge)
-      else if (sensor->in_pulse_ && !sensor->last_pin_val_) {
-        sensor->set_->last_detected_edge_us_ = sensor->last_edge_candidate_us_;
-        sensor->set_->count_++;
-        sensor->in_pulse_ = false;
-      }
-    }
 
-    sensor->last_intr_ = now;
+  if (length && state.latched_ && !state.last_pin_val_) {  // Long enough low edge
-    sensor->last_pin_val_ = pin_val;
+    state.latched_ = false;
+  } else if (length && !state.latched_ && state.last_pin_val_) {  // Long enough high edge
+    state.latched_ = true;
+    set.last_detected_edge_us_ = state.last_intr_;
+    set.count_++;
   }
+
+  // Due to order of operations this includes
+  //    length && latched && rising   (just reset from a long low edge)
+  //    !latched && (rising || high)  (noise on the line resetting the potential rising edge)
+  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;
 }
 
 }  // namespace pulse_meter

esphome/components/pulse_meter/pulse_meter_sensor.h

@@ -43,6 +43,7 @@ class PulseMeterSensor : public sensor::Sensor, public Component {
   // Variables used in the loop
   enum class MeterState { INITIAL, RUNNING, TIMED_OUT };
   MeterState meter_state_ = MeterState::INITIAL;
+  bool peeked_edge_ = false;
   uint32_t total_pulses_ = 0;
   uint32_t last_processed_edge_us_ = 0;
 
@@ -53,6 +54,7 @@ class PulseMeterSensor : public sensor::Sensor, public Component {
   // (except for resetting the values)
   struct State {
     uint32_t last_detected_edge_us_ = 0;
+    uint32_t last_rising_edge_us_ = 0;
     uint32_t count_ = 0;
   };
   State state_[2];
@@ -61,10 +63,20 @@ class PulseMeterSensor : public sensor::Sensor, public Component {
 
   // Only use these variables in the ISR
   ISRInternalGPIOPin isr_pin_;
-  uint32_t last_edge_candidate_us_ = 0;
+
-  uint32_t last_intr_ = 0;
+  /// Filter state for edge mode
-  bool in_pulse_ = false;
+  struct EdgeState {
-  bool last_pin_val_ = false;
+    uint32_t last_sent_edge_us_ = 0;
+  };
+  EdgeState edge_state_{};
+
+  /// Filter state for pulse mode
+  struct PulseState {
+    uint32_t last_intr_ = 0;
+    bool latched_ = false;
+    bool last_pin_val_ = false;
+  };
+  PulseState pulse_state_{};
 };
 
 }  // namespace pulse_meter