[pulse_meter] Fix early edge detection (#12360)

Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>
Co-authored-by: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com>
This commit is contained in:
Lukáš Maňas
2026-02-12 18:20:54 +01:00
committed by Jesse Hills
parent ead7937dbf
commit e9bf9bc691
2 changed files with 44 additions and 42 deletions
@@ -38,8 +38,7 @@ void PulseMeterSensor::setup() {
} }
void PulseMeterSensor::loop() { void PulseMeterSensor::loop() {
// Reset the count in get before we pass it back to the ISR as set State state;
this->get_->count_ = 0;
{ {
// Lock the interrupt so the interrupt code doesn't interfere with itself // Lock the interrupt so the interrupt code doesn't interfere with itself
@@ -58,31 +57,35 @@ void PulseMeterSensor::loop() {
} }
this->last_pin_val_ = current; this->last_pin_val_ = current;
// Swap out set and get to get the latest state from the ISR // Get the latest state from the ISR and reset the count in the ISR
std::swap(this->set_, this->get_); state.last_detected_edge_us_ = this->state_.last_detected_edge_us_;
state.last_rising_edge_us_ = this->state_.last_rising_edge_us_;
state.count_ = this->state_.count_;
this->state_.count_ = 0;
} }
const uint32_t now = micros(); const uint32_t now = micros();
// If an edge was peeked, repay the debt // If an edge was peeked, repay the debt
if (this->peeked_edge_ && this->get_->count_ > 0) { if (this->peeked_edge_ && state.count_ > 0) {
this->peeked_edge_ = false; this->peeked_edge_ = false;
this->get_->count_--; // NOLINT(clang-diagnostic-deprecated-volatile) state.count_--;
} }
// If there is an unprocessed edge, and filter_us_ has passed since, count this edge early // 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_ && // Wait for the debt to be repaid before counting another unprocessed edge early.
now - this->get_->last_rising_edge_us_ >= this->filter_us_) { if (!this->peeked_edge_ && state.last_rising_edge_us_ != state.last_detected_edge_us_ &&
now - state.last_rising_edge_us_ >= this->filter_us_) {
this->peeked_edge_ = true; this->peeked_edge_ = true;
this->get_->last_detected_edge_us_ = this->get_->last_rising_edge_us_; state.last_detected_edge_us_ = state.last_rising_edge_us_;
this->get_->count_++; // NOLINT(clang-diagnostic-deprecated-volatile) state.count_++;
} }
// Check if we detected a pulse this loop // Check if we detected a pulse this loop
if (this->get_->count_ > 0) { if (state.count_ > 0) {
// Keep a running total of pulses if a total sensor is configured // Keep a running total of pulses if a total sensor is configured
if (this->total_sensor_ != nullptr) { if (this->total_sensor_ != nullptr) {
this->total_pulses_ += this->get_->count_; this->total_pulses_ += state.count_;
const uint32_t total = this->total_pulses_; const uint32_t total = this->total_pulses_;
this->total_sensor_->publish_state(total); this->total_sensor_->publish_state(total);
} }
@@ -94,15 +97,15 @@ void PulseMeterSensor::loop() {
this->meter_state_ = MeterState::RUNNING; this->meter_state_ = MeterState::RUNNING;
} break; } break;
case MeterState::RUNNING: { case MeterState::RUNNING: {
uint32_t delta_us = this->get_->last_detected_edge_us_ - this->last_processed_edge_us_; uint32_t delta_us = state.last_detected_edge_us_ - this->last_processed_edge_us_;
float pulse_width_us = delta_us / float(this->get_->count_); float pulse_width_us = delta_us / float(state.count_);
ESP_LOGV(TAG, "New pulse, delta: %" PRIu32 " µs, count: %" PRIu32 ", width: %.5f µs", delta_us, ESP_LOGV(TAG, "New pulse, delta: %" PRIu32 " µs, count: %" PRIu32 ", width: %.5f µs", delta_us, state.count_,
this->get_->count_, pulse_width_us); pulse_width_us);
this->publish_state((60.0f * 1000000.0f) / pulse_width_us); this->publish_state((60.0f * 1000000.0f) / pulse_width_us);
} break; } break;
} }
this->last_processed_edge_us_ = this->get_->last_detected_edge_us_; this->last_processed_edge_us_ = state.last_detected_edge_us_;
} }
// No detected edges this loop // No detected edges this loop
else { else {
@@ -141,14 +144,14 @@ void IRAM_ATTR PulseMeterSensor::edge_intr(PulseMeterSensor *sensor) {
// This is an interrupt handler - we can't call any virtual method from this method // 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 // Get the current time before we do anything else so the measurements are consistent
const uint32_t now = micros(); const uint32_t now = micros();
auto &state = sensor->edge_state_; auto &edge_state = sensor->edge_state_;
auto &set = *sensor->set_; auto &state = sensor->state_;
if ((now - state.last_sent_edge_us_) >= sensor->filter_us_) { if ((now - edge_state.last_sent_edge_us_) >= sensor->filter_us_) {
state.last_sent_edge_us_ = now; edge_state.last_sent_edge_us_ = now;
set.last_detected_edge_us_ = now; state.last_detected_edge_us_ = now;
set.last_rising_edge_us_ = now; state.last_rising_edge_us_ = now;
set.count_++; // NOLINT(clang-diagnostic-deprecated-volatile) state.count_++; // NOLINT(clang-diagnostic-deprecated-volatile)
} }
// This ISR is bound to rising edges, so the pin is high // This ISR is bound to rising edges, so the pin is high
@@ -160,26 +163,26 @@ void IRAM_ATTR PulseMeterSensor::pulse_intr(PulseMeterSensor *sensor) {
// Get the current time before we do anything else so the measurements are consistent // Get the current time before we do anything else so the measurements are consistent
const uint32_t now = micros(); const uint32_t now = micros();
const bool pin_val = sensor->isr_pin_.digital_read(); const bool pin_val = sensor->isr_pin_.digital_read();
auto &state = sensor->pulse_state_; auto &pulse_state = sensor->pulse_state_;
auto &set = *sensor->set_; auto &state = sensor->state_;
// Filter length has passed since the last interrupt // Filter length has passed since the last interrupt
const bool length = now - state.last_intr_ >= sensor->filter_us_; const bool length = now - pulse_state.last_intr_ >= sensor->filter_us_;
if (length && state.latched_ && !sensor->last_pin_val_) { // Long enough low edge if (length && pulse_state.latched_ && !sensor->last_pin_val_) { // Long enough low edge
state.latched_ = false; pulse_state.latched_ = false;
} else if (length && !state.latched_ && sensor->last_pin_val_) { // Long enough high edge } else if (length && !pulse_state.latched_ && sensor->last_pin_val_) { // Long enough high edge
state.latched_ = true; pulse_state.latched_ = true;
set.last_detected_edge_us_ = state.last_intr_; state.last_detected_edge_us_ = pulse_state.last_intr_;
set.count_++; // NOLINT(clang-diagnostic-deprecated-volatile) state.count_++; // NOLINT(clang-diagnostic-deprecated-volatile)
} }
// Due to order of operations this includes // Due to order of operations this includes
// length && latched && rising (just reset from a long low edge) // length && latched && rising (just reset from a long low edge)
// !latched && (rising || high) (noise on the line resetting the potential rising 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_rising_edge_us_ = !pulse_state.latched_ && pin_val ? now : state.last_detected_edge_us_;
state.last_intr_ = now; pulse_state.last_intr_ = now;
sensor->last_pin_val_ = pin_val; sensor->last_pin_val_ = pin_val;
} }
@@ -46,17 +46,16 @@ class PulseMeterSensor : public sensor::Sensor, public Component {
uint32_t total_pulses_ = 0; uint32_t total_pulses_ = 0;
uint32_t last_processed_edge_us_ = 0; uint32_t last_processed_edge_us_ = 0;
// This struct (and the two pointers) are used to pass data between the ISR and loop. // This struct and variable are used to pass data between the ISR and loop.
// These two pointers are exchanged each loop. // The data from state_ is read and then count_ in state_ is reset in each loop.
// Use these to send data from the ISR to the loop not the other way around (except for resetting the values). // This must be done while guarded by an InterruptLock. Use this variable to send data
// from the ISR to the loop not the other way around (except for resetting count_).
struct State { struct State {
uint32_t last_detected_edge_us_ = 0; uint32_t last_detected_edge_us_ = 0;
uint32_t last_rising_edge_us_ = 0; uint32_t last_rising_edge_us_ = 0;
uint32_t count_ = 0; uint32_t count_ = 0;
}; };
State state_[2]; volatile State state_{};
volatile State *set_ = state_;
volatile State *get_ = state_ + 1;
// Only use the following variables in the ISR or while guarded by an InterruptLock // Only use the following variables in the ISR or while guarded by an InterruptLock
ISRInternalGPIOPin isr_pin_; ISRInternalGPIOPin isr_pin_;