FS#5772 - BL Hold Switch Option (for all) and LCD Sleep (for x5)
Attached to Project:
Rockbox
Opened by Michael Sevakis (MikeS) - Saturday, 05 August 2006, 07:53 GMT
Opened by Michael Sevakis (MikeS) - Saturday, 05 August 2006, 07:53 GMT
|
DetailsThere's a fair amount here in this one.
* Adds a Backlight (On Hold Switch) option for all platforms where HAS_BUTTON_HOLD is defined (Adapted from * Adds LCD Sleep (After Backlight Off) for those defining HAVE_LCD_SLEEP in config (RaeNye's idea). (x5) * Adds HAVE_LCD_ENABLE (x5, 3G, H300) and BL_X5. * Turns off visible display op. with backlight for x5. * Makes nescessary LCD driver changes to implement sleep and disp. shutdown plus a few tweaks that save IRAM and RAM without perf. sacrifice. * Adds a hack to make sure bl flash doesn't happen if bl is off...namely make sure shutting down screen is always displayed (TODO: skip that if low batt shutdown), bl thread is stopped, and no yields occur once the system is flushed (x5). * Starts some things to add the hold setting to the remote. No functionality is lost; just moved some code. * Contains a few miscellaneous changes that my text editor made, namely tabs->spaces, strip trailing whitespace. Not too much but a big pain in the behind to remove. :) * Other things I can't remember that I did. 
new-hold-switch-and-lcd-sleep...
(52.8 KiB)
|
This task depends upon
Closed by Rani Hod (RaeNye)
Tuesday, 08 August 2006, 22:06 GMT
Reason for closing: Accepted
Additional comments about closing: w/o the \"halt\" part.
Tuesday, 08 August 2006, 22:06 GMT
Reason for closing: Accepted
Additional comments about closing: w/o the \"halt\" part.
1) You should try to split different 'features' into different tasks. Often you'll be asked to split them anyway, because one is worth committing and another isn't, or whatnot.
2) With the changes your text editor made, does this patch adhere to the posted guidelines for Rockbox code or not? Again, if it doesn't you'll be the one who needs to change it.
2) The text editor changes actually make it more compliant since I set my options according to guidelines: Tabs->four Spaces, End file in linefeed (for gcc), UNIX linefeeds. No rule about trailing whitespace but it shouldn't really be there (maybe helps with <80 char lines).
* Had to resync to CVS in misc.c.
* Changed the thread handling for the x5 backlight shutdown code 'cause I hated it. Still hate that hack but it's a bit better.
* Cleaned it up a lot (Things accidentally left in not untimately needed and such).
Additional lesson: Turn off "Remove trailing whitespace".
two comments:
- the while(1) yield --> halt change is actually unrelated and should be discussed separately (I'll commit the patch without this change).
- the setting description that appears is 'Sleep (after BL is off)' and I'll change it to 'LCD sleep (after BL is off)'.
I'll test the new version later (I was just about to commit it...)
The use of halt was meant to prevent loss of control from the main thread. With the BL thread killed, I don't think it will matter though yes, I will definitely discuss that if you must remove it.
No real difference in the new version except instead of creating a new thread.c function get_thread_id(), I just save the backlight thread id returned by create_thread and use that to kill it.
I thought about using "LCD Sleep (After Backlight Off)" but felt the LCD part to be as redundant as saying "LCD Contrast" or "LCD Brightness" would be from the LCD menu and is also why I placed it next to those.
BTW: I added a second instance of the word "Normal" apart from the one used by the LCD Flip option since I'm not sure the sense and hence the word would be the same in all translations (hence the use of LANG_NORMAL_LCD_FLIP instead of LANG_FLIP by the CVS code). That of course can be changed at any time if they should be the same.