Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10284 - Sansa e200v2 & Fuze: Merge the button driver into 1 file

Attached to Project: Rockbox
Opened by Dustin Skoracki (sko) - Saturday, 06 June 2009, 13:24 GMT+2
Last edited by Thomas Martitz (kugel.) - Tuesday, 23 June 2009, 00:26 GMT+2
Task Type Patches
Category Drivers
Status Closed
Assigned To No-one
Player Type Sansa e200
Severity Low
Priority Normal
Reported Version Version 3.2
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

This patch renames the variable for wheel acceleration and changes some comments to make it more equal to the fuze's button driver. I also implements the button_delay() function from the fuze driver.
The aim of this patch is to make it easier to read the source of both drivers (why call the same things by two names ;) ) and maybe unify them to one driver later.
I tested it with  FS#10048  and with clean svn and I couldn't find any problems, works like before.
   button_e200v2.diff (3.2 KiB) 
 firmware/target/arm/as3525/sansa-e200v2/button-e200v2.c |   35 +++++++++-------
 1 file changed, 21 insertions(+), 14 deletions(-)

This task depends upon

Closed by  Thomas Martitz (kugel.)
Tuesday, 23 June 2009, 00:26 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  Committed in r21474. Thanks for the good work!
Comment by Dustin Skoracki (sko) - Sunday, 07 June 2009, 23:10 GMT+2
Here is a new patch: overlooked one place for using button_delay() (button_read_device() also has an delay). Works also fine for me, but should be tested by someone else too, because the delay in button_read_device() is a little longer now (8 in svn, now 10). Fuze does it the same way, so I applied it for e200v2 too.
   button_e200v2_b.diff (3.8 KiB) 
 firmware/target/arm/as3525/sansa-e200v2/button-e200v2.c |   41 +++++++++-------
 1 file changed, 24 insertions(+), 17 deletions(-)

Comment by Rafaël Carré (funman) - Monday, 08 June 2009, 02:05 GMT+2
Now the fuze and e200v2 driver are identical, except these few differences:

COSMETICS:
- some comments
- how button_hold() is positioned

FUNCTIONAL:
- some settings (acceleration, how much hardware changes = 1 software change)
- how lcd and button concurrent access to DBOP are managed (e200v2 skips whole button reading if lcd is used, fuze only skips buttons read by dbop)

Please kugel since you wrote all of this, can you detail what are the differences here, and what prevent us from merging the 2 files ?

sko: did you try to use the button-fuze.c file on e200v2 ?
Comment by Jack Halpin (FlynDice) - Monday, 08 June 2009, 07:16 GMT+2
I tried the first patch on the way home tonight and it works fine. If the 2 button drivers are finally coming together should we just wait until we can merge them?
Comment by Dustin Skoracki (sko) - Monday, 08 June 2009, 09:00 GMT+2
@funman: - "sko: did you try to use the button-fuze.c file on e200v2 ?"

I tried long time ago and it was not working, but now they're very similar, I'll try again (but will need a little adaption since e200v2 has no BUTTON_HOME). Anyway I startet merging the two drivers, it's a little more complex then I thought due the differences you mentioned in your comment (well it's a little "#if defined()"-hell right now^^).
Comment by Rafaël Carré (funman) - Monday, 08 June 2009, 19:05 GMT+2
I was suggesting that the functional differences could be removed as well, especially the settings.

Perhaps both models have a decent user experience with respect to the buttons (especially the wheel), but this experience is a bit different. Does anyone have both an e200v2 and a fuze?
Comment by Jack Halpin (FlynDice) - Monday, 08 June 2009, 19:30 GMT+2
"Does anyone have both an e200v2 and a fuze?"

Saratoga?
Comment by Richard (kronflux) - Tuesday, 09 June 2009, 13:37 GMT+2
I have an e280v2 and a fuze, but recently my fuze's screen got stepped on, so using it is next to impossible.
Comment by Dustin Skoracki (sko) - Monday, 22 June 2009, 21:45 GMT+2
Here is my actual patch for a unified driver. It's working good on e200v2, maybe someone with a fuze should have a look at it.
   button-e200v2-fuze.diff (28.1 KiB) 
 firmware/SOURCES                                        |    4 
 firmware/target/arm/as3525/sansa-e200v2/button-e200v2.c |  248 -----------
 firmware/target/arm/as3525/button-e200v2-fuze.c         |  340 ++++++++++++++++
 firmware/target/arm/as3525/sansa-fuze/button-fuze.c     |  284 -------------
 4 files changed, 342 insertions(+), 534 deletions(-)

Comment by Thomas Martitz (kugel.) - Monday, 22 June 2009, 21:54 GMT+2
Looks good, I'm thinking this can actually go in. I will test on my fuze!

Loading...