Notice: A non well formed numeric value encountered in /sites/rockbox.org/flyspray/includes/class.flyspray.php on line 96 Notice: A non well formed numeric value encountered in /sites/rockbox.org/flyspray/includes/class.flyspray.php on line 96 Notice: A non well formed numeric value encountered in /sites/rockbox.org/flyspray/includes/class.flyspray.php on line 96 Deprecated: Function create_function() is deprecated in /sites/rockbox.org/flyspray/includes/class.flyspray.php on line 104 Deprecated: The each() function is deprecated. This message will be suppressed on further calls in /sites/rockbox.org/flyspray/adodb/adodb.inc.php on line 845 Deprecated: Function create_function() is deprecated in /sites/rockbox.org/flyspray/includes/class.user.php on line 111 FS#5772 : BL Hold Switch Option (for all) and LCD Sleep (for x5)

Rockbox

Tasklist

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
Task Type Patches
Category Settings
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

There'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  FS#5735  by Bernych). (H100, H300, 3G, 4G, iFP-790, X5)
* 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.
Comment by Paul Louden (darkkone) - Saturday, 05 August 2006, 15:10 GMT
A few notes:
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.
Comment by Michael Sevakis (MikeS) - Saturday, 05 August 2006, 19:14 GMT
1) I was asked to put this together as a unit.
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).
Comment by Michael Sevakis (MikeS) - Sunday, 06 August 2006, 05:58 GMT
Update:
* 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".
   new-hold-switch-and-lcd-sleep... (45.4 KiB)
Comment by Rani Hod (RaeNye) - Sunday, 06 August 2006, 05:59 GMT
I tested this patch and it works well.
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)'.
Comment by Rani Hod (RaeNye) - Sunday, 06 August 2006, 06:17 GMT
Of course I meant the first patch...
I'll test the new version later (I was just about to commit it...)
Comment by Michael Sevakis (MikeS) - Sunday, 06 August 2006, 06:22 GMT
Actually I hate that hack just because it's dirty to have to kill the BL thread/ticks like that and it adds code just for one player in a few spots but no real choice unless you like to use your X5 as a strobe light.
Comment by Michael Sevakis (MikeS) - Sunday, 06 August 2006, 08:02 GMT
Got some comments in while I wasn't watching huh? :) Some important notes follow:

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.



Loading...