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

Helpers rework - rounding #1529

Merged
merged 8 commits into from
Jan 3, 2025
Merged

Conversation

folfy
Copy link
Collaborator

@folfy folfy commented Dec 20, 2024

Motivation:

A lot of code is unused and redundant, so cleaned this up before fixing #1475. I know a lot of these conversions back and forth were done to avoid the rounding error from floating point arithmetic, but I'm proposing easier ways to achieve this.

The 0.0001 offset for correcting this error is a bit of a hack as well, but I think it's the simplest solution, given that we do not need arbitrary precision or working with extremly large numbers.

Changes:

  • removed "int" from type defs - redundant according to PEP-0484
  • converted "round_by_step" to be a generic function for all usecases
  • Replaced all other rounding functions with a call to round_by_step
  • removed all unused rounding functions (so the previous change is useless, but in case you wanna keep some functions for some reason...)
  • Changed rounding method for target temperature calibration from down to nearest (Does not round correctly #1475)
  • Changed rounding method for target temperature calibration from fixed 0.5°C to actual step size (Does not round correctly #1475)

Related issue (check one):

Checklist (check one):

  • I did not change any code (e.g. documentation changes)
  • The code change is tested and works locally.

Test-Hardware list (for code changes)

HA Version: 2024.12.1
Zigbee2MQTT Version: N/A (ZHA)
TRV Hardware: Demo and HmIP

@folfy folfy marked this pull request as ready for review December 20, 2024 17:54
@folfy
Copy link
Collaborator Author

folfy commented Dec 20, 2024

Tested on Demo-Instance, would ask you for some feedback @RubenKelevra or @KartoffelToby
Will test it on production with HmIP once I got feedback, and after knowing if it should be based on v1.7.0 / merged there instead.

@KartoffelToby
Copy link
Owner

@folfy Thanks! <3
I'll do the Linting, try to use the Lint validation for your next PRs :)

@KartoffelToby KartoffelToby merged commit 6d64590 into KartoffelToby:master Jan 3, 2025
@folfy
Copy link
Collaborator Author

folfy commented Jan 6, 2025

@KartoffelToby Sure, but you're talking about real linting or just formattting, because I just see black in the config, which would be a formatter? Have to figure out if/how I could get it running on my PRs, probably I don't have permission if it's not started automatically and will have to get and run it locally then or set it up on my repo.

@KartoffelToby
Copy link
Owner

@folfy you should have the permissions, as i added you to the Contributors anyway.

I setup a pre commit hook in the repo (you need to have installed it on your maschine) https://pre-commit.com/

Or if you have docker, you can run: docker run --rm --volume $(pwd):/src --workdir /src pyfound/black:latest_release black --safe .

@folfy
Copy link
Collaborator Author

folfy commented Jan 6, 2025

@KartoffelToby Ohhh, those options both looks nice and as straight-forward as l like it to be, great! Ofc also running all my stuff in docker, thanks a lot ❤️

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.

Does not round correctly
2 participants