-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
settings: use 2024 as default year #2067
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
Conversation
|
Build checks have not completed. Possible reasons for this are:
|
NeroBurner
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.
We probably have to update the variable year by year then, but I think it makes sense to have the default year close the the release date of the firmware
|
One option would be using the compiler to insert the current year at time of compilation, I'm sure that's possible as the build date page in settings already exists Edit: looks like |
mark9064
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.
This is the wrong place to introduce a default year. By changing the counter parameter instead the minimum year the spinbox can show is adjusted. The spinboxes otherwise display the current time. A better and more correct way to do this is to initialise the default time on boot to 1 Jan on the year of the compile time. This means that the time can still be adjusted freely. I would suspect that main.cpp where the time is loaded from the persistent RAM is the correct place to set the time as a fallback.
The compile time could be used as the default date and time, but I think this is confusing whereas setting to 1 Jan YEAR 00:00 is unambiguously a default time.
NeroBurner
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.
@mark9064 makes a very good point. Although changing the year to a time before now is esoteric I think, but taking away this option isn't nice as well.
|
I like the idea of using the |
|
This attempt at constexpr atoi looks pretty bleak https://stackoverflow.com/a/59537554 😅 |
constexpr int isdigit_constexpr(int c) {
return c >= '0' && c <= '9';
}
constexpr int atoi_constexpr(char const *str) {
int result = 0;
while (isdigit_constexpr(*str)) {
result = result * 10 + *str - '0';
str++;
}
return result;
}I have no idea why that implementation looks so difficult, this is all it takes. I guess it could be that this isn't standards-compliant, and in that post they were trying to be as compliant as possible. |
|
That looks much more reasonable and does the job here 👍 |
|
@mark9064 thanks, good suggestions. however i would not be able to make all those changes. my reasoning was to to make this PR because it is vary easy to change constant, instead of opening an issue about this. please feel free to take over this PR or close it and make another one. |
|
With #2133 this is integrated now and can be closed |
|
@vladopajic Thank you for the idea of setting the year by default! And for the initial implementation, it was very helpful for working out the final solution. |
🙏 kudos to you guys! |
having 2024 as default year will make it less annoying to configure year