Skip to content

[pulse_counter] Fix PCNT glitch filter calculation off by 1000x#14132

Merged
swoboda1337 merged 1 commit intoesphome:devfrom
swoboda1337:fix-pulse-counter-glitch-filter
Feb 20, 2026
Merged

[pulse_counter] Fix PCNT glitch filter calculation off by 1000x#14132
swoboda1337 merged 1 commit intoesphome:devfrom
swoboda1337:fix-pulse-counter-glitch-filter

Conversation

@swoboda1337
Copy link
Member

What does this implement/fix?

The max_glitch_ns calculation used 1000000 (10^6) instead of 10^9 when converting PCNT_LL_MAX_GLITCH_WIDTH ticks to nanoseconds. This made the computed maximum 1000x too small (12ns instead of 12787ns), causing std::min to cap the user's requested filter (e.g. 13000ns for 13µs) to just 12ns. ESP-IDF then converts that to 0 APB clock cycles — effectively disabling the glitch filter entirely.

With the filter disabled, the PCNT hardware counts electrical noise as pulses, producing wildly inflated readings (e.g. ~280,000 pulses/min instead of ~1,010 on an ESP32-C6).

This fix uses the same split-division approach as ESP-IDF's own pulse_cnt.c:404 to stay in 32-bit arithmetic:

// ESP-IDF (ns → cycles): cycles = (freq / 1000000) * ns / 1000
// ESPHome (cycles → ns): ns     = cycles * 1000 / (freq / 1000000)

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Developer breaking change (an API change that could break external components)
  • Code quality improvements to existing code or addition of tests
  • Other

Related issue or feature (if applicable):

Pull request in esphome-docs with documentation (if applicable):

  • N/A

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx
  • LN882x
  • nRF52840

Example entry for config.yaml:

sensor:
  - platform: pulse_counter
    pin: GPIO18
    name: "Pulse Counter"
    internal_filter: 13us

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

The max_glitch_ns calculation used 1000000 (1e6) instead of 1e9 to
convert PCNT_LL_MAX_GLITCH_WIDTH ticks to nanoseconds, making the
computed maximum 1000x too small (12ns instead of 12787ns). This
caused std::min to cap the user's requested filter (e.g. 13000ns for
13us) to just 12ns, which ESP-IDF then converted to 0 APB clock
cycles — effectively disabling the glitch filter entirely.

Use the same split-division approach as ESP-IDF's own pulse_cnt.c to
stay in 32-bit arithmetic: multiply ticks by 1000, divide freq by 1M.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings February 19, 2026 23:57
@github-actions
Copy link
Contributor

To use the changes from this PR as an external component, add the following to your ESPHome configuration YAML file:

external_components:
  - source: github://pr#14132
    components: [pulse_counter]
    refresh: 1h

(Added by the PR bot)

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes the ESP32 PCNT glitch filter maximum duration calculation in the pulse_counter component so user-configured internal_filter values are no longer incorrectly clamped ~1000× too small (which effectively disables the hardware glitch filter).

Changes:

  • Correct max_glitch_ns conversion from PCNT clock ticks to nanoseconds (uses 10^9 scaling via split-division instead of 10^6).
  • Align the conversion approach with ESP-IDF’s integer-arithmetic strategy to avoid overflow while keeping the value in-range for uint32_t.

@swoboda1337 swoboda1337 enabled auto-merge (squash) February 20, 2026 00:09
@swoboda1337
Copy link
Member Author

Thanks

@swoboda1337 swoboda1337 merged commit 1b4de55 into esphome:dev Feb 20, 2026
44 checks passed
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.11%. Comparing base (cceb109) to head (9cd3f77).
⚠️ Report is 4 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev   #14132   +/-   ##
=======================================
  Coverage   74.11%   74.11%           
=======================================
  Files          55       55           
  Lines       11590    11590           
  Branches     1578     1578           
=======================================
  Hits         8590     8590           
  Misses       2598     2598           
  Partials      402      402           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

swoboda1337 added a commit that referenced this pull request Feb 20, 2026
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
@swoboda1337 swoboda1337 mentioned this pull request Feb 20, 2026
@github-actions github-actions bot locked and limited conversation to collaborators Feb 21, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compile error with pulse_counter

4 participants