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

Fix RC calibration with inverted throttle #21576

Closed
wants to merge 1 commit into from

Conversation

abarcis
Copy link
Contributor

@abarcis abarcis commented May 10, 2023

Solved Problem

RC calibration with the inverted throttle leads to wrong values being sent and eventually prevents arming of the vehicle.

Fixes #21575

@junwoo091400
Copy link
Contributor

@abarcis this PR is almost the same as this PR: #21542

Copy link
Contributor

@junwoo091400 junwoo091400 left a comment

Choose a reason for hiding this comment

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

Nice catch! Could you rebase to main branch to make sure the CI passes?

Also, would be nice to get a quick review from @MaEtUgR cuz this was introduced from your PR 🙏

@junwoo091400 junwoo091400 requested a review from MaEtUgR May 23, 2023 10:26
@DronecodeBot
Copy link

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/px4-maintainers-call-may-23-2023/32245/1

@abarcis
Copy link
Contributor Author

abarcis commented May 23, 2023

Nice catch! Could you rebase to main branch to make sure the CI passes?

Also, would be nice to get a quick review from @MaEtUgR cuz this was introduced from your PR pray

Thanks! I've rebased to main but it looks like some checks still did not pass. Is it expected?

@junwoo091400
Copy link
Contributor

Yes, tailistter VTOL SITL and MacOS CIs are expected to fail!

@MaEtUgR
Copy link
Member

MaEtUgR commented Jun 5, 2023

_parameters.rev[throttle_channel] is a boolean value and gets implicitly cast to an unsigned integer in this pr which of course works without warning in C. Also I'd explicitly split the cases of non-reversed && trim == min; reversed && trim == max. Sorry for the delay, I didn't understand it immediately 😇
My suggestion is this one: #21682

@junwoo091400
Copy link
Contributor

Thanks for this PR @abarcis!

We will continue with this PR: #21682, since it cleans up the logic further. Let me know if you have any comments!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RC calibration with the inverted throttle doesn't work
4 participants