This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#11172 - Fuzev2: Read the scrollwheel scrollwheel via IRQ
Attached to Project:
Rockbox
Opened by Thomas Martitz (kugel.) - Sunday, 04 April 2010, 15:07 GMT+2
Last edited by Thomas Martitz (kugel.) - Tuesday, 27 April 2010, 12:13 GMT+2
Opened by Thomas Martitz (kugel.) - Sunday, 04 April 2010, 15:07 GMT+2
Last edited by Thomas Martitz (kugel.) - Tuesday, 27 April 2010, 12:13 GMT+2
|
DetailsWith this patch the wheel is read via IRQ (like on the e200v1). It feels a tad bit better than the polling solution from the fuzev1 IMO (the difference is smaller than I expected though).
I'm not entirely happy with the coding style, but it's difficult since the isr for GPIOA must be shared with the sd driver for microsd monitoring. Please give it a try, scrollwheel acceleration has lots of parameters which can be used to tweak it. Also comment on my solution to share the isr. |
This task depends upon
Closed by Thomas Martitz (kugel.)
Tuesday, 27 April 2010, 12:13 GMT+2
Reason for closing: Accepted
Additional comments about closing: r25736
Tuesday, 27 April 2010, 12:13 GMT+2
Reason for closing: Accepted
Additional comments about closing: r25736
I think the VIC_INT_ENABLE = INTERRUPT_GPIOA should be moved in system_init() after setup_vic() : setup_vic() only setups the VIC, not individual interrupts
Also I don't understand the need for EXT_SD_BITS: it's not clear to me, leaving it hardcoded to 1<<2 is fine IMO
You could replace "irq_handler" by "isr" in the function names, it's a bit shorter
Else sharing the 2 ISR is fine, perhaps put the function prototypes in system-target.h ?
I replaced get_wheel_bits() by one load which reads pins 7 and 6 at the same time : seems to work fine.
Also I reduced the type of scrollwheel acceleration to what it was (2 -> 1, not sure what the real effect is in drivers/button.c)
Also removed if (hold_button) { big block } and instead:
if (hold_button) return;
So the big block can be less indented and easier to read.
wheel value isn't saved with my diff, but I guess it doesn't matter since we don't read the wheel the hold switch is set ?
Same for btn == BUTTON_NONE (but here I save the current value to old value)
It works fine without acceleration but with it it seems too fast.
I can not compare very good with my Fuzev1 since the scrollwheel is physically broken : it still works but it's not as smooth as when it was new, the wheel sometimes get stuck.
As the comment states, I copied heavily from the e200v1 and didn't dare to optimize too much. But once that's done I'll happily backport :)
The scrollwheel acceleration type was intended at 2. It's results in faster acceleration (as the comment explains) which fits me but if it's too fast for others I can live with 1. Eventually it should be a user setting, non-scrollwheel targets have a similar setting.
We can revert the EXT_SD_BITS changes, I did that at first because I had in my mind that it's gonna be needed but it turned out it wasn't (although I like the #define approach more in general, f.e. it took me a while to figure out what the 1000000 means in the e200v1 scrollwheel function).
- "It works fine without acceleration but with it it seems too fast." We can probably tweak that a bit, but it fits me well. Do you think the acceleration kicks in too early or is too fast in general? The fuzev1 acceleration gets insane quickly but has a very nice transition which I didn't quite manage to achieve I think.
I find the current behavior of the patch nice. If someone disagrees speak up.
It's a matter of getting used to it. I spend a lot of time making the fuzev1 work well with polling, and now I'm used to it. But I remember the switch from e200v1 was hard because the e200v1 uses the algorithm I'm trying to implement here and I was used to it. And once you get the transition between slow mode and acceleration mode right, the irq algorithm is much more intuitive to use.