fix: VehicleCameraControl firmwareVersion is backwards from MAVLink spec and doesn't include dev#14285
fix: VehicleCameraControl firmwareVersion is backwards from MAVLink spec and doesn't include dev#14285jnomikos wants to merge 1 commit intomavlink:masterfrom
Conversation
…pec and doesn't include dev
There was a problem hiding this comment.
Pull request overview
Fixes camera firmware version parsing in VehicleCameraControl to match the MAVLink CAMERA_INFORMATION.firmware_version encoding, and optionally includes the “dev” component when present.
Changes:
- Decode
firmware_versionbytes according to MAVLink spec (Major in lowest byte). - Return an empty string when
firmware_versionis0(unknown). - Append the dev component as a 4th version field when non-zero.
| int major = (_mavlinkCameraInfo.firmware_version) & 0xFF; | ||
| int minor = (_mavlinkCameraInfo.firmware_version >> 8) & 0xFF; | ||
| int patch = (_mavlinkCameraInfo.firmware_version >> 16) & 0xFF; | ||
| int dev = (_mavlinkCameraInfo.firmware_version >> 24) & 0xFF; | ||
| if (dev != 0) { |
There was a problem hiding this comment.
The new MAVLink-spec firmware_version decoding logic should be covered by a unit test (e.g., cases for 0 -> empty/unknown, dev==0 -> "major.minor.patch", dev!=0 -> "major.minor.patch.dev") so regressions don’t silently reintroduce the byte-order issue.
| if (_mavlinkCameraInfo.firmware_version == 0) { | ||
| return {}; | ||
| } |
There was a problem hiding this comment.
The PR description checklist indicates tests were added/updated, but this change set only updates production code. Either add a corresponding test (likely under test/Camera/VehicleCameraControlTest.cc) or update the PR checklist to reflect what was actually done.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14285 +/- ##
=========================================
Coverage ? 25.62%
=========================================
Files ? 754
Lines ? 68005
Branches ? 31514
=========================================
Hits ? 17429
Misses ? 37898
Partials ? 12678
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Build ResultsPlatform Status
All builds passed. Pre-commit
Pre-commit hooks: 4 passed, 36 failed, 7 skipped. Test Resultslinux-coverage: 78 passed, 0 skipped Code CoverageCoverage: 58.2% No baseline available for comparison Artifact Sizes
Updated: 2026-04-16 18:40:39 UTC • Triggered by: Android |
|
I don't think tests are necessary due to how simple the logic is, but I can add if needed |
Description
According to MAVLink spec for CAMERA_INFORMATION, Version of the camera firmware, encoded as: (Dev & 0xff) << 24 + (Patch & 0xff) << 16 + (Minor & 0xff) << 8 + (Major & 0xff). Use 0 if not known.
When viewing the version of my camera server in a custom QGC, I noticed the firmware version was reversed. This is because it isn't following the MAVLink spec correctly.
As a note, I made the intentional choice to make dev optional, because not everyone uses it. Even in the MAVSDK camera server example, they don't use dev and instead just do "1.0.0".
Type of Change
Testing
Platforms Tested
Flight Stacks Tested
Screenshots
Checklist
Related Issues
By submitting this pull request, I confirm that my contribution is made under the terms of the project's dual license (Apache 2.0 and GPL v3).