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

Virtual Joystick not working #10664

Closed
beniaminopozzan opened this issue Apr 20, 2023 · 11 comments
Closed

Virtual Joystick not working #10664

beniaminopozzan opened this issue Apr 20, 2023 · 11 comments

Comments

@beniaminopozzan
Copy link

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

  • Operating System: Ubuntu 22.04.2
  • QGC Version: 4.2.6
  • QGC build: daily : a5baac0
  • Flight Controller: SITL
  • Autopilot (with version): PX4 main branch, and PX4 v1.13.3

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
Copy link
Contributor

JMare commented Jul 19, 2023

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 6ab6ef1 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
Copy link
Collaborator

zdanek commented Jul 19, 2023

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
Copy link
Contributor

JMare commented Jul 19, 2023

@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:
1b53ed3 bad
afc1f52 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
Copy link
Collaborator

zdanek commented Jul 19, 2023 via email

@JMare
Copy link
Contributor

JMare commented Jul 20, 2023

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

commit 434e76a
Author: Christopher Vo [email protected]
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
Copy link
Contributor

JMare commented Jul 20, 2023

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
Copy link

@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
Copy link
Contributor

JMare commented Jul 21, 2023

@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.

  • Joystick input and joystick output should be decoupled. Joystick input should be a global configuration, you should access it in the application settings, not in the vehicle settings. When joystick input is enabled, the joystick thread should always be running, regardless of whether a vehicle is using it.
  • The handling of joystick output enable/disable makes most sense to handle in the vehicle class. Each vehicle can decide whether to send vehicle data or not without affecting the global joystick object.
  • I realize there are some issues with the above due to the handling of actions/flight mode, which are vehicle/firmware dependent, but I think that could be worked around
  • Perhaps virtual joysticks should be handled through the joystick interface rather than bypassed into the vehicle.

@zdanek
Copy link
Collaborator

zdanek commented Jul 22, 2023

@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
Copy link
Contributor

JMare commented Jul 24, 2023

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

@MaEtUgR
Copy link
Collaborator

MaEtUgR commented Aug 15, 2023

Big kudos to @JMare ! I just verified and his pr #10758 resolved this problem 🙏👍🎉

@MaEtUgR MaEtUgR closed this as completed Aug 15, 2023
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

No branches or pull requests

5 participants