nuttxpr commented on PR #16087: URL: https://github.com/apache/nuttx/pull/16087#issuecomment-2759166196
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) The PR summary is missing some key information required by the NuttX guidelines. Here's a breakdown of what's missing and how to improve it: **Missing Information & Suggested Improvements:** * **Summary - Why change is necessary:** While the summary mentions adding a driver, it doesn't explicitly state *why* this driver is needed. Is there a specific application or use case this enables? Does it replace a less effective solution? Example: "This change is necessary to provide support for the Nuvoton NAU7802 ADC, enabling high-resolution analog-to-digital conversion for applications requiring precise measurements, such as load cell readings." * **Summary - What functional part of the code is being changed:** Be more specific than just "the code." Mention the affected subsystems. Example: "This change adds a new driver to the `drivers/sensors` directory and introduces a new I2C device driver." * **Summary - How does the change exactly work:** While it mentions I2C and UORB, provide a slightly more detailed explanation of the driver's implementation. Example: "The driver communicates with the NAU7802 via I2C, configures the ADC settings, reads the raw sensor data, converts it to a force measurement, and publishes the data using the UORB `sensor_force` topic." * **Impact - Is new feature added?**: YES. State this explicitly. * **Impact - Is existing feature changed?**: Probably NO (unless this replaces/modifies an existing ADC driver). State this explicitly. * **Impact on user (will user need to adapt to change)?**: Likely YES. Explain how. Example: "Users will need to configure the I2C bus and device address in their board configuration file to use the NAU7802 driver." * **Impact on build (will build process change)?**: Likely YES. Explain how. Example: "Users will need to select the NAU7802 driver in their build configuration (e.g., using `make menuconfig`) to include it in the firmware." * **Impact on hardware (will arch(s) / board(s) / driver(s) change)?**: YES. Specify which architectures and boards are supported. Example: "This change adds support for the NAU7802 on platforms with I2C capabilities, specifically tested on the Raspberry Pi Pico W (ARM architecture)." * **Impact on documentation (is update required / provided)?**: YES. It's great that documentation is included! Mention where it's located. Example: "Documentation for the NAU7802 driver is provided in the `docs/drivers/sensors` directory, including calibration instructions." * **Impact on security (any sort of implications)?**: Likely NO. State this explicitly and explain why if relevant. * **Impact on compatibility (backward/forward/interoperability)?**: Likely NO. State this explicitly and explain why if relevant. * **Testing - Build Host(s):** Provide details about your build host. Example: "macOS Ventura, Apple M1 Pro, GCC 12.2" * **Testing - Target(s):** Be more specific. Example: "ARM: Raspberry Pi Pico W, `boardconfig`" (replace `boardconfig` with the actual board configuration used). * **Testing - Logs before change:** These logs aren't strictly necessary for a new driver, but if there was a previous method for reading similar data, include logs from that for comparison. * **Testing logs after change:** The provided logs are good. It might be beneficial to include logs demonstrating different load cell values to show the sensor's range and accuracy. **Revised Summary Example (concise):** This PR adds a new driver for the Nuvoton NAU7802 24-bit ADC to enable high-resolution analog-to-digital conversion, specifically for applications like load cell readings. It implements an I2C driver and publishes data via the `sensor_force` UORB topic. This impacts the `drivers/sensors` directory. Users will need to configure their board and build configurations to use the driver. Testing was performed on a Raspberry Pi Pico W (ARM) with a 100kg CZL601J load cell, demonstrating successful data acquisition and calibration. Documentation is included in `docs/drivers/sensors`. See [related issue # if applicable]. By addressing these points, your PR will be much clearer and easier for reviewers to assess. Remember to be concise while providing all the necessary information. -- 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: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org