nuttxpr commented on PR #16327: URL: https://github.com/apache/nuttx/pull/16327#issuecomment-2855246855
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR description mostly meets the NuttX requirements. Here's a breakdown and suggestions for improvement: **Strengths:** * **Clear Summary:** The summary explains the "why" (need for external debugger/trace compatibility), "what" (USART2 as console), and "how" (configuration changes). * **Impact Section Addresses Key Points:** It covers user impact, build impact (implicitly through config changes), hardware impact (specific to Nucleo H563ZI), and documentation (implicitly by stating no changes needed). * **Testing Information Provided:** Host, compiler, configuration, and confirmation of NSH functionality are all helpful. Including the `savedefconfig` output is excellent for reproducibility. **Areas for Improvement:** * **Explicitly Address All Impact Points:** Even if the answer is "NO," explicitly state it for each impact item (build, documentation, security, compatibility). For example: * `Impact on documentation: NO (No documentation updates required)` * `Impact on security: NO (This change does not have security implications)` * `Impact on compatibility: NO (This change maintains backward compatibility)` * **"Testing logs before change" and "Testing logs after change":** While you describe the testing, you haven't provided actual logs. Show a snippet of NSH output before and after the change, even if it's just a simple command like `echo hello`. This proves the console is working. If the "before" is impossible due to the hardware limitations, explain why and focus on the "after" logs. * **Consider a more descriptive title for the PR:** Something like "Nucleo-H563ZI: Enable USART2 as console for external debugger compatibility" would be more informative. Your current PR description makes a good starting point. * **Relate to NuttX Issue (if any):** If this addresses a reported issue, link it in the Summary section. **Example of Improved Testing Section:** ``` ## Testing I confirm that changes are verified on local setup and works as intended: * Build Host(s): Ubuntu 24.04 Docker container on Ubuntu 24.04 host (WSL2) * Target(s): arm (STM32H563ZI on Nucleo-H563ZI board) * Compiler: `arm-none-eabi-gcc (Arm GNU Toolchain 13.3.Rel1 (Build arm-13.24)) 13.3.1 20240614` * Configuration: `nucleo-h563zi:nsh` with modifications to enable USART2 console (see savedefconfig output below). Testing logs before change: N/A - Using USART2 as console is required for functionality when using an external debugger with JP1 set, as the ST-Link VCOM port RX is disabled in this configuration. Testing logs after change: ``` nsh> echo hello hello nsh> ``` ``` By making these small improvements, your PR will be even clearer, easier to review, and better aligned with the NuttX contribution guidelines. -- 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
