Skip to content
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

feat: Added max_expected_flow_w configuration #34

Merged
merged 2 commits into from
Mar 31, 2023
Merged

Conversation

shadow7412
Copy link
Contributor

@shadow7412 shadow7412 commented Mar 27, 2023

The idea here is that that max_expected_flow_w config will set a hard minimum for the total value when calculating animations.

This should result in linearly slower animations under that value (but the current scaling when over).

Note that I wasn't entirely sure how to quickly test my changes, so they are just eyeballed for now. I figured it'd be worth raising the PR anyway, in case you wouldn't mind testing it. Otherwise it can stay stagnant until I confirm it actually works as expected.

The biggest assumption I've made is that circleRate's total is in watts.

Also my IDE apparently cleaned up some trailing whitespace. I can revert this if you feel strongly about it.

@shadow7412 shadow7412 changed the title Added max_expected_flow_w configuration feat: Added max_expected_flow_w configuration Mar 27, 2023
@flixlix
Copy link
Owner

flixlix commented Mar 28, 2023

Thank you for your pull request.
I'll review it when I have some free time

@flixlix
Copy link
Owner

flixlix commented Mar 31, 2023

I have reviewed and tested your PR and I overall I'm pretty happy with the new formula. 👍
I only changed the default value for max_expected_flow_w to 5000W, since I think most peoples homes wont exceed this value regularly. I also changed a few things in the readme.
Thank you very much for the PR, I'm going to have it introduced in the next beta and see what people think before releasing a new official version.

@flixlix flixlix merged commit 89dfb99 into flixlix:main Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants