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

Jump to bottom

Joysticks not supported in Android #742

Closed
SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Closed

Joysticks not supported in Android #742

SDLBugzilla opened this issue Feb 10, 2021 · 0 comments
Labels
enhancement New feature or request

Comments

@SDLBugzilla
Copy link
Collaborator

SDLBugzilla commented Feb 10, 2021 •
edited

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.0
Reported for operating system, platform: Android (All), All

Comments on the original bug report:

On 2013-01-21 13:56:42 +0000, Joseba García Echebarria wrote:

Created attachment 1024
Android joystick support

Right now SDLHG doesn't support joysticks in Android (It only supports one accelerometer).

The attached patch adds joystick support in Android.
In order to create the initial list of available joysticks, it requires functions available only in android-14 or over, but the code should be runnable in earlier versions of Android (the user will just not have joystick support).

This patch also allows the user to get the real name for the accelerometer, as given by Android.

I'd say the patch is not final and needs some tweaking in order to function properly, do please let me know of any problems you see.

Regards

On 2013-01-21 22:56:33 +0000, Philipp Wiesemann wrote:

In the patch there is a comment hat InputDevice.getDeviceIds() is available since API 16 and there is also a check for this level.
According to the documentation [1] it is available since API 9.

Some of the methods used in the patch are available since API 12. Maybe the required API level could be lowered to 12 which would make the joystick functionality available on more devices (than API 16 or 14 as mentioned in your post).

[1] http://developer.android.com/reference/android/view/InputDevice.html#getDeviceIds%28%29

On 2013-01-21 23:02:46 +0000, Philipp Wiesemann wrote:

The patch also changes the handling of the VOLUME keys in a way that they are passed to SDL and Android (only SDL before). Using the keys now has the side effect of changing the devices volume itself.

This is may useful but it should (in my opinion) not be the default (see also Bug 1569). And if it would be the default there should be an easy way to disable it.

On 2013-01-22 01:56:18 +0000, Joseba García Echebarria wrote:

(In reply to comment # 2)

The patch also changes the handling of the VOLUME keys in a way that they are
passed to SDL and Android (only SDL before). Using the keys now has the side
effect of changing the devices volume itself.

This is may useful but it should (in my opinion) not be the default (see also
Bug 1569). And if it would be the default there should be an easy way to
disable it.

Ups, sorry.

That's part of my code and shouldn't have gone into the patch.
I agree that it shouldn't be the default for SDL.

I'll update the patch this evening.

On 2013-01-22 01:59:39 +0000, Joseba García Echebarria wrote:

(In reply to comment # 1)

In the patch there is a comment hat InputDevice.getDeviceIds() is available
since API 16 and there is also a check for this level.
According to the documentation [1] it is available since API 9.

Some of the methods used in the patch are available since API 12. Maybe the
required API level could be lowered to 12 which would make the joystick
functionality available on more devices (than API 16 or 14 as mentioned in your
post).

[1]
http://developer.android.com/reference/android/view/InputDevice.html#getDeviceIds%28%29

Thanks for your comment.
I'll have a look to try to see why I thought the minimum Android level should be 16 and will update the patch accordingly.

On 2013-02-01 19:39:14 +0000, Joseba García Echebarria wrote:

Created attachment 1031
Update to the joystick patch as of 2013-02-02

The joystick code should now work in Android 9 and over.
The Android >= 16 requirement was wrong apparently and only functionality from Android >= 12 is needed.

I have, however, isolated those to only two spots:

  • A constant: I just entered its value and left a comment to where it came from
  • A method: On lower versions of Android there's a sane default value, return it

On 2013-02-02 06:35:02 +0000, Philipp Wiesemann wrote:

Thank you for the changes to support older Android versions.

There still is a use of View's setOnGenericMotionListener() which (according to documentation [1]) is not available on devices < API 12. Maybe this needs a check with Build.VERSION.SDK_INT too.

The newly added method getJoyId() contains an re-implementation of ArrayList's indexOf(). [2] Maybe the library method could be used to shorten the source.

Another not really needed simplification: The variable mJoyListCreated could be replaced with a check for mJoyIdList != null which makes one variable less to care about.

[1] http://developer.android.com/reference/android/view/View.html#setOnGenericMotionListener%28android.view.View.OnGenericMotionListener%29
[2] http://developer.android.com/reference/java/util/ArrayList.html#indexOf%28java.lang.Object%29

On 2013-02-03 15:18:16 +0000, Joseba García Echebarria wrote:

Thank you very much for the reply and for having alook at the code.

I'll implement your changes and re-upload the patch.

Joseba

(In reply to comment # 6)

Thank you for the changes to support older Android versions.

There still is a use of View's setOnGenericMotionListener() which (according to
documentation [1]) is not available on devices < API 12. Maybe this needs a
check with Build.VERSION.SDK_INT too.

The newly added method getJoyId() contains an re-implementation of ArrayList's
indexOf(). [2] Maybe the library method could be used to shorten the source.

Another not really needed simplification: The variable mJoyListCreated could be
replaced with a check for mJoyIdList != null which makes one variable less to
care about.

[1]
http://developer.android.com/reference/android/view/View.html#setOnGenericMotionListener%28android.view.View.OnGenericMotionListener%29
[2]
http://developer.android.com/reference/java/util/ArrayList.html#indexOf%28java.lang.Object%29

On 2013-02-17 17:46:48 +0000, Joseba García Echebarria wrote:

Created attachment 1046
Latest version of joystick patch. Should work in android-9 and later

I've attached the latest version of the patch. I've tested this (in the emulator) to work in android-9 and later.

The proposed patch includes a way of getting around the fact that android 9-11 don't implement OnGenericMotionListener. I don't know if that particular piece of code can be made any cleaner.

Anyway, waiting on your comments and thanks for your patience.

Joseba

On 2013-02-18 14:22:04 +0000, Philipp Wiesemann wrote:

(In reply to comment # 8)

The proposed patch includes a way of getting around the fact that android 9-11
don't implement OnGenericMotionListener. I don't know if that particular piece
of code can be made any cleaner.

As far as I understand OnGenericMotionListener does not need to be implemented by SDLActivity itself. It can be implemented by a new (maybe anonymous) class. This way there would be no need for two versions of SDLActivity and writing some parts twice (e.g. onCreate() or createEGLSurface()).

On 2013-02-18 14:24:00 +0000, Philipp Wiesemann wrote:

(In reply to comment # 9)

(In reply to comment # 8)

The proposed patch includes a way of getting around the fact that android 9-11
don't implement OnGenericMotionListener. I don't know if that particular piece
of code can be made any cleaner.

As far as I understand OnGenericMotionListener does not need to be implemented
by SDLActivity itself. It can be implemented by a new (maybe anonymous) class.
This way there would be no need for two versions of SDLActivity and writing
some parts twice (e.g. onCreate() or createEGLSurface()).

Sorry, I meant SDLSurface not SDLActivity.

On 2013-02-18 16:04:24 +0000, Joseba García Echebarria wrote:

Hi,

To be honest, this is the first piece of Java code I've written in my life and I have no formal Java training. Could you help me a bit with how to write that code?

I'm feeling that I should just create the class implementing the onGenericMotion event in the right way and then do something like:

if(Build.SDK_INT>=12) {
setOnGenericMotionListener(thatclass);
}

But I've tried a couple of different things today and didn't succeed.

Thanks a lot.
Joseba

(In reply to comment # 9)

(In reply to comment # 8)

The proposed patch includes a way of getting around the fact that android 9-11
don't implement OnGenericMotionListener. I don't know if that particular piece
of code can be made any cleaner.

As far as I understand OnGenericMotionListener does not need to be implemented
by SDLActivity itself. It can be implemented by a new (maybe anonymous) class.
This way there would be no need for two versions of SDLActivity and writing
some parts twice (e.g. onCreate() or createEGLSurface()).

On 2013-02-19 13:12:50 +0000, Philipp Wiesemann wrote:

I would implement it as an inner class in SDLSurface:

class SDLSurface extends SurfaceView implements /* ... */

private static class SDLOnGenericMotionListener implements View.OnGenericMotionListener {
    @Override
    public boolean onGenericMotion(View view, MotionEvent event) {
        /* ... */
        return true;
    }
}

/* ... */

public SDLSurface(Context context) {

    /* ... */

    setOnTouchListener(this);

    if (Build.VERSION_CODES.HONEYCOMB < Build.VERSION.SDK_INT) {
        setOnGenericMotionListener(new SDLOnGenericMotionListener());
    }

    /* ... */
}

/* ... */

}

This way the onGenericMotion() method may be placed close to the other on*() methods which might make it easier to maintain.

If an anonymous class would be used (like in audioStartThread()) it would be possible to have the complete implementation inside the if statement (in the constructor). I would not do this because (apart from being connected the outer class) the constructor gets so large (in source not in byte code). But there may be different opinions.

There are some more possibilities for classes in Java but I think these two are the most useful in this case.

I have not tested the above source on a real device.

On 2013-02-25 06:13:31 +0000, wrote:

Hey,

Thanks for the patch adding joystick support.
After checking out the most recent version of the patch, there are a few suggestions I have now. Truly, I have mentioned a couple of these in the mailing list (and forums) before; I admit it has possibly been a bit messy over there, though, so let's hope it doesn't happen again.

Now, the suggestions themselves:

  • Support motion input from axes other than AXIS_X and AXIS_Y, using getMotionRanges() to enumerate all axes if it helps.
  • Furthermore, use getMotionRanges() (or getMotionRange()) to tell the correct motion ranges (like [-1.0,1.0]).
  • Ignore synthesized key events, using the FLAG_FALLBACK flag as a hint. At least for me, when a specific analog joystick or a HAT switch is used (from some USB controller), additional D-Pad keycode events are generated. For instance, such events with keycode KEYCODE_DPAD_LEFT are generated when the HAT switch is pushed to the left. Arbitrary controller buttons also generate KEYCODE_DPAD_CENTER events (in addition to the key events you'd expect). One possible reason for the existence of such events is the emulation of a standalone D-Pad (as found in the very first Android devices), so native Android GUIs can be navigated using the controller. Indeed, I can navigate these with the left analog stick and HAT switch of the controller mentioned above.
  • I'd have a lower priority for this: Treat AXIS_HAT_X and AXIS_HAT_Y as a HAT switch, rather than two analog axes. That may remind some of a familiar behavior on GNU/Linux desktop environments: When read from /dev/input/event*, a HAT switch is what you'd expect; From /dev/input/js*, though, the HAT switch appears to be detected as two additional analog axes.

It's good to see how has the compatibility with older APIs gotten improved. Thanks again for the patch!

On 2013-03-17 14:38:03 +0000, wrote:

Created attachment 1070
A suggested update to the patch (March 17th 2013), should support more analog axes

On 2013-03-17 14:38:58 +0000, wrote:

Hi again,

I hope nobody here minds a little change in plans, as I have decided to revise the given patch. Since it is a revision of an existing patch to a different project, I don't mind what would other individuals do with this.

List of changes:

  • The project target API level (in the Android project template) is now set to 12, the API level introducing game controller support. In contrast, the minimum API level has not changed.
  • Some code restructuring has been done within the Java and C sources altogether. These include Philipp's suggestion in regards to the surface class, changes to joystick subsystem initialization and shutdown, and more.
  • Input from all joystick axes supported by Android should be handled now.
  • Given an InputDevice which has a joystick as a source (InputDevice.SOURCE_CLASS_JOYSTICK), ignore any analog axis (technically a MotionRange) not coming from such a source. I'm not sure about this, but confusion may arise if a single InputDevice has, say, an analog stick and a trackpad altogether.
  • KeyEvent handling: If a KeyEvent comes from an InputDevice which has a joystick as a source (InputDevice.SOURCE_CLASS_JOYSTICK), assume a game controller button. Otherwise, assume a keyboard key. An example for the former kind of an InputDevice is any standard USB game controller with at least one analog axis. Considering the latter kind, physical keyboards fall under that class. Furthermore, if for some reason one wants to manually toggle a soft keyboard and use it like a physical keyboard (i.e. not via the SDL text input API), it can work in the same way, at least with some soft keyboards.

Furthermore, now that we have a proposed sensor API with an Android implementation, thanks to Joseba, I have decided to omit/comment-out code related to accelerometer handling via the joystick API, mainly for the reason of simplicity. Truly, the API has recently been frozen and backwards compatibility should be retained, but I'm not aware of a good way that has been decided upon. For instance: Should attached game controllers be ignored in currently available apps, implying that setting a hint is going to be required just for using them in newer apps?
To be more specific about the changes:

  • Accelerometer related code is now commented out in SDLActivity.java. It is also (sort of) commented out in SDL_android.cpp and SDL_android.h.
  • On the other hand, accelerometer specific code originally present in the revised SDL_androidjoystick.c file is now gone.
  • As a consequence, I have removed the dependency on the "android" library from Android.mk (in comparison to the last revision of the patch).

Here are a few notes of mine in regards to my earlier comment (which is about a month old now):

  • I have realized there is no real need to check if a specific key event is synthesized (i.e. FLAG_FALLBACK is set). These shouldn't be reproduced for an analog axis or a HAT switch as long as we take care of all relevant motion events.
  • For now, the HAT switch of a USB controller is detected as two analog axes (AXIS_HAT_X and AXIS_HAT_Y). I've considered handling these separately, but then I've realized there are the kinds of USB controllers that have more than one HAT switch (for each of them). Truly, though, these do not seem to be common, especially when it comes to mobile devices. Furthermore, I don't know if the Android platform properly supports additional HAT switches at all.
  • I have decided to avoid from using getMotionRanges() for the purpose of normalizing an analog axis range. So, a range of [-1.0,1.0] is now (roughly) mapped to [-32767,32767], while a range of [0.0,1.0] is mapped to [0,32767].

Some more notes, partially relevant to earlier revisions of the patch as well:

  • For reference, I have tested the code with an old USB controller that has 9 buttons, 5 analog axes and 1 HAT switch. The analog axes on the Android side are AXIS_X, AXIS_Y, AXIS_Z, AXIS_RZ and AXIS_THROTTLE (in that order). The same order of axes also applies on Windows and GNU/Linux. The buttons are numbered by KEYCODE_BUTTON_1 to KEYCODE_BUTTON_16.
  • For a given joystick, SDL_JoystickNumButtons currently returns some positive value. Unfortunately, there are great chances that the Android platform does not give a way to properly implement this function.
  • Nevertheless, distinct controller buttons can still be distinguished, which is often sufficient.
  • Note that in the way it's done now, the very first button of a USB controller like the one I've used has the key code KEYCODE_BUTTON_1, which is translated to an SDL button value of 20 at the moment. The second button (KEYCODE_BUTTON_2) is translated to 21, and so it continues.
  • I don't have any XInput capable controller. A good guess for the key codes can be found here, though: http://blog.sagaoftherealms.net/?p=161. The relevant SDL buttons values are given by the range 5..19.
  • I suppose the key codes KEYCODE_BUTTON_1..KEYCODE_BUTTON_16 can rather be translated to 0..15, in order to be consistent with the behavior on Windows and GNU/Linux desktop platforms. However, there are still the XInput controllers, so it probably doesn't worth the effort.
  • In fact, the motion range of a controller's axis on a desktop environment may differ from the one on an Android device. I can tell it is the case here with the 5th controller axis: -32768..32767 on a GNU/Linux desktop, but 0..32767 on Android (since it is reported to be AXIS_THROTTLE).

Finally, if one hasn't tried it before, you should check the API Demos sample app (for API 12 or later). It has a great deal of ready tests that can be used out of the box, one of them being a game controller test. Of course, you can find even more potentially useful test within the same app.

Thanks again to Joseba for the patch!

On 2013-03-17 17:07:45 +0000, Philipp Wiesemann wrote:

(In reply to comment # 14)

Created attachment 1070 [details]
A suggested update to the patch (March 17th 2013), should support more
analog axes

In SDL_SYS_JoystickInit() of SDL_androidjoystick.c are two checks for SYS_Joysticks not being NULL after SDL_malloc() while SYS_JoystickNames is not checked. I think this is a fault.

On 2013-03-17 18:25:10 +0000, wrote:

Created attachment 1072
A suggested update to the patch (March 18th 2013), should support more analog axes

On 2013-03-17 18:25:33 +0000, wrote:

Oops, fixed this now!

On 2013-03-20 02:04:32 +0000, Sam Lantinga wrote:

Thanks! I don't have any way to test this right now, but it compiles and looks good.
http://hg.libsdl.org/SDL/rev/9cef1005df5f

On 2013-03-20 03:57:32 +0000, wrote:

Thanks for applying this patch!

Unfortunately, there is still a little problem. You can only blame me for posting a too long comment!
[I've actually planned to add another relevant comment tonight or this morning...oh well]

Basically, I think one great issue is the accelerometer support as an emulated joystick. In my latest update to the patch it is currently disabled.

Question is, how to handle it, now when there is a proposed sensor API? I suppose some SDL hint can be useful. Furthermore, should joystick support be totally disabled unless that hint is set to a different value, or not?
To compare, in Joseba's joystick patch (without my changes) joysticks are always enumerated and the accelerometer is enumerated as the "last" joystick, which I suppose shouldn't be a problem with existing apps even if a controller is available?)

P.S. Should I change the status of the patch? For now I don't, since the reported bug itself (joysticks not supported) is solved.

On 2013-03-20 04:07:13 +0000, wrote:

Great, a typo in the very end of my own recent comment!
Please let me correct this:

"P.S. Should I change the status of the bug report? For now I don't, since the reported bug itself (joysticks not supported) is solved."

On 2013-03-20 07:22:06 +0000, wrote:

Created attachment 1074
Restore accelerometer-as-joystick emulation (patch against a500a9dbfb41); March 20th, 2013

After thinking about it a bit, I have decided to:

  • Apply Joseba's approach and add the accelerometer back as the "last" joystick.
  • Re-open the bug, since the now-frozen API is broken without that.

Originally I have removed the accelerometer stuff since it made things somewhat simpler for me, and I've assumed more changes are going to be required anyway with the Sensor API.

Anyway, what the patch does is:

  • Always add an accelerometer as the last joystick, with three analog axes.
  • Fix another typo in SDL_SYS_JoystickInit (double initialization of a loop variable).

With this patch applied, an old app assuming an accelerometer as a joystick may not act as expected if a device with a source of SOURCE_CLASS_JOYSTICK is found (e.g. a USB game controller). Otherwise, backwards compatibility should be preserved.

On 2013-03-20 07:24:22 +0000, wrote:

Re-opened since the (frozen) API is currently broken without the accelerometer-as-joystick emulation.

On 2013-03-20 07:45:30 +0000, wrote:

Oh, I have almost forgotten: As the situation seems to be with the last patch (restoring accelerometer support), no SDL joystick motion events are sent for the accelerometer, while they are sent for actual joysticks. I suppose this is a good compromise for some level of backwards compatibility.

On 2013-03-20 08:53:02 +0000, wrote:

Yet two more such points that I shouldn't forget:

  • A proper Android implementation of SDL_JoystickNumButtons may simply be impossible. Hence, some positive value is returned at the moment for actual joysticks (and 0 for the accelerometer).
  • If the given approach is accepted, with the accelerometer being the last joystick in the list, it may have an implication on backwards compatibility with future Android apps that do support game controllers.

On 2013-03-20 17:58:56 +0000, Philipp Wiesemann wrote:

(In reply to comment # 17)

Created attachment 1072 [details]
A suggested update to the patch (March 18th 2013), should support more
analog axes

Thank you for the updating the patch. Here are some more things I would change:

In SDL_androidjoystick.c the return values of Android_JNI_GetNumJoysticks() and Android_JNI_GetJoystickNumOfAxes() are used assuming a positive value but they may be -1 in case of error. This may lead to unexpected behaviour (though I think they may return -1 not very often).

In SDL_androidjoystick.c the SDL_SYS_JoystickQuit() needs to delete the referenced strings. Else there would be a memory leak (though most of the time the subsystem is only quit on application exit so it may not matter).

In AndroidManifest.xml "android:targetSdkVersion" should be set to "12".

On 2013-03-22 13:02:46 +0000, Joseba García Echebarria wrote:

Hi,

Thanks ny00@outlook.com for taking the time to look at the patch.
I'll test the code in my device ASAP :)

And thanks Sam for committing the change, this'll really be helpful to people developing games for the OUYA and similar devices.

Joseba

On 2013-03-23 08:08:00 +0000, wrote:

Hi all,

Thanks to Philip for your bug (sub)reports.
And thanks to Joseba for bringing this patch out!

As hinted before, there are chances that the recent commit of the patch will be reverted, since it currently breaks the API when it comes to accelerometer input handling.

I am going to attach an updated patch against revision 7c2eb015a6d7 (right before the commit).

On 2013-03-23 08:31:32 +0000, wrote:

Created attachment 1077
Patch against 7c2eb015a6d7 (patch uploaded on March 23rd), supporting more joystick axes, backwards compatible with current apps, adding a hint

I have just attached an update to the patch, this time against revision 7c2eb015a6d7 (which still has no joystick support and no relevant API breakage).

List of changes, in comparison to "A suggested update to the patch (March 18th 2013), should support more analog axes":

  • As done in another file I have attached, another typo in SDL_SYS_JoystickInit is fixed (double initialization of a loop variable).
  • Philip's bug reports should be fixed.
  • A new hint named SDL_HINT_EMULATE_ACCELEROMETER_AS_JOYSTICK has been added. The default behavior of the hint (or equivalently when set to "1") is accelerometer-as-joystick emulation. Furthermore, no actual joystick device can be used. To use actual joystick devices and disable the emulation, set the hint to "0".
  • Reason for disabling actual joystick support, rather than merely adding the accelerometer as an additional joystick, is that current apps that use the accelerometer may malfunction if a real joystick is found.

Some notes about the hint:

*** If one wants to set the hint to "0", it should be done before the joystick subsystem is initialized. I have some feeling this is not a good idea, but I am not sure.

  • Furthermore, the hint currently does nothing on platforms differing from Android, although I see no reason for not changing that.
  • Internally, the actual string describing the hint is "SDL_ANDROID_EMULATE_ACCELEROMETER_AS_JOYSTICK". If someone implements that on a different platform one day, maybe a different name is preferred.

On 2013-03-23 13:12:14 +0000, wrote:

Right, may I beg your pardon (mainly Philipp's I guess) for these typos I have snipped into my last comments:

  • Yeah, I have realized your name has a double 'p'.
  • Technically, rather than fixing reports, I have corrected the reported bugs.

On 2013-03-25 05:40:58 +0000, wrote:

Alright, sorry for all the mess I have done so far. A couple of examples:

  • Committing against 7c2eb015a6d7 rather than the latest revision, since I have thought a revert is going to be applied.
  • I don't know why haven't I remembered this, but one revision of the proposed Sensor API patch has already had a similar hint for the accelerometer-as-joystick behavior. Better ignore my own suggestion for such a hint..

The latter point implies that, if the Sensor API and its implementation get committed (including the hint), then we are mostly set. All that may be missing is a few coding bugs reports here.

Thus, I would better halt for now, at least until the Sensor API and the hint get committed, if it happens.

On 2013-04-02 01:36:41 +0000, Sam Lantinga wrote:

Okay, we went ahead and rolled back your changes.
We need to keep SDL working at API rev 10.

On 2013-04-03 04:31:28 +0000, wrote:

(In reply to comment # 32)

Okay, we went ahead and rolled back your changes.
We need to keep SDL working at API rev 10.

Thanks for rolling back the changes and making existing apps work as expected again.

I don't agree on the claims of some developers for breaking compatibility with API level 10, though. I have never changed the minimum API level required, just the target API.

Reference: http://forums.libsdl.org/viewtopic.php?t=8958

On 2013-07-12 22:15:27 +0000, Ryan C. Gordon wrote:

(Sorry if you get a lot of copies of this email, we're touching dozens of bug reports right now.)

Tagging a bunch of bugs as target-2.0.0, Priority 2.

This means we're in the final stretch for an official SDL 2.0.0 release! These are the bugs we really want to fix before shipping if humanly possible.

That being said, we don't promise to fix them because of this tag, we just want to make sure we don't forget to deal with them before we bless a final 2.0.0 release, and generally be organized about what we're aiming to ship.

Hopefully you'll hear more about this bug soon. If you have more information (including "this got fixed at some point, nevermind"), we would love to have you come add more information to the bug report when you have a moment.

Thanks!
--ryan.

On 2013-07-15 13:33:36 +0000, Joseba García Echebarria wrote:

I've got the patch running in android-10 and over through a very dirty solution.
I basically have two classes: one to be called when onGenericMotion is available (android-12 and over) which is a subclass of the SDLSurface and SDLSurface itself.

When running the code in android-10 and android-11 I call SDLSurface and when I'm in Android-12 and over I create an object of this new class.

It works, but it's crappy.

I'm sure there must be a solution which is cleaner in Java. If someone helps me with that, I can try to re-submit the patch.

Regards,
Joseba

(In reply to comment # 34)

(Sorry if you get a lot of copies of this email, we're touching dozens of
bug reports right now.)

Tagging a bunch of bugs as target-2.0.0, Priority 2.

This means we're in the final stretch for an official SDL 2.0.0 release!
These are the bugs we really want to fix before shipping if humanly possible.

That being said, we don't promise to fix them because of this tag, we just
want to make sure we don't forget to deal with them before we bless a final
2.0.0 release, and generally be organized about what we're aiming to ship.

Hopefully you'll hear more about this bug soon. If you have more information
(including "this got fixed at some point, nevermind"), we would love to have
you come add more information to the bug report when you have a moment.

Thanks!
--ryan.

On 2013-07-16 04:13:44 +0000, wrote:

Hey,

Actually, after the reports of the patch refusing to work with apps that target API level 10 (without changing it), I have thought of a way to solve this which is not that different from what you have done. It is more general and can help with adding more features than joystick support.

It basically goes like this:

  • The current SDLActivity.java file with class SDLActivity should stay and have no more than minor modifications, used for any app that targets API level 10 or 11.
  • A new class, say, SDLActivity12, can be implementation in a file named SDLActivity12.java as a subclass of SDLActivity, to be used by any app that targets API level 12 or later. It should be possible, at least in theory, to take advantage of any feature added in API level 12 (like game controller support) using this class, rather than SDLActivity directly.
  • Similarly, a subclass of SDLActivity12 can be added; Say, SDLActivity13. And so on. In the same way there can be a subclass of SDLActivity named SDLActivity11, while SDLActivity12 is a subclass of SDLActivity11.
  • Not sure about this, but maybe in such a case it worths to rename SDLActivity to SDLActivity10?
  • Of course, if any of the suggestions is actually applied, the README file should be updated appropriately.
  • Finally, it should still be possible to use a default accelerometer as a joystick device. (It's probably better to do using the hint added by you in the Sensor API patch, rather than the hint I have independently added here before for some strange reason.)

On 2013-07-16 04:27:38 +0000, wrote:

On a side note, since this is indirectly related to the Sensor API patch (via the accelerometer "joystick device"), originally I've thought the sensor patch should have a higher priority. It may be a bit late to add new things to the API now, though, and there is something I have recently realized about the sensor patch regarding resource usage (after encountering a different bug regarding such a thing). I should add a description of that over there soon.

On 2013-07-26 13:52:09 +0000, Gabriel Jacobo wrote:

Joseba, can you resubmit your updated Android-10 compatible patch? I'll try to help you out getting it ready for acceptance, what you did seems to be along the lines of what Android devs suggest here: http://android-developers.blogspot.com.ar/2010/07/how-to-have-your-cupcake-and-eat-it-too.html So, hopefully with a little massaging we can add joystick support and remain compatible with android-10 for those who need it.

Keep in mind that SDL_android.cpp was recently moved to be SDL_android.c

Thanks!

On 2013-07-26 15:17:17 +0000, Joseba García Echebarria wrote:

I'll have to rewrite parts of it, but yeah, I'll upload it ASAP.

Cheers.

On 2013-08-01 17:56:15 +0000, Gabriel Jacobo wrote:

Joseba, do you have an ETA for the updated patch? I know this issue has been dormant for a while but ideally we want to get this in before the 2.0 release.

On 2013-08-01 18:06:13 +0000, Joseba García Echebarria wrote:

I should be able to create and submit the patch next week before the weekend.

Cheers,
Joseba

(In reply to comment # 40)

Joseba, do you have an ETA for the updated patch? I know this issue has been
dormant for a while but ideally we want to get this in before the 2.0
release.

On 2013-08-05 15:13:18 +0000, Joseba García Echebarria wrote:

Created attachment 1272
New patch for latest version of SDL

The attached patch should work in Android 9 and later and applies to current SDL.

I have modified the code so that instead of using two different SDLSurface classes depending on the Android version you're running, I now only have one SDL_Surface class with a class inside of it that handles onGenericMotion events and only gets instanced if the user's running a supported version of Android.

This patch should also emulate the accelerometer as a joystick unless the user sets a given hint.

It's fairly big, so please let me know what you think of it.

Regards,
Joseba

On 2013-08-05 16:21:23 +0000, Philipp Wiesemann wrote:

(In reply to comment # 42)

Created attachment 1272 [details]
New patch for latest version of SDL

Thanks for working on getting joystick functionality included again.

I have not tested the patch but but here are some comments from reading:

Java:

  • You could shorten the getJoyId() method by using indexOf() from ArrayList (see the older patches linked above). [1]
  • Also use Integer.valueOf() then. [2]

C:

  • You could write "return SDL_OutOfMemory();" because it returns -1.
  • You could use SDL_calloc() instead of SDL_malloc()/SDL_memset().

[1] http://developer.android.com/reference/java/util/ArrayList.html#indexOf(java.lang.Object)

[2] http://developer.android.com/reference/java/lang/Integer.html#valueOf(int)

On 2013-08-05 17:00:24 +0000, wrote:

Yeah, thanks for the updated patch.

While I have not even applied it, and I'm also far from being an actual SDL developer/maintainer, here are a few comments I may add:

  • InputDevice.getDeviceIds requires SDK 9, rather than 16. (On a side note, this comment apparently applied to the very first revision of the patch as well.)
  • While getJoystickAxes may return what you expect it to, the given implementation of onGenericMotion does its job only under the assumption there are exactly two axes: MotionEvent.AXIS_X and MotionEvent.AXIS_Y.
  • Shouldn't the call to the Java function getJoystickAxes from the C code be static?
  • I have seen that other pointers to (static) Java methods are stored in global variables, rather than retrieved whenever there is the need for such. See SDL_android.c for a few examples. It probably worths to do the same here, although there may also be a good reason to not do so which I haven't thought about.

On 2013-08-05 17:15:56 +0000, Gabriel Jacobo wrote:

Thanks Joseba, I'll test this ASAP and incorporate the comments made above where appropriate.

On 2013-08-06 04:41:56 +0000, wrote:

Oh yeah, some more comments:

  • (Possibly the most important) As SDL_HINT_ACCEL_AS_JOY is set to 1 by default, existing programs that use the Joystick API for retrieving accelerometer input will malfunction if an actual game controller is found. If the accelerometer's joystick ID is forced to be 0, though, most of them will probably continue to function as usual. It may still be desired to ignore all actual controllers and force** the value of SYS_numjoysticks to 1 in such a case.

** One problem with the very last suggestion: How to support accelerometer and controller input altogether? (I guess the Sensor API should solve that one day.)

  • What is the purpose of the "final" variable in the body of the Java function keycode_to_SDL? I guess it should be used for returning 0 in case the keycode is unrecognized.
  • Although not mandatory, it may worth to rename the "final" variable, since "final" is actually a Java keyword.
  • There are great chances that the Android platform does not give out a reliable way to query the number of buttons in a game controller. This is not different from the situation with keyboards, which are not that different on that platform when it comes to the SDKs. For now, the returned number of buttons is 36.
  • Maybe it worths to add a comment to README-android.txt and SDL_joystick.h, informing that SDL_JoystickNumButtons is not supported on the platform in its usual form.
  • In SDL_SYS_JoystickInit, the accelerometer's name is copied twice if SDL_HINT_ACCEL_AS_JOY is set to 1.

On 2013-08-06 14:03:33 +0000, Gabriel Jacobo wrote:

Do any of you have a test project I can use to try this? My engine doesn't have Joystick support, so any help in this regard is appreciated!

On 2013-08-06 16:06:24 +0000, wrote:

(In reply to comment # 47)

Do any of you have a test project I can use to try this? My engine doesn't
have Joystick support, so any help in this regard is appreciated!

For a non-SDL project, try a recent revision of API Demos. Among other things, you may choose something like "View -> Game Controller Input" and see reports of joystick axes motions and button presses/releases.

As for SDL code, you can check out test/testjoystick.c. You'd probably want to change the value of SDL_HINT_ACCEL_AS_JOY, depending on what you exactly want to do. Furthermore, if there is a problem with running this then maybe it can be solved by modifying SCREEN_WIDTH and/or SCREEN_HEIGHT.

On 2013-08-06 19:29:10 +0000, Joseba García Echebarria wrote:

Thank you all from your comments and for taking the time to read the patch.
Here's my thoughts on some of your comments:

  • (Possibly the most important) As SDL_HINT_ACCEL_AS_JOY is set to 1 by
    default, existing programs that use the Joystick API for retrieving
    accelerometer input will malfunction if an actual game controller is
    found. If the accelerometer's joystick ID is forced to be 0, though, most of
    them will probably continue to function as usual. It may still be desired to
    ignore all actual controllers and force** the value of SYS_numjoysticks to 1
    in such a case.

** One problem with the very last suggestion: How to support accelerometer
and controller input altogether? (I guess the Sensor API should solve that
one day.)
That's a problem, yes. One shouldn't have to renounce to joystick support in Android just to be able to get accelerometer support. And since the default is to have the accelerometer as a joystick, we'd probably get flooded with questions like "why aren't joysticks working with my code?", so the final solution should include both behaviours by default, in my opinion.
What we could do is set the accelerometer as fake joy 0 (instead of the last one, as it currently is) but codes that choose joystick 0 as the default controller would probably not work as expected either.

Since the SDL2 API is frozen now, maybe the second option is best as existing code would probably continue to work as expected and it can be disabled through the hint.

  • What is the purpose of the "final" variable in the body of the Java
    function keycode_to_SDL? I guess it should be used for returning 0 in case
    the keycode is unrecognized.
  • Although not mandatory, it may worth to rename the "final" variable, since
    "final" is actually a Java keyword.
    That's not needed anymore. I was storing the value in "final" and returned it at the end of the function, but having multiple returns makes it unneeded, yeah. Still, if I'm not mistaken, it's C code :)
  • There are great chances that the Android platform does not give out a
    reliable way to query the number of buttons in a game controller. This is
    not different from the situation with keyboards, which are not that
    different on that platform when it comes to the SDKs. For now, the returned
    number of buttons is 36.
  • Maybe it worths to add a comment to README-android.txt and SDL_joystick.h,
    informing that SDL_JoystickNumButtons is not supported on the platform in
    its usual form.
    Yep, that's kind of crappy. If there's a reliable way to detect the number of buttons, please let me know.
  • In SDL_SYS_JoystickInit, the accelerometer's name is copied twice if
    SDL_HINT_ACCEL_AS_JOY is set to 1.
    Absolutely true, yes. The if condition after the for loop in the function is useless.

(In reply to comment # 47)

Do any of you have a test project I can use to try this? My engine doesn't
have Joystick support, so any help in this regard is appreciated!
The code I took the patch from is not directly applicable, since it's too big and relies on some of my other patches (sensor API, real mouse support and some other unrelated patches to SDL).
If the code pointed in comment # 48 doesn't work, I'll try to write some small sample tomorrow. Nothing fancy, just an app with a lot of SDL_Log's

Cheers,
Joseba

On 2013-08-07 04:24:08 +0000, wrote:

Thanks for your reply. Regarding a few of your comments:

  1. You're right, keycode_to_SDL is a C function! Well, I've thought the purpose of the "final" variable is returning 0 in case an unrecognized key code is queried. On a second thought, it may be better to return, say, a known out-of-range value, and not call SDL_PrivateJoystickButton at all in such a case.

  2. About the accelerometer, basically I've referred to programs that assume there is a single "joystick" device: An accelerometer. Thinking about it, my opinion about supporting accelerometer and actual joystick input altogether leads into two possibilities, where one of them has not been mentioned before.

So, two suggestions for handling joystick and accelerometer input:

  1. Just don't do it. It should really be done using something like the propsed sensor API, if at all. The hint SDL_HINT_ACCEL_AS_JOY exists simply because there are already applications taking advantage of the joystick API in a specific way, and it may be better to not add another example of that kind.
  2. Change SDL_HINT_ACCEL_AS_JOY into a 3-value enum. A suggestion for values and their meanings: "0" - List actual joysticks only, "1" - List accelerometer only, "2" - List joysticks and accelerometer altogether. Technically, it seems like the technical difference between "1" and "2" can be as little as in the total amount of reported joystick devices.

On 2013-08-09 10:08:51 +0000, wrote:

Oh yeah, one more thing:

Right now, when a joystick motion event arrives, a float value representing the state of an analog axis is simply multiplied by 32767, before getting casted to a (signed) integer. This should work well as long as the axis' motion range returned by the Java function getMotionRange (and also found using getMotionRanges) is [-1.0,1.0]. Problem is that an axis may have a different range.

For instance, there is this USB gamepad I have gotten (which is admittedly considered old these days) with 5 analog axes: 0-1 for a left stick, 2 for a throttle/slider and 3-4 for a right stick. (Yeah, good choice of enumeration.)

When I tested it once on an Android setup or two (admittedly in quite unofficial ways involving a custom ROM or so), all axes were reported to have a motion range of [-1.0,1.0], with the exception of the last, having the range of [0.0,1.0].

Should that be fixed? Well, I wasn't sure about that before. But, now I think it's better to do so, especially considering the fact that per-controller axis scalings may theoretically be corrected with the help of the SDL_GameController API, as I saw at one point.

On 2013-11-05 17:57:02 +0000, Gabriel Jacobo wrote:

Created attachment 1408
Joystick support for Android RC1

I'm waiting for word from Sam on bumping the required SDK level for building to 12 to commit this into mainline, so in the meantime feel free to test this patch and report back. I tested this on two x86 VMs (Android 2.3 and Android 4.3) and the 2.3.3 ARM emulator VM, using the testjoystick.c test. It seems to work fine, though it explodes when joysticks are plugged/unplugged, so I guess that's the next thing in the todo list.

On 2013-11-05 23:08:31 +0000, Gabriel Jacobo wrote:

Fixed in http://hg.libsdl.org/SDL/rev/24b4e98c6010

Thanks to everyone that contributed!

On 2013-11-06 08:31:09 +0000, wrote:

It has surely been a kind of a journey. Thanks to bringing the patch in its initial form, and later (yesterday here) committing it!

Hoping it's not too late, I do have a few comments about it. Truly, some of them may seem a bit old, but let me mention these anyway.

  • Just a bit about wording, I guess (as described in the modified txt files): "minimum SDK version required to build SDL" refers to targetSdkVersion (or the project target in default.properties), while "required runtime version" is minSdkVersion? :)

  • Write at some place (say README-android.txt and/or SDL_joystick.h) that SDL_JoystickNumButtons doesn't do what you'd expect it to do. Of course, if the SDL_GameController API is ever used on the Android platform then it shouldn't be a problem, I believe.

  • Hope that existing applications still work as expected. Yes, this is why we have the hint SDL_HINT_ACCEL_AS_JOY. Problem is, there are great chances that, if you build such an app using the current revision of SDL 2.0 at the HG repository (with the patch applied), and then run the app while a controller is connected, the app will get input from the controller even though it rather expects accelerometer input. Reason: The app opens SDL_Joystick no. 0. Relocating the accelerometer to the beginning of the list may help, but not necessarily, since some app may try to open the "last" SDL_Joystick in the list for the purpose of obtaining accelerometer input (although the chances of this are probably small). The safest approach is listing the accelerometer only. Since it looks like there's no Sensor API (and implementation), though, an additional value may be required for the hint to do so, to be the new default for the hint.

  • Since hotplugging is unsupported, ensure that there is no case of buffer overflow in the event handling (say if one controller is swapped with another).

  • Considering the call to keycode_to_SDL, maybe make sure that no unrecognized keycode is handled in an unexpected way? (Although it could mean that less buttons are supported...)

On 2013-11-06 12:33:50 +0000, Gabriel Jacobo wrote:

Feel free to open new ticket for particular issues, trying to make them as atomic as possible (a ticket/patch for README changes, etc).

Regarding your other comments, hotplugging will require API >=16. It's doable but right now it's 50% of the market, so perhaps not the most pressing issue (but if someone contributes a patch... :) )

Given the "static" nature of the joystick code, I don't think having an unplugged joystick is going to be a problem, you just stop receiving events for it. I actually tried this (only in a VM), and it seemed to work without failing spectacularly. Real world testing is needed though.

Re the accelerometer stuff, I think it's not worth overthinking it. SDL on Android is distributed by the developer of the app, so there's no chance (like it's the case with Steam games, or Linux games in general) for the library to change under you. Also identifiying the accelerometer by name is simple enough.

On 2013-11-06 16:07:12 +0000, wrote:

Alright, guess I can open a few tickets, at least for a portion of the points.

(In reply to Gabriel Jacobo from comment # 55)

Given the "static" nature of the joystick code, I don't think having an
unplugged joystick is going to be a problem, you just stop receiving events
for it. I actually tried this (only in a VM), and it seemed to work without
failing spectacularly. Real world testing is needed though.

My worry has rather been about the possibility of a new controller added by the device's user after the app is started (and similar situations). Doesn't matter if it's done based on USB (OTG), Bluetooth or any other form of connectivity.

On a side-note, if this means that lots of function bodies (like the one of SDLJoystickHandler_API12.getJoystickAxes) need to be modified, I wonder if it's better to call SDLJoystickHandler.createJoystickList only one, after initializing the joystick subsystem.

On 2013-11-06 22:53:24 +0000, Joseba García Echebarria wrote:

Thank you very much for taking the time to review our code and push it online, really :)

(In reply to Gabriel Jacobo from comment # 53)

Fixed in http://hg.libsdl.org/SDL/rev/24b4e98c6010

Thanks to everyone that contributed!
@SDLBugzilla SDLBugzilla added the enhancement New feature or request label Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@SDLBugzilla