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
Scale manual control setpoint up/down stick -1 to 1 #15949
Changes from all commits
8073a70
34729cf
03f8842
3835c51
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2714,8 +2714,8 @@ void Commander::manualControlCheck() | |
|
||
if (manual_control_updated && manual_control_setpoint.valid) { | ||
|
||
_is_throttle_above_center = (manual_control_setpoint.z > 0.6f); | ||
_is_throttle_low = (manual_control_setpoint.z < 0.1f); | ||
_is_throttle_above_center = (manual_control_setpoint.throttle > 0.2f); | ||
_is_throttle_low = (manual_control_setpoint.throttle < -0.8f); | ||
Comment on lines
+2717
to
+2718
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not directly related, but this I think may be better handled if we have a function to just calculate these flags off of raw manual control setpoint struct data (preferably a class): // If it is a C++ Class
manual_control_setpoint.isThrottleAboveCenter() const
{
return (_data.throttle > 0.2f);
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely. I plan to do a library next that allows to get this information and directly scaled throttle z, ... from the raw manual_control_setpoint message. I think that's much nicer than having repeated thresholds everywhere. But shouldn't be part of this pr. |
||
|
||
if (_arm_state_machine.isArmed()) { | ||
// Abort autonomous mode and switch to position mode if sticks are moved significantly | ||
|
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.
Previously the mappings were vehicle and flight mode agnostic. An MC vehicle in position mode does not roll or pitch - it move forward or left-right. So you've made this very unintuitive except for fixed wing vehicles.
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 do not agree, previously the naming according to the cartesian coordinate system's axes
x
,y
,z
only fully made sense for multirotor position mode where the values moved the drone along the respective body axis.r
I could never make sense of.I agree it doesn't fit entirely and it's in my eyes impossible to find a naming that fully fits every single possible mode, that's why I favored consistency across drone remotes.
I disagree. For someone who never worked with drones before and has no reference from any other project or product you could argue the previous naming was equal as suitable. But this is clearly not the case. As I elaborated in #15949 (comment) people come with previous knowledge and or use a remote of some brand and the roll, pitch, yaw, throttle naming is the most popular and consistent one out in the hobby and industry. So it's not just me who got confused when joining PX4 why it needs to have different naming than everyone else, every time has to think twice about which axis it now is and keep a translation table in the comments but numerous other people I asked and found reference for in the code.
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.
After 5 years working with PX4 I still had to look up every time what x,y,z and r are linked to, so for me these new namings are massively more intuitive.