Android Joystick Handling #3536

Merged
merged 6 commits into from Nov 2, 2013

Conversation

Projects
None yet
4 participants
@davilla
Contributor

davilla commented Nov 1, 2013

Add native android joystick handling, Works OOTB with Ouya, GameStick and Nvidia/Shield.

Much thanks to Koying for putting up with the old man, Nvidia for donating a few Shields for development work as well as Pivos/GameStick for the initial implementation and Ouya for dev units.

@garbear

This comment has been minimized.

Show comment Hide comment
@garbear

garbear Nov 1, 2013

Member

perfect, thank you for creating this PR davilla. When I finish the Joystick API pr I'll be able to migrate over the joystick processing from CWinEventsAndroid.

Getting to know the Android layer is still something I've been meaning to do, so there's little I can check but I'll do a quick review.

Member

garbear commented Nov 1, 2013

perfect, thank you for creating this PR davilla. When I finish the Joystick API pr I'll be able to migrate over the joystick processing from CWinEventsAndroid.

Getting to know the Android layer is still something I've been meaning to do, so there's little I can check but I'll do a quick review.

@davilla

This comment has been minimized.

Show comment Hide comment
@davilla

davilla Nov 1, 2013

Contributor

merged and ready to go.

Contributor

davilla commented Nov 1, 2013

merged and ready to go.

xbmc/android/activity/AndroidKey.cpp
XBMC_Key((uint8_t)keycode, sym, modifiers, true);
+#endif

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Nov 2, 2013

Member

is the ifdef block supposed to surround the XBMC_Key() routine (or is only the debug stuff?)

@jmarshallnz

jmarshallnz Nov 2, 2013

Member

is the ifdef block supposed to surround the XBMC_Key() routine (or is only the debug stuff?)

This comment has been minimized.

Show comment Hide comment
@davilla

davilla Nov 2, 2013

Contributor

no, that's a bug

@davilla

davilla Nov 2, 2013

Contributor

no, that's a bug

xbmc/android/activity/EventLoop.cpp
+ int32_t type = AInputEvent_getType(event);
+ int32_t source = AInputEvent_getSource(event);
+ int32_t repeat = AKeyEvent_getRepeatCount(event);
+ int32_t keycod = AKeyEvent_getKeyCode(event);

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Nov 2, 2013

Member

is there a plan to use these last two?

@jmarshallnz

jmarshallnz Nov 2, 2013

Member

is there a plan to use these last two?

This comment has been minimized.

Show comment Hide comment
@davilla

davilla Nov 2, 2013

Contributor

when debug is enabled, yes.

@davilla

davilla Nov 2, 2013

Contributor

when debug is enabled, yes.

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Nov 2, 2013

Member

Not in this function though - appears only to be a switch on type and on source.

@jmarshallnz

jmarshallnz Nov 2, 2013

Member

Not in this function though - appears only to be a switch on type and on source.

+ {
+ ret |= g_application.ProcessJoystickEvent(input_device.name,
+ item, input_type, amount, holdTime);
+ }

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Nov 2, 2013

Member

Amount is always 1.0 or 0.0 as things are. This seems a bit odd to me, as I'd have thought that 0.5 should be a valid value for one of the axes to take?

Maybe there is supposed to be a line in the XBMC_JOYAXISMOTION case with amount = pumpEvent.jaxis.fvalue; ?

@jmarshallnz

jmarshallnz Nov 2, 2013

Member

Amount is always 1.0 or 0.0 as things are. This seems a bit odd to me, as I'd have thought that 0.5 should be a valid value for one of the axes to take?

Maybe there is supposed to be a line in the XBMC_JOYAXISMOTION case with amount = pumpEvent.jaxis.fvalue; ?

This comment has been minimized.

Show comment Hide comment
@davilla

davilla Nov 2, 2013

Contributor

As you should know, one never should try for == with floats, they just don't work that way in hardware. So >= ALMOST_ZERO means +/- 1.0 in this case.

@davilla

davilla Nov 2, 2013

Contributor

As you should know, one never should try for == with floats, they just don't work that way in hardware. So >= ALMOST_ZERO means +/- 1.0 in this case.

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Nov 2, 2013

Member

ATM, amount can only ever possibly be 0 or 1. Should you maybe have a line of code assigning pumpEvent.jaxis.fvalue to amount ?

@jmarshallnz

jmarshallnz Nov 2, 2013

Member

ATM, amount can only ever possibly be 0 or 1. Should you maybe have a line of code assigning pumpEvent.jaxis.fvalue to amount ?

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Nov 2, 2013

Member

@davillla, @koying? Surely as things are the analog sticks are functioning as dpads. No analog at all as amount is never set to anything other than 0 or 1.

@jmarshallnz

jmarshallnz Nov 2, 2013

Member

@davillla, @koying? Surely as things are the analog sticks are functioning as dpads. No analog at all as amount is never set to anything other than 0 or 1.

This comment has been minimized.

Show comment Hide comment
@koying

koying Nov 3, 2013

Contributor

Indeed. To be discussed with @garbear for his refactor, I guess.
XBMC analog seek stuff seems broken, now, so if a stick has no dpad, that renders it useless. IMO, ideally, we should be able to assign up/down/... to a true analog stick.

@koying

koying Nov 3, 2013

Contributor

Indeed. To be discussed with @garbear for his refactor, I guess.
XBMC analog seek stuff seems broken, now, so if a stick has no dpad, that renders it useless. IMO, ideally, we should be able to assign up/down/... to a true analog stick.

@jmarshallnz

This comment has been minimized.

Show comment Hide comment
@jmarshallnz

jmarshallnz Nov 2, 2013

Member

Looks good other than those minors.

Member

jmarshallnz commented Nov 2, 2013

Looks good other than those minors.

davilla added a commit that referenced this pull request Nov 2, 2013

@davilla davilla merged commit a17e191 into xbmc:master Nov 2, 2013

@koying

This comment has been minimized.

Show comment Hide comment
@koying

koying Nov 7, 2013

Contributor

@davilla Any specific reasons for filtering key events? Right mouse button sends a KeyEvent.KEYCODE_BACK and it doesn't seem to be trapped by us anymore, so I suspect this filters it out.

@davilla Any specific reasons for filtering key events? Right mouse button sends a KeyEvent.KEYCODE_BACK and it doesn't seem to be trapped by us anymore, so I suspect this filters it out.

This comment has been minimized.

Show comment Hide comment
@koying

koying Nov 7, 2013

Contributor

I confirm this cause right mouse button to go back to desktop

Contributor

koying replied Nov 7, 2013

I confirm this cause right mouse button to go back to desktop

This comment has been minimized.

Show comment Hide comment
@davilla

davilla Nov 9, 2013

Contributor

k

Contributor

davilla replied Nov 9, 2013

k

@davilla davilla deleted the davilla:joystick branch Feb 27, 2014

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