diff --git a/esphome/components/pulse_meter/pulse_meter_sensor.cpp b/esphome/components/pulse_meter/pulse_meter_sensor.cpp index 836a84b391..b82cb7a15c 100644 --- a/esphome/components/pulse_meter/pulse_meter_sensor.cpp +++ b/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->pulse_state_.last_pin_val_; + this->pulse_state_.latched_ = this->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; - // 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 - auto *temp = this->set_; - this->set_ = this->get_; - this->get_ = temp; + { + // 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 + std::swap(this->set_, this->get_); + } + + 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 diff --git a/esphome/components/pulse_meter/pulse_meter_sensor.h b/esphome/components/pulse_meter/pulse_meter_sensor.h index 76c4a35f03..748bab29ac 100644 --- a/esphome/components/pulse_meter/pulse_meter_sensor.h +++ b/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. - // 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) + // Use these to send data from the ISR to the loop not the other way around (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_{}; };