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

FR: Add rumble support #5374

Closed
Frenzie opened this issue Sep 13, 2019 · 8 comments · Fixed by #5454
Closed

FR: Add rumble support #5374

Frenzie opened this issue Sep 13, 2019 · 8 comments · Fixed by #5454

Comments

@Frenzie
Copy link
Member

Frenzie commented Sep 13, 2019

Frenzie added a commit to Frenzie/koreader-base that referenced this issue Sep 14, 2019
Frenzie added a commit to Frenzie/koreader-base that referenced this issue Sep 14, 2019
Frenzie added a commit to koreader/koreader-base that referenced this issue Sep 14, 2019
Frenzie added a commit to Frenzie/koreader that referenced this issue Sep 15, 2019
@Frenzie Frenzie added this to the 2019.10 milestone Sep 15, 2019
@pazos
Copy link
Member

pazos commented Sep 15, 2019

@Frenzie: I do like the idea but after a quick search I think the implementation on android could have some problems:

  1. differences between api levels
  2. overhead of the JNI calls (about 1ms)
  3. Is the call blocking the program until the vibration stops? In that case vibrate on keyboard taps will become impossible.

Also I would like to finish koreader/android-luajit-launcher#183 first and these days I'm very busy trying to find time for both my daily job and my family.

@Frenzie
Copy link
Member Author

Frenzie commented Sep 16, 2019

Is the call blocking the program until the vibration stops?

Certainly not in SDL. If it's blocking in Android then right now I'd be inclined to think of that as an issue for the device implementation to deal with.

Also I would like to finish koreader/android-luajit-launcher#183 first and these days I'm very busy trying to find time for both my daily job and my family.

I'm just asking for some feedback on the device abstraction logic. ;-) Job & family take precedence of course!

Frenzie added a commit to Frenzie/koreader that referenced this issue Sep 27, 2019
Frenzie added a commit that referenced this issue Sep 29, 2019
@pazos
Copy link
Member

pazos commented Sep 30, 2019

@Frenzie: I did a first implementation in koreader/android-luajit-launcher#190

We're fine running vibration in the Ui Thread so the call doesn't block and works great on the keyboard. But needs a lot of testing and tweaking because default intensity values are far from perfect (at least in 8.0+)

I'm currently testing with this changes on the frontend:

diff --git a/frontend/device/android/device.lua b/frontend/device/android/device.lua
index 4deee2c1..b194da09 100644
--- a/frontend/device/android/device.lua
+++ b/frontend/device/android/device.lua
@@ -70,6 +70,7 @@ local Device = Generic:new{
     canRestart = no,
     firmware_rev = android.app.activity.sdkVersion,
     display_dpi = android.lib.AConfiguration_getDensity(android.app.config),
+    isHapticFeedbackEnabled = function() return android.canVibrate() end,
     hasClipboard = yes,
     hasOTAUpdates = canUpdateApk,
     canOpenLink = yes,
@@ -225,6 +226,15 @@ function Device:initNetworkManager(NetworkMgr)
     end
 end
 
+function Device:performHapticFeedback(type)
+    local short, long = 80, 160 -- milliseconds
+    if type == "CONTEXT_CLICK" or type == "LONG_PRESS" then
+        android.vibrate(long)
+    elseif type == "KEYBOARD_TAP" then
+        android.vibrate(short)
+    end
+end
+
 function Device:retrieveNetworkInfo()
     local ssid, ip, gw = android.getNetworkInfo()
     if ip == "0" or gw == "0" then

@pazos pazos added the Android label Sep 30, 2019
@poire-z
Copy link
Contributor

poire-z commented Sep 30, 2019

If it comes to Android, it would be nice if it was toggable via a menu checkbox.

@pazos
Copy link
Member

pazos commented Sep 30, 2019

If it comes to Android, it would be nice if it was toggable via a menu checkbox.

Sure, I'm the first that want vibration disabled. But I'm starting to change my mind about the keyboard feedback because it can be really nice.

In any case android.prop.vibration is a boolean referencing if the current device has vibration capabilities. We can disable it at launch just by android.prop.vibration = false and won't hurt anymore.

Having other things (duration, intensity) modificable via UI would be nice too.

@Frenzie
Copy link
Member Author

Frenzie commented Sep 30, 2019

it would be nice if it was toggable via a menu checkbox.

@poire-z @pazos That's an advantage I neglected to mention in koreader/android-luajit-launcher#190 (comment)

Because then it is, in system settings.

@pazos
Copy link
Member

pazos commented Sep 30, 2019

Because then it is, in system settings.

That is awesome 👍 Also the implementation is really simple.

New PR in koreader/android-luajit-launcher#192

Works with the following change in the frontend:

diff --git a/frontend/device/android/device.lua b/frontend/device/android/device.lua
index 4deee2c1..885731c6 100644
--- a/frontend/device/android/device.lua
+++ b/frontend/device/android/device.lua
@@ -70,6 +70,7 @@ local Device = Generic:new{
     canRestart = no,
     firmware_rev = android.app.activity.sdkVersion,
     display_dpi = android.lib.AConfiguration_getDensity(android.app.config),
+    isHapticFeedbackEnabled = yes,
     hasClipboard = yes,
     hasOTAUpdates = canUpdateApk,
     canOpenLink = yes,
@@ -225,6 +226,16 @@ function Device:initNetworkManager(NetworkMgr)
     end
 end
 
+function Device:performHapticFeedback(type)
+    if type == "CONTEXT_CLICK" then
+        android.hapticFeedback(C.AHAPTIC_CONTEXT_CLICK)
+    elseif type == "LONG_PRESS" then
+        android.hapticFeedback(C.AHAPTIC_LONG_PRESS)
+    elseif type == "KEYBOARD_TAP" then
+        android.hapticFeedback(C.AHAPTIC_KEYBOARD_TAP)
+    end
+end
+
 function Device:retrieveNetworkInfo()
     local ssid, ip, gw = android.getNetworkInfo()
     if ip == "0" or gw == "0" then

@Frenzie
Copy link
Member Author

Frenzie commented Oct 1, 2019

Given that I named them after the Android constants because they seemed sensible enough, you should be able to just use C["AHAPTIC_"..type].

mwoz123 pushed a commit to mwoz123/koreader that referenced this issue Mar 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants