Rockbox

  • Status Unconfirmed
  • Percent Complete
    0%
  • Task Type Bugs
  • Category Drivers
  • Assigned To No-one
  • Operating System Another
  • Severity Low
  • Priority Very Low
  • Reported Version Release 3.7.1
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by BenjaminBrown - 2011-01-24

FS#11903 - Android does not handle keypresses correctly - potential fix but I can't java

I've hunted this down here is what I found out.

from
http://developer.android.com/reference/android/view/KeyEvent.Callback.html


public abstract boolean onKeyDown (int keyCode, KeyEvent event)
Since: API Level 1

Called when a key down event has occurred. If you return true, you can first call KeyEvent.startTracking() to have the framework track the event through its onKeyUp(int, KeyEvent) and also call your onKeyLongPress(int, KeyEvent) if it occurs.


We need to return true and call KeyEvent.startTracking() from /android/src/org/rockbox/RockboxFramebuffer.java then set the interrupt flag from /firmware/target/hosted/button-android.c and find out what is passed back after a key release and handle that. I may not be a great programmer but this looks like the ticket to removing the BUTTON_REL work around from /firmware/target/hosted/android/app/button-application.c

Here is what I have so far it does not work it needs eyes with more experience in java.

  public boolean onKeyDown(int keyCode, KeyEvent event)
  {
      event.startTracking();
      buttonHandler(keyCode, true);
      return true;
  }
I'm still a dork,
this still does nothing except compile what am I missiing?

Ok still not sure about event.startTracking() this latest patch does not use it.

This patch fixes up the logic handler for key events in firmware/target/hosted/android/button-android.c and removes an unneeded function as well as removes the |BUTTON_REL work around from firmware/target/hosted/android/app/button-application.c

My kasiar's DPAD is behaving nicely with the patch and even gets long press repeats through to the button handlers.
I need to know if it effected any other button functions on other devices like ones with scroll wheels and navigation balls, before its ready for commit.

I found a bug key presses are being eaten by the logic some how has something to do with last_btns

Here is the fix

Or would

  unsigned button = 0;
  unsigned last_btns = 0;

be better?

Nevermind unsigned doesn't work any way

another bug still not getting the first keydown event

Before today I had no interest in learning java and I still don't.
Good news is that this bug is crushed.

Dang you guys must hate me

this.requestFocus(); was missing from the constructor
from http://www3.ntu.edu.sg/home/ehchua/programming/android/Android_2D.html

I know most of my code looks like a barrel full of monkey sh!t typed by a blind person with split personalities, but this one is damn nice
but still a ways to go. I could really use a hand on this java.
tested on my side scroll wheel and dpad.

todo:
there are no long key press events
some times two key presses are need to change direction.

android 1.5 compatible its just buggy,

Much better this time. I need some one with optical and ball scrolling androids to test this and see if I'm on to something good.

patches:
BUG_MISSING_BETTER_button-android.c.patch
BUG_MISSING_RockboxFramebuffer.java.patch
BUG_MISSING_fixed.button-application.c.patch

Your patch looks somewhat strange:

      button = key_to_button(keycode);
      if (!button)
      {   
          button = key_to_button(keycode);
      }

I still think the queue_post() call you add is wrong. Also please note that multimedia buttons need some special handling.

Thanks Thomas
This one works much better.
I did notice that when entering the plugins menu two different key presses were needed to "wake up"? the buttons. last key presses are stored some where I guess. I removed last_btns from this patch is that needed some where else?
I did not have this issue on the settings menu I don't think it is a bug in this code but one in the "list plugins funtion" what ever file that may be in, but of course I'm always wrong just ask my wife.

You know if someone has input code they want tested my tilt has:
A. has a scroll wheel you know for doing scrollie stuff
B. has touch screen for that all important touch screen interface and
C. every freakin button imaginable! It has to many really.
let me count 1 2 3 oh you know what just pull up android-event.h and I think every thing less than 84 or less just let me know.

the BUTTON_REL work around was supposed to be remove so here sorry test it now.

nope not working

I can live with the work around. It doesn't effect the behavior as far as I can tell, holding the button results in key press being sent one after another. and pressing the button once results in one button event So it plays nice.
I did try

if (!state)

   queue_post(&button_queue, "BUTTON_REL", 0);
   button = !button;
   return true; 

which, I thought, should have worked and I tried |BUTTON_REL as well with no quotes.
Unless someone knows why, I can't get rid of |BUTTON_REL from button-application.c.

So as it is right now these patches work but still contain the original work around. tested with my dpad and scroll wheel.

button-android.c
works better with last_btns rather than queue_post() hmm now I can remove |BUTTON_REL

this patch
http://www.rockbox.org/tracker/task/11903?getfile=23212 and the following work best so far and without the work around.

Latest without queue_post() in !state.

dang it
be right back

synced again r29143

nope

BUTTON_UNKNOWN must remain 0 for the back button to work properly in other words BUTTON_NONE cannot be false.

unless I set the value of button as 0 instead of !button to reset the run, this is caused by button = !button
either way it does the same thing and I feel changing the values of android-keyevents.h to be wrong if this the way it was downloaded.

feel free to test sorry about all the trouble I can cause :)

the volume up/down is where my scroll wheel lives.
Do any other android phones have volume up/down that is not mapped to the left/right soft keys?
I don't really care cause I never want to look at java again but I will :)

never mind this is junk back to square 1 1/2 a few minutes in of pressing buttons and my initial presses disappear and I'm left with button releases only…. sigh.

It's not so much a bug anymore. More like the ugly stray dog that smells bad, and keeps coming back no matter how many miles you drive to dump it in some one's field, behind their barn, late at ight.
So I decided to keep it, as it is now my pet.

this patch is IT hopefully :)

I did not need to add requestFocus() to the constructor.
It works without it, I'm sure you don't want extra cruft in there so I left it out. cheers.

Oh and I tested it with my scroll wheel for me it works beautifully.

I was missing button releases from !state plugins act sane now

I was getting two back presses for every one press. This one feels good Thomas! let me know.

Just have a look at a generic android phone (like nexus one); e.g. they have volume buttons on the side and not at all on the front.

Designing this patch according to your hacked phone is not the way to go I'm afraid.

Oh no the volume keys were for me to test the scroll not in the patch mate sorry

When you have time kugel I know your busy but if you want me to put it all back and just try adding
queue_post(&button_queue, button|BUTTON_REL, 0);
Instead of trying to rewrite the logic that was there before, I understand and I will. but it was hard to follow.
that last patch really works great trust me :) no bull
I know you hate looking at my ugly monkey shit code but it's the first time I programmed since I wrote a 3dcube like program way way way back in the day, and java is so new to me its not even funny. Just look at the first post ^
Anyway as always it's your call of course.

with formating help

I suck I'll fix after I wake up

Im gonna try one last his java damn

opps I meant in java

I found most of this at http://blog.7touchgroup.com/tag/keyevent-starttracking/ since action.c locks waiting for release events a nicer way to track them must be necessary I'm gonna try this instead of just onKeyUp or onKeyDown

  public boolean dispatchKeyEvent(KeyEvent event)
  {
      // Tell the framework to start tracking this event.
      getKeyDispatcherState().startTracking(event, this);
             if (event.getAction() == KeyEvent.ACTION_DOWN && event.getRepeatCount() == 0)
              {
                  getKeyDispatcherState().handleDownEvent(event);
                  keyCode = event.getKeyCode()
                  return buttonHandler(keyCode, true);
              }
              else if (event.getAction() == KeyEvent.ACTION_UP && event.getRepeatCount() == 0)
              {
                  getKeyDispatcherState().handleUpEvent(event);
                  keyCode = event.getKeyCode()
                  return buttonHandler(keyCode, false);
              }
       return super.dispatchKeyEvent(event);
  }

As you have witnessed I can find no way to code around this bug in c. The silly bug would get pushed to some other part of the code.
I'll submit another patch only after I get the hang of java and write the right code to get this done or if I write a patch to split the Aaap port into dpad and track ball android targets. cheers

I'm actually not sure if there's a bug. Obviously your device vastly differs from all other android devices.

public boolean onKeyDown(int keyCode, KeyEvent event)

  {
      return buttonHandler(keyCode, true);
  }

Doesn't this return immediately after being called and get called again directly afterwards, if the button event is still active. So it sends rapid key presses, can a "do while loop" work sort of better. I see where you coded around this, but it doesn't make sense to do this if we can make java send one keydown event then wait until the key is released, and then send one keyup event. This is maybe not a real "bug" but if I can find away to handle this better I'll jump on in.

http://pastebin.com/g4BecsAq none working example of my idea.

lol dang it back here again! lol

If I do ever find the right flag to watch thats 1.5 compatible. I'll come back to this, but all this recursion is making my head spin. moving on.

Just a note what I think when you say all other android devices.
http://forum.xda-developers.com/ sorry just my way of thinking about it, no offense intended mate.

I don't understand.

Sorry that was an incomplete thought, big surprise huh? I'm trying to quit smoking but these e-cigarettes are not working!
What I meant, was that since android is open source, there will be some one else who comes a long the way, with another legacy device running android.
And is going to come in here and do the same thing I did. Except they probably won't read the wiki let alone the source code, and they definitely would never consider a patch.
So until rockbox is android 2.0. How about a legacy option for those poor souls with legacy devices?
I'll send a patch sometime. ;)
Now time to put android on the tilt II and see what happens when I run rockbox.

But I do say, you must have a vividly twisted mind to come up that one.

One last thought, rockbox Aaapp is android 1.5 compatible because we want to support a wide range of handsets. The button handling code side steps around an interrupt that will not be interrupted, making the use of buttons almost useless on devices that have them.
I know the "why" it's coded this way, maybe not the "how" did you come up with that, but it is brilliant only looking for button releases.
The startTracking() call is api 2.0 and up, but handles the interrupt line differently than the existing code. Making it hard for me to just drop it in and walk away. If we used starttracking it would mostly insure rockbox worked smoothly on any device as long as it was 2.0 compliant.
So by excluding phones below 2.0 we can actually open up the door to their hardware as long as they can upgrade to 2.0 and up. Actually 2.3 runs much faster.

Here is what I'll use on my Kasiar, as far as I know it should work just fine for other devices as well, but raises the API level to 2.0.

Warning for those who travel this way, DO NOT open the box Thomas Martitz so kindly wrapped around the most hideous demon one could conceive of, and just use this last patch if you can.

This came to me in my sleep. Now it does the same thing at API 1.0 :)

How does this one feel on a track/optical android? My wheel is worn out so I can't tell, it never really works reliably.

opp let me fix that be right back

There now, all better now, Kugel my friend, you have one convoluted mind.

If you have a sliding keyboard you can test why this patch is important by compiling pac-man without this patch and playing pac-man with it but you will need keymap to do that see FS#11898 the last patch.

pacbox*

To my surprise the cycles we get back by not calling button_handler every tick from the frame buffer, and has removed most of my complaints from Aaapp.

It taste good, trust me people. test it, try it but don't try to understand it.

Kugel's problem was that, onKeyDown and onKeyUP are treated differently by android.
If onKeyDown is evaluated true button_handler is called and the keypress is sent to rockbox for processing. On the next tick if onKeyDown is still true another keypress is sent. thus multiple key presses are sent.
But if an onKeyup event is detected button_handler is called only once. This is what Kugel looked for and used evil evil code to go back in time a see which key was pressed before hand, dragging down the frame buffer while it did that.
My way fixes the assumption that only one onKeyDown event is sent and stops sending them after the first one. I maybe foolsh but I'm not stupid. Commit this, it is good code kugel, I stepped through the registers on my white board to figure it out.

The int wasKeypressed in RockboxFrambuffer.java, I feel should remain an int and not bol. Have you looked in the box of demons? What if they get out?

It is intended (and correct) that down is reported multiple times while up is only reported once. This way the generic button code detects repeat and release events.

No repeat events should be handled by leaving the keycode in last_btns and letting rockbox take care of it.

If your way is correct WHY? did you work around it in button-application.c?
Test it bud, it works better this way.

Listen your way was a valiant ingenious way to work around what data you had, but in the end it was to much. You over thought the problem. Listen Big Dog, I do not want your spot on the porch, I want Aaapp to run correctly and optimally for any Android device, thats all I want.

If you cannot understand why mine works, don't worry I can see how your's works. But I do see how mine works correctly and optimally…..let me have this one kugel…. think of all the other optimizations I could find…. I want to make Aaapp lord over all other apps for android, It could be awesome.

don't worry I can't* see how your's works

I said this thing was satanic utterly demonic indeed, stupid widgets are written in java I'm most certainly fuXt

maybe I can put/move the function to the other side of the equation but do I really want to open the box again? here we go again…

Here's a patch… again…. this time I touched kugels logic only lightly and add a proper interrupt signal. This should work with ALL android devices but I can not get the widgets to talk to rockbox this way. Please help me.

The patching spree continues.. I don't know why but removing the interrupt had no effect on function? but I did use my new logic in button-handler. Either way the widgets half way work now, they send an intent the first time, then you have to tap a different button to get another intent passed, this is the last hurdle I hope.

Ok I have tested this thing into the ground!
It's out of my hands until some body tests it on a track pad/ball.

What this patch does:
rewrites the input code (sorry kugel) in order to remove a work around.

fixes:
widgets work
buttons work for plugins again
frambuffer seems faster
It "should" work on every android out there - needs testers
It is 1.5 API compliant

Have you ever looked at firmware/drivers/button.c? It handles BUTTON_REL on it's own. I'm not sure what you do about the queue_post() calls; you normally shouldn't do that at all since it's done by the generic code (multimedia buttons are an exception).

all good points I'll check, but have tested it? :)

I couldn't get the widgets to work right with out the BUTTON_REL I tried taking out everything I did not need. You would be surprised how often that helps.

Widgets work in SVN without some extra BUTTON_REL.

nope the extra BUTTON_REL in svn is located in button-application…. test it silly

Only for the DPAD buttons (that's the infamous workaround), but not all other buttons (touchscreen, menu, back, …)

Test it kugel, it's a rewrite it is not going to look the same.
It will act the same for track pads/balls, but fix the problem with dpads.
Its ready buddy trust me.
please guy, trust the sage old programmer who had to learn java, to get rockbox working on his android ;) please

Thanks guy
I'm Sorry
Truce?
works nice over here now, how about you?

Flyspray is not a chat channel.

Last time I hope :)

hoping wasn't good enough last one….. (O_o)….maybe

hmm?

There is still some dog food in some plugins? maybe? on exit.
It might not be from this patch I'll dig in tomorrow.. again, and see what I dig up.

this fixes that… maybe I haven't tested it yet

nah forget this last one

Context change is having trouble when exiting from a plugin, it sends an extra button release, at least I think I'm sure. You can not see it, until you compile the plugins, though. Thomas can you think of any other button releases that I don't need in action.c?

Let me explain what I think happened with the old input method, while my head is clear this morning.
First, it is supposed to be a virtual hardware construct, so that the virtual hardware looks like real hardware.
In real hardware you cannot split a one bit interrupt into two or more path ways.
By splicing the interrupt, in button-application, it basically hooked the output line, from the gpio, directly up to three individual virtual chips, sort of.
The media, key, and dpad buttons can all go in one matrix, and should. That is how real "non-virtual" hardware operates.
This caused most of the trouble.

Exactly, well observed. But the multimedia keys need to be handled separately (they just work differently throughout rockbox, they're not normal buttons), and the dpad buttons currently have a work around due to the (normally) trackball behavior.

How is the track pad/ball behavior anyways? If you tested it. ;) maybe it can be changed to a motion event.
I'll pull multimedia_to_button out to the side and see what I can bring back.

dispatchTrackballEvent(MotionEvent event) API 1.0
from
http://developer.android.com/reference/android/view/MotionEvent.html look just right

Maybe just grouping the keys/dpad in under multimedia_keys (button-application) would work since (dpad/keys) are only called from button-handler.
That leaves multimedia_keys exposed to rockbox. And put in a dispatchTrackballEvent and see if works for track pads too, not just balls.

http://developer.android.com/reference/android/text/method/ScrollingMovementMethod.html

Ok this has one interrupt, with multimedia keys on the side.

I duplicated onTouchEvent for onTrackballEvent, but still pass it off to touchhandler. This is wrong, it needs it's own handler, but I don't have this hardware so I can't really see what happens.

gah! Pulling multimedia keys out to the side breaks the widgets. Thomas why is the multimedia key so different than regular keys? Can I call it from some where else besides the widget, to test if functionality changes by handling them from queue_post?

  public boolean onTrackballEvent(MotionEvent me)
  {
      switch (me.getAction())
      {
      case MotionEvent.ACTION_CANCEL:
      case MotionEvent.ACTION_MOVE:
      }
      return false;
  }

might work better

multimedia keys are handled differently, they're global events and don't go through the action system for example. This is so they work regardless of the current context.

Oh I see now,
If I assign one of the many buttons on this phone to lets say, BUTTON_MULTIMEDIA_PREV it should change to the previous track, no matter what is happening in rockbox. Like if I was changing something on the lcd setting screen and pressed it.
I'm gonna try this if it works does that mean it auto-magically fixed itself? (O_o) that…. would be awesome

Or I can remap one to KEYCODE_MEDIA_PREVIOUS in android and get a better test subject.

http://developer.motorola.com/docstools/library/Basics_of_Event_Management/

This is It! this page talks about the problem at detail Thomas I found it!

Dang I wish I knew java like you Thomas this would be gone in five or ten minutes. I'll just start pecking at it till I understand it.

(Trackball Event Handler

Trackball events are handled by the onTrackBallEvent() callback. If you don’t add this callback to your application, trackball events fall through to key press events. You can use the onKeyUp() or onKeyDown() event handler callbacks, or implement the OnKeyListenerand onKey() listener callback to intercept trackball motion events.)

and this other bit

(On trackballs, the pointer coordinates specify relative movements as X/Y deltas. A trackball gesture consists of a sequence of movements described by motion events with ACTION_MOVE interspersed with occasional ACTION_DOWN or ACTION_UP motion events when the trackball button is pressed or released)

when the deltas add up to 1 you get a button press!! missing BUTTON_REL!!! do you see it yet?

sorry I don't make much sense :(
I hope you see it, if not I will try harder to …..say……things

Let me apologize,
Things go in my head real easy
—→(o,o)——-?
but they don't comeback out so clearly.
but it's in there you can find it.
I need sleep so, tomorrow then.

To be more precise,
The RockboxFramebuffer view is not receiving scroll events, because

(Trackball events are handled by the onTrackBallEvent() callback. If you don’t add this callback to your application, trackball events fall through to key press events. You can use the onKeyUp() or onKeyDown() event handler callbacks, or implement the OnKeyListenerand onKey() listener callback to intercept trackball motion events.)

But we did not see those events because

(Event Listeners are must more specific. They “listen” for an event generated on a View or ViewGroup. You must register the listener for a View or Viewgroup with the appropriate setOnxxxListener(). The event is then “triggered” when one of the input types is activated. The callback that is launched depends on which View or ViewGroup has focus.)

we know how it supposed to work

(On trackballs, the pointer coordinates specify relative movements as X/Y deltas. A trackball gesture consists of a sequence of movements described by motion events with ACTION_MOVE interspersed with occasional ACTION_DOWN or ACTION_UP motion events when the trackball button is pressed or released)

But they look like they are missing BUTTON_REL lastly because

(The relative movement of the trackball since the last event can be retrieve with MotionEvent.getX() and MotionEvent.getY(). These are normalized so that a movement of 1 corresponds to the user pressing one DPAD key (so they will often be fractional values, representing the more fine-grained movement information available from a trackball))

and we get only one dpad event for every delta = 1

These two links:
http://developer.motorola.com/docstools/library/Basics_of_Event_Management/ http://developer.android.com/reference/android/view/View.html#onTrackballEvent%28android.view.MotionEvent%29

svn checkout http://scidonthego.googlecode.com/svn/trunk/ scidonthego-read-only

Ah this code implements setOnTrackballListener. It should be easy peasy now

Some progress, I implemented a simple trap to test the functionality, I just need to fill in the blanks….. I hope.

dang it!

Ugg, the override reports false positives. It's back to deciphering the views and view groups.
I did notice the view is removed and reattached to the framebuffer, at RockboxActivity.java:110-114
Seems like the source of the trouble.

It looks like trying to add setOnTrackballListener() and registering the view/viewgroup in the manifest, is the last effort available. Before trying to hurt myself coming up with another layer of work around.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing