Skip to content

Remove RC Throttle trim calibration value getting hard-set to Minimum value #10497

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

junwoo091400
Copy link
Contributor

@junwoo091400 junwoo091400 commented Nov 19, 2022

Descrption

A hotfix commit from 7 years ago was forcing RC transmitters to always have a trim value set to 'Min', effectively only allowing the [0, 1000] range.

While for legacy reasons & to bypass cases of spring-loaded transmitters, this may be ok, it is certainly not true regarding reversible thrust vehicles (e.g. Boat / Rovers), and it is confusing why only the throttle would be in the range [0, 1], and not [-1 ,1], as defined in the MANUAL_CONTROL MAVLink message definition.

Changes

Removed this hotfix, and allows setting the trim values automatically in the state machine (need to check logic once again)

Context

The discussion that discovered this flaw is here: PX4/PX4-Autopilot#15949 (comment)

We need to also clearly define the point about the definition of MANUAL_CONTROL's throttle (z) definition. As it doesn't really make sense to have throttle vector's orientation interfere with the RC transmitter's setpoint values (it should be agnostic to vehicle's behavior to be basic).

Also, PX4 will be able to handle this change using this logic (although some legacy parameter translations / user awarenesses would be needed): PX4/PX4-Autopilot#15949

And we need to make sure for ArduPilot this will work out as well. Judging from the way Steer / Throttle Manual override isn't differentiated in Rover code, I guess ArduPilot already uses full range (-1000 ~ +1000) definition for the internal RC throttle data, @peterbarker. Did you not have problem with QGC, regarding this point?

TODOs

  • I am still unclear how the rcTrim value would then get set for throttle, need to check the state machine logic to be sure.

hotfix

- A hotfix commit from 7 years ago was forcing RC transmitters to
  always have a trim value set to 'Min', effectively only allowing the
  [0, 1] range of the `manual_control_setpoint.throttle`.

  This was the commit:
  mavlink@0577af2
- This removes this hotfix, and allows setting the trim values
  automatically in the state machine (need to check logic once again)
- Discussion was here:
  PX4/PX4-Autopilot#15949 (comment)
@DonLakeFlyer
Copy link
Contributor

I don't quite understand what the original code that was removed was actually doing. That said as long as spring loaded throttle still works fine.

But this comment is concering: "although some legacy parameter translations / user awarenesses would be needed"

In general there is pretty much no user awareness on anything.

@MaEtUgR
Copy link
Collaborator

MaEtUgR commented Nov 21, 2022

In short:

We have to be careful with:

  • backward compatibility, if the trim is set to the center value and it's used with an older PX4 version then the lower half of the stick input will get used for negative thrust which is almost nowhere in the code. So for most cases, the lower half would just get capped and be unusable without resetting the trim value.
  • what the trim value gets even set to if it's not hardcoded to the minimum. Is it the center of the stick? When is that measured? Or is it just the position of the stick when the calibration starts (what I assume).

An alternative would be to replace the calibration with an autopilot-based one compared to the ground station doing all the logic based on the telemetry of the autopilot. That way compatibility on the exact ranges and values becomes less of an issue and e.g. also MAVSDK could easier trigger a calibration and just follow the steps. Just a suggestion.

That said as long as spring loaded throttle still works fine.

@DonLakeFlyer With compatible PX4 that's not an issue. From PX4 perspective it currently doesn't matter if the throttle is spring-loaded or not. It has its full range and in certain modes a spring-loaded throttle just makes more sense than in others. We could add this information in the future to possibly enhance UX in some cases but the pure control input is independent.

@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/driving-backwards-with-a-boat-rover-in-manual-mode/25714/6

@mrpollo
Copy link
Member

mrpollo commented Jun 6, 2023

Should we get this in?

@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-june-06-2023/32473/1

@DonLakeFlyer
Copy link
Contributor

Resurrecting this...

So to my understanding is according to the docs for parameters *_TRIM is supposed to be the neutral point for the channel. This is true for the PX4 and ArduPilot with respect to the fact that they use the same name for this parameter. I guess the meaning of "neutral" is not super clear with respect to what it means for the throttle channel. How is it the concept of "neutral" effected by spring loaded throttle sticks. And/or how is effected by vehicles which support reverse throttle?

The current QGC code sets the trim values as the center stick position except for the throttle which is set as the throttle full down position.

It's still not clear to me from this discussion above what is correct/incorrect with respect to what the throttle trim value should be? There are links to PRs all over the place. Some in firmware some in mavlink spec. Some merged, some not.

So my questions to firmware folks:

  • What should the RC#_TRIM be set to for throttle?
  • Is it the same for both PX4 and ArduPilot?
  • What are the back-compat issue with changing this in QGC?
  • Does QGC RC cal need specific calibration modes for spring-loaded throttle and/or vehicles which support reverse thrust?

@peterbarker
Copy link
Contributor

So to my understanding is according to the docs for parameters *_TRIM is supposed to be the neutral point for the channel. This is true for the PX4 and ArduPilot with respect to the fact that they use the same name for this parameter. I

There's actually a neutral "zone" around the _TRIM value, its width given by the matching _DZ parameter, but otherwise yeah.

guess the meaning of "neutral" is not super clear with respect to what it means for the throttle channel. How is it the concept of "neutral" effected by spring loaded throttle sticks. And/or how is effected by vehicles which support reverse throttle?

There's this important parameter from Copter which changes behaviour on the ground when you are using a sprung throttle: https://ardupilot.org/copter/docs/parameters.html#pilot-thr-bhv-throttle-stick-behavior

AFAIK TRIM isn't actually used at all for throttle channel on Plane, we always assume MIN for now.

Both Copter and Rover can use throttle TRIM in their calculations. And with dual-paddle controls in Rover, I guess, two trims! Rover is rather problematic in that some Rovers can only go forward so the trim should be the same as min - but the vast majority would have trim in the middle.

The current QGC code sets the trim values as the center stick position except for the throttle which is set as the throttle full down position.

Which is fine. As mentioned, AP doesn't necessarily use the value - I'm not sure we

It's still not clear to me from this discussion above what is correct/incorrect with respect to what the throttle trim value should be? There are links to PRs all over the place. Some in firmware some in mavlink spec. Some merged, some not.

RC throttle trim is complicated, it seems! I'm unsure if you can reasonably expect to provide a fancy interface around setting the appropriate value.

So my questions to firmware folks:

* What should the `RC#_TRIM` be set to for throttle?

Zero on some vehicles. Half way on others.... I should also mention that some Copters have weird RC throttle setups to account for differences in control between auto-throttle modes like LOITER modes and manual-throttle modes like ACRO.

* Is it the same for both PX4 and ArduPilot?

I very much doubt it; our RC across all of vehicles is very varied - and varied even within the vehicle. One example of that is Copter's "toymode" which uses a different mechanism for determining throttle mid-point to account for poor-quality hardware!

* Does QGC RC cal need specific calibration modes for spring-loaded throttle and/or vehicles which support reverse thrust?

It would be nice to have a wizard which walks the user through setting up their RC input - so a stage saying, "put all sticks into their trim position". I'm actually quite keen on a wizard of some description as I'd also like to kill ArduPilot's "RCMAP_" parameters at some stage. We have a set of RCn_OPTION parameters which are to take the place of RCMAP_, but there's a lot of GCS work to do between now and when we can make that change.

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.

6 participants