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

Reply via email to