Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Drivers
  • Assigned To No-one
  • Operating System Another
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by csavery - 2010-05-18
Last edited by kugel. - 2010-05-20

FS#11288 - Sansa Fuze v2 scrollwheel lockup after 20 minutes - fix

Here's a patch to test for the scroll wheel lockup bug.
It appears to be working fine for me but I'm not completely sure about wheel acceleration and any subtle behaviours. It does work for me after about 30 minutes.
Tested against r26214.
Please feel free to test and let me know if something is wrong.
:)

Closed by  kugel.
2010-05-20 15:42
Reason for closing:  Fixed
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

r26175, also see comments

Oops. Incorrect patch info but I can't edit…

I can't test it now but I have remarks:

The loop interval is changed from 3ms (+/- 1ms) to 10ms (+/- 10ms), doesn't it change the wheel behaviour too much ?

Also when giving the intervals you assume HZ=100.
While it's unlikely to change, it's better if you do something like (30*HZ) / 1000 for 30ms

Thanks for fixing the patch info.
I didn't notice any difference in the feel of the scrolling but I did notice that wheel acceleration was ineffective.
I think this is due to the low resolution of the HZ timer which is made worse by the inversion into a velocity value.
I was modifying the define values as suggested when it occurred to me of a better solution that keeps the high res kernel timer.
I'm just testing this method now and will post shortly if it works.
It should maintain the res needed for useful acceleration, and would be a much simpler change.

btw that's a mistake up above, actual test build was r26114. I seem to be all butter fingers today.

Ok. Here's a new patch.
Wheel feel and acceleration both work the same and it didn't lock up after 30 minutes.
This one makes a very simple change using the current hi-res timer.
Normally we compare the time difference "v" with WHEEL_LOOP_INTERVAL but now if it is less than zero I assume a wrap has occurred and I re-sync the last_wheel_post time before returning. This throws away the first wheel click if after about 23 minutes but I think we can live with that as the following clicks are back in sync and get responded to.
Let me know if any problems.

I think it's not the optimal solution but it should be good enough, so it could be committed if you resync it.

It looks like in current svn the WHEEL_LOOP_INTERVAL test has been removed.
This makes this patch no longer relevant.

Wasn't the problem that "long time = TIMER2_VALUE + current_tick*TIMER_TICK; /* to timer unit */" overflows? I think that problem still applies.

Yes, but the overflow only affects one "click" because the last_time is sync'd. It was the test and return that caused the lockup since no matter how many "clicks" followed the overflow they would all cause a return, until the overflow value once again became positive. Without the test condition and return no lockup occurs any more.

I don't think the test condition for WHEEL_LOOP_INTERVAL really did much because in my reading of the code it seems that hardware polling for the wheel occurred at 200Hz. This meant that the minimum interval between possible clicks was 5ms. Since the test condition detected and removed clicks below 3ms I think it would have made very little difference to how the wheel ultimately behaved. Removing the test condition is likely just as good as my fix which prevented it from locking on negative values (overflows).

I sync'd my code to r26185 today and didn't notice any change in wheel use but I didn't try a 30 minute test because this code is now reverted to r26059, before the test condition was inserted, and that code worked ok.

OK, I'll close this one then, you explained that we don't need to worry about the overflow. Is that OK?

On the fuzev2, there's no polling. Each wheel change (4 per physical click) generates an interrupt, i.e. the function is run. The fuzev1 wheel is read by polling.

Ok with me. I believe this to be fine now that the code was changed back.
I probably got mixed up in the code regarding polling. It's still very new to me.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing