-
Notifications
You must be signed in to change notification settings - Fork 31
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
Separate idle watchers for AC and Battery #228
Conversation
Config and src implementation. First attempt.
- set minimum time - display mm:ss - load right values
- clean up code - ensure discharging state is updated - set a minimum difference on AC/Battery timeout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything perfectly OK, but, please, implement the fallback mechanism in settings...
Thank you
@@ -46,8 +46,10 @@ namespace PowerManagementSettingsConstants | |||
const QString POWER_LOW_LEVEL_KEY { QL1S("powerLowLevel") }; | |||
const QString SHOW_ICON_KEY { QL1S("showIcon") }; | |||
const QString USE_THEME_ICONS_KEY { QL1S("useThemeIcons") }; | |||
const QString IDLENESS_ACTION_KEY { QL1S("idlenessAction") }; | |||
const QString IDLENESS_TIME_SECS_KEY { QL1S("idlenessTimeSecs") }; | |||
const QString IDLENESS_AC_ACTION_KEY { QL1S("idlenessACAction") }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor note.
In this new version we completely ignore the pre-existing "legacy" configuration.
IMO we should fallback to legacy configuration for both AC/baterry if there is no new configuration defined in the config yet.
E.g. something like
return value(IDLENESS_AC_ACTION_KEY, value(IDLENESS_LEGACY_ACTION_KEY, -1)).toInt();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok.
I'm going to set the old values to be the actual settings of both AC/Battery watchers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GTM
should fix #60