New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatic setting for 'warmth' #3820

Merged
merged 4 commits into from Apr 1, 2018

Conversation

Projects
None yet
3 participants
@dengste

dengste commented Mar 31, 2018

Support for setting 'warmth' according to current time.

@dengste

This comment has been minimized.

dengste commented Mar 31, 2018

This should finally fix #2285 (at least I won't add anything more).

I decided to not go the plugin route, because changing the frontlightwidget dynamically from a plugin (like disabling the manual 'warmth' progress bar) would have been awkward, IMHO. Same goes for syncing with Nickel configuration. I solved the circular requires by not calling UIManager myself but using the backgroundrunner plugin instead, which is easier anyway.

@Frenzie

This comment has been minimized.

Member

Frenzie commented Mar 31, 2018

I solved the circular requires by not calling UIManager myself but using the backgroundrunner plugin instead, which is easier anyway.

Sounds good (haven't looked yet). BackgroundRunner should become part of the core anyway, imo.

if self.auto_warmth then
self:setWarmth()
end
end

This comment has been minimized.

@Frenzie

Frenzie Mar 31, 2018

Member

Could you stick a comma at the end? Just in case we want to add something else in the future. :-)

@Frenzie

I add this nitpick only because of the high quality of the commit but "automatical" is not an English word. That should be "automatic."

if new_autocolor then
return (new_autocolor == "true")
end
return nil

This comment has been minimized.

@Frenzie

Frenzie Mar 31, 2018

Member

Seems redundant?

This comment has been minimized.

@dengste

dengste Mar 31, 2018

If the setting exists, the function should return a boolean, and 'nil' otherwise. I was under the impression that 'nil' and 'false' are different things in Lua?

This comment has been minimized.

@Frenzie

Frenzie Mar 31, 2018

Member

nil ~= false, but the default return value of a function is nil, not false, so that distinction isn't relevant here. ;-) Any return nil at the end of a function is redundant. (Just like in the one a few lines above. I either missed that or decided not to nitpick it.)

This comment has been minimized.

@dengste

dengste Apr 1, 2018

Thank you for your patience, now I get it. I'm learning Lua as I stumble along... :-)

dbg:guard(NickelConf.autoColorEnabled, 'set',
function(self, new_autocolor)
assert(type(new_autocolor) == "boolean",
"Wrong autocolor value given (expected boolean)!")

This comment has been minimized.

@Frenzie

Frenzie Mar 31, 2018

Member

I find the error message slightly confusing. It's not so much a wrong value (which makes me think that, for example, it's too large) as the wrong type.

width = self.screen_width * 0.1
}
local button_minus_one_hour = Button:new{
text = "-",

This comment has been minimized.

@Frenzie

Frenzie Mar 31, 2018

Member

A real minus () or less correctly an en-dash () might look better than a hyphen (-).

@dengste

This comment has been minimized.

dengste commented Mar 31, 2018

BTW, this is how it looks:
automatic_warm

@dengste

This comment has been minimized.

dengste commented Apr 1, 2018

Let me know if the fixups are OK, then I(!) will squash them. ;-)

@Frenzie

This comment has been minimized.

Member

Frenzie commented Apr 1, 2018

Alright, looks good to me. Could you please rebase it? I'm not at my main computer atm.

@dengste

This comment has been minimized.

dengste commented Apr 1, 2018

I'll also fix the "automatical" thingies during squash.

David Engster added some commits Mar 31, 2018

David Engster
[feat] kobo/powerd: Support for automatic warmth
This is the first step to support "automatic warmth", meaning that
warmth will be set according to the current time. The user can set a
"bedtime" at which warmth should be maximal. Warmth will increase
towards approaching bedtime and decrease afterwards.

Add new members 'auto_warmth' and 'max_warmth_hour' which tell if this
feature is enabled and the "bedtime", resp. Add a method
'calculateAutoWarmth' which will set the current warmth according to
the current time. The progression is linear but not symmetrical: we
start 5h before "bedtime", but turn back warmth to '0' 2h after it, to
make sure that warmth is '0' in the morning.

For automatically setting warmth in the background we use the
backgroundrunner plugin, because not only is it more comfortable to
use, but we also cannot require 'uimanager' during device
initialization.
David Engster
[feat] kobo/powerd, kobo/nickel_conf: Saving of auto-warmth settings
Add support in nickel_conf to save/retrieve setting of
'autoColorEnabled', which is the automatic warmth feature in
Nickel. We do not support reading of 'BedTime', because it is encoded
as a QVariant. This setting is hence saved/loaded solely in/from
G_reader_settings.
David Engster
[feat, UX] frontlighwidget: Support for automatic warmth
If the device supports it, add a checkbox for enabling automatic
warmth, and a widget for setting the hour at which warmth should be
maximal ("bedtime").

The hour can be changed through +/- buttons; holding them will change
with 30min steps. As soon as automatic warmth is enabled, the 'warmth'
cannot be changed manually anymore, so we make its widget grey, but
the current value is still displayed correctly. Changing the hour
changes warmth accordingly.
David Engster
[UX] plugins/kobolight: Handle automatic warmth
If warmth is handled automatically, do not try to change warmth but
display a message instead.

@dengste dengste force-pushed the dengste:automatic_warmth branch from aa2b4e3 to fbfbbaf Apr 1, 2018

@dengste

This comment has been minimized.

dengste commented Apr 1, 2018

I squashed the fixups, corrected the commit messages and rebased onto current master.

@dengste

This comment has been minimized.

dengste commented Apr 1, 2018

BTW, is there some way to donate to the project? There are bug bounties on #2285 and IMO those should go to the project, since without it there would be nothing to hack on...

@Frenzie

This comment has been minimized.

Member

Frenzie commented Apr 1, 2018

You deserve that $20 bounty fair and square. :-) It's a bounty for a specific bug after all. I haven't looked into Bountysource. Perhaps you could redirect it to something you'd like to see fixed?

But there isn't a way to donate really. If you have any suggestions as to how that could or should work please let us know. For now all I can say is that I'm keeping an eye on FreshRSS/FreshRSS#1694

I have personally accepted a €15 donation recently but presently there's no clear plan of how it could be used or how to distribute it fairly.

@Frenzie Frenzie merged commit ba46376 into koreader:master Apr 1, 2018

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
@helmus

This comment has been minimized.

helmus commented Apr 1, 2018

Very cool! I suggest you claim the bounty here and then donate it to the project if you want to do that. Someone needs to claim the bounty to unlock the funds tough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment