nuttxpr commented on PR #15428: URL: https://github.com/apache/nuttx/pull/15428#issuecomment-2572572689
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) This PR does not meet the NuttX requirements. The provided information is insufficient and lacks crucial details. Here's a breakdown of the issues and how to address them: **Summary:** * **Insufficient Justification:** "There is no driver... I am using this driver, so it is just submitted" is not a valid justification. Explain the *benefit* of adding this driver. Who else might use it? What functionality does it unlock? * **Missing Technical Details:** The summary mentions SPI support and STM32G4, but doesn't connect them to the BMI088 driver itself. Clarify how the driver interacts with the SPI bus and what specific changes were made to the STM32G4 SPI driver (if any). Be explicit. * **Missing Links:** Provide links to the actual PR and any related issues. **Impact:** * **Vague Statements:** "Adds a new device driver function, convenient for everyone to use" is too general. Specify which users will benefit and how. Does this enable new applications or improve existing ones? * **Incomplete Analysis:** The impact section is almost entirely empty. Address *all* the points: Impact on the user, build, hardware, documentation, security, and compatibility. Even if the answer is "NO," state it explicitly and briefly explain why. For example, "Impact on build: NO. No changes to Kconfig or build system are required." **Testing:** * **Insufficient Host Information:** Specify the *exact* build host OS, version, compiler, and version. Example: "Linux Ubuntu 20.04, GCC 9.4.0". * **Insufficient Target Information:** Be more specific about the target. "STM32G474" is a microcontroller family, not a board. Provide the board name or configuration. Example: "STM32G474RE Nucleo board". Also mention if you tested on a simulator (QEMU). * **Missing "Before" Logs:** You haven't provided any "before" logs. This makes it impossible to assess the impact of the changes. * **Unclear "After" Logs:** The provided code snippet is from the test application, not a log. Show actual output from the driver, demonstrating that it's functioning correctly (e.g., sensor readings). * **Lack of Test Cases:** The test application seems basic. Demonstrate testing of different driver functionalities (different sensor ranges, data rates, etc.). Consider adding error handling tests. **Example of a Better Summary:** > This PR adds a new driver for the Bosch BMI088 accelerometer and gyroscope sensor, utilizing the SPI bus. This sensor is commonly used in robotics and drone applications and provides high-precision inertial measurement. The driver supports [list specific features: e.g., various data rates, power modes, interrupt generation]. This PR also includes updates to the STM32G4 series SPI driver to support the BMI088's specific communication requirements. This change is necessary because NuttX currently lacks support for this sensor. Related Issue: [#Issue Number](Link to Issue) **Example of a Better Impact Section:** * **Impact on user:** YES. Users can now interface with the BMI088 sensor through the provided driver. Example applications are provided in `apps/examples/bmi088`. * **Impact on build:** YES. Users will need to enable the BMI088 driver in their configuration using `CONFIG_SENSORS_BMI088`. * **Impact on hardware:** YES. This driver currently supports the STM32G4 series MCUs connected to the BMI088 via SPI. * **Impact on documentation:** YES. Documentation for the driver is included in the README file within the driver directory. Updates to the board documentation may be necessary to describe the BMI088 connection. * **Impact on security:** NO. This driver does not introduce any new security vulnerabilities. * **Impact on compatibility:** NO. This driver is a new addition and does not affect existing functionality. By addressing these points, you'll significantly improve the quality of your PR and increase the likelihood of it being accepted. Remember to be thorough, precise, and provide all the necessary information to allow reviewers to understand and evaluate your changes effectively. -- 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]
