Ci: testing frame order and fps - #330
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive FPS testing for V4L2 cameras, verifying frame timing, sequencing, and delivery. The implementation validates that frames arrive at the correct rate, in proper order, and without significant drops.
Key changes:
- New
test_fps.pyvalidates frame rates, detects dropped/repeated frames, and ensures frame counts match expectations - Enhanced
test_fw_version.pywith proper Python 3 string handling using.decode() - Added Jenkins pipeline configuration for automated CI testing
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_fps.py | New test validating FPS accuracy, frame ordering, and completeness across different video formats |
| test/test_fw_version.py | Fixed Python 3 compatibility by decoding subprocess output to strings |
| test/test_metadata/build.sh | Added debug flags and removed test execution from build script |
| test/run_ci.py | Improved error reporting and added missing exit code on usage error |
| test/pytest.ini | Added pytest configuration with d457 marker for device-specific tests |
| test/groovy/LRS_libci_pipeline.groovy | New Jenkins pipeline for automated artifact deployment and testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # print(f"{frame}", end='') | ||
| if last: | ||
| assert frame > last, f"Repeated frame: {frame}" | ||
| assert (frame - last) < 3 , f"Frames dropped between: {last} and {frame}" |
There was a problem hiding this comment.
This assertion allows up to 2 dropped frames, but the PR description states that only 1 drop is allowed. Change < 3 to < 2 to match the specification.
| assert (frame - last) < 3 , f"Frames dropped between: {last} and {frame}" | |
| assert (frame - last) < 2 , f"Frames dropped between: {last} and {frame}" |
| timeout=timeout).stderr.splitlines() | ||
| last = None | ||
| skip = True # skip first FPS measurement | ||
| kpi = 5 # [%] |
There was a problem hiding this comment.
The KPI threshold of 5% is a magic number that should be extracted as a constant or test parameter to improve maintainability and make it easier to adjust.
| f"{frames}", | ||
| "--verbose", | ||
| ] | ||
| timeout = 4.0 * frames / FPS |
There was a problem hiding this comment.
The timeout multiplier of 4.0 is a magic number. Consider extracting it as a named constant (e.g., TIMEOUT_MULTIPLIER) to clarify its purpose and make it easier to adjust.
| check=True ) | ||
| except Exception as e: | ||
| print( "Exception occurred.") | ||
| print("Exception occurred: {}".format( e )) |
There was a problem hiding this comment.
Inconsistent spacing: there's an extra space before the closing parenthesis in .format( e ). Should be .format(e) to match Python style conventions.
Functional V4L2 camera FPS test verifies: