Skip to content

feat(rmt): improves RMT documentation #11992

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(rmt): improves RMT documentation #11992

wants to merge 1 commit into from
+439 −9

Conversation

@SuGlider
Copy link
Collaborator

@SuGlider SuGlider commented Nov 7, 2025

Description of Change

Improves the RMT documentation for the Arduino API

Test Scenarios

CI Only

Related links

N/A

@SuGlider SuGlider added this to the 3.3.0 milestone Nov 7, 2025
@SuGlider SuGlider self-assigned this Nov 7, 2025
@SuGlider SuGlider added Type: Documentation Issue pertains to Documentation of Arduino ESP32 Area: Peripherals API Relates to peripheral's APIs. labels Nov 7, 2025
@SuGlider SuGlider moved this from Todo to In Progress in Arduino ESP32 Core Project Roadmap Nov 7, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025

Messages
📖 🎉 Good Job! All checks are passing!

👋 Hello SuGlider, 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.
- 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.

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 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.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against deb4e93

@SuGlider SuGlider added the Status: Review needed Issue or PR is awaiting review label Nov 7, 2025
@SuGlider SuGlider moved this from In Progress to In Review in Arduino ESP32 Core Project Roadmap Nov 7, 2025
@lucasssvaz lucasssvaz requested a review from Copilot November 7, 2025 11:50
Copilot AI reviewed Nov 7, 2025
View reviewed changes
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

This PR adds comprehensive documentation for the RMT (Remote Control Transceiver) peripheral API in the Arduino-ESP32 framework. The documentation replaces a placeholder "work in progress" message with complete reference material covering all RMT functionality.

Key changes:

  • Added detailed explanations of RMT peripheral capabilities and use cases (IR protocols, WS2812 LEDs, custom protocols)
  • Documented all RMT API functions including initialization, transmission (blocking/async/looping), reception, and configuration
  • Included memory block allocation details for different ESP32 SoC variants
  • Added code examples and helper macro documentation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

docs/en/api/rmt.rst
Constant defining the number of RMT symbols per memory block.

* ESP32/ESP32-S2: 64 symbols per block
* ESP32-S3/ESP32-C3/ESP32-C6/ESP32-H2: 48 symbols per block
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

Incomplete list of SoCs. Line 39 mentions that ESP32-C5 and ESP32-P4 have 48 symbols per block, but line 424 omits these two SoCs from the list. The complete list should be: "ESP32-S3/ESP32-C3/ESP32-C5/ESP32-C6/ESP32-H2/ESP32-P4: 48 symbols per block"

Suggested change
* ESP32-S3/ESP32-C3/ESP32-C6/ESP32-H2: 48 symbols per block
* ESP32-S3/ESP32-C3/ESP32-C5/ESP32-C6/ESP32-H2/ESP32-P4: 48 symbols per block

Copilot uses AI. Check for mistakes.
docs/en/api/rmt.rst

**Non-blocking mode:** Returns immediately after execution. Use ``rmtTransmitCompleted()`` to check if transmission is finished.

**Note:** If more than one ``rmtWriteAsync()`` is called in sequence, it will wait for the first transmission to finish, returning ``false`` to indicate failure.
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

Unclear error condition description. The note states "If more than one rmtWriteAsync() is called in sequence, it will wait for the first transmission to finish, returning false to indicate failure." This is contradictory - if it waits for the first transmission to finish, that's blocking behavior, which conflicts with the non-blocking nature described above. Please clarify whether subsequent calls fail immediately (returning false) or wait (making it blocking).

Suggested change
**Note:** If more than one ``rmtWriteAsync()`` is called in sequence, it will wait for the first transmission to finish, returning ``false`` to indicate failure.
**Note:** If ``rmtWriteAsync()`` is called while a previous transmission is still in progress, it will return ``false`` immediately to indicate failure; it does not wait for the previous transmission to finish.

Copilot uses AI. Check for mistakes.
docs/en/api/rmt.rst

**Note:**
* If ``rmtWrite()`` times out or ``rmtWriteAsync()`` is called, this function will return ``false`` until all data is sent out.
* ``rmtTransmitCompleted()`` will always return ``true`` when ``rmtWriteLooping()`` is active, because looping mode never completes.
Copy link

Copilot AI Nov 7, 2025

Choose a reason for hiding this comment

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

Incorrect behavior description for looping mode. The note states that rmtTransmitCompleted() will always return true when rmtWriteLooping() is active. However, this contradicts the function's purpose - during looping mode, the transmission never completes (it loops infinitely), so rmtTransmitCompleted() should return false, not true. Please verify and correct this description.

Suggested change
* ``rmtTransmitCompleted()`` will always return ``true`` when ``rmtWriteLooping()`` is active, because looping mode never completes.
* ``rmtTransmitCompleted()`` will always return ``false`` when ``rmtWriteLooping()`` is active, because looping mode never completes.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@me-no-dev me-no-dev me-no-dev approved these changes

@lucasssvaz lucasssvaz lucasssvaz approved these changes

@P-R-O-C-H-Y P-R-O-C-H-Y P-R-O-C-H-Y approved these changes

@pedrominatel pedrominatel Awaiting requested review from pedrominatel pedrominatel is a code owner

At least 0 approving reviews are required to merge this pull request.

Assignees

@SuGlider SuGlider

Labels

Area: Peripherals API Relates to peripheral's APIs. Status: Review needed Issue or PR is awaiting review Type: Documentation Issue pertains to Documentation of Arduino ESP32

Projects

Arduino ESP32 Core Project Roadmap
Status: In Review

Milestone

3.3.0

Development

Successfully merging this pull request may close these issues.

5 participants

@SuGlider @me-no-dev @lucasssvaz @P-R-O-C-H-Y