nuttxpr commented on PR #14549: URL: https://github.com/apache/nuttx/pull/14549#issuecomment-2445133375
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) __Fill In The Commit Message:__ This PR contains a Commit with an Empty Commit Message. Please fill in the Commit Message with the PR Summary. ## NuttX PR Requirements Review This PR summary provides a good starting point, but it needs some work to meet all the NuttX requirements. Here's a breakdown: **Strengths:** * **Clear explanation of the problem:** The summary clearly identifies the issue with ACL packets not reaching the host layer and the connection issues it causes. * **Solution description:** The fix of setting the 'Controller to Host Flow Control' parameter is well-explained, including the reasoning behind making it user-configurable. * **Additional improvement:** The improvement to the pairing process by handling `LL_CONNECTION_PARAM_REQ` is a valuable addition. * **Testing details:** You mention the target board and configuration used for testing, along with the tool used for connection. **Areas for Improvement:** * **Summary:** * **Why is the change necessary?** While the problem is stated, explicitly mention the impact on users (e.g., inability to connect to Bluetooth devices reliably). * **What functional part of the code is being changed?** Be more specific than "Bluetooth stack." Mention specific files or modules impacted (e.g., HCI layer, L2CAP). * **Issue references:** Include links to any relevant NuttX or NuttX Apps issues or pull requests. * **Impact:** * **New feature:** The configurable 'Controller to Host Flow Control' is a new feature. State this explicitly. * **User impact:** Users with non-ESP32 devices may need to experiment with this parameter to achieve successful connections. * **Hardware impact:** Specify that the changes primarily affect Bluetooth-enabled boards. * **Documentation:** Documentation will need updating to describe the new configuration option and its implications. * **Testing:** * **Build Host(s):** Provide details about your development environment (OS, CPU, compiler). * **Testing logs:** The provided logs are placeholders. Include actual logs showing the behavior before and after the changes. * **Bug reports:** * The issues with advertising and transmission seem significant. * Each bug should be reported as a separate issue on the NuttX issue tracker with detailed descriptions and steps to reproduce. * Consider addressing these bugs in separate PRs for better clarity and review. **Conciseness:** * The summary is a bit long. Try to be more concise while still providing all necessary information. **Overall:** The PR addresses important issues and introduces useful improvements to the Bluetooth stack. However, you need to provide more specific details, complete the required sections, and address the identified bugs to ensure a smooth review and merge process. -- 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]
