fix(rmt): set rx filter resolution_hz to rmt_sclk (IDFGH-13418)

[LonerDan]

Fixes assignment of filter_clock_resolution_hz to rx_channel->base.resolution_hz, which is the clock frequency after the fractional divider RMT_SCLK_DIV (also referenced as rmt_sclk in the TRM), instead of the previously used value from group->resolution_hz, which represents the full undivided frequency from clock selected by RMT_SCLK_SEL (APB, RC_FAST or XTAL).
Before this fix, this mixup resulted in an error signal_range_min_ns too big being returned by the rmt_receive function due to this check failing no matter the resolution_hz used in the configuration of the channel.

[github-actions]

Warnings

⚠️

Some issues found for the commit messages in this PR: the commit message "fix(rmt): set rx filter resolution_hz to rmt_sclk": body's lines must not be longer than 100 characters footer must have leading blank line footer's lines must not be longer than 100 characters Please fix these commit messages - here are some basic tips: follow Conventional Commits style correct format of commit message should be: <type/action>(<scope/component>): <summary>, for example fix(esp32): Fixed startup timeout issue allowed types are: change,ci,docs,feat,fix,refactor,remove,revert,test sufficiently descriptive message summary should be between 20 to 72 characters and start with upper case letter avoid Jira references in commit messages (unavailable/irrelevant for our customers) TIP: Install pre-commit hooks and run this check when committing (uses the Conventional Precommit Linter).

👋 Hello LonerDan, we appreciate your contribution to this project!

---

📘 Please review the project's Contributions Guide for key guidelines on code, documentation, testing, and more.

---

🖊️ Please also make sure you have read and signed the Contributor License Agreement for this project.

---

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Resolve all warnings (⚠️ ) before requesting a review from human reviewers - they will appreciate it.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests via this public GitHub repository.

This GitHub project is public mirror of our internal git repository

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved, we synchronize it into our internal git repository.
4. In the internal git repository we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
5. If the change is approved and passes the tests it is merged into the default branch.
5. On next sync from the internal git repository merged change will appear in this public GitHub repository.

Generated by 🚫 dangerJS against f92bff6

[LonerDan]

NOTE: I couldn't verify this fix's correctness for the ESP32-C5 and ESP32-P4, since I could not find their TRMs (I suppose those are not available yet).

[suda-morris]

Hi @LonerDan I'm afraid this is not a mistake. The RX channel can filter out glitches in the signals, so it needs a high frequency base clock. The hardware uses the group's clock as the filter's tick clock.

[LonerDan]

Hi @suda-morris
I was going off of the TRM (more specifically for the ESP32-S3) figure 37-1, where the label rmt_sclk is put after the fractional divider and the same label is used with the arrow going into the filter block. Also, the text about the RX filter mentions that

The filter samples input signals continuously, and detects the signals which remain unchanged for a continuous RMT_RX_FILTER_THRES_CHm rmt_sclk cycles as valid, otherwise, the signals will be detected as invalid.

If that's not really the case, It could be beneficial to clarify this in the documentation.

[suda-morris]

@LonerDan The rmt_sclk is exactly the group clock. So rx_channel->filter_clock_resolution_hz = group->resolution_hz; is correct.
rx_channel->base.resolution_hz; is actually the frequency of the clk_div in the TRM diagram.

[LonerDan]

@suda-morris Ah, I see. Thank you for the correction. I looked closer into the source code and found the part where the rmt_sclk divider is always configured to no division:

esp-idf/components/esp_driver_rmt/src/rmt_common.c

Lines 210 to 214 in 8e4454b

	// no division for group clock source, to achieve highest resolution
	RMT_CLOCK_SRC_ATOMIC() {
	rmt_ll_set_group_clock_src(group->hal.regs, channel_id, clk_src, 1, 1, 0);
	rmt_ll_enable_group_clock(group->hal.regs, true);
	}

In my opinion, this should be configurable since the resolution of the filter is application-specific. In my case, the signal has shortest pulse length around 12 us and there is a lot of short spikes/noise mixed in, so I would like to filter out anything less than say 8 us to limit the number of data rmt rx channel generates for me to process.
I am willing to try to implement this, I just wanted to ask where you think is the best place to incorporate this parameter into? Not sure if rmt_rx_channel_config_t is the correct place since the sclk divider is common to the group.