Closed grahamwhaley closed 1 year ago
It'd be great to get some code review on this, given last time I messed it up! I've read through the code multiple times, and have it running locally, and I believe it is now doing the right thing...
Looks OK
Thanks @G6EJD . I think this PR got closed rather than merged - was that the intention? I think that happened on my last PR as well - must be something my PRs throw up that then doesn't do what we expect?
That’s strange, I merged it then checked the master files were showing updated now but it’s been unwound. Not sure what’s going on!
I re-opened the pull request and submitted again, it’s showing as successful like last time, but time will tell now.
Thanks! Yeah, I’ve no idea what happened - but this time I got the merge email notification, so hopefully it should ’stick’ this time. Most strange...
On 19 Jun 2023, at 17:07, G6EJD @.***> wrote:
I re-opened the pull request and submitted again, it’s showing as successful like last time, but time will tell now. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>
It looks like I really messed up the sleep timer code in the original commit when I tried to add the 'overnight long sleep'.
Rewrite that code, to cater for both big sleeps that start either before or after midnight.
Whilst there, make the SleepDuration/WakeupTime/SleepTime variables consts, which specifically in this new code allows the compiler to drop out the case that we are obviously not using. In my local tests this dropped ~100bytes from the image size (and probably saved us a single bit of math/test in the code itself).
Fixes: #217