Re: [PR] examples/elf: add possibility to create romfs image [nuttx-apps]
xiaoxiang781216 merged PR #3062: URL: https://github.com/apache/nuttx-apps/pull/3062 -- 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]
Re: [PR] examples/elf: add possibility to create romfs image [nuttx-apps]
Laczen commented on code in PR #3062: URL: https://github.com/apache/nuttx-apps/pull/3062#discussion_r2049483212 ## examples/elf/Kconfig: ## @@ -39,7 +39,7 @@ config EXAMPLES_ELF_EXTERN The file system is assumed to reside on some external media such as an SD card or a USB FLASH drive. In this case, that external file system must be created manually by copying the - files in apps/examples/elf/tests/romfs to the volume. + files in apps/examples/elf/tests/extfs to the volume. Review Comment: Thanks for the comment. It is a bit confusing, there are 2 possible uses for romfs: internal (included in nuttx.bin/hex) and external (not in nuttx.bin/hex). In the above option it is using an external romfs (another option is to use external vfat). The old help message was an error, the files would be found under cromfs. To correct this error I decided to use romfs and cromfs for internal (in nuttx.bin/hex) fs, and to use extfs for external fs. The external fs can be of type vfat or romfs. I hope this makes it a little clearer. -- 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]
Re: [PR] examples/elf: add possibility to create romfs image [nuttx-apps]
Laczen commented on PR #3062: URL: https://github.com/apache/nuttx-apps/pull/3062#issuecomment-2813782060 > Thank you @Laczen very nice feature!! :-) > > Some trivial code formatting to be fixed (https://nuttx.apache.org/docs/latest/contributing/coding_style.html). > > Would it be possible to add nuttx/documentation in this area please? :-) I didn't realize the coding_style was also valid for the `nuttx-apps`, I thought the requirements where relaxed to allow direct integration of external code. I will correct the errors. Regarding the documentation, is it correct to add sample documentation for nuttx-apps in the NuttX/documentation? This seems very fragile and risks to get out of sync. -- 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]
Re: [PR] examples/elf: add possibility to create romfs image [nuttx-apps]
cederom commented on PR #3062: URL: https://github.com/apache/nuttx-apps/pull/3062#issuecomment-2813840163 > > Thank you @Laczen very nice feature!! :-) > > Some trivial code formatting to be fixed (https://nuttx.apache.org/docs/latest/contributing/coding_style.html). > > Would it be possible to add nuttx/documentation in this area please? :-) > > I didn't realize the coding_style was also valid for the `nuttx-apps`, I thought the requirements where relaxed to allow direct integration of external code. I will correct the errors. The coding standard is both for NuttX and the Apps, so we have all clean, thanks :-) > Regarding the documentation, is it correct to add sample documentation for nuttx-apps in the NuttX/documentation? This seems very fragile and risks to get out of sync. Yes, docs are in nuttx repo, also for the apps part, with smaller updates in the code coupled with the documentation updates we should be able to keep things in sync :-) Thus my question to document this nice feature I am sure there are other folks that may find it useful :-) -- 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]
Re: [PR] examples/elf: add possibility to create romfs image [nuttx-apps]
cederom commented on code in PR #3062: URL: https://github.com/apache/nuttx-apps/pull/3062#discussion_r2049495857 ## examples/elf/Kconfig: ## @@ -39,7 +39,7 @@ config EXAMPLES_ELF_EXTERN The file system is assumed to reside on some external media such as an SD card or a USB FLASH drive. In this case, that external file system must be created manually by copying the - files in apps/examples/elf/tests/romfs to the volume. + files in apps/examples/elf/tests/extfs to the volume. Review Comment: Thanks @Laczen :-) If we have a full nuttx/doc on that part, which is really interesting, thigs would be clear for everyone I suppose :-) -- 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]
Re: [PR] examples/elf: add possibility to create romfs image [nuttx-apps]
cederom commented on code in PR #3062: URL: https://github.com/apache/nuttx-apps/pull/3062#discussion_r2049442404 ## examples/elf/Kconfig: ## @@ -39,7 +39,7 @@ config EXAMPLES_ELF_EXTERN The file system is assumed to reside on some external media such as an SD card or a USB FLASH drive. In this case, that external file system must be created manually by copying the - files in apps/examples/elf/tests/romfs to the volume. + files in apps/examples/elf/tests/extfs to the volume. Review Comment: As romfs is used (i.e. `CONFIG_EXAMPLES_ELF_ROMFS`) why change to extfs? Can you also provide some nuttx/Documentation in this area please? :-) -- 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]
Re: [PR] examples/elf: add possibility to create romfs image [nuttx-apps]
nuttxpr commented on PR #3062: URL: https://github.com/apache/nuttx-apps/pull/3062#issuecomment-2812938926 [**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues) Yes, this PR appears to meet the NuttX requirements, although some minor improvements could be made. **Strengths:** * **Clear Summary:** The summary explains the change, its purpose, and how it works. * **Impact Assessment:** While concise, it addresses all the points. "None" is acceptable for most if the change is truly isolated. * **Testing:** Includes host/target details and provides *before* and *after* logs demonstrating functionality. **Areas for Improvement:** * **Relate to an issue:** Even if this is a new feature without a prior issue, it's good practice to create an issue first to discuss the idea and get feedback before implementing it. This facilitates community involvement and avoids wasted effort. The PR can then reference the issue. * **More detail on *how* it works:** The summary mentions using the ELF loader to generate a romfs.img. A little more detail here would be helpful. Does this involve new Kconfig options? Changes to the build system? Mentioning specific files modified would make it easier for reviewers. * **Testing - Clarity on *before*:** The "before" logs are unclear. What was the previous behavior? Since this is a new feature, the before might simply be the absence of the feature, but it's worth explicitly stating that. Or perhaps the "before" is a system without the external romfs, and you're showing that the elf loader still works as expected for internal romfs. **Conclusion:** The PR is well-structured and provides the necessary information. Adding an associated issue and slightly expanding the technical details and "before" test explanation would strengthen it further. -- 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]
