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

Stop sending MAVLink MANUAL_CONTROL commands when there's no pilot input #713

Conversation

rafaellehmkuhl
Copy link
Member

Fix #307

Copy link
Contributor

@ES-Alexander ES-Alexander left a comment

Choose a reason for hiding this comment

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

This seems reasonable and helpful - nice! :-)

Copy link
Member

@patrickelectric patrickelectric left a comment

Choose a reason for hiding this comment

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

Not sending manual controls will make the vehicle to disarm, we should send the controls periodically to avoid such scenario.

@ES-Alexander
Copy link
Contributor

Documentation note:
Clarify the difference between this and FS_PILOT_TIMEOUT - the autopilot's pilot timeout can never be triggered if the control station continues to send control updates even once it's no longer receiving input from the pilot.

@ES-Alexander ES-Alexander added the docs-needed Change needs to be documented label Jan 31, 2024
@ES-Alexander
Copy link
Contributor

Not sending manual controls will make the vehicle to disarm, we should send the controls periodically to avoid such scenario.

@patrickelectric I think that's the point of this PR - if the joystick disconnects or the window gets hidden then the control station shouldn't be pretending it's receiving input from the pilot, and the vehicle should disarm if it's set up to do so using a pilot input failsafe (e.g. FS_PILOT_INPUT in ArduSub).

@patrickelectric
Copy link
Member

Not sending manual controls will make the vehicle to disarm, we should send the controls periodically to avoid such scenario.

@patrickelectric I think that's the point of this PR - if the joystick disconnects or the window gets hidden then the control station shouldn't be pretending it's receiving input from the pilot, and the vehicle should disarm if it's set up to do so using a pilot input failsafe (e.g. FS_PILOT_INPUT in ArduSub).

The code points that will only send joystick data if there is joystick events, so if there are no joystick inputs from the user, the vehicle will disarm.

@ES-Alexander ES-Alexander changed the title Stop sending repeated MAVLink MANUAL_CONTROL commands Stop sending MAVLink MANUAL_CONTROL commands when there's no pilot input Jan 31, 2024
@ES-Alexander
Copy link
Contributor

The code points that will only send joystick data if there is joystick events, so if there are no joystick inputs from the user, the vehicle will disarm.

My assumption was that updateControllerData runs frequently/regularly while there's a joystick connected. If it instead works by only triggering when the joystick state changes then that is indeed a problem that needs to be addressed.

@rafaellehmkuhl
Copy link
Member Author

This patch makes Cockpit stops sending MANUAL_CONTROL messages if the joystick is not updated, indeed. I didn't know about the auto-disarm behavior. Will fix accounting for that.

@ES-Alexander
Copy link
Contributor

I didn't know about the auto-disarm behavior. Will fix accounting for that.

An ideal solution would account for whether there is a pilot input failsafe configured, and if so send repeated inputs only as often as necessitated by the timeout parameter (to minimise resource wastage). That said, I'm not sure whether non-ArduSub vehicles actually have such a failsafe - could be good to check that, and/or offer the repeated message frequency as a user-configurable parameter.

@rafaellehmkuhl
Copy link
Member Author

I didn't know about the auto-disarm behavior. Will fix accounting for that.

An ideal solution would account for whether there is a pilot input failsafe configured, and if so send repeated inputs only as often as necessitated by the timeout parameter (to minimise resource wastage). That said, I'm not sure whether non-ArduSub vehicles actually have such a failsafe - could be good to check that, and/or offer the repeated message frequency as a user-configurable parameter.

Agree. Can you open an issue for us to track that?

@rafaellehmkuhl rafaellehmkuhl force-pushed the stop-sending-repeated-mavlink-command branch 3 times, most recently from 8a86e7f to 18f8b25 Compare January 31, 2024 19:00
@rafaellehmkuhl
Copy link
Member Author

rafaellehmkuhl commented Jan 31, 2024

@patrickelectric @ES-Alexander @Williangalvani changed accordingly to the discussion here (and some in particular):

  1. We now disable every joystick forwarding when the window/tab is not visible (changed/minimized)
  2. We also disable it when there's no joystick connected
  3. We allow the user to decide if they want to override the behavior on 1 (and just keep sending the last state) if he wants to

This is because there's no such thing as "zeroing" a MANUAL_CONTROL message. It can be that a zero value is a motor spinning, so we don't want to send "zeroed" messages to the vehicle when that can mean "accelerate the motor". So in the end our default behavior will be indeed to stop sending those messages if the window is hidden or the joystick is disconnected, as this allows for the vehicle to decide on what to do (that's why the failsafe function exists, in the end).

@rafaellehmkuhl rafaellehmkuhl force-pushed the stop-sending-repeated-mavlink-command branch from 3262a0b to a7c25e5 Compare January 31, 2024 21:47
@rafaellehmkuhl rafaellehmkuhl merged commit 2b1a21b into bluerobotics:master Feb 1, 2024
7 checks passed
@rafaellehmkuhl rafaellehmkuhl deleted the stop-sending-repeated-mavlink-command branch February 1, 2024 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-needed Change needs to be documented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Last joystick command keeps getting send if user changes tab and stops clicking
3 participants