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

Haptics and Controller power-off #9663

Merged
merged 8 commits into from Aug 10, 2016
Merged

Haptics and Controller power-off #9663

merged 8 commits into from Aug 10, 2016

Conversation

garbear
Copy link
Member

@garbear garbear commented Apr 22, 2016

This PR activates controller rumble motors when the user receives a notification. Also, thanks to Montellese, we have the ability to power off controllers on exit.

Two settings are added:

  • "Test rumble"
  • "Power off controllers on exit"

This PR currently relies on the retroplayer branch of peripheral.joystick. These changes will be merged to master when this PR is merged.

ATM, rumble and power-off-on-exit are only supported via the XInput API on windows. Additional APIs can be added at a later date independent of Kodi.

@@ -39,15 +42,25 @@ CAddonInputHandling::CAddonInputHandling(CPeripheral* peripheral, IInputHandler*
{
m_buttonMap.reset(new CAddonButtonMap(peripheral, addon, handler->ControllerID()));
if (m_buttonMap->Load())
m_driverHandler.reset(new CInputHandling(handler, m_buttonMap.get()));
else
m_buttonMap.reset();

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@Montellese
Copy link
Member

BTW the Android API supports vibration patterns, see http://developer.android.com/reference/android/os/Vibrator.html. Maybe that might be something that would be useful for other stuff as well? I don't really know how libretro provides rumble events though.

@Montellese Montellese added Type: Feature non-breaking change which adds functionality v17 Krypton Component: Input labels Apr 22, 2016
@Hedda
Copy link
Contributor

Hedda commented Apr 22, 2016

Will haptics (rumble) have GUI options to be disabled and enabled? Maybe even have GUI settings to enable or disable rumble for different things? I know that at least I would not want to have rumble enabled for notifications by default but I would like to have it enabled in games by default.

@Montellese
Copy link
Member

@garbear is there a branch in peripheral.joystick with just the rumble stuff in it? I wanted to update my "turn off controller on shutdown" work and thought it best to do it on top of this work but I also need to update peripheral.joystick.

@Montellese
Copy link
Member

nvm I pulled together the necessary commits, squashed them as good as possible and added mine on top, see https://github.com/Montellese/peripheral.joystick/commits/joystick_poweroff in case you are interested in the squashed commits. And https://github.com/Montellese/xbmc/commits/joystick_shutdown has my commits on top of this PR.

@garbear
Copy link
Member Author

garbear commented Apr 22, 2016

@Hedda I'm wary of settings creep, but I can see your point, it might be annoying if all 4 controllers rumble every time a notification is shown. What if we drop the commit and only rumble in games? I added rumble on notifications as more of a proof of concept anyway.

@Montellese you're right, the rumble command should go somewhere else in case multiple notifications are queued. Thanks for the power off support, should I tack that on and make this a dual-feature PR?

@Montellese
Copy link
Member

You can include it here or I can provide it as a separate PR once this has gone in, I don't really care.

@garbear
Copy link
Member Author

garbear commented Apr 23, 2016

OK, I'll save you the trouble and add it here.

So what's the consensus? Do we drop rumble-on-notification for now? Once rumble functionality is in it'll be trivial to add it later. Also, rumble can be made less annoying by shortening the duration or lowering the intensity (motors support values from 0.0 to 1.0)

@garbear garbear changed the title Haptics Haptics and Controller power-off Apr 23, 2016
@Montellese
Copy link
Member

I'd either make the rumble on notifications configurable or remove it completely.

@garbear garbear force-pushed the rumble branch 2 times, most recently from f38d1b7 to 6ca89b4 Compare April 23, 2016 19:32
{
CPeripheralJoystick* joystick = static_cast<CPeripheralJoystick*>(peripheral.second);
if (joystick->SupportsPowerOff())
PowerOffJoystick(joystick->RequestedPort());

This comment was marked as spam.

@garbear
Copy link
Member Author

garbear commented Apr 23, 2016

I'll add a setting. The input page in system settings is getting a little crowded, but this is fixed in my RetroPlayer work - all controller settings are moved to a "game input" page

@@ -15568,7 +15568,19 @@ msgctxt "#35050"
msgid "Controller profiles"
msgstr ""

#empty strings from id 35051 to 35054
#. Setting to test rumble

This comment was marked as spam.

@garbear
Copy link
Member Author

garbear commented Jul 3, 2016

jenkins build this please (to test if I rebased on alpha 2 properly)

@MartijnKaijser
Copy link
Member

@garbear nudge

@garbear
Copy link
Member Author

garbear commented Aug 5, 2016

The major thing stopping this was a crash if you disconnected the controller shortly after connecting it. My wireless 360 controller had low batteries, and when it connected Kodi told it to rumble and the rumble drew too much power and killed the controller and crashed Kodi :)

Fixing this will require a bit of refactoring. I rebased this patch on master last night, I'll try to look into the crash soon.

@MartijnKaijser
Copy link
Member

@garbear well v16 crashed when even a controller was connected.
Also you can now drop the visual Studio files as windows now uses cmake

@garbear
Copy link
Member Author

garbear commented Aug 6, 2016

Rebased on master. I'll look into the crash

@garbear
Copy link
Member Author

garbear commented Aug 8, 2016

Fixed the crash. jenkins build this please

@garbear
Copy link
Member Author

garbear commented Aug 9, 2016

@MartijnKaijser ok to merge?

@MartijnKaijser MartijnKaijser merged commit 27c22fe into xbmc:master Aug 10, 2016
@MartijnKaijser MartijnKaijser added this to the Krypton 17.0-beta1 milestone Aug 10, 2016
@garbear garbear deleted the rumble branch August 13, 2016 19:32
@nille02
Copy link

nille02 commented Oct 13, 2016

Is there a way to disable the rumble? its horrible for each notification. The Pad lies on a table and its extreme loud when it rumbles on max speed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Input Type: Feature non-breaking change which adds functionality v17 Krypton
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants