benchmarks/cyclictest: the rt-tests cyclictest NuttX Port

[zdebanos]

Despite the existence of the patch in benchmarks/rt-tests, this commit adds the NuttX Official cyclictest utility.

The main difference is the introduction of different waiting methods next to POSIX clock_nanosleep:

• The thread can wait for a g_waitsem, posted by board_timerhook() if CONFIG_SYSTEMTICK_HOOK is defined. Since the semaphore is only one, only one thread can wait.
• The thread can wait for a Timer Device to timeout. The timer's timeout determines the waiting time of the thread. Since the timer is only one, again, only one thread can wait.

The user can measure the elapsed time using clock_gettime or the timer device itself. The different waiting and measuring methods were introduced because NuttX, by default, does not offer fine measuring capabilities using POSIX time functions (as of Feb 25).

Summary

Instead of a simple patch, NuttX now has an official cyclictest utility port. The new cyclictest was introduced due to limited fine measuring capabilities using POSIX time functions (clock_gettime and clock_nanosleep). Also this port adheres to the NuttX coding style.

Impact

Might be used in CI/CD to test the RT capabilities of NuttX.

Testing

Locally on a custom ATSAMV7 board.

[nuttxpr]

[Experimental Bot, please feedback here]

This PR description, while a good start, doesn't fully meet the NuttX requirements. Here's a breakdown and suggestions for improvement:

Missing/Insufficient Information:

• Summary: While you explain the why, you need to be more explicit about what changed and how. Mention specific files modified or added. For example: "This PR adds a cyclictest utility under tools/testing/cyclictest. It provides alternative waiting mechanisms using semaphores (when CONFIG_SYSTEMTICK_HOOK is enabled) and timer devices, along with measurement options using clock_gettime and timer devices. This allows for more accurate latency measurements than the existing patch in benchmarks/rt-tests."

• Impact: Be more specific and answer all the questions with YES/NO and explanations. Your current statement is vague. Examples:

  • Is new feature added? YES (cyclictest utility)
  • Is existing feature changed? NO
  • Impact on user: YES (Users can now utilize a dedicated cyclictest utility with enhanced features)
  • Impact on build: YES (Need to add tools/testing/cyclictest to the build system - describe how, e.g., a new Kconfig option)
  • Impact on hardware: Potentially YES (if using the timer device method - specify which timers are supported). If no specific hardware changes are needed, explicitly state NO and briefly explain why.
  • Impact on documentation: YES (Documentation needs to be added explaining how to build and use the new cyclictest utility, including configuration options).
  • Impact on security: NO (Provide a brief justification. E.g., "No new system calls or privileged operations are introduced.")
  • Impact on compatibility: NO (Provide a brief justification. E.g., "No breaking changes to existing APIs.")

• Testing: Provide much more detail. "Locally on a custom ATSAMV7 board" is not sufficient.

  • Build Host(s): Specify OS version, CPU architecture, compiler version (e.g., "Linux Ubuntu 20.04, x86_64, GCC 9.4.0").
  • Target(s): Be specific about the ATSAMV7 board and its configuration (e.g., "ATSAMV71-XULT, configured with CONFIG_SYSTEMTICK_HOOK=y and using Timer/Counter 0").
  • Testing logs: You must include actual log output demonstrating the cyclictest's functionality, both before (showing the limitations of the existing approach or the absence of the tool) and after the change. Show different test cases (different wait/measurement methods). Even a few lines demonstrating successful execution and basic output would be better than nothing.

Revised PR Description Example (incorporating the above suggestions):

## Summary

This PR adds a `cyclictest` utility under `tools/testing/cyclictest`. It provides alternative waiting mechanisms using semaphores (when `CONFIG_SYSTEMTICK_HOOK` is enabled) and timer devices, along with measurement options using `clock_gettime` and timer devices. This allows for more accurate latency measurements than the existing patch in `benchmarks/rt-tests`.  Files added: `tools/testing/cyclictest/cyclictest.c`, `tools/testing/cyclictest/Kconfig`, `tools/testing/cyclictest/Makefile`.

## Impact

* **Is new feature added?** YES (cyclictest utility)
* **Is existing feature changed?** NO
* **Impact on user:** YES (Users can now utilize a dedicated cyclictest utility with enhanced features for measuring latency.)
* **Impact on build:** YES (A new Kconfig option `CONFIG_TOOLS_CYCLICTEST` has been added to enable building the cyclictest utility.)
* **Impact on hardware:** YES (The timer device wait method utilizes Timer/Counter 0.  This could potentially conflict with other uses of this timer.)
* **Impact on documentation:** YES (Documentation has been added to the `tools/testing` directory explaining how to build and use the cyclictest utility, including its configuration options.)
* **Impact on security:** NO (No new system calls or privileged operations are introduced.)
* **Impact on compatibility:** NO (No breaking changes to existing APIs.)

## Testing

I confirm that changes are verified on a local setup and work as intended:
* **Build Host(s):** Linux Ubuntu 22.04, x86_64, GCC 11.2.0
* **Target(s):** ATSAMV71-XULT, configured with `CONFIG_SYSTEMTICK_HOOK=y` and `CONFIG_TOOLS_CYCLICTEST=y`, using Timer/Counter 0.

Testing logs before change (demonstrating lack of dedicated cyclictest and limitations of existing methods):

No cyclictest utility found in NuttX

$ cyclictest
bash: cyclictest: command not found

Using existing inadequate method (e.g., from benchmarks/rt-tests - provide a concrete example)

... (output showing limitations in precision or functionality) ...

Testing logs after change:

Using semaphore wait method

$ cyclictest -p 90 -d 0 -n -h 0 -t 1 -i 200000 -l 1000

... output showing consistent latency measurements ...

Using timer device wait method

$ cyclictest -p 90 -d 1 -n -h 0 -t 1 -i 200000 -l 1000

... output showing consistent latency measurements ...

By providing this level of detail, you significantly increase the chances of your PR being accepted quickly. Remember to adapt this example to accurately reflect your specific changes and testing results.

[acassis]

Nice work @zdebanos !!! Please submit also a Documentation/ to the NuttX repository show usages examples, expected results, how to spot issues using the cyclictest, etc.

[cederom]

Thank you @zdebanos great work! :-)

• Please resolve discussion with @xiaoxiang781216 :-)
• Please provide documentation :-)

[zdebanos]

Thank you very much for the review! I've been going through the code and there are still some bugs I did not think about. But as I have time, I'll prepare the documentation and fix some of those things so it is stable.

[zdebanos]

I've managed to prepare the documentation for the cyclictest utility, I've created a pull request here. Concerning the semaphore systemtickhook discussion, I've decided to delete such measurement from this cyclictest, so I mark this as resolved @xiaoxiang781216.

I actually did not think twice before implemententing such thing because I found out that measuring the latency would probably be too much of a hassle (in the board_timerhook function, one would need to measure the first timestamp, so that would require putting the board_timerhook function definition in the cyclictest.c file, which is not ideal). Since this cyclictest program allows for up to 4 measure-waiting combinations, I've also fixed the calculations.

However, I do not have time right now to do proper testing with the POSIX time functions, because it would require project reconfiguration to the tickless mode.

The command cyclictest -p 150 -T /dev/timer1 -m 1 -n 1 -h 20 -D 100 -i 50 (it's mentioned in the documentation) uses the timer device to measure the latencies. Its output is:

  # Histogram
  000000 000000
  000001 000000
  000002 000000
  000003 000000
  000004 000000
  000005 000000
  000006 000000
  000007 000000
  000008 000000
  000009 000000
  000010 603045
  000011 1395782
  000012 000804
  000013 000153
  000014 000034
  000015 000083
  000016 000030
  000017 000000
  000018 000000
  000019 000000
  # Total: 001999931
  # Min Latencies: 00010
  # Avg Latencies: 00010
  # Max Latencies: 00016
  # Histogram Overflows: 00000

Tested on ATSAMV71Q21B 300 MHz, Cortex M7. The units are in us.

[cederom]

Thank you @zdebanos for the update and documentation!! :-)

I think this is a good starting point and for sure may be updated soon wit other options / switches / methods? What you you think @xiaoxiang781216 can we accept it in current form ?

[cederom]

@acassis you have merged PR with "changes requested" and unresolved discussion :-( :-(

[acassis]

@cederom sorry, but I think he fixed it