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

Sticks : Add utility functions to get pitch, roll, throttle and yaw values more intuitively #19309

Merged
merged 4 commits into from
Mar 23, 2022

Conversation

junwoo091400
Copy link
Contributor

Describe problem solved by this pull request
'Sticks' utility library is used by Flight Tasks. While implementing a Follow-Target feature (#19260) with RC-controllable behavior, I found that it is unclear as to which 'user input' each input vector corresponds to.

Describe your solution
Added functions each returning user input values in a clear, intuitive naming.

  • Also renamed the 'checkAndSetStickInputs' function name to 'checkAndUpdateStickInputs', since 'set' doesn't imply that the stick values are update, which is the most important aspect of this function, thus avoiding possible confusions ;)

…alues intuitively and rename ambiguously named function checkAndSetStickInputs()
@dagar dagar requested a review from MaEtUgR March 10, 2022 16:15
…StickInputs() functions and correct Throttle returned value inverse issue
… throttle const to specify that it's not changing Sticks' internal variables
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

I was planning to introduce such getters so I'm really much in favor. Can we quickly discuss the details?

bool isAvailable() { return _input_available; };

// Position : 0 : pitch, 1 : roll, 2 : throttle, 3 : yaw
Copy link
Member

Choose a reason for hiding this comment

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

The definition and order of the sticks comes from the global definition here:

float32 x # stick position in x direction -1..1
# in general corresponds to forward/back motion or pitch of vehicle,
# in general a positive value means forward or negative pitch and
# a negative value means backward or positive pitch
float32 y # stick position in y direction -1..1
# in general corresponds to right/left motion or roll of vehicle,
# in general a positive value means right or positive roll and
# a negative value means left or negative roll
float32 z # throttle stick position 0..1
# in general corresponds to up/down motion or thrust of vehicle,
# in general the value corresponds to the demanded throttle by the user,
# if the input is used for setting the setpoint of a vertical position
# controller any value > 0.5 means up and any value < 0.5 means down
float32 r # yaw stick/twist position, -1..1
# in general corresponds to the righthand rotation around the vertical
# (downwards) axis of the vehicle

How about referencing it here instead of only explaining roll, pitch, yaw and throttle?

Copy link
Member

Choose a reason for hiding this comment

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

Closely related my goal is to unify the stick definitions and make the description easier to read:
https://github.com/PX4/PX4-Autopilot/pull/15949/files#diff-cdbc310b47e9f8baaa9198133a1adc0cb4ae571a0ca91c2ef3df45db4a1e216dR16-R25

src/modules/flight_mode_manager/tasks/Utility/Sticks.hpp Outdated Show resolved Hide resolved
Copy link
Member

@MaEtUgR MaEtUgR left a comment

Choose a reason for hiding this comment

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

We just discussed and the conclusion was the only thing that should change for clarity is the name of getThrottle() to be very explicit that it's [-1, 1].

src/modules/flight_mode_manager/tasks/Utility/Sticks.hpp Outdated Show resolved Hide resolved
@MaEtUgR MaEtUgR merged commit 8975062 into PX4:master Mar 23, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants