Skip to content
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

AP_Camera: CAMERA_FOV_STATUS includes horizontal and vertical field-of-view #25441

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

rmackay9
Copy link
Contributor

@rmackay9 rmackay9 commented Nov 2, 2023

This small change built on top of PR #25390 populates the hfov and vfov fields in the CAMERA_FOV_STATUS message with values from two new CAMx_HFOV and CAMx_VFOV parameters.

This has been lightly tested in SITL and below are screen shots showing the two fields are populated.
camera-fov

Copy link
Contributor

@khancyr khancyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, match the mavlink spec

Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really convinced by the Range: documentation entries, but that's a minor thing

@rmackay9
Copy link
Contributor Author

rmackay9 commented Nov 2, 2023

@peterbarker,

Ah right, so the HFOV range should probably be 0 to 360 and VFOV should be 0 to 180? This PR is pretty new so I'm happy to fix it now.

@rmackay9
Copy link
Contributor Author

rmackay9 commented Nov 2, 2023

I've updated the HFOV and VFOV parameter ranges to be 0 to 360 and 0 to 180 respectively. Txs the feedback, I might merge after this passes CI.

@rmackay9 rmackay9 merged commit 73589a2 into ArduPilot:master Nov 2, 2023
86 checks passed
@rmackay9 rmackay9 deleted the camera-fov branch November 2, 2023 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants