nuttxpr commented on PR #16276: URL: https://github.com/apache/nuttx/pull/16276#issuecomment-2834648570
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR description does not fully meet the NuttX requirements. Here's why and how to fix it: **Missing/Insufficient Information:** * **Summary:** While it mentions a new DAC device, it lacks crucial details. Which specific MCP47X6 chip(s) are supported? What functional part of the code is changed (e.g., new driver in `drivers/dac`)? How *exactly* does the change work (I2C communication details, register mapping, etc.)? Provide a related NuttX issue if one exists (or create one!). * **Impact:** Almost all impact sections are missing details. At minimum, you should specify: * **Impact on hardware:** Specifically state "rp2040" and list the board name. Mention the I2C peripheral used. * **Impact on documentation:** Since it's a new driver, documentation MUST be added. State where (e.g., `docs/drivers/dac`). * **Impact on compatibility:** If the driver depends on specific features in the I2C framework, mention that. * **Testing:** * **Build Host(s):** Provide details (OS, CPU architecture, compiler version). Example: "Linux, x86_64, GCC 12.2.1". * **Target(s):** Be more specific than "custom rp2040 board." Give the board name or a descriptive identifier. State the NuttX configuration used (e.g., `nsh`). Crucially, mention the I2C bus number used by the DAC. Example: `rp2040:my_custom_board:nsh (I2C1)`. * **Testing Logs:** The provided logs are placeholders. Include *actual* logs showing the DAC functionality before and after the change. Demonstrate setting different DAC values, using different voltage references, and enabling/disabling gain. The logs should clearly show the expected behavior. **How to Improve the PR Description:** 1. **Be Specific:** Provide precise details in every section. Avoid vague terms like "custom board." 2. **Show, Don't Tell:** Use the testing logs to *prove* that the driver works. Include commands used and the resulting output. 3. **Follow the Template:** Carefully address each point in the NuttX PR requirements. **Example of Improved Summary:** This PR adds support for the Microchip MCP4706, MCP4716, and MCP4726 series 8/10/12-bit I2C DACs. A new driver is implemented in `drivers/dac` that handles I2C communication, register configuration (including gain, power-down mode, and voltage reference selection), and provides a standard DAC interface. This addresses [NuttX Issue #XYZ] (if applicable). By providing more specific information and actual testing logs, you will significantly improve the quality of your PR and make it easier for reviewers to understand and accept your changes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
