[Merged] Power Saver feature

Sano

@Hex compiled via @meleu script, yes !

Hex

Need testers to update and build to verify all changes. Finally please report any bugs (undocumented behaviour)

Final changes are made:

PS enabled by default
Removed PS enable warning
Works nicely with all screensavers
Gameinfo is shown with PS enabled too
mVersion no longer shows PS state. Restored to default.

cyperghost

@Hex
I've uploaded binary to my github >> https://github.com/crcerror/emulationstation-binary-archive (only PI2/3)

This is an merged version with your and @pjft favorite branch!

sadly it's an older release but it works the same way as @sano got -- CPU load is only 0,3%. Thank you mate. I'm sure this will be implented it's better as my suggestion ... the Standard ES branch still got a CPU load of 3% if Screensaver is enabeld.

Hex

Testers please check if GameInfo (XX Games bar) is properly shown when PS is enabled and all screensavers (video,dim,blank) are properly triggered. Thanks

Sano

@Hex Compiled, tested. GameInfo display is back to normal.
Edit : and the screen just dimmed by itself after screensaver timeout.

cyperghost

@Sano Please set screensaver to 1 min and wait if Dimming, Video and Blank are working
EDIT: Thanks for testing dimming - what about BLANK and Video?

Sano

@cyperghost Black screen is ok too, but this minute is pretty long, I think the timer is buggy.
Currently I don't have any video to test video screensaver.

cyperghost

@Sano Thank you can you make further tests please? It's important to hunt down possible bugs before a merge to the main branch can be done.

Hex

@Sano The minute is correct. I have timed it. It is just counting after some delay which you dont know about. The delay is present to finish any remaining animations and delayed displays like game count extra.

meleu

@felleg said in Testers needed :: Power Saver features :: PR #172:

This is a problem if you want to avoid screen burn-in. I'd much rather have the screensaver be able to start automatically, although, don't get me wrong, being able to start it manually is great in its own right.

I think that playing a video is not a "power saving" action. But now I think I better understood what the @Hex 's change is about.

It seems to be a fix in the logic, and not an addition of a new optional feature.

@Hex am I right saying that?

Hex

@meleu said in Testers needed :: Power Saver features :: PR #172:

.

Yes, It was a logic problem but such things get overlooked quite often. As I said, Aloshi's work might not have been optimized for Pi but it can be.

I agree Video screensaver is not a power saving feature but it is not intended to be. It is for systems that have a constant source of power and hence not battery friendly. But that does not mean that the feature is useless. A lot of effort and time went into making it worthwhile. :)

The option to enable or disable is just for your satisfaction. The functionality is not compromised at all. It just got better at doing nothing :P

meleu

Once you fixed all those "issues" related with screensaver (video/dim/blank), then will it be enabled by default and with no option to toggle it?

Hex

@meleu All bugs are fixed. The option is still there. Do you think the option should not be present?

meleu

@Hex here's my reasoning:

your change does power saving (optimizes the logic).
dim/blank/video screensaver works fine no matter if the PS is enabled (by the way, you should update this info on OP).
there isn't any advantage in leaving PS disabled.

Then here's the big question: Why the heck would anyone leave this option disabled?

If there's no answer for that question, then the conclusion is "leave it always enabled and remove the option to disable it"

Hex

@meleu Okay let me remove the option.

EDIT : Why are you highlighting it?? I wrote a lengthy answer and deleted everything. In short the answer is to disable PS if it misbehaves and yet not be shackled by a broken feature. On the other hand we have lots of Options already so screw it :D

meleu

@Hex said in Testers needed :: Power Saver features :: PR #172:

Why are you highlighting it?

I don't mean to sound like screaming, sorry if I sounded like that. :-)

I was just trying to emphasize the conclusion. Maybe we can continue this conversation on the PR thread to get Jools opinion. I'll paste my reasoning above there.

Hex

That would be a good idea

jdrassa

@Hex @meleu I haven't had a chance to test this yet, but based on my understanding of how it works, it seems likely that power saver will disable text scrolling for game descriptions and game names in the gamelist. If I am correct, this would be a reason to keep it as an option.

Another reason to do so would be to prevent any conflicts with future development. For example, there have been requests for the ability to include animated images in themes. If power saver was not an option, implementation of this feature would be dependent on implementing a way to detect and disable PS when necessary. I would hate to see a potential contributor decide not to implement something because of the extra effort involved with making it compatible with PS.

Hex

@jdrassa Firstly the text scrolling works as expected. That feature is what convoluted the entire logic from being very simple. I am glad that i got to implement this.

Secondly I concur with you on this but.... Features like animated images would be useful and it woult be easy to implement PS+animations as I have already done it for videos and users can just copy how i did it for Videos. Its two lines, one to stop powersaver when images are animated/video and another to start it again when images end. I thought about it a lot. Remember we are developing for Raspberry pi primarily. Thus we should strive to save as much processing as possible. Had it been difficult to incorporate in further development I would have said that switch would be ideal.

See the systemscreensaver.cpp here and tell me if i can make it easier.

jdrassa

@Hex I saw that is was easy enough to enable/disable. What I was referring to was the process of determining when it should be enabled vs. disabled. For video view, the logic is pretty simple, is the current view video view? However, if any animated element was supported as a theme extra, that logic would be slightly more complex. Whenever, switching views or themes, one would have to scan the list of "extra" elements to see if one was animated. It is not terribly difficult logic, but it is more complex. It would also lead to the scenario where a user may have to choose between a theme and power saver. An option would give the user the choice to still use the theme with power saver mode, even if some animated elements were disabled.

With regards to text scrolling, for the game description, the animation runs in a loop, It scrolls to the end, pauses for a bit, then resets back to the top and scrolls again. As I said, I haven't tested it yet, but I don't see how it could work based on reviewing the code. I am guessing that you have set the delay before power saving mode is enabled so that a single scroll is completed. I think that is a great way to approach it, but some users may still want/expect the old behavior, which is why I think keeping it as an option is a good idea.