mavlink / qgroundcontrol

Cross-platform ground control station for drones (Android, iOS, Mac OS, Linux, Windows)
http://qgroundcontrol.io
3.27k stars 3.6k forks source link

Virtual Joystick not working #10664

Closed beniaminopozzan closed 1 year ago

beniaminopozzan commented 1 year ago

Expected Behavior

On a PX4 simulation (gazebo classic) manual input is possible using virtual joystick.

Current Behavior

Even if virtual joystick is enabled in QGC and RC_COM_IN_MODE is set to Joystick only, PX4 does not detect any manual control input

Steps to Reproduce:

Please provide an unambiguous set of steps to reproduce the current behavior

  1. Start a PX4 simulation in gazebo-classic https://docs.px4.io/main/en/sim_gazebo_classic/#running-the-simulation
  2. Open QGC and enable Virtual Joystick
  3. Make sure that RC_COM_IN_MODE is set to Joystick only
  4. Observe how the arming check reports notifies "No manual control input"

System Information

When posting bug reports, include the following information

Detailed Description

QGC v4.2.6 stable version works as expected.

Log Files and Screenshots

image Virtual Joystick is present but still No manual control input.

image COM_RC_IN_MODE is set to Joystick only.

JMare commented 1 year ago

I think I am seeing the same issue as this:

Some more information from my testing: Enviroment: Ubuntu 22.04.2 Autopilot: Arducopter 4.3.7 SITL QGC: Daily 6ab6ef179 2023-07-17

I have compared the operation of the above daily build to the latest stable QGC V4.2.8.

In both cases, I ensured all settings were as default, then enabled the virtual joystick. On QGC v4.2.8, I can control the aircraft, and I see MANUAL_CONTROL messages in wireshark. On the Daily build, I cant see MANUAL_CONTROL messages in wireshark, and I cant control the vehicle.

zdanek commented 1 year ago

This might be connected to issue with multivehicle joystick control problem. I mean somewhere with handling joystick, not regarding amount of vehicles.

Can you also check for sim of two vehicles and try to control them with joystick? Are able to check problem you described with real joystick? For example Playstation or MS XBOX controller?

JMare commented 1 year ago

@zdanek I will test both those things tomorrow. I presume you are referring to #10544?

Today I started bisecting where the issue arose. So far I have narrowed it down to: 1b53ed35d bad afc1f5230 good This means that none of the code in the joystick folder has changed since it was last working - which suggests the issue is in the vehicle code. (edit: or I guess in the virtual joystick qml)

But I will finish bisecting tommorow and then test with a bluetooth gamepad as well as test multi vehicle.

zdanek commented 1 year ago

Yes. I was referring https://github.com/mavlink/qgroundcontrol/issues/10544

Thanks, I'm on my smartphone.

Thank you for your support. Zdanek

śr., 19 lip 2023, 11:57 użytkownik James Mare @.***> napisał:

@zdanek https://github.com/zdanek I will test both those things tomorrow. I presume you are referring to #10544 https://github.com/mavlink/qgroundcontrol/issues/10544?

Today I started bisecting where the issue arose. So far I have narrowed it down to: 1b53ed3 https://github.com/mavlink/qgroundcontrol/commit/1b53ed35d9fa902efa898dd6d051eca52e85af08 bad afc1f52 https://github.com/mavlink/qgroundcontrol/commit/afc1f52302b5e5384de6f2d009431c707fc66dd2 good This means that none of the code in the joystick folder has changed since it was last working - which suggests the issue is in the vehicle code.

But I will finish bisecting tommorow and then test with a bluetooth gamepad as well as test multi vehicle.

— Reply to this email directly, view it on GitHub https://github.com/mavlink/qgroundcontrol/issues/10664#issuecomment-1641787998, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADLSJS324TOC77ZQQCLEHLXQ6VSFANCNFSM6AAAAAAXF4RQZY . You are receiving this because you were mentioned.Message ID: @.***>

JMare commented 1 year ago

I finished bisecting and found the first commit at which the virtual joystick doesn't work

commit 434e76a747c037f09ee07c798ebc47a6b93f7303 Author: Christopher Vo vo@sentienrobotics.com Date: Wed Mar 29 16:11:16 2023 -0400

Enable joystick automatically on the active vehicle when the active vehicle is changed

When there are multiple vehicles, and you select a vehicle, the joystick should switch over to the newly active vehicle, and only the active vehicle (so that joystick inputs are not sent to a vehicle you do not intend to control).

To resolve this, we connect Vehicle to the activeVehicleAvailableChanged and activeVehicleChanged signals produced by the MultiVehicleManager. ALL vehicles disable joystick when activeVehicleAvailableChanged(false), because this signals when there is no longer an active vehicle available. When activeVehicleChanged is signalled, ONLY the currently active vehicle enables joystick.

See issue #10544.
JMare commented 1 year ago

Here are the results of my investigations on this issue: commit 434e76a adds code which enables the joystick for a vehicle when it becomes the active vehicle.

void Vehicle::_activeVehicleChanged(Vehicle *newActiveVehicle)
{
    if(newActiveVehicle == this) {
        // this vehicle is the newly active vehicle
        setJoystickEnabled(true);
    }
}

However, this enables the joystick even if there is no joystick configured or enabled which causes the following code to fail, because _joystickEnabled is now true when it should be false:

void Vehicle::virtualTabletJoystickValue(double roll, double pitch, double yaw, double thrust)
{
    // The following if statement prevents the virtualTabletJoystick from sending values if the standard joystick is enabled
    if (!_joystickEnabled) {
        sendJoystickDataThreadSafe(
                    static_cast<float>(roll),
                    static_cast<float>(pitch),
                    static_cast<float>(yaw),
                    static_cast<float>(thrust),
                    0);
    }
}

The simplest fix for this is a check in Vehicle::setJoystickEnabled to avoid enabling the Joystick if none are connected. I have tested this fix with multi vehicles and virtual joysticks, and it fixes the problem.

diff --git a/src/Vehicle/Vehicle.cc b/src/Vehicle/Vehicle.cc
index 8ae6b6b9d..0b35b4a21 100644
--- a/src/Vehicle/Vehicle.cc
+++ b/src/Vehicle/Vehicle.cc
@@ -2125,6 +2125,11 @@ bool Vehicle::joystickEnabled() const

 void Vehicle::setJoystickEnabled(bool enabled)
 {
+    // Don't enable the joystick if none are connected
+    if (enabled && !_toolbox->joystickManager()->joysticks().count()) {
+        return;
+    }
+
     _joystickEnabled = enabled;
     _startJoystick(_joystickEnabled);
     _saveSettings();

An interesting edge case appears to be what happens if there is a joystick connected but is uncalibrated. In this case, Vehicle::activevehiclechanged will call Vehicle::setJoystickEnabled(true), which will call Vehicle::_startJoystick, which will call Joystick::startPolling, which will call Vehicle::setJoystickEnabled(false).

So we have a recursive call to setJoystickEnabled (but will only call recursively once) I can't see how it will cause an issue besides a double call to _saveSettings and double emit of joystickEnabledChanged but might be worth a look over.

This affects the fix in #10544

Edit: I have not yet tested this with a real joystick, I will try and test that tomorrow, forgot to bring my joystick to work today.

vorobotics commented 1 year ago

@JMare Thanks for finding this and for your testing in #10544. In my opinion it looks like the joystick support in QGC needs some rework, I think some of the logic is confusing.

JMare commented 1 year ago

@vorobotics, no problem. I agree that the logic is confusing. Now that I've been deep in this joystick code for a couple of days, I think the following issues are adding surface area for bugs.

  1. the term, methods and variables named "joystickEnabled" is used interchangeably to indicate whether a joystick is enabled globally, and whether a particular vehicle should output the joystick data. Many parts of the code base are accessing these functions.
  2. The Joystick class and the Vehicle class are both handling activeVehicleChanged messages to do the same job - which is redirect joystick output to the active vehicle.
  3. The Joystick input configuration is somewhat associated with the vehicle - which doesn't make sense when there is one joystick, one gcs, but many vehicles.

I would make the following suggestions, and I would be willing to put work in on either coding or testing as needed.

zdanek commented 1 year ago

@JMare this is a great insight. I'd love to cooperate to fix those issues. My goal at first is to make joystick work with multiple vehicles through one connection. It seems that this is seriously broken. Affects both SITL sim and real connection (one physical com that handles many remote devices). This makes QGC unusable when need to control a few drones, by joystick, through single physical connection (my case).

Next I'd love to engage more and make some rework of joystick code, exactly as you described. Especially, to pass virtual joystick through the same "out channel". Otherwise it can be messy.

JMare commented 1 year ago

Sounds like a good plan - maybe once we have fixed these issues we can make a new issue to discuss reworking some things.

MaEtUgR commented 1 year ago

Big kudos to @JMare ! I just verified and his pr https://github.com/mavlink/qgroundcontrol/pull/10758 resolved this problem 🙏👍🎉