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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed Unix joystick stuff #838

Merged
merged 2 commits into from Mar 31, 2015

Conversation

Projects
None yet
5 participants
@binary1248
Member

binary1248 commented Mar 23, 2015

Fixes #776.

Also makes joystick handling on Unix more robust in general, with more fallbacks than before. You will need to have a really exotic set up for this not to work 馃榿.

Also replaced inotify polling with a udev monitor, so the amount of polling done should be decreased, by how much depends on the system I guess.

@zmertens

This comment has been minimized.

Show comment
Hide comment
@zmertens

zmertens Mar 24, 2015

The phantom joystick message is gone which is awesome. I haven't used the joystick class in any of my projects before so I had to put a quick one together. I noticed that the initial value for the Y / X axis isn't zero (my static Y axis value is -3.87575, and static X axis is 14.2827). Here's the code I used to test (Ubuntu 14.04, XBox 360 wired controller).

std::cout << "Initial axis positions" << std::endl; sf::Joystick::update(); posY_Axis = sf::Joystick::getAxisPosition(0, sf::Joystick::Y); posX_Axis = sf::Joystick::getAxisPosition(0, sf::Joystick::X); std::cout << "posY_Axis: " << posY_Axis << " posX_Axis: " << posX_Axis << std::endl;

I also created a big block of if statements to test button clicks and they all seemed to work except for the D-Pad and the A button. When I did a sf::Joystick::getButtonCount(0) I noticed that I only get 11 buttons present (and I thought it was weird that the A button didn't work but X, Y, and B showed up). I can do more testing later; I want to put together a proper joystick testing program. I've been wanting to make use of SFML's joystick class for a while so this is a good chance for me to learn more about it.

zmertens commented Mar 24, 2015

The phantom joystick message is gone which is awesome. I haven't used the joystick class in any of my projects before so I had to put a quick one together. I noticed that the initial value for the Y / X axis isn't zero (my static Y axis value is -3.87575, and static X axis is 14.2827). Here's the code I used to test (Ubuntu 14.04, XBox 360 wired controller).

std::cout << "Initial axis positions" << std::endl; sf::Joystick::update(); posY_Axis = sf::Joystick::getAxisPosition(0, sf::Joystick::Y); posX_Axis = sf::Joystick::getAxisPosition(0, sf::Joystick::X); std::cout << "posY_Axis: " << posY_Axis << " posX_Axis: " << posX_Axis << std::endl;

I also created a big block of if statements to test button clicks and they all seemed to work except for the D-Pad and the A button. When I did a sf::Joystick::getButtonCount(0) I noticed that I only get 11 buttons present (and I thought it was weird that the A button didn't work but X, Y, and B showed up). I can do more testing later; I want to put together a proper joystick testing program. I've been wanting to make use of SFML's joystick class for a while so this is a good chance for me to learn more about it.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 24, 2015

Member

Pushed new version which fixed a small buffer overflow that would occur if parsed joystick numbers were larger than sf::Joystick::Count.

Member

binary1248 commented Mar 24, 2015

Pushed new version which fixed a small buffer overflow that would occur if parsed joystick numbers were larger than sf::Joystick::Count.

@zsbzsb

This comment has been minimized.

Show comment
Hide comment
@zsbzsb

zsbzsb Mar 25, 2015

Member

Tested with a XBox360 controller on Ubuntu (with xboxdrv) through virtual box and it worked great. If it works through that it should be good 馃憤

Member

zsbzsb commented Mar 25, 2015

Tested with a XBox360 controller on Ubuntu (with xboxdrv) through virtual box and it worked great. If it works through that it should be good 馃憤

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 25, 2015

Member

The latest commit should take care of the remaining false positives. The lower level implementation also supports an unlimited number of devices, if that is ever needed higher up one day.

Member

binary1248 commented Mar 25, 2015

The latest commit should take care of the remaining false positives. The lower level implementation also supports an unlimited number of devices, if that is ever needed higher up one day.

@zmertens

This comment has been minimized.

Show comment
Hide comment
@zmertens

zmertens Mar 26, 2015

I just tested the last changes (22303f4) and I noticed I'm getting the "Unable to get joystick attribute. Could not find USB device for joystick at index 0." message again.

zmertens commented Mar 26, 2015

I just tested the last changes (22303f4) and I noticed I'm getting the "Unable to get joystick attribute. Could not find USB device for joystick at index 0." message again.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 26, 2015

Member

Uhh... but... that error message doesn't even exist in the code any more... 馃槙

@zwookie Can you double check if you are testing the right stuff? 馃榿 If you are dynamic linking, switch to static linking for testing, since it will be easier to keep track of which version you are currently using that way.

Member

binary1248 commented Mar 26, 2015

Uhh... but... that error message doesn't even exist in the code any more... 馃槙

@zwookie Can you double check if you are testing the right stuff? 馃榿 If you are dynamic linking, switch to static linking for testing, since it will be easier to keep track of which version you are currently using that way.

@zmertens

This comment has been minimized.

Show comment
Hide comment
@zmertens

zmertens Mar 26, 2015

@binary1248 Oops, I had built the Release libraries when I intended to build the Debug libraries... (but I was linking to the Debug libraries which are an older version of SFML 2.2)

zmertens commented Mar 26, 2015

@binary1248 Oops, I had built the Release libraries when I intended to build the Debug libraries... (but I was linking to the Debug libraries which are an older version of SFML 2.2)

@zmertens

This comment has been minimized.

Show comment
Hide comment
@zmertens

zmertens Mar 28, 2015

I'm getting an error Failed to open joystick /dev/input/event4: 24using 22303f4 version of the branch. This is using the same testing setup I had posted about previously. Anyone else on Ubuntu 14.04 using Xbox 360 controller get this message? Here's the testing code I'm using for reference:

while(1) 
{
    sf::Joystick::update();
    if(sf::Joystick::isConnected(0))
    {
        std::cout << "Joystick 0 supports "
                  << sf::Joystick::getButtonCount(0)
                  << " buttons" << std::endl;
    } 
}

The console seems to stall for a second and then that error message I posted spits out repedeatley until I get a -1 return error.

zmertens commented Mar 28, 2015

I'm getting an error Failed to open joystick /dev/input/event4: 24using 22303f4 version of the branch. This is using the same testing setup I had posted about previously. Anyone else on Ubuntu 14.04 using Xbox 360 controller get this message? Here's the testing code I'm using for reference:

while(1) 
{
    sf::Joystick::update();
    if(sf::Joystick::isConnected(0))
    {
        std::cout << "Joystick 0 supports "
                  << sf::Joystick::getButtonCount(0)
                  << " buttons" << std::endl;
    } 
}

The console seems to stall for a second and then that error message I posted spits out repedeatley until I get a -1 return error.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 28, 2015

Member

As stated here, can you provide the output of:

find /dev/input/* -not -path "/dev/input/by-path*" -exec udevadm info -q all '{}' \;

and

udevadm monitor --property

The second command will only print out useful information when you connect/disconnect your devices, so just unplug/plug it back in once or twice and it should be enough information for me.

Member

binary1248 commented Mar 28, 2015

As stated here, can you provide the output of:

find /dev/input/* -not -path "/dev/input/by-path*" -exec udevadm info -q all '{}' \;

and

udevadm monitor --property

The second command will only print out useful information when you connect/disconnect your devices, so just unplug/plug it back in once or twice and it should be enough information for me.

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 28, 2015

Member

I just pushed a fixed version that properly ignores evdev device nodes. It should work properly now.

Member

binary1248 commented Mar 28, 2015

I just pushed a fixed version that properly ignores evdev device nodes. It should work properly now.

@zsbzsb

This comment has been minimized.

Show comment
Hide comment
@zsbzsb

zsbzsb Mar 28, 2015

Member

Tested again and 馃憤

Member

zsbzsb commented Mar 28, 2015

Tested again and 馃憤

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 28, 2015

Member

Rebased onto master and fixed an issue that @zsbzsb still had when unplugging/replugging joysticks multiple times.

Should be ready for merge now. Time to test/review 馃槈.

Member

binary1248 commented Mar 28, 2015

Rebased onto master and fixed an issue that @zsbzsb still had when unplugging/replugging joysticks multiple times.

Should be ready for merge now. Time to test/review 馃槈.

@binary1248 binary1248 added this to the 2.3 milestone Mar 28, 2015

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 28, 2015

Member

@Bromeon care to test this as well? I recall from the previous joystick work that you were able to provide extensive testing on Linux.

Member

binary1248 commented Mar 28, 2015

@Bromeon care to test this as well? I recall from the previous joystick work that you were able to provide extensive testing on Linux.

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Mar 29, 2015

Member

I'll try to have a look this evening :)

Member

Bromeon commented Mar 29, 2015

I'll try to have a look this evening :)

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 29, 2015

Member

This evening can be very long... 馃槢

Member

binary1248 commented Mar 29, 2015

This evening can be very long... 馃槢

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Mar 29, 2015

Member

Hey, I had to find out all the new dependencies and upgrade all my packages, because you guys changed so much recently, and other things got broken :D
(Ok, I admit I also got home late... but I'll get to it soon ;))

By the way, do you have a test code at hand, or should I search the one I used last time?

Member

Bromeon commented Mar 29, 2015

Hey, I had to find out all the new dependencies and upgrade all my packages, because you guys changed so much recently, and other things got broken :D
(Ok, I admit I also got home late... but I'll get to it soon ;))

By the way, do you have a test code at hand, or should I search the one I used last time?

@binary1248

This comment has been minimized.

Show comment
Hide comment
@binary1248

binary1248 Mar 29, 2015

Member

It was nothing but a refactor/fix, so the same one as last time should do. Otherwise, there might be some code here.

Member

binary1248 commented Mar 29, 2015

It was nothing but a refactor/fix, so the same one as last time should do. Otherwise, there might be some code here.

@Bromeon

This comment has been minimized.

Show comment
Hide comment
@Bromeon

Bromeon Mar 29, 2015

Member

Works fine. I tested two different joysticks, exactly these two were recognized, including vendor and product ID. The error messages "Unable to get joystick attribute" don't appear anymore.

馃憤

Member

Bromeon commented Mar 29, 2015

Works fine. I tested two different joysticks, exactly these two were recognized, including vendor and product ID. The error messages "Unable to get joystick attribute" don't appear anymore.

馃憤

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Mar 29, 2015

Member

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

Member

eXpl0it3r commented Mar 29, 2015

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

binary1248 added some commits Mar 23, 2015

Replaced inotify joystick polling with udev monitoring, added more pr鈥
鈥cise checking whenever connection/disconnection occurs so full scans are no longer needed, fixed up USB attribute querying and added udev property querying as the primary method of getting joystick property data, added a fallback method of getting the joystick name if JSIOCGNAME fails.
Replaced Unix joystick enumeration with a fully native udev implement鈥
鈥tion which supports an unlimited number of devices (still limited higher up by sf::Joystick::Count though).

@eXpl0it3r eXpl0it3r merged commit 0076ea5 into master Mar 31, 2015

@eXpl0it3r eXpl0it3r deleted the bugfix/joystick_unix branch Mar 31, 2015

@Bromeon Bromeon added the s:accepted label Mar 31, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment