FS#11172 - Fuzev2: Read the scrollwheel scrollwheel via IRQ

Attached to Project: Rockbox
Opened by Thomas Martitz (kugel.) - Sunday, 04 April 2010, 13:07 GMT
Last edited by Thomas Martitz (kugel.) - Tuesday, 27 April 2010, 10:13 GMT
Task Type Patches
Category Drivers
Status Closed
Assigned To No-one
Operating System Another
Severity Low
Priority Normal
Reported Version Release 3.4
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


With 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, 10:13 GMT
Reason for closing:  Accepted
Additional comments about closing:  r25736
Comment by Rafaël Carré (funman) - Sunday, 04 April 2010, 18:03 GMT
SOURCES has a missing #ifdef HAVE_SCROLLWHEEL

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.
Comment by Thomas Martitz (kugel.) - Sunday, 04 April 2010, 18:17 GMT
We could also mask the interrupt once we detect the hold button so we wouldn't need the hold_button check at all.

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.
Comment by Rafaël Carré (funman) - Sunday, 04 April 2010, 22:18 GMT
I think it's both: I can see scroll through the main menu (8 items) and the last move will skip over 4 items: early acceleration and fast final speed
Comment by Thomas Martitz (kugel.) - Monday, 05 April 2010, 21:36 GMT
Fixed some problems after recent svn commits.

I find the current behavior of the patch nice. If someone disagrees speak up.
Comment by Rafaël Carré (funman) - Tuesday, 06 April 2010, 09:55 GMT
It's better than with first patch: wheel accelerates but not too much
Comment by Thomas Martitz (kugel.) - Tuesday, 06 April 2010, 10:04 GMT
Re: comparing with fuzev1. The fuzev1 wheel behavior is exactly same same as svn fuzev2 (maybe a tad bit more fluent due to higher fps).

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.