Preparing a Pull Request for Apache NuttX RTOS

📝 28 Nov 2022

Typical Pull Request for Apache NuttX RTOS

This article explains how I prepared my Pull Requests for submission to Apache NuttX RTOS. So if we’re contributing code to NuttX, just follow these steps and things will (probably) go Hunky Dory!

(Like the fish)

Before we begin, please swim over to the official Development Workflow for NuttX…

Ready? Let’s dive in! (Like the fish)

Create Fork

1 NuttX Repositories

We begin by creating our forks for the nuttx and apps repositories…

  1. Browse to NuttX Repository

    github.com/apache/nuttx

    Click “Fork” to create our fork. (Pic above)

    Click “Actions” and enable workflows…

    Enable Workflows

    (This will check that our code compiles OK at every commit)

  2. Do the same for the NuttX Apps Repository

    github.com/apache/nuttx-apps

    Click “Fork” to create our fork.

    Click “Actions” and enable workflows.

  3. As a principle, let’s keep our master branch always in sync with the NuttX Mainline master branch.

    (This seems cleaner for syncing upstream updates into our repo)

    Let’s create a branch to make our changes…

  4. In our NuttX Repository, click master.

    Enter the name of our new branch.

    Click “Create Branch

    Create Branch

    (I named my branch gic for Generic Interrupt Controller)

  5. Do the same for our NuttX Apps Repository

    (Because we should sync nuttx and apps too)

  6. Download the new branches of our nuttx and apps repositories…

    ## Download the "gic" branch of "lupyuen2/wip-nuttx"
    ## TODO: Change the branch name and repo URLs
    mkdir nuttx
    cd nuttx
    git clone \
      --branch gic \
      https://github.com/lupyuen2/wip-nuttx \
      nuttx
    git clone \
      --branch gic \
      https://github.com/lupyuen2/wip-nuttx-apps \
      apps
    

2 Build and Test

We’re ready to code!

  1. Consider breaking our Pull Request into Smaller Pull Requests.

    This Pull Request implements One Single Feature (Generic Interrupt Controller)…

    “arch/arm64: Add support for Generic Interrupt Controller Version 2”

    That’s called by another Pull Request…

    “arch/arm64: Add support for PINE64 PinePhone”

    Adding a NuttX Arch (SoC) and Board might need 3 Pull Requests (or more)…

    “Add the NuttX Arch and Board”

  2. Modify the code in nuttx and apps to implement our awesome new feature.

  3. Build and test the modified code.

    (I configured F1 in VSCode to run this Build Script)

  4. Capture the Output Log and save it as a GitHub Gist

    “NuttX QEMU Log for Generic Interrupt Controller”

    We’ll add this to our Pull Request. Test Logs are super helpful for NuttX Maintainers!

    (Because we can’t tell which way the train went… Unless we have the Test Logs!)

  5. Commit the modified code to our repositories.

    Sometimes the GitHub Actions Workflow will fail with a strange error (like below). Just re-run the failed jobs. (Like this)

    Error response from daemon:
    login attempt to https://ghcr.io/v2/
    failed with status: 503 Service Unavailable"
    

2.1 Regression Test

Will our modified code break other parts of NuttX?

That’s why it’s good to run a Regression Test (if feasible) to be sure that other parts of NuttX aren’t affected by our modified code.

For our Pull Request…

We tested with QEMU Emulator our new implementation of Generic Interrupt Controller v2…

tools/configure.sh qemu-armv8a:nsh_gicv2 ; make ; qemu-system-aarch64 ...

(See the NuttX QEMU Log)

And for Regression Testing we tested the existing implementation of Generic Interrupt Controller v3…

tools/configure.sh qemu-armv8a:nsh ; make ; qemu-system-aarch64 ...

(See the NuttX QEMU Log)

Remember to capture the Output Log, we’ll add it to our Pull Request.

(Yeah it will be hard to run a Regression Test if it requires hardware that we don’t have)

2.2 Documentation

Please update the Documentation. The Documentation might be in a Text File

Or in the Official NuttX Docs

To update the Official NuttX Docs, follow the instructions here…

Check Coding Style with nxstyle

3 Check Coding Style

Our NuttX Code will follow this Coding Standard…

NuttX provides a tool nxstyle that will check the Coding Style of our source files…

## Compile nxstyle
## TODO: Change "$HOME/nuttx" to our NuttX Project Folder
gcc -o $HOME/nxstyle $HOME/nuttx/nuttx/tools/nxstyle.c

## Check coding style for our modified source files
## TODO: Change the file paths
$HOME/nxstyle $HOME/nuttx/nuttx/arch/arm64/Kconfig
$HOME/nxstyle $HOME/nuttx/nuttx/arch/arm64/src/common/Make.defs
$HOME/nxstyle $HOME/nuttx/nuttx/arch/arm64/src/common/arm64_gic.h
$HOME/nxstyle $HOME/nuttx/nuttx/arch/arm64/src/common/arm64_gicv2.c

(nxstyle.c is here)

(How I run nxstyle in my Build Script)

Will nxstyle check Kconfig and Makefiles?

Not yet, but maybe someday? That’s why we passed all the modified files to nxstyle for checking.

How do we fix our code?

The pic above shows the output from nxstyle. We’ll see messages like…

- C++ style comment
- Long line found
- Mixed case identifier found
- Operator/assignment must be followed/preceded with whitespace
- Upper case hex constant found
- #include outside of 'Included Files' section

We modify our code so that this…

// Initialize GIC. Called by CPU0 only.
int arm64_gic_initialize(void) {
  // Verify that GIC Version is 2
  int err = gic_validate_dist_version();
  if (err) { sinfo("no distributor detected, giving up ret=%d\n", err); return err; }

  // CPU0-specific initialization for GIC
  arm_gic0_initialize();

  // CPU-generic initialization for GIC
  arm_gic_initialize();
  return 0;
}

(Source)

Becomes this…

/****************************************************************************
 * Name: arm64_gic_initialize
 *
 * Description:
 *   Initialize GIC. Called by CPU0 only.
 *
 * Input Parameters
 *   None
 *
 * Returned Value:
 *   Zero (OK) on success; a negated errno value is returned on any failure.
 *
 ****************************************************************************/

int arm64_gic_initialize(void)
{
  int err;

  /* Verify that GIC Version is 2 */

  err = gic_validate_dist_version();
  if (err)
    {
      sinfo("no distributor detected, giving up ret=%d\n", err);
      return err;
    }

  /* CPU0-specific initialization for GIC */

  arm_gic0_initialize();

  /* CPU-generic initialization for GIC */

  arm_gic_initialize();

  return 0;
}

(Source)

If we see this…

/* */ not balanced

Check that our stars “*” are aligned (heh)…

/******************************
 * Name: missing_asterisk_below
 *
 *****************************/

After fixing, test our code one last time.

4 Write the Pull Request

Our Pull Request will have…

  1. Title: NuttX Subsystem and One-Line Summary

  2. Summary: What’s the purpose of the Pull Request? What files are changed?

  3. Impact: Which parts of NuttX are affected by the Pull Request? Which parts won’t be affected?

  4. Testing: Provide evidence that our Pull Request does what it’s supposed to do.

    (And that it won’t do what it’s not supposed to do)

To write the above items, let’s walk through these Pull Requests…

4.1 Title

Inside our Title is the NuttX Subsystem and One-Line Summary

“arch/arm64: Add support for Generic Interrupt Controller Version 2”

(Source)

Let’s write this into a Markdown File for easier copying…

We’ll add the following sections to the Markdown File…

4.2 Summary

In the Summary we write the purpose of the Pull Request

“This PR adds support for GIC Version 2, which is needed by Pine64 PinePhone.”

(Source)

And we list the files that we changed

arch/arm64/Kconfig: Under “ARM64 Options”, we added an integer option ARM_GIC_VERSION (“GIC version”) that selects the GIC Version. Valid values are 2, 3 and 4, default is 3.“

(Source)

If it’s a long list, we might break into subsections like this…

For Code Provenance it’s good to state how we created the code

“This 64-bit implementation of GIC v2 is mostly identical to the existing GIC v2 for 32-bit Armv7-A (arm_gicv2.c, gic.h), with minor modifications to support 64-bit Registers (Interrupt Context).”

(Source)

(Adding GPL Code? Please check with the NuttX Maintainers)

4.3 Impact

Under “Impact”, we write which parts of NuttX are affected by the Pull Request.

(And which parts won’t be affected)

“With this PR, NuttX now supports GIC v2 on Arm64.

There is no impact on the existing implementation of GIC v3, as tested below.“

(Source)

4.4 Testing

Under “Testing”, we provide evidence that our Pull Request does what it’s supposed to do. We fill in the…

Like this…

“We tested with QEMU our implementation of GIC v2:

tools/configure.sh qemu-armv8a:nsh_gicv2 ; make ; qemu-system-aarch64 ...

(See the NuttX QEMU Log)

The log shows that GIC v2 has correctly handled interrupts“

(Source)

If we have done a Regression Test, provide the details too…

“For Regression Testing: We tested the existing implementation of GIC v3…”

tools/configure.sh qemu-armv8a:nsh ; make ; qemu-system-aarch64 ...

(See the NuttX QEMU Log)

(Source)

Test Logs are super helpful for NuttX Maintainers!

(Because we can’t tell which way the train went… By staring at the track!)

Now we tidy up our commits…

Squash Commits with GitHub Desktop

5 Squash the Commits

What’s this “squashing”?

“Squashing” means we’re combining Multiple Commits into One Single Commit.

Our Commit History can get awfully messy during development…

- Initial Commit
- Fixing Build
- Build OK!
- Oops fixing bug
- Tested OK yay!

(Source)

So we always Squash the Commits into One Single Commit (to help future maintainers)…

- arch/arm64: Add support for Generic Interrupt Controller Version 2

(Source)

How do we squash the commits?

We’ll use GitHub Desktop (because I’m terrible with the Git Command Line)…

  1. Install GitHub Desktop and launch it

  2. Click “File → Add Local Repository

    Select our downloaded nuttx folder.

    Click “Add Repository

  3. Click the “History” tab to reveal the Commit History

    (Pic above)

  4. Select the Commits to be Squashed.

    Right-click the Commits.

    Select “Squash Commits

    (Pic above)

  5. Copy the Title of our Pull Request and paste into the Title Box

    arch/arm64: Add support for Generic Interrupt Controller Version 2
    

    In the Description Box, erase the old Commit Messages.

    Copy the Summary of our Pull Request and paste into the Description Box

    This PR adds support for GIC Version 2.
    - `boards/arm64/qemu/qemu-armv8a/configs/nsh_gicv2/defconfig`: Added the Board Configuration
    

    Click “Squash Commits

    Squash Commits with GitHub Desktop

  6. Click “Begin Squash

    Squash Commits with GitHub Desktop

  7. Click “Force Push Origin

    Squash Commits with GitHub Desktop

  8. Click “I’m Sure

    Squash Commits with GitHub Desktop

    And we’re ready to merge upstream! (Like the salmon)

What if we prefer the Git Command Line?

Here are the steps to Squash Commits with the Git Command Line

Taking a long walk

6 Meditate

Breathe. Take a break.

We’re about to make NuttX History… Our changes will be recorded for posterity!

Take a long walk and ponder…

(I walked 12 km for 3 hours while meditating on my Pull Request)

If we get an inspiration or epiphany, touch up the Pull Request.

(And resquash the commits)

What’s your epiphany?

Through my Pull Requests, I hope to turn NuttX into a valuable tool for teaching the internals of Smartphone Operating Systems.

That’s my motivation for porting NuttX to PINE64 PinePhone.

But for now… Let’s finish our Pull Request!

Submit the Pull Request

7 Submit the Pull Request

Finally it’s time to submit our Pull Request!

  1. Create the Pull Request (pic above)

  2. Verify that it has only One Single Commit (pic above)

    (Squash the Commits)

  3. Copy these into the Pull Request…

    Title

    Summary

    Impact

    Testing

    (Like this)

  4. Submit the Pull Request

  5. Wait for the Automated Checks to be completed (might take an hour)…

    Automated Checks for Pull Request

  6. Fix any errors in the Automated Checks

  7. Wait for the NuttX Team to review and comment on our Pull Request.

    This might take a while (due to the time zones)… Grab a coffee and standby for fixes!

    (I bake sourdough while waiting)

    If all goes Hunky Dory, our Pull Request will be approved and merged! 🎉

Sometimes we need to Rebase To The Latest Master due to updates in the GitHub Actions Workflow (Continuous Integration Script). Here’s how…

  1. Browse to the master branch of our nuttx repository.

    Click “Sync Fork → Update Branch

    (Pic below)

  2. Launch GitHub Desktop

    Click “File → Add Local Repository

    Select our downloaded nuttx folder.

    Click “Add Repository

  3. Check that the Current Branch is our Working Branch for the Pull Request.

    (Like gic branch)

  4. Click “Fetch Origin

  5. Click “Branch → Rebase Current Branch

    Select the master branch

    Click “Rebase” and “Begin Rebase

  6. Click “Force Push Origin” and “I’m Sure

    (See the official docs)

Is it really OK to Rebase To The Latest Master?

Normally when the GitHub Actions build fails (not due to our code), I take a peek at other recent Pull Requests.

If they didn’t fail the build, then it’s probably OK to Rebase with Master to force the Rebuild.

Pull Updates from NuttX Mainline

8 Update Our Repositories

After our Pull Request has been merged into NuttX Mainline, pull the updates into our repositories…

  1. Browse to the master branch of our nuttx repository.

    Click “Sync Fork → Update Branch

    (Pic above)

  2. Do the same for the master branch of our apps repository.

    Click “Sync Fork → Update Branch

  3. Test the code from the master branch of our nuttx and apps repositories.

When we’re ready to add our next awesome feature

  1. Pull the updates from NuttX Mainline into our nuttx repository…

    Select the master branch.

    Click “Sync Fork → Update Branch

    (Pic above)

  2. Do the same for our apps repository.

  3. In our nuttx repository, click master.

    Enter the name of our new branch.

    Click “Create Branch

    Create Branch

  4. Do the same for our apps repository.

  5. Modify the code in the new branch of our nuttx and apps repositories.

    Build, test and submit our new Pull Request.

And that’s the Complete Lifecycle of a Pull Request for Apache NuttX RTOS!

One last thing: Please help to validate that the NuttX Release works OK! We really appreciate your help with this… 🙏

9 What’s Next

I hope this article will be helpful for folks contributing code to NuttX for the very first time.

In the next article we’ll explain this complex Pull Request that adds a new SoC and new Board to NuttX. Stay Tuned!

Many Thanks to my GitHub Sponsors for supporting my work! This article wouldn’t have been possible without your support.

Got a question, comment or suggestion? Create an Issue or submit a Pull Request here…

lupyuen.github.io/src/pr.md

10 Notes

  1. When we modify the Kconfig Configuration Files, remember to update the defconfig Configuration Files!

    (Like this)

    What happens if we forget to update defconfig

    Configuration/Tool: pinephone/sensor
    Building NuttX...
    Normalize pinephone/sensor
    71d70
    < CONFIG_UART1_SERIAL_CONSOLE=y
    Saving the new configuration file
    HEAD detached at pull/9243/merge
    Changes not staged for commit:
    (use "git add <file>..." to update what will be committed)
    (use "git restore <file>..." to discard changes in working directory)
        modified:   boards/arm64/a64/pinephone/configs/sensor/defconfig
    

    (Source)

  2. How do we create a new defconfig?

    In the “nuttx/nuttx” folder, run this…

    make savedefconfig
    

    This creates the file defconfig that contains the changed settings.

    Copy the defconfig file to the new config subfolder…

    mkdir boards/<archname>/<chipname>/<boardname>/config/<mynewcustomconfig>
    
    mv defconfig boards/<archname>/<chipname>/<boardname>/config/<mynewcustomconfig>/
    

    (Thanks to Alan C. Assis for the tip!)

  3. Some Default Settings in .config are missing from defconfig. Can we copy them ourselves to defconfig?

    Sorry it won’t work. Suppose we copy these Default UART3 Settings from .config to defconfig (to hard-code the UART3 Baud Rate)…

    CONFIG_UART3_BAUD=115200
    CONFIG_UART3_BITS=8
    CONFIG_UART3_PARITY=0
    CONFIG_UART3_2STOP=0
    

    The “Linux (Other)” Build will fail with this error…

    Configuration/Tool: pinephone/modem
    Building NuttX...
    Normalize pinephone/modem
    69,72d68
    < CONFIG_UART3_BAUD=115200
    < CONFIG_UART3_BITS=8
    < CONFIG_UART3_PARITY=0
    < CONFIG_UART3_2STOP=0
    Saving the new configuration file
    HEAD detached at pull/9304/merge
    Changes not staged for commit:
    (use "git add <file>..." to update what will be committed)
    (use "git restore <file>..." to discard changes in working directory)
        modified:   boards/arm64/a64/pinephone/configs/modem/defconfig
    

    (Source)

    Thus we can’t copy any Default Settings in .config to defconfig.

  4. If the auto-build fails with “Untracked etctmp”

    HEAD detached at pull/11379/merge
    Untracked files: (use "git add <file>..." to include in what will be committed)
    boards/risc-v/k230/canmv230/src/etctmp.c
    boards/risc-v/k230/canmv230/src/etctmp/
    

    (Source)

    Check that we have added “etctmp” to the Board-Specific Git Ignore: boards/risc-v/jh7110/star64/src/.gitignore

    etctmp
    etctmp.c
    
  5. Here’s an excellent guide for the Git Command Line

    “Flight rules for Git”

  6. Converting our code to the “NuttX C Coding Standard”

    Can we automate this with a VSCode Extension?

    Maybe like the Linux checkpatch Extension, but it will actually auto-reformat our lines?

  7. QEMU Emulator is incredibly helpful for Regression Testing…

    Can we extend it to emulate PinePhone? Maybe just the UART Hardware? Check out the articles…

    “(Possibly) Emulate PinePhone with Unicorn Emulator”

    “(Clickable) Call Graph for Apache NuttX Real-Time Operating System”

11 Appendix: Validate NuttX Release

For each Official Release of NuttX, how do we check if it runs OK on all devices? Like PinePhone, ESP32, BL602, …

NuttX needs to be tested on every device, and we need your help! 🙏

Before every Official Release of NuttX, a Validation Request will be broadcast on the NuttX Developers Mailing List

Follow the instructions here to validate that the NuttX Release builds correctly and runs OK on your device…

Here’s the script I run to validate NuttX on PinePhone

And here’s the output of the validation script

Boot NuttX on our device, run “uname -a” and “free”…

NuttShell (NSH) NuttX-12.1.0
nsh> uname -a
NuttX 12.1.0 d40f4032fc Apr 12 2023 07:11:20 arm64 pinephone
nsh> free
      total     used   free      largest   nused nfree
Umem: 133414240 550768 132863472 132863376 56    2

The NuttX Hash (like “d40f4032fc” above) should match the Validation Request.

Copy the above into a Validation Response email…

And send back to the Mailing List. (Assuming all is hunky dory)

Since there are so many NuttX devices, we really appreciate your help with the NuttX Validation! 🙏

What are the updates to the NuttX Validation Instructions?

The NuttX Validation Instructions should be updated…