Skip to content

Cmake: Implement CMake build of Tiva arch #16238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 19, 2025

Conversation

simbit18
Copy link
Contributor

@simbit18 simbit18 commented Apr 18, 2025

Summary

added TI/Stellaris Tiva

CMake added tm4c1294-launchpad and tm4c123g-launchpad board.

this implementation #16195 is incorrect and broken.

Impact

Impact on user: This PR adds tm4c1294-launchpad and tm4c123g-launchpad board with CMake build

Impact on build: This PR Implement CMake build of Tiva arch

Impact on hardware: NO

Impact on documentation: NO

Impact on security: NO

Impact on compatibility: NO

Testing

Locally and on GitHub

Locally

D:\nuttx>cmake -B build -DBOARD_CONFIG=tm4c1294-launchpad:nsh -GNinja
-- Initializing NuttX
--   ENV{PROCESSOR_ARCHITECTURE} = AMD64
  Select HOST_WINDOWS=y
  Select WINDOWS_NATIVE=y
--   CMake:  3.31.5
--   Ninja:  1.12.1
--   Board:  tm4c1294-launchpad
--   Config: nsh
--   Appdir: D:/apps
-- The C compiler identification is GNU 13.2.1
-- The CXX compiler identification is GNU 13.2.1
-- The ASM compiler identification is GNU
-- Found assembler: D:/nx20250410/tools/gcc-arm-none-eabi/bin/arm-none-eabi-gcc.exe
-- Configuring done (9.5s)
-- Generating done (2.1s)
-- Build files have been written to: D:/nuttx/build

D:\nuttx>cmake --build build
[1447/1448] Linking C executable nuttx
Memory region         Used Size  Region Size  %age Used
           flash:      139172 B         1 MB     13.27%
            sram:       29384 B       256 KB     11.21%
[1448/1448] Generating nuttx.bin

GitHub

tm4c1294-launchpad:nsh
https://github.com/simbit18/nuttx_test_pr/actions/runs/14536972193/job/40787247827#logs

@github-actions github-actions bot added Arch: arm Issues related to ARM (32-bit) architecture Board: arm Size: L The size of the change in this PR is large labels Apr 18, 2025
added TI/Stellaris Tiva

CMake added tm4c1294-launchpad and tm4c123g-launchpad board

Signed-off-by: simbit18 <[email protected]>
@github-actions github-actions bot added Size: M The size of the change in this PR is medium and removed Size: L The size of the change in this PR is large labels Apr 18, 2025
@nuttxpr
Copy link

nuttxpr commented Apr 18, 2025

[Experimental Bot, please feedback here]

This PR description mostly meets the NuttX requirements, but could be improved for clarity and completeness. Here's a breakdown and suggestions:

Strengths:

  • Clear Summary of Changes: The summary clearly states the addition of Tiva support and the two new boards. Mentioning the previous incorrect implementation (boards/arm/tiva/tm4c123g-launchpad: add CMakeLists.txt #16195) is helpful.
  • Concise Impact Assessment: The impact section is generally well-structured, though some parts could be more detailed.
  • Build Host Information: Provides information about the local build environment.
  • Target Information: Specifies target architecture and configuration.
  • Inclusion of Build Logs: Includes relevant build logs, demonstrating successful compilation.
  • Link to CI Run: Provides a link to a GitHub Actions run, which is excellent.

Weaknesses & Suggestions:

  • Summary - "Why" is Missing: The summary explains what changed, but not why. Was this a requested feature? Does it replace a broken implementation (besides the mentioned boards/arm/tiva/tm4c123g-launchpad: add CMakeLists.txt #16195)? Adding a sentence about the motivation for adding Tiva support would be beneficial.
  • Summary - "How" Needs Detail: While you mention CMake, elaborate on how the CMake build system is integrated. Did you add new files? Modify existing ones? What specific changes were made to enable Tiva support?
  • Impact - Be More Specific:
    • User Impact: While it mentions adding boards, specify how a user would utilize these new boards. Are there any configuration changes required? Any new example applications?
    • Build Impact: You mention implementing a CMake build. Does this mean the older build system is no longer supported for Tiva? Are there any dependencies users need to be aware of for the CMake build?
    • Hardware Impact: You say "NO," but you are adding support for new hardware. Clarify if this means no existing hardware configurations are affected.
    • Documentation: Even if no documentation updates are required, consider whether adding some basic documentation about the new boards and how to use them would be beneficial. If you believe no documentation is necessary, explicitly state that and the rationale behind it (e.g., "Documentation is not required as these boards use standard NuttX drivers and configurations").
  • Testing - Insufficient "Before" Logs: You've provided "after" logs showing a successful build, but no "before" logs to demonstrate the previous state (presumably a build failure or lack of Tiva support). This makes it harder to assess the impact of your changes. Since you reference boards/arm/tiva/tm4c123g-launchpad: add CMakeLists.txt #16195 as a broken implementation, provide logs or output demonstrating that broken behavior.
  • Testing - More Comprehensive Tests: Building the code is a good first step. However, ideally, you should provide logs demonstrating the boards functioning correctly. This could include booting the board, running basic shell commands, or testing specific peripherals. The CI link is good, but summarize the key tests performed there within the PR description itself.

Revised Example (incorporating the suggestions):

Summary

This PR adds support for Texas Instruments Stellaris Tiva TM4C1294 LaunchPad and TM4C123G LaunchPad boards to NuttX. This new support is provided to address the community's need for Tiva platforms and to correct the broken implementation in #16195, which [briefly describe the issue with #16195, e.g., "failed to build due to...," "did not correctly initialize...," etc.]. The changes introduce CMake build configurations for both boards, enabling users to easily build and deploy NuttX on these platforms. New board configuration files (configs/tm4c1294-launchpad/ and configs/tm4c123g-launchpad/) and CMakeLists.txt entries were added to integrate these boards into the build system.

Impact

  • User Impact: YES. Users can now build and run NuttX on the TM4C1294 LaunchPad and TM4C123G LaunchPad. Example configurations using the NSH shell are provided. Users can select these boards using the -DBOARD_CONFIG CMake option.
  • Build Impact: YES. CMake build support is now implemented for the Tiva architecture. The previous (non-existent/broken - be specific) build system is no longer applicable. Users must have CMake installed to build for these targets.
  • Hardware Impact: NO. This PR adds support for new hardware but does not affect any existing hardware configurations.
  • Documentation Impact: NO. While no documentation updates are strictly required, as these boards use standard NuttX drivers, we recommend adding a brief section to the documentation in a future PR to guide users on configuring and utilizing these boards.
  • Security Impact: NO.
  • Compatibility Impact: NO.

Testing

I confirm that changes are verified locally and through GitHub Actions CI.

Build Host: Windows, AMD64, GCC 13.2.1, CMake 3.31.5, Ninja 1.12.1

Targets: Tiva (TM4C1294 LaunchPad and TM4C123G LaunchPad), nsh configuration

Testing Logs Before Change (demonstrating the issue with #16195 or lack of previous Tiva support):

[Insert logs showing build failure or other problems with previous state]
e.g., Output of attempting to build for Tiva before this PR, showing errors.

Testing Logs After Change (local build):

[Your successful build logs as before]

GitHub Actions CI: [Link to CI Run]

The CI run performs the following tests:

  • Builds NuttX for both the TM4C1294 LaunchPad and TM4C123G LaunchPad using the nsh configuration.
  • [Describe any other tests performed by the CI, e.g., flashing to a board, running basic commands, testing peripherals]

By addressing these points, you'll create a more comprehensive and informative PR, making it easier for reviewers to understand and approve your changes. Remember to always be as specific and detailed as possible to minimize ambiguity and facilitate a smoother review process.

@simbit18
Copy link
Contributor Author

simbit18 commented Apr 18, 2025

I only tested Cmake's build, if anyone has one of these two boards and can test it, that would be helpful.

@raiden00pl @acassis @roskuttan @hartmannathan @xiaoxiang781216 @sydeney @ldube

@acassis
Copy link
Contributor

acassis commented Apr 18, 2025

I don't have this board, maybe @roskuttan and @hartmannathan has it

@acassis acassis merged commit 047a409 into apache:master Apr 19, 2025
25 checks passed
@simbit18 simbit18 deleted the simbit18-tiva branch April 21, 2025 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: arm Issues related to ARM (32-bit) architecture Board: arm Size: M The size of the change in this PR is medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants