Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 100 additions & 2 deletions src/Camera/QGCCameraManager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@
#include "QGCVideoStreamInfo.h"
#include "SimulatedCameraControl.h"

#include <cmath>
#include "GimbalControllerSettings.h"
#include "SettingsManager.h"
#include <numbers>

constexpr double kPi = std::numbers::pi_v<double>;

QGC_LOGGING_CATEGORY(CameraManagerLog, "Camera.QGCCameraManager")

namespace {
Expand All @@ -28,6 +35,34 @@ namespace {

QVariantList QGCCameraManager::_cameraList;

static void _requestFovOnZoom_Handler(
void* user,
MAV_RESULT result,
Vehicle::RequestMessageResultHandlerFailureCode_t failureCode,
const mavlink_message_t& message)
{
auto* mgr = static_cast<QGCCameraManager*>(user);

if (result != MAV_RESULT_ACCEPTED) {
qCDebug(CameraManagerLog) << "CAMERA_FOV_STATUS request failed, result:"
<< result << "failure:" << failureCode;
return;
}

if (message.msgid != MAVLINK_MSG_ID_CAMERA_FOV_STATUS) {
qCDebug(CameraManagerLog) << "Unexpected msg id:" << message.msgid;
return;
}

mavlink_camera_fov_status_t fov{};

mavlink_msg_camera_fov_status_decode(&message, &fov);

if (!mgr) return;
if (auto* cam = mgr->currentCameraInstance()) {
}
Comment on lines +62 to +63
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

This function contains dead code. After checking if (!mgr) return; on line 61, and then checking if (auto* cam = mgr->currentCameraInstance()) on line 62, the body of the if statement (line 63) is empty.

Either:

  1. This code is incomplete and the handler should process the FOV data, or
  2. The handler is unnecessary since the actual processing happens in _handleCameraFovStatus().

If the handler is meant to be empty, consider removing it entirely and using a simpler callback, or add a comment explaining why it's empty.

Suggested change
if (auto* cam = mgr->currentCameraInstance()) {
}

Copilot uses AI. Check for mistakes.
}

/*===========================================================================*/

QGCCameraManager::CameraStruct::CameraStruct(QGCCameraManager *manager_, uint8_t compID_, Vehicle *vehicle_)
Expand Down Expand Up @@ -148,6 +183,9 @@ void QGCCameraManager::_mavlinkMessageReceived(const mavlink_message_t &message)
case MAVLINK_MSG_ID_CAMERA_TRACKING_IMAGE_STATUS:
_handleTrackingImageStatus(message);
break;
case MAVLINK_MSG_ID_CAMERA_FOV_STATUS:
_handleCameraFovStatus(message);
break;
default:
break;
}
Expand Down Expand Up @@ -289,6 +327,25 @@ void QGCCameraManager::_handleCameraInfo(const mavlink_message_t& message)
_cameraInfoRequest[sCompID]->backoffTimer.stop();
qCDebug(CameraManagerLog) << "Success for compId" << QGCMAVLink::compIdToString(message.compid) << "- reset retry counter";
}

double aspect = std::numeric_limits<double>::quiet_NaN();

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);
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
}

_aspectByCompId.insert(message.compid, aspect);

const double sensorH = static_cast<double>(info.sensor_size_h);
const double sensorV = static_cast<double>(info.sensor_size_v);
const double focal = static_cast<double>(info.focal_length);

if (sensorH > 0.0 && sensorV > 0.0 && focal > 0.0) {
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;
Comment on lines +346 to +347
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

Inconsistent use of PI constant. Lines 346-347 use M_PI, while the new code in lines 771-773 uses kPi (defined at line 26 as std::numbers::pi_v<double>).

For consistency within this file, either:

  1. Use kPi everywhere (lines 346, 347), or
  2. Use M_PI everywhere (lines 771, 773)

Since kPi is explicitly defined at the top of the file using C++20's std::numbers, it's preferred to use kPi consistently throughout.

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;
const double hfovDeg = 2.0 * std::atan(sensorH / (2.0 * focal)) * 180.0 / kPi;
const double vfovDeg = 2.0 * std::atan(sensorV / (2.0 * focal)) * 180.0 / kPi;

Copilot uses AI. Check for mistakes.
Comment on lines +346 to +347
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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:

  1. Remove these lines if they're not needed, or
  2. 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)

Copilot uses AI. Check for mistakes.
}
}

void QGCCameraManager::_checkForLostCameras()
Expand Down Expand Up @@ -364,13 +421,16 @@ void QGCCameraManager::_handleStorageInfo(const mavlink_message_t &message)
}
}

void QGCCameraManager::_handleCameraSettings(const mavlink_message_t &message)
void QGCCameraManager::_handleCameraSettings(const mavlink_message_t& message)
{
MavlinkCameraControl *pCamera = _findCamera(message.compid);
auto pCamera = _findCamera(message.compid);
if (pCamera) {
mavlink_camera_settings_t settings{};
mavlink_msg_camera_settings_decode(&message, &settings);
pCamera->handleSettings(settings);

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

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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:

  1. Making zoomValueCurrent an atomic variable (std::atomic<int>), or
  2. Protecting access with a mutex, or
  3. Using Qt's thread-safe signaling mechanism to update the value on the main thread

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
requestCameraFovForComp(message.compid);
}
}

Expand Down Expand Up @@ -680,3 +740,41 @@ const QVariantList &QGCCameraManager::cameraList() const
}
return _cameraList;
}

void QGCCameraManager::requestCameraFovForComp(int compId) {
_vehicle->requestMessage(_requestFovOnZoom_Handler, /*user*/this,
compId, MAVLINK_MSG_ID_CAMERA_FOV_STATUS);
}
Comment on lines +744 to +747
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

Missing defensive check: requestCameraFovForComp() calls _vehicle->requestMessage() without checking if _vehicle is valid. While it's unlikely to be null during normal operation, following QGC's defensive coding guidelines, this should be validated.

Add a null check:

void QGCCameraManager::requestCameraFovForComp(int compId) {
    if (!_vehicle) {
        qCWarning(CameraManagerLog) << "requestCameraFovForComp: vehicle is null";
        return;
    }
    _vehicle->requestMessage(_requestFovOnZoom_Handler, /*user*/this,
                             compId, MAVLINK_MSG_ID_CAMERA_FOV_STATUS);
}

Copilot generated this review using guidance from repository custom instructions.

//-----------------------------------------------------------------------------
double QGCCameraManager::aspectForComp(int compId) const {
auto it = _aspectByCompId.constFind(compId);
return (it == _aspectByCompId.cend())
? std::numeric_limits<double>::quiet_NaN()
: it.value();
}

double QGCCameraManager::currentCameraAspect() const {
if (auto* cam = const_cast<QGCCameraManager*>(this)->currentCameraInstance()) {
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

Using const_cast to call a non-const method from a const method is a design issue. The method currentCameraInstance() is declared as non-const (line 74 in the header), but is being called from the const method currentCameraAspect().

Consider either:

  1. Making currentCameraInstance() const if it doesn't modify state, or
  2. Adding a const overload: const MavlinkCameraControl* currentCameraInstance() const;

The const_cast here breaks const-correctness guarantees and should be avoided.

Suggested change
if (auto* cam = const_cast<QGCCameraManager*>(this)->currentCameraInstance()) {
if (auto* cam = currentCameraInstance()) {

Copilot uses AI. Check for mistakes.
return aspectForComp(cam->compID());
}
return std::numeric_limits<double>::quiet_NaN();
}
void QGCCameraManager::_handleCameraFovStatus(const mavlink_message_t& message)
{
mavlink_camera_fov_status_t fov{};
mavlink_msg_camera_fov_status_decode(&message, &fov);
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;
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
const double hfovRad = fov.hfov * kPi / 180.0;
const double vfovRad = 2.0 * std::atan(std::tan(hfovRad * 0.5) * aspect);
const double vfovDeg = vfovRad * 180.0 / kPi;
SettingsManager::instance()->gimbalControllerSettings()->CameraHFov()->setRawValue(QVariant::fromValue(fov.hfov));
SettingsManager::instance()->gimbalControllerSettings()->CameraVFov()->setRawValue(QVariant::fromValue(vfovDeg));
Comment on lines +773 to +775
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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 generated this review using guidance from repository custom instructions.
}

int QGCCameraManager::currentZoomLevel() const {
return zoomValueCurrent;
}
10 changes: 10 additions & 0 deletions src/Camera/QGCCameraManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,15 @@ class QGCCameraManager : public QObject

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

int zoomValueCurrent = 0;
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
int currentZoomLevel() const;
double aspectForComp(int compId) const;
double currentCameraAspect() const;
Q_INVOKABLE void requestCameraFovForComp(int compId);

private:
QHash<int, double> _aspectByCompId;
Comment on lines +91 to +92
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.

signals:
void camerasChanged();
void cameraLabelsChanged();
Expand Down Expand Up @@ -118,6 +127,7 @@ protected slots:
void _handleBatteryStatus(const mavlink_message_t& message);
void _handleTrackingImageStatus(const mavlink_message_t& message);
void _addCameraControlToLists(MavlinkCameraControl* cameraControl);
void _handleCameraFovStatus(const mavlink_message_t& message);

QPointer<Vehicle> _vehicle;
QPointer<SimulatedCameraControl> _simulatedCameraControl;
Expand Down
2 changes: 1 addition & 1 deletion src/Gimbal/Gimbal.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,4 @@ class Gimbal : public FactGroup
bool _retracted = false;
bool _haveControl = false;
bool _othersHaveControl = false;
};
};
59 changes: 59 additions & 0 deletions src/Gimbal/GimbalController.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
#include "QmlObjectListModel.h"
#include "SettingsManager.h"
#include "Vehicle.h"
#include <cmath>
#include "Gimbal.h"
#include "QGCCameraManager.h"

QGC_LOGGING_CATEGORY(GimbalControllerLog, "Gimbal.GimbalController")

Expand Down Expand Up @@ -591,6 +594,62 @@ void GimbalController::sendRate()
}
}

void GimbalController::sendGimbalRate(float pitch_rate_deg_s, float yaw_rate_deg_s)
{
if (!_tryGetGimbalControl()) {
return;
}

_sendGimbalAttitudeRates(pitch_rate_deg_s, yaw_rate_deg_s);

if (pitch_rate_deg_s == 0.f && yaw_rate_deg_s == 0.f) {
_rateSenderTimer.stop();
} else {
_rateSenderTimer.start();
}
}

void GimbalController::_sendGimbalAttitudeRates(float pitch_rate_deg_s,
float yaw_rate_deg_s)
{

auto sharedLink = _vehicle->vehicleLinkManager()->primaryLink().lock();
if (!sharedLink) {
qCDebug(GimbalControllerLog) << "_sendGimbalAttitudeRates: primary link gone!";
return;
}

uint32_t flags =
GIMBAL_MANAGER_FLAGS_ROLL_LOCK |
GIMBAL_MANAGER_FLAGS_PITCH_LOCK |
GIMBAL_MANAGER_FLAGS_YAW_IN_VEHICLE_FRAME; // use vehicle/body frame

// Preserve current yaw-lock state instead of changing it:
if (_activeGimbal->yawLock()) {
flags |= GIMBAL_MANAGER_FLAGS_YAW_LOCK;
}

const float qnan[4] = {NAN, NAN, NAN, NAN};
mavlink_message_t msg;

mavlink_msg_gimbal_manager_set_attitude_pack_chan(
MAVLinkProtocol::instance()->getSystemId(),
MAVLinkProtocol::getComponentId(),
sharedLink->mavlinkChannel(),
&msg,
_vehicle->id(),
static_cast<uint8_t>(_activeGimbal->managerCompid()->rawValue().toUInt()),
flags,
static_cast<uint8_t>(_activeGimbal->deviceId()->rawValue().toUInt()),
qnan,
NAN,
qDegreesToRadians(pitch_rate_deg_s),
qDegreesToRadians(yaw_rate_deg_s)
);

_vehicle->sendMessageOnLinkThreadSafe(sharedLink.get(), msg);
}

void GimbalController::_rateSenderTimeout()
{
// Send rate again to avoid timeout on autopilot side.
Expand Down
4 changes: 4 additions & 0 deletions src/Gimbal/GimbalController.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ class GimbalController : public QObject
Q_INVOKABLE void releaseGimbalControl();
Q_INVOKABLE void sendRate();

Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
Q_INVOKABLE void sendGimbalRate(float pitch_rate_deg_s, float yaw_rate_deg_s);

signals:
void activeGimbalChanged();
void showAcquireGimbalControlPopup(); // This triggers a popup in QML asking the user for aproval to take control
Expand Down Expand Up @@ -105,6 +107,8 @@ private slots:
bool _tryGetGimbalControl();
bool _yawInVehicleFrame(uint32_t flags);

void _sendGimbalAttitudeRates(float pitch_rate_deg_s, float yaw_rate_deg_s);

QTimer _rateSenderTimer;
Vehicle *_vehicle = nullptr;
Gimbal *_activeGimbal = nullptr;
Expand Down
18 changes: 16 additions & 2 deletions src/Settings/GimbalController.SettingsGroup.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@
"name": "CameraVFov",
"shortDesc": "Vertical camera field of view",
"type": "uint32",
"default": 70,
"default": 80,
"units": "deg"
},
{
"name": "CameraHFov",
"shortDesc": "Horizontal camera field of view",
"type": "uint32",
"default": 70,
"default": 80,
"units": "deg"
},
{
Expand Down Expand Up @@ -63,6 +63,20 @@
"type": "uint32",
"default": 20,
"units": "deg/s"
},
{
"name": "zoomMinSpeed",
"shortDesc": "Maximum gimbal speed for min zoom (deg/sec)",
"type": "uint32",
"default": 5,
"units": "deg/s"
},
{
"name": "zoomMaxSpeed",
"shortDesc": "Minimum gimbal speed for max zoom (deg/sec)",
"type": "uint32",
"default": 60,
"units": "deg/s"
}
]
}
2 changes: 2 additions & 0 deletions src/Settings/GimbalControllerSettings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,5 @@ DECLARE_SETTINGSFACT(GimbalControllerSettings, showAzimuthIndicatorOnMap)
DECLARE_SETTINGSFACT(GimbalControllerSettings, toolbarIndicatorShowAzimuth)
DECLARE_SETTINGSFACT(GimbalControllerSettings, toolbarIndicatorShowAcquireReleaseControl)
DECLARE_SETTINGSFACT(GimbalControllerSettings, joystickButtonsSpeed)
DECLARE_SETTINGSFACT(GimbalControllerSettings, zoomMaxSpeed)
DECLARE_SETTINGSFACT(GimbalControllerSettings, zoomMinSpeed)
2 changes: 2 additions & 0 deletions src/Settings/GimbalControllerSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,6 @@ class GimbalControllerSettings : public SettingsGroup
DEFINE_SETTINGFACT(toolbarIndicatorShowAzimuth)
DEFINE_SETTINGFACT(toolbarIndicatorShowAcquireReleaseControl)
DEFINE_SETTINGFACT(joystickButtonsSpeed)
DEFINE_SETTINGFACT(zoomMaxSpeed)
DEFINE_SETTINGFACT(zoomMinSpeed)
};
26 changes: 26 additions & 0 deletions src/UI/toolbar/GimbalIndicator.qml
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,32 @@ Item {
}
}

SettingsGroupLayout {
heading: qsTr("Zoom speed")
showDividers: false

QGCLabel {
text: qsTr("Max speed (min zoom):")
visible: true
}

FactTextField {
fact: QGroundControl.settingsManager.gimbalControllerSettings.zoomMaxSpeed
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

Inconsistent reference style. This code uses the full path QGroundControl.settingsManager.gimbalControllerSettings.zoomMaxSpeed, while the surrounding code (lines 248, 253, 259, 265, 271, 305) uses the local _gimbalControllerSettings reference.

For consistency with the rest of the file, use:

fact: _gimbalControllerSettings.zoomMaxSpeed
Suggested change
fact: QGroundControl.settingsManager.gimbalControllerSettings.zoomMaxSpeed
fact: _gimbalControllerSettings.zoomMaxSpeed

Copilot uses AI. Check for mistakes.
visible: true
}

QGCLabel {
text: qsTr("Min speed (max zoom):")
visible: true
}

FactTextField {
fact: QGroundControl.settingsManager.gimbalControllerSettings.zoomMinSpeed
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
visible: true
}
Comment on lines +280 to +298
Copy link

Copilot AI Nov 15, 2025

Choose a reason for hiding this comment

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

Inconsistent indentation and style in this section. The QGCLabel and FactTextField elements have extra indentation (lines 280-298) compared to the surrounding code pattern.

Compare with the pattern used in lines 257-273 where LabelledFactTextField is used consistently. Consider using the same pattern:

LabelledFactTextField {
    label:      qsTr("Max speed (min zoom)")
    fact:       _gimbalControllerSettings.zoomMaxSpeed
}

LabelledFactTextField {
    label:      qsTr("Min speed (max zoom)")
    fact:       _gimbalControllerSettings.zoomMinSpeed
}

Also note the hardcoded visible: true properties are unnecessary as visibility defaults to true.

Suggested change
QGCLabel {
text: qsTr("Max speed (min zoom):")
visible: true
}
FactTextField {
fact: QGroundControl.settingsManager.gimbalControllerSettings.zoomMaxSpeed
visible: true
}
QGCLabel {
text: qsTr("Min speed (max zoom):")
visible: true
}
FactTextField {
fact: QGroundControl.settingsManager.gimbalControllerSettings.zoomMinSpeed
visible: true
}
LabelledFactTextField {
label: qsTr("Max speed (min zoom)")
fact: QGroundControl.settingsManager.gimbalControllerSettings.zoomMaxSpeed
}
LabelledFactTextField {
label: qsTr("Min speed (max zoom)")
fact: QGroundControl.settingsManager.gimbalControllerSettings.zoomMinSpeed
}

Copilot uses AI. Check for mistakes.

}

SettingsGroupLayout {
LabelledFactTextField {
label: qsTr("Joystick buttons speed:")
Expand Down
2 changes: 0 additions & 2 deletions src/Vehicle/VehicleSetup/JoystickConfigCalibration.qml
Original file line number Diff line number Diff line change
Expand Up @@ -199,5 +199,3 @@ Item {
}
}
}


Loading