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
Quiet the chatty sun.sun #23832
Merged
Merged
Quiet the chatty sun.sun #23832
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
e629c4c
Split up method to allow caching event
Swamp-Ig 934806c
Lower frequency of updates.
Swamp-Ig 4f55c6c
Code review patches.
Swamp-Ig 5f0d529
Minor changes to test
Swamp-Ig 36e5328
Skip end of period at fixed multiple of delta.
Swamp-Ig File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Instead of using hard-coded update times, why not round the angles? Then the state machine would take care of de-duplicating values (because of force_update=False).
The sun position calculation is really quick - so computation time is not a problem. Database storage however is.
We could also have different rounding accuracy based on the value. So for example
Sounds like a much simpler approach to me.
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.
I did try that. The thing is that the azimuth swings around rapidly at near noon, and so you end up with a bunch of pretty useless updates if you're tracking the azimuth too.
You do need to do that too at high latitudes, at the extreme case if you are at the north pole the elevation doesn't change much during 24hrs, only the azimuth. This affects what windows the dun shines into for example.
That's what lead to total arc of travel, rather than change with respect to the horizon. We do care more about changes near the horizon because they're more important, but mostly we want to know that the sun has moved somewhere new in the sky.
I'm not trying to minimise the calculation, the goal is to reduce the number of writes to the database.
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.
I've added a more detailed description above to what the code is doing. It does take a bit to get your head around 😁