Gimbal speed auto fov #13612

Niki-dev12 · 2025-10-29T10:59:59Z

Auto-adjust gimbal FOV and zoom-dependent control
Description

This change requests updated camera field-of-view (FOV) information from the vehicle whenever the gimbal zoom level changes.
By using CAMERA_FOV_STATUS, QGC dynamically updates the horizontal and vertical FOV values and adjusts the click-to-point gimbal movement accordingly. When zoomed in, the gimbal motion becomes finer and moves a shorter distance per click, improving precision.

In addition, pitch and yaw rate limits are scaled based on zoom level to maintain smooth and intuitive control.

Test Steps

Connect to a drone equipped with a zoom gimbal.
Enable click-to-point functionality in Gimbal Settings.
Change the zoom level.
Verify that HFOV/VFOV values update automatically.
Click on the video stream and confirm that gimbal motion becomes more precise at higher zoom levels.

Checklist:

[ x] Review Contribution Guidelines.
[ x] Review Code of Conduct.
[ x] I have tested my changes.

Related Issue

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

(cherry picked from commit 43d58af)

…r-Repository

HTRamsey · 2025-11-01T14:17:43Z

Still got some // dev comments to remove

Niki-dev12 · 2025-11-03T06:22:43Z

Still got some // dev comments to remove

Absolutely, good catch. I will clean up the remaining // dev comments

Copilot

Pull Request Overview

This PR adds dynamic gimbal control speed adjustment based on camera zoom level, automatically updating FOV values when zoom changes to enable more precise gimbal movement at higher zoom levels.

Introduces zoom-dependent gimbal speed settings (zoomMinSpeed, zoomMaxSpeed)
Implements automatic FOV updates via CAMERA_FOV_STATUS MAVLink messages when zoom level changes
Adds new gimbal rate control method for direct rate specification

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 16 comments.

Show a summary per file

File	Description
src/Settings/GimbalController.SettingsGroup.json	Adds zoom speed settings with default values and updates default camera FOV from 70° to 80°
src/Settings/GimbalControllerSettings.h	Declares new SETTINGFACT entries for zoomMaxSpeed and zoomMinSpeed
src/Settings/GimbalControllerSettings.cc	Implements the new zoom speed settings
src/UI/toolbar/GimbalIndicator.qml	Adds UI controls for configuring zoom-dependent speed limits
src/Gimbal/GimbalController.h	Declares new public sendGimbalRate() and private _sendGimbalAttitudeRates() methods
src/Gimbal/GimbalController.cc	Implements direct gimbal rate control via MAVLink GIMBAL_MANAGER_SET_ATTITUDE messages
src/Camera/QGCCameraManager.h	Adds zoom level tracking, aspect ratio storage, and FOV request methods
src/Camera/QGCCameraManager.cc	Implements CAMERA_FOV_STATUS handling to update FOV settings dynamically based on zoom, includes aspect ratio calculation from camera info
src/Vehicle/VehicleSetup/JoystickConfigCalibration.qml	Removes trailing blank lines (formatting cleanup)
src/Gimbal/Gimbal.h	Removes trailing blank line (formatting cleanup)

Copilot · 2025-11-15T02:31:04Z

src/Camera/QGCCameraManager.cc

+    if (!std::isfinite(fov.hfov) || fov.hfov <= 0.0 || fov.hfov >= 180.0) return;
+    double aspect = aspectForComp(message.compid);
+
+    if (!std::isfinite(aspect) || aspect <= 0.0) aspect = 9.0/16.0;


The default aspect ratio fallback appears to be incorrect. Line 770 uses 9.0/16.0 (= 0.5625), which is actually width/height.

However, based on the calculation in line 334 where aspect = double(info.resolution_v) / double(info.resolution_h), the aspect ratio is defined as vertical/horizontal (height/width).

The fallback should be 16.0/9.0 (≈ 1.778) for a standard 16:9 display, not 9.0/16.0.

Change to:

if (!std::isfinite(aspect) || aspect <= 0.0) aspect = 16.0/9.0;

Suggested change

if (!std::isfinite(aspect) || aspect <= 0.0) aspect = 9.0/16.0;

if (!std::isfinite(aspect) || aspect <= 0.0) aspect = 16.0/9.0;

Copilot · 2025-11-15T02:31:04Z

src/Camera/QGCCameraManager.cc

+    if (info.resolution_h > 0 && info.resolution_v > 0) {
+        aspect = double(info.resolution_v) / double(info.resolution_h);
+    } else if (info.sensor_size_h > 0.f && info.sensor_size_v > 0.f) {
+        aspect = double(info.sensor_size_v / info.sensor_size_h);


Inconsistent type handling in aspect ratio calculation. Line 334 correctly casts both operands to double before division, but line 336 performs the division in float precision first (both operands are float), then casts the result to double.

For consistency and to avoid potential precision loss, line 336 should match line 334's pattern:

aspect = double(info.sensor_size_v) / double(info.sensor_size_h);

This ensures both calculations use the same precision approach.

Suggested change

aspect = double(info.sensor_size_v / info.sensor_size_h);

aspect = double(info.sensor_size_v) / double(info.sensor_size_h);

Copilot · 2025-11-15T02:31:04Z

src/Camera/QGCCameraManager.cc

        mavlink_msg_camera_settings_decode(&message, &settings);
        pCamera->handleSettings(settings);
+
+        zoomValueCurrent = static_cast<double>(settings.zoomLevel);


Potential thread safety issue: zoomValueCurrent is accessed from _handleCameraSettings() which is called from the MAVLink message handler (potentially on a different thread), and also from currentZoomLevel() which could be called from the main/QML thread.

According to the QGC guidelines, "MAVLink messages processed on LinkManager thread" and "UI updates must happen on main thread". If currentZoomLevel() is called from QML, there's a potential race condition.

Consider:

Making zoomValueCurrent an atomic variable (std::atomic<int>), or

Protecting access with a mutex, or

Using Qt's thread-safe signaling mechanism to update the value on the main thread

Copilot · 2025-11-15T02:31:05Z

src/Camera/QGCCameraManager.h


    CameraStruct* findCameraStruct(uint8_t compId) const { return _cameraInfoRequest.value(QString::number(compId), nullptr); }

+    int zoomValueCurrent = 0;


Public member variable zoomValueCurrent breaks encapsulation. This should be a private member with proper accessor methods, following the pattern used elsewhere in the class.

Consider making it private:

private: int _zoomValueCurrent = 0;

And keeping only the getter method public (line 86) which already exists.

Suggested change

int zoomValueCurrent = 0;

private:

int _zoomValueCurrent = 0;

public:

Copilot · 2025-11-15T02:31:05Z

src/UI/toolbar/GimbalIndicator.qml

+                    }
+
+                    FactTextField {
+                        fact:               QGroundControl.settingsManager.gimbalControllerSettings.zoomMinSpeed


Inconsistent reference style. This code uses the full path QGroundControl.settingsManager.gimbalControllerSettings.zoomMinSpeed, while the surrounding code uses the local _gimbalControllerSettings reference.

For consistency with the rest of the file, use:

fact: _gimbalControllerSettings.zoomMinSpeed

Suggested change

fact: QGroundControl.settingsManager.gimbalControllerSettings.zoomMinSpeed

fact: _gimbalControllerSettings.zoomMinSpeed

Copilot · 2025-11-15T02:31:07Z

src/Camera/QGCCameraManager.cc

        mavlink_msg_camera_settings_decode(&message, &settings);
        pCamera->handleSettings(settings);
+
+        zoomValueCurrent = static_cast<double>(settings.zoomLevel);


Incorrect type cast: settings.zoomLevel is a float according to MAVLink definition, but it's being cast to double and then assigned to zoomValueCurrent which is declared as int in the header file (line 85 of QGCCameraManager.h).

This should be:

zoomValueCurrent = static_cast<int>(settings.zoomLevel);

Or consider changing the type of zoomValueCurrent to float if fractional zoom levels are meaningful.

Suggested change

zoomValueCurrent = static_cast<double>(settings.zoomLevel);

zoomValueCurrent = static_cast<int>(settings.zoomLevel);

Copilot · 2025-11-15T02:31:07Z

src/Camera/QGCCameraManager.h

+private:
+    QHash<int, double> _aspectByCompId;


The private: section on line 91 appears before signals: on line 94, which breaks the logical flow of the class declaration. The _aspectByCompId member should be moved to the main private section at the bottom of the class (after line 130), along with other private members like _vehicle, _cameras, etc.

This maintains consistency with the existing class structure where public interface comes first, then signals, then protected slots, then all private members together.

Copilot · 2025-11-15T02:31:08Z

src/Gimbal/GimbalController.h

    Q_INVOKABLE void acquireGimbalControl();
    Q_INVOKABLE void releaseGimbalControl();
    Q_INVOKABLE void sendRate();



Missing documentation for the new public method sendGimbalRate(). This Q_INVOKABLE method is exposed to QML but has no comment explaining its purpose or parameters.

Consider adding a brief comment above line 53, similar to other public methods in the class. For example:

/// Send gimbal attitude rates directly without using active gimbal's rate properties /// @param pitch_rate_deg_s Pitch rate in degrees per second /// @param yaw_rate_deg_s Yaw rate in degrees per second Q_INVOKABLE void sendGimbalRate(float pitch_rate_deg_s, float yaw_rate_deg_s);

Suggested change

/// Send gimbal attitude rates directly without using active gimbal's rate properties

/// @param pitch_rate_deg_s Pitch rate in degrees per second

/// @param yaw_rate_deg_s Yaw rate in degrees per second

Copilot · 2025-11-15T02:31:08Z

src/Camera/QGCCameraManager.cc

+    const double vfovDeg = vfovRad * 180.0 / kPi;
+    SettingsManager::instance()->gimbalControllerSettings()->CameraHFov()->setRawValue(QVariant::fromValue(fov.hfov));
+    SettingsManager::instance()->gimbalControllerSettings()->CameraVFov()->setRawValue(QVariant::fromValue(vfovDeg));


Missing validation for calculated vertical FOV. While fov.hfov is validated on line 767, the calculated vfovDeg (line 773) is not validated before being stored in settings (line 775).

Although the calculation should produce valid results for valid inputs, defensive coding suggests validating the output:

const double vfovDeg = vfovRad * 180.0 / kPi; if (!std::isfinite(vfovDeg) || vfovDeg <= 0.0 || vfovDeg >= 180.0) { qCWarning(CameraManagerLog) << "Invalid calculated VFOV:" << vfovDeg; return; }

Copilot · 2025-11-15T02:31:08Z

src/Camera/QGCCameraManager.cc

+        const double hfovDeg = 2.0 * std::atan(sensorH / (2.0 * focal)) * 180.0 / M_PI;
+        const double vfovDeg = 2.0 * std::atan(sensorV / (2.0 * focal)) * 180.0 / M_PI;


Dead code: the calculated FOV values hfovDeg and vfovDeg are computed but never used. These local variables are created in lines 346-347 but have no effect.

Either:

Remove these lines if they're not needed, or

Store/use these values somewhere (e.g., update settings or store in _aspectByCompId alongside the aspect ratio).

Suggested change

const double hfovDeg = 2.0 * std::atan(sensorH / (2.0 * focal)) * 180.0 / M_PI;

const double vfovDeg = 2.0 * std::atan(sensorV / (2.0 * focal)) * 180.0 / M_PI;

// FOV calculation removed: values were unused (dead code)

Niki-dev12 added 6 commits October 28, 2025 15:41

gimbal speed control

a9be1b6

(cherry picked from commit 43d58af)

auto FOV

4914bd6

fix

881656a

auto FOV

22f759a

Remove accidental submodule src/FirmwarePlugin/APM/ArduPilot-Paramete…

227de6f

…r-Repository

clean

1c4f721

dev clean up

76ad02d

HTRamsey requested a review from Copilot November 15, 2025 02:24

Copilot started reviewing on behalf of HTRamsey November 15, 2025 02:25 View session

Copilot finished reviewing on behalf of HTRamsey November 15, 2025 02:29

Copilot AI reviewed Nov 15, 2025

View reviewed changes

	if (!std::isfinite(aspect) \|\| aspect <= 0.0) aspect = 9.0/16.0;
	if (!std::isfinite(aspect) \|\| aspect <= 0.0) aspect = 16.0/9.0;

	aspect = double(info.sensor_size_v / info.sensor_size_h);
	aspect = double(info.sensor_size_v) / double(info.sensor_size_h);


		CameraStruct* findCameraStruct(uint8_t compId) const { return _cameraInfoRequest.value(QString::number(compId), nullptr); }

		int zoomValueCurrent = 0;

	fact: QGroundControl.settingsManager.gimbalControllerSettings.zoomMinSpeed
	fact: _gimbalControllerSettings.zoomMinSpeed

	zoomValueCurrent = static_cast<double>(settings.zoomLevel);
	zoomValueCurrent = static_cast<int>(settings.zoomLevel);

+    /// Send gimbal attitude rates directly without using active gimbal's rate properties
+    /// @param pitch_rate_deg_s Pitch rate in degrees per second
+    /// @param yaw_rate_deg_s Yaw rate in degrees per second

		const double hfovDeg = 2.0 * std::atan(sensorH / (2.0 * focal)) * 180.0 / M_PI;
		const double vfovDeg = 2.0 * std::atan(sensorV / (2.0 * focal)) * 180.0 / M_PI;

	const double hfovDeg = 2.0 * std::atan(sensorH / (2.0 * focal)) * 180.0 / M_PI;
	const double vfovDeg = 2.0 * std::atan(sensorV / (2.0 * focal)) * 180.0 / M_PI;
	// FOV calculation removed: values were unused (dead code)

Gimbal speed auto fov #13612

Are you sure you want to change the base?

Gimbal speed auto fov #13612

Conversation

Niki-dev12 commented Oct 29, 2025

Auto-adjust gimbal FOV and zoom-dependent control Description

Test Steps

Checklist:

Related Issue

Uh oh!

HTRamsey commented Nov 1, 2025

Uh oh!

Niki-dev12 commented Nov 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Auto-adjust gimbal FOV and zoom-dependent control
Description