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

commander_params: COM_RC_IN_MODE - comment out of date #19634

Merged
merged 7 commits into from
Jul 13, 2022

Conversation

hamishwillee
Copy link
Contributor

@hamishwillee hamishwillee commented May 11, 2022

The comment for COM_RC_IN_MODE does not match the current value options.
I have removed the invalid part, but we need to update with information about the new options.

I can't do that yet because I don't know for each case what protections are enabled/disabled.

@MaEtUgR can you clarify?

Options with questions/comments about behaviour as bullets.
@value 0 RC Transmitter only

  • Is it true that this requires valid transmitter setup and will failsafe (unless circuit breaker) if transmitter data is lost?
  • No requirements for Joystick and it will be ignored.

@value 1 Joystick only

  • Joystick is enabled and RC is disabled.
  • All the RC checks are disabled - so you don't have to set a circuit breaker on them or anything.
  • Joystick checks are enabled - so either you will have to have a joystick OR a virtual joystick enabled right?
    • What if you don't - is there a circuit breaker?
    • How do you enable virtual joysticks (i.e. why not here?)

@value 2 RC and Joystick with fallback

  • I assume this means that you have both RC and Joystick connected it will take inputs from either/both.
  • Are checks enabled? I.e. if you don't have RC set, what will happen? What about if you don't have joystick set?
  • What does "with fallback" mean?

@value 3 RC or Joystick keep first

  • What does "keep first" mean. My guess would be "whatever it detects first it uses". If it detects RC then it is as though 0 were set, with all the same safety checks etc. Otherwise it is Joystick - same as if 1 was set.

@value 4 Stick input disabled

  • I think this means no RC or Joystick and no safety checks - ie it won't ask for you to connect RC or Joystick or have virtual joystick. BUT this would mean you would only be able to fly auto modes. Right?

@hamishwillee hamishwillee requested a review from MaEtUgR May 11, 2022 01:36
@hamishwillee
Copy link
Contributor Author

PS When this goes in need to fix https://docs.px4.io/master/en/config/joystick.html#enabling-px4-joystick-support and https://docs.qgroundcontrol.com/master/en/SetupView/Joystick.html and do a general search on COM_RC_IN_MODE in the docs.

@julianoes
Copy link
Contributor

@hamishwillee I think the questions whether RC is required is somewhat orthogonal, so covered by other parameters. This parameter really only configures what is used as an input.

@hamishwillee
Copy link
Contributor Author

@hamishwillee I think the questions whether RC is required is somewhat orthogonal, so covered by other parameters. This parameter really only configures what is used as an input.

@julianoes You might be right, but in the old version setting joystick disabled RC checks. So it is worth confirming everything in one go.

@MaEtUgR
Copy link
Member

MaEtUgR commented May 12, 2022

@hamishwillee Thanks for bringing this up! I wasn't even aware of this part of the parameter description. It was folded in on the review page of #17404 and that's how it went unnoticed 🤦
image

My dream:
One stick input tab in QGC that specifically requires you to choose an input configuration option on the first setup. When you chose any option that uses RC it lets you calibrate RC and PX4 requires it. If you chose any option that uses a joystick you can choose if virtual or real joystick and it requires calibration. As a result it's clear to the user what is in use and expected and there's less magic and unexpected behavior.

Current state:
@value 0

  • Makes PX4 only accept stick input from RC protocols like e.g. SBUS ignoring any joystick input.
  • If this value is set the preflight check requires valid RC calibration parameters (available after QGC RC calibration, not joystick).
  • If this value is set the MAVLink message SYS_STATUS.onboard_control_sensors_... reports the health status for the MAV_SYS_STATUS_SENSOR_RC_RECEIVER
  • If this value is set I think QGC does additional checks and shows a red circle next to RC setup and a message you need to set RC up first. I'm not sure how exactly this works, possibly related to the point above.

Is it true that this requires valid transmitter setup and will failsafe (unless circuit breaker) if transmitter data is lost?

The failsafes are independent of the configuration here. They will kick in when losing the last valid stick input from RC or Joystick in a mode that is piloted by sticks and will trigger also when stick input was present before and later on lost in all other atuonomous modes if not disabled by https://docs.px4.io/v1.12/en/advanced_config/parameter_reference.html#COM_RCL_EXCEPT.

No requirements for Joystick and it will be ignored.

There are requirements on stick input see the answer just above but joystick data is ignored.

@value 1

  • Makes PX4 only accept joystick input through MAVLink like QGC sending input from a USB joystick or the virtual joystick.
  • If this or any other value than 0 is set RC calibration is not required and the MAV_SYS_STATUS_SENSOR_RC_RECEIVER is reported to not be present.

All the RC checks are disabled - so you don't have to set a circuit breaker on them or anything.

Checks are disabled yes. What circuit breaker are you referring to? What would it be set for?

Joystick checks are enabled - so either you will have to have a joystick OR a virtual joystick enabled right?

No requirement for a joystick, RC input is ignored, joystick accepted, stick input requirements stay like I wrote above for the failsafe question.

How do you enable virtual joysticks (i.e. why not here?)

Virtual joystick can not be distinguished from a real joystick PX4 side. Both are accepted. If I remeber correctly QGC preferably sends real joystick data if both are enabled. UX is not nicely predictable from the configuration.

@value 2

  • Makes PX4 accept RC and joystick input. It will latch onto the first one available and fall back to the other valid stream if the first one is lost.

Are checks enabled? I.e. if you don't have RC set, what will happen? What about if you don't have joystick set?

No additional checks or requirements. Preflight check is like with any value other than 0.

What does "with fallback" mean?

Fallback means it uses the first available valid source and keeps on using that until it gets invalid and then switches to any other valid source that is/gets available. From there on you can repeatedly switch back and forth if sources get unavailable/available. All of this without landing, disarming or rebooting so it works mid-air.

@value 3

  • Makes PX4 accept RC or joystick input. Whatever source comes in first it latches on and it can only regain the same source again and not fall back to another. This was reintroduced because it's the original behavior that PX4 had before all these options were added with manual control selection and cleanup #17404

If it detects RC then it is as though 0 were set, with all the same safety checks etc. Otherwise it is Joystick - same as if 1 was set.

No additional checks. RC calibration is not required for takeoff.

@value 4

  • Makes PX4 not accept any stick input at all. Everything incoming is ignored.

I think this means no RC or Joystick and no safety checks

No more or fewer checks than with 1, 2, 3.

BUT this would mean you would only be able to fly auto modes. Right?

Only modes that don't require stick input, correct. That's a result of you not being able to have any stick input than checking the value of this parameter directly.

src/modules/commander/commander_params.c Outdated Show resolved Hide resolved
src/modules/commander/commander_params.c Outdated Show resolved Hide resolved
hamishwillee and others added 2 commits May 13, 2022 08:40
Co-authored-by: Matthias Grob <maetugr@gmail.com>
Co-authored-by: Matthias Grob <maetugr@gmail.com>
@hamishwillee hamishwillee marked this pull request as ready for review May 18, 2022 07:48
@hamishwillee
Copy link
Contributor Author

Thanks @MaEtUgR , Those are all better than before. I've made some counter-suggestions "to my taste", with one question

@hamishwillee hamishwillee requested a review from MaEtUgR May 19, 2022 02:39
@hamishwillee
Copy link
Contributor Author

@MaEtUgR Can you please merge this - assuming it is all as we agreed!

@hamishwillee hamishwillee merged commit 5f953fd into main Jul 13, 2022
@hamishwillee hamishwillee deleted the hamishwillee-COM_RC_IN_MODE branch July 13, 2022 05:26
@hamishwillee
Copy link
Contributor Author

Merged, since we know it is more accurate than what was there! Thanks for approving @julianoes

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.

3 participants