-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Feature: Add QML-configurable gimbal max speed and deadzones #13280
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
base: master
Are you sure you want to change the base?
Conversation
9546ecf
to
9b14afc
Compare
Could you look at the GimbalController and see if you can use that instead of changing Vehicle? |
src/Joystick/Joystick.cc
Outdated
static int zeroPitchCount = 0; | ||
static int zeroYawCount = 0; | ||
|
||
if (std::abs(gimbalPitchOut) == 0) { | ||
zeroPitchCount++; | ||
} else { | ||
zeroPitchCount = 0; | ||
} | ||
|
||
if (std::abs(gimbalYawOut) == 0) { | ||
zeroYawCount++; | ||
} else { | ||
zeroYawCount = 0; | ||
} | ||
|
||
if (zeroPitchCount >= 3 && zeroYawCount >= 3) { | ||
|
||
}else{ | ||
_activeVehicle->sendGimbalAbsolutePosition(gimbalPitchOut, gimbalYawOut); | ||
// qDebug() << gimbalYawOut << gimbalPitchOut; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a bit weird. Can you add a comment how it works. And also remove avoid the empty if clause.
And don't use static
there but instead it should be a class member variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gonna fix the static
. I use this code to stop overflowing MAVLink messages to the gimbal.
Without it QGroundControl cannot use the onscreen gimbal control.
When the slider is in the neutral / 0 position, the code stops sending gimbal commands.
The slider goes to neutral when you release it, because a spring centers it.
This prevents spamming the gimbal with useless "zero movement" commands when the stick is centered.
src/Joystick/Joystick.cc
Outdated
// float gimbalPitchDeg = gimbalPitchNorm * 160.0f - 80.0f; | ||
// float gimbalPitchDeg = gimbalPitchNorm * (2.0f * gimbalPitchMax) - gimbalPitchMax; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's this commented out code for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was a hard-coded max speed for gimbal movement. I forgot to remove it. It should not be there.
src/Gimbal/GimbalController.cc
Outdated
|
||
uint32_t flags = GIMBAL_MANAGER_FLAGS_ROLL_LOCK | GIMBAL_MANAGER_FLAGS_PITCH_LOCK; | ||
if (_activeGimbal && _activeGimbal->yawLock()) { | ||
flags |= GIMBAL_MANAGER_FLAGS_YAW_LOCK; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means the yaw mode is relative to North and not relative to forward. Is that expected?
src/Gimbal/GimbalController.cc
Outdated
} | ||
|
||
mavlink_message_t msg; | ||
mavlink_msg_command_long_pack_chan( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming we're sending this continuously? If so, a command is the wrong way, and we need to use the message GIMBAL_MANAGER_SET_ATTITUDE instead.
src/Gimbal/GimbalController.cc
Outdated
{ | ||
auto sharedLink = _vehicle->vehicleLinkManager()->primaryLink().lock(); | ||
if (!sharedLink) { | ||
qCDebug(GimbalControllerLog) << "_sendGimbalRateCommandLong: primary link gone!"; | ||
return; | ||
} | ||
|
||
uint32_t flags = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should set GIMBAL_MANAGER_FLAGS_ROLL_LOCK | GIMBAL_MANAGER_FLAGS_PITCH_LOCK
.
If you want to lock to some angle relative to North, you would also set GIMBAL_MANAGER_FLAGS_YAW_LOCK
, otherwise it's relative to vehicle forward and will turn with the vehicle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the note. My intent was vehicle-relative yaw; I have kept YAW_LOCK unset and now explicitly set ROLL_LOCK | PITCH_LOCK to make that clear.
src/Gimbal/GimbalController.cc
Outdated
void GimbalController::_sendGimbalRateCommandLong(float pitch_rate_deg_s, | ||
float yaw_rate_deg_s, | ||
uint32_t flags) | ||
float yaw_rate_deg_s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method name is now misleading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did change it
src/Gimbal/GimbalController.cc
Outdated
@@ -17,6 +17,8 @@ | |||
#include "Vehicle.h" | |||
#include <cmath> // for NAN | |||
|
|||
#define M_DEG_TO_RAD_F 0.0174532925f |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Define is unused.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is starting to look good.
Have you thought about the case where we don't have control over the gimbal yet and need to acquire it?
I suggest handling this in a follow up PR, since it feels a bit out of scope for the changes in this one. |
Title:
Add QML-configurable gimbal max speed and yaw/pitch deadzone settings
Description
This PR adds new joystick configuration features to allow runtime adjustment of:
Gimbal pitch and yaw max speed, using a single gimbalMaxSpeed parameter exposed to QML
Deadzone control for pitch and yaw gimbal axes via gimbalPitchDeadzone and gimbalYawDeadzone
Enable/disable axis-based gimbal control (gimbalAxisEnabled)
This allows users to fine-tune gimbal behavior without recompiling, which is especially useful when mapping joystick axes for dynamic camera control.
Changes include:
Test Steps
Tested in:
Checklist:
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.