nuttxpr commented on PR #15205: URL: https://github.com/apache/nuttx/pull/15205#issuecomment-2544796639
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) No, this PR description does not meet the NuttX requirements. It's far too brief and lacks critical information. Here's why and what's missing: **Summary:** * **Insufficient detail:** "Support for the mouse ioctl interface" is vague. What *specific* ioctl calls are added? What functionality do they provide? What problem does this solve (e.g., was there no way to control the mouse before)? * **Missing context:** Which part of the codebase is affected? Is this a new driver? Modification to an existing one? A new system call? * **No explanation of how it works:** How is the ioctl interface implemented? What are the internal changes? * **Missing issue references:** Are there any related issues this PR addresses? **Impact:** * **Incomplete answers:** Saying "mouse driver" doesn't explain the impact. All the "NO / YES" questions need proper answers with explanations if "YES." For example: * **Impact on user:** Will users need to change their applications to use the new ioctl calls? * **Impact on hardware:** Does this only affect certain mouse hardware or architectures? * **Impact on documentation:** Does this require documentation updates? Has documentation been added? **Testing:** * **Inadequate testing description:** "qemu" is not enough. What specific qemu configuration was used? What tests were run? What were the *expected* results? * **Missing logs:** The "Testing logs before change" and "Testing logs after change" sections are empty. These are crucial for demonstrating that the change works and doesn't introduce regressions. Include relevant log output showing the mouse functionality before and after the change. * **Missing build host details:** What operating system, compiler, and versions were used for the build? **Example of a better Summary:** > This PR adds support for the `MOUSE_SET_SAMPLERATE` and `MOUSE_GET_STATUS` ioctl calls to the existing mouse driver in `drivers/input/mouse.c`. These ioctls allow user applications to control the mouse sampling rate and retrieve the current mouse status (buttons, position). This addresses the lack of a standardized interface for interacting with the mouse device, as described in [NuttX Issue #123]. **Example of a better Impact Section:** > * **Is new feature added?** YES - Adds ioctl interface for mouse control. > * **Impact on user?** YES - Users can now use the new ioctls to control the mouse sampling rate and get status. Applications will need to be updated to use these features. > * **Impact on build?** NO > * **Impact on hardware?** NO - Works with existing supported mouse hardware. > * **Impact on documentation?** YES - Documentation has been added to the `doc/drivers/input.txt` file describing the new ioctl calls and their usage. > * **Impact on security?** NO > * **Impact on compatibility?** NO **Example of a better Testing Section:** > * **Build Host:** Linux (Ubuntu 20.04), x86_64, GCC 9.4.0 > * **Target:** qemu-system-arm, sim:nsh config > **Testing logs before change:** > ``` > $ nsh > nsh> mouse_test # No output, as ioctls are not implemented > ``` > **Testing logs after change:** > ``` > $ nsh > nsh> mouse_test > Mouse sampling rate set to 100Hz > Mouse status: Left button pressed, X=100, Y=200 > ``` By providing more specific details and filling in the missing information, the PR will be much easier to review and understand, increasing the chances of it being accepted. -- 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]
