nuttxpr commented on PR #15626: URL: https://github.com/apache/nuttx/pull/15626#issuecomment-2603836674
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) The PR summary is good, but lacks some detail on *how* the changes work. For instance, *how* is memory reduced? What specific changes enable graceful shutdown and more ostest cases? The Impact and Testing sections are too brief and don't meet the requirements. Here's a revised version suggesting how to improve the PR description: ## Summary This PR adjusts the `qemu-armv7a:knsh` configuration to improve testing and resource usage. Specifically: * **Reduced memory usage:** The default configuration's memory allocation is reduced [explain how, e.g., by adjusting the configuration variable X to Y] allowing the QEMU `-m 1024` option to be omitted. This streamlines execution and reduces resource requirements. * **ELF Executables for apps:** Applications are now built as ELF executables instead of [previous format]. This change enables easier debugging with tools like GDB. * **Graceful Shutdown from NSH:** A new command/mechanism [explain what command or mechanism was added] has been implemented to allow graceful shutdown of the simulated system from the NSH console. This facilitates cleaner testing and avoids abrupt termination. * **Expanded ostest coverage:** The configuration is updated to enable more ostest cases, including `vfork`, `pthread_exit` [and any others]. This provides more thorough testing of core OS functionalities. ## Impact * **Is new feature added?** YES (Graceful shutdown from NSH, Expanded ostest coverage) * **Is existing feature changed?** YES (Memory usage, App executable format) * **Impact on user:** YES. Users running the `qemu-armv7a:knsh` target will experience reduced memory usage and have access to the new shutdown command [explain the command and how to use it]. They can also leverage debugging tools more effectively with ELF executables. * **Impact on build:** NO. * **Impact on hardware:** NO (Only impacts the qemu-armv7a simulated hardware). * **Impact on documentation:** YES. Documentation should be updated to reflect the new shutdown command and explain the benefits of the ELF executable change. (A documentation update is included in this PR). * **Impact on security:** NO. * **Impact on compatibility:** NO known compatibility issues. * **Anything else to consider?** None. ## Testing I confirm that changes are verified on local setup and works as intended: * **Build Host:** macOS Ventura, Intel Core i9, clang-14 * **Target:** qemu-armv7a:knsh **Testing logs before change:** ``` [Show output of relevant commands demonstrating the previous behavior, e.g., memory usage with `free`, trying to debug an application, attempting to shut down, and running ostest showing missing tests.] ``` **Testing logs after change:** ``` [Show output of the same commands demonstrating the improved behavior, e.g., reduced memory usage, successful debugging session, clean shutdown using the new command, and ostest output including the newly enabled tests.] ``` -- 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
