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

Android-TV: no more handling of back button on remote #2264

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

Android-TV: no more handling of back button on remote #2264

SDLBugzilla opened this issue Feb 11, 2021 · 0 comments

Comments

@SDLBugzilla
Copy link
Collaborator

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: don't know
Reported for operating system, platform: Android (All), x86_64

Comments on the original bug report:

On 2016-10-04 08:45:04 +0000, Sylvain wrote:

I am trying the latest trunk today.

on Android-TV the remote is not fully supported anymore:

I received "SDL_JOYBUTTONDOWN" with button=11/12/13/14/19
which are arrows up/down/left/right/up and "OK".

But there are no more handling of the "back" button.
nor all the other keys (back, 0-9, etc.)

It used to works because I think the remote was producing SDL_KEYDOWN events.

On 2016-10-04 08:57:09 +0000, Sylvain wrote:

It has been introduced with this commit:
https://bugzilla.libsdl.org/show_bug.cgi?id=3426

Before, the remote control on Android-TV was handling both with:
SDL_JOYBUTTONDOWN events for arrows and "OK" button.
SDL_KEYDOWN events for others keys

On 2016-10-04 16:33:09 +0000, Sylvain wrote:

The reason seems to be that the return codes of "onNativePadDown/Up" are not checked anymore, so they they just fails silently...
Please add them back like:

    if (SDLActivity.isDeviceSDLJoystick(event.getDeviceId())) {
        // Note that we always process a pressed/released button here, as an unopened
        // SDL_Joystick's button press should not be processed as a keyboard's key press
        if (event.getAction() == KeyEvent.ACTION_DOWN) {
           if (SDLActivity.onNativePadDown(event.getDeviceId(), keyCode) == 0) { 
              return true;
           }    
        } else if (event.getAction() == KeyEvent.ACTION_UP) {
            if (SDLActivity.onNativePadUp(event.getDeviceId(), keyCode) == 0) {
              return true;
           } 
        }    
    }

On 2016-10-05 07:47:24 +0000, Sylvain wrote:

more investigation:

Native function Android_OnPadDown() fails because "keycode" cannot be translated.
This line produce a value of "-1":
"int button = keycode_to_SDL(keycode);"

For instance, I observed:
keycode 7 to 16 for the remote buttons 0 to 9.
keycode 4 for the back button.

Though, there are correctly interpreted as scancode with "SDL_Scancode Android_Keycodes[]".

The remote device has an "event.getSource()" of 0x301 which is: SOURCE_DPAD | SOURCE_KEYBOARD

On 2016-10-05 18:10:01 +0000, wrote:

Unfortunately, simply checking the return codes of "onNativePadDown/Up" as previously done has its own issue:

If an SDL joystick is connected and opened, then a proper KeyEvent, say with keycode KEYCODE_BUTTON_1, should lead to an SDL joystick button event as expected.

If, however, the joystick was not opened, then "onNativePadDown/Up" will return a negative value, so before the commit from bug 3426, you could unexpectedly get a keyboard event. (In practice, you'll just get a log message, since KEYCODE_BUTTON_1 has no mapping to a proper SDL_ScanCode value, but it's still an problem).

What should still be done, though, is checking the key code itself. We do have the KeyEvent.isGamepadButton method, but according my test, it returns "true" exactly (and only) for the KEYCODE_BUTTON* values, and not for KEYCODE_DPAD* or any other key code.

Here is a possible solution:

  • Do check the return codes of "onNativePadDown/Up" as previously done.
  • In addition, in "Android_OnPadDown/Up" from src/joystick/android/SDL_sysjoystick.c, 0 should always be returned in case the key code can be translated to an SDL_joystick button; Even if no matching joystick can be found.

On 2016-10-05 18:46:30 +0000, wrote:

Created attachment 2575
Patch

A patch with the suggested changes should be attached.

On 2016-10-05 19:38:25 +0000, Sylvain wrote:

Thanks for the explanation!

I think the patch will work (haven't tested yet), because you smartly catch the case when the joystick is not open.
But I don't see it really readable in the future.

Just want to suggest potential other (distinct) ways:

  • what about extending the function "keycode_to_SDL" of "SDL_sysjoystick.c" so that it also include some of the keyboard ScanCode ?
    (without error code check of "SDLActivity.onNativePadUp/Down").
    that would mean extending the header "SDL_gamecontroller.h" of the library "SDL_GameControllerButton", maybe not wanted ...

  • in C function "Android_OnPadDown/Up()", if the keycode cannot be decoded, but the joystick is opened, use the keyboard scancode (call SDL_SendKeyboardKey(...)) ?
    (without error code check of "SDLActivity.onNativePadUp/Down").

  • in SDLActivity.java, make "isDeviceSDLJoystick()" immediately return false if source is/contains a InputDevice.SOURCE_KEYBOARD ?
    what would happen ? maybe the remote would be fully seen as keyboard ? would it break other stuff?

  • in SDLActivity.java:
    like your patch, but in something more read-able I think.
    (and it would have the benefit of ignoring the "remote" if it has not been opened. (currently, if it's not open, 0-9 and back should still work!))

    export the c function "JoystickByDeviceId" to Java to know in the java code whether the joystick is open or not.
    and re-write the java side like this:

if (SDLActivity.isDeviceSDLJoystick(event.getDeviceId())) {

if (SDLActivity.onNativeJoystickIsOpened(event.getDeviceId())) {

  if (event.getAction() == KeyEvent.ACTION_DOWN) {
     if (SDLActivity.onNativePadDown(event.getDeviceId(), keyCode) == 0) {
        return true;
     }
  } else if (event.getAction() == KeyEvent.ACTION_UP) {
     if (SDLActivity.onNativePadUp(event.getDeviceId(), keyCode) == 0) {
        return true;
     }
  }

  // Keyboard side of the Joystick
  if ((event.getSource() & InputDevice.SOURCE_KEYBOARD) != 0) {
     if (event.getAction() == KeyEvent.ACTION_DOWN) {
        SDLActivity.onNativeKeyDown(keyCode);
        return true;
     }
     else if (event.getAction() == KeyEvent.ACTION_UP) {
        SDLActivity.onNativeKeyUp(keyCode);
        return true;
     }
  }

}

// Always return true
return true;
}

... other Keyboard only, and Mouse only

On 2016-10-06 10:40:05 +0000, Sylvain wrote:

Created attachment 2576
patch

Indeed, there is an issue with SDL and this composite device "gamepad|keyboard".
When the SDL_Joystick is not opened, the "keyboard" part still works. Which is bad.

I think that either this kind of device should be fully handled as a keyboard, and it can be done with following line:
In the java function "isDeviceSDLJoystick()":

  • if ((sources & InputDevice.SOURCE_KEYBOARD) == InputDevice.SOURCE_KEYBOARD) {
  • return false;
  • }

Or it can be handled as a joystick, with a fall-back to keyboard for the extra key-codes:
But in that case, if the Joystick is not opened, the keyboard should not be send neither.

The attached patch is longer than your but it essentially the same.
It's a little more clear the way events are sent, and it solves the opening thing.

On 2016-10-07 06:25:50 +0000, wrote:

I see what you're hinting at. Unfortunately, the way the SDL2 API is currently defined (which is obviously based on 1.2 and earlier), I believe that keyboard events should always arrive, and not be directly related to SDL2's joystick subsystem. Well, sure, you can filter the events in your app in order to effectively "disable" them, but that's unrelated.

Also, checking if ((sources & InputDevice.SOURCE_KEYBOARD) == InputDevice.SOURCE_KEYBOARD) in isDeviceSDLJoystick() has an issue. For some weird reason, when I've recently made tests in an Android x86 VM with a USB controller (actually a PSX controller + USB adapter), the reported sources for the device have been SOURCE_KEYBOARD and SOURCE_JOYSTICK. So it's really SOURCE_JOYSTICK which I depend on in this case.

This may be a bug, since SOURCE_GAMEPAD might make more sense, but this is what I've gotten. Furthermore, when I've pressed on a button, I've gotten a KeyEvent with source SOURCE_KEYBOARD. So I've really had to ignore the event's sources, and rather check the input device's sources with isDeviceSDLJoystick(). Using the key code as a filter is not a good idea, since a few KEYCODE_DPAD* keycodes may arrive from a PC keyboard (the arrow keys) or a game controller.

If you really want to get events only after opening the device, then adding even more button mappings to keycode_to_SDL may be the only solution for now. It doesn't mean you have to modify SDL_GameController; You'll simply add more SDL_CONTROLLER_BUTTON_MAX+offset values, and modify ANDROID_MAX_NBUTTONS accordingly. This will also have the advantage of supporting separate inputs from different devices. Unfortunately, it won't cover other sources, like trackpad/mouse, unless they're faked as joysticks maybe.

So yeah, I agree there's a problem. At the least, my suggested patch should make the "non-joystick" buttons usable again (using keyboard events), hopefully without introducing other issues.

On 2016-10-07 07:57:05 +0000, Sylvain wrote:

Those are not strictly "keyboard" events. They are events coming from a device which tells itself to be both JOYSTICK and KEYBOARD. I mean "detected" by "isDeviceSDLJoystick()" and whose keycode is not handled by the SDL Joystick sub-system, so that we can handled it by the SDL keyboard sub-system.

For this reason, it make sense to me to activate/desactive those very keyboard event based on the joystick opening. At least, it's not weird to do so.

In thhe case of the real keyboard:
I have attached a plain Desktop USB keyboard to my smartphone.
And it is not detected "isDeviceSDLJoystick()", so that it really behave as a keyboard.
And it's not affected by the opening/closing of joysticks.

I don't need a patch for myself nor I need to modify my applications. I have already have many solutions. But I'd like SDL to evolve to handle correctly those devices.

Also, there is this issue / side effect:
https://bugzilla.libsdl.org/show_bug.cgi?id=3237

With a composite device JOYSTICK|KEYBOARD. At first arrow keys come from the SDL keyboard subsystem. Then, once you open the SDL Joystick, arrow keys arrive from the Joystick subsystem.

On 2016-10-11 12:09:25 +0000, wrote:

ok, I think that we can summarize that there are basically three suggested solutions at the moment:

  1. On key event, if isDeviceSDLJoystick returns true, process event based on keycode. If we consider it a joystick/pad keycode, send SDL joystick button event, but only if joystick is opened. Otherwise, continue with non-joystick event processing. (This is the approach in the patch I've attached to here earlier.)

  2. On key event, if isDeviceSDLJoystick returns true, ignore event if joystick is not opened. Otherwise, process it according to the key code (can be an SDL joystick event, an SDL keyboard event or anything else). I think the patch can be similar to Sylvain's code from Comment 6 (posted at 2016-10-05 19:38:25 UTC), although maybe without the duplication of keyboard handling.

  3. On key event, if isDeviceSDLJoystick returns true, always process the event as a joystick button event, and only if the joystick is opened. This means that the function keycode_to_SDL from src/joystick/android/SDL_sysjoystick.c should be modified to handle all possible key codes from Android.

In all of the above, if isDeviceSDLJoystick returns false, the non-joystick cases should be checked as usual.

Furthermore:

(In reply to Sylvain from comment # 9)

Also, there is this issue / side effect:
https://bugzilla.libsdl.org/show_bug.cgi?id=3237

With a composite device JOYSTICK|KEYBOARD. At first arrow keys come from the
SDL keyboard subsystem. Then, once you open the SDL Joystick, arrow keys
arrive from the Joystick subsystem.

It is indeed a similar bug, showing the issues with having combo "SDL keyboard+joystick" input devices (or similar). It should be fixed as of this commit: https://bugzilla.libsdl.org/show_bug.cgi?id=3426

On 2016-10-12 09:42:14 +0000, Sylvain wrote:

I agree, the three solutions seem correctly summed up! hopefully it gives enough information to fix the issue.

About https://bugzilla.libsdl.org/show_bug.cgi?id=3237
It may be currently fixed, but I hope it won't occur again with one of the three solution.

I mean, the issue would be:
if joystick is opened, it produces "SDL_JOYBUTTONUP", but if it's closed, it produces SDLK_UP".

On 2016-10-18 05:09:47 +0000, Sam Lantinga wrote:

Fixed, thanks!
https://hg.libsdl.org/SDL/rev/a5bc0d82dca4

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

1 participant