-
-
Notifications
You must be signed in to change notification settings - Fork 130
Fix status message #312
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
Fix status message #312
Conversation
38a7a6f to
45a74af
Compare
|
I tested and it's working. I'm not sure which is the most readable style for the
|
DL6ER
left a comment
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.
Tested as well, I suggest to add consistent empty lines and comments. I also looked at the style proposed by @rdwebdesign and am undecided what might be better. It is a lot more condensed (half the number of lines), however, this also means it is very condensed. In contrast, the current design feel a lot more "light" and I don't think this hurts here. Good that I don't have to decide this :-)
Signed-off-by: Christian König <[email protected]>
Signed-off-by: Christian König <[email protected]>
Signed-off-by: Christian König <[email protected]>
Signed-off-by: Christian König <[email protected]>
|
Actually, I wasn't sure which style I should use when I first wrote the code. I thought the |
What does this PR aim to accomplish?:
We use the "status message field" to show various, independent messages (e.g. System ist hot, Updates available, Everything is fine,...). Currently, the message is set for every event in the respective
GetXXXXInformationfunction and the order of the variousGetXXXXInformationdetermines the final output. This made the code hard to follow, as it was not clear when which message will be shown. It also led to a bug described here, because we forget to set the message back toEverything is fineonce something was not fine.This PR disentangles the code by setting flags if certain conditions are met (e.g. updates available, system is hot) and setting the final status message in a separate function, depending on their "severity" (it's more important that the device is hot than that updates are available).
Additionally, the PR
a) Creates a new function
GetPADDInformationwhere we check for PADD updates only. This allows to runGetPADDInformationupdates more often (30 seconds) so that "Update available" does not stay for 24hours if users have been upgrading their Pi-holes inbetweenb) Only tries to get
FTLs DNS port ifFTLis known to be runningc) Fixes a small visual glitch in
minid) Removed the
version_statusfrom the startup sequence (shown only for 3 seconds), if updates are available it is shown on the dashboard anyway.By submitting this pull request, I confirm the following:
git rebase)