-
-
Notifications
You must be signed in to change notification settings - Fork 30
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
Add support for more sensors & basic Siren (switch) support #84
Conversation
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.
None of my comments are game changing. feel free to ignore if you disagree.
"{:.2f}".format( | ||
self.tahoma_device.active_states.get(CORE_RELATIVE_HUMIDITY_STATE) | ||
) | ||
"{:.2f}".format(states.get(CORE_RELATIVE_HUMIDITY_STATE)) |
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 should use f-strings here. I think this will be required for a merge in core.
f'{states.get(CORE_RELATIVE_HUMIDITY_STATE):%.2f}'
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.
@tetienne do we need fstrings for all floats here? home-assistant/core#36258 (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.
What do you have in entry and what do you want in output?
To have a float from a string you can do:
>>>a = "2.3"
>>>float(a)
2.3
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.
do we actually convert a float to string then to float?
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.
To be honest, I am not sure how HA would like to have the values. Do they require a float or is a floating value in a string the right approach?
In the end, what should happen in sensor.py
is that we make round the value and make it an int or float.
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.
@vlebourl, is states.get(CORE_RELATIVE_HUMIDITY_STATE) already a float? If a string, the round method can not be applied.
>>> a = "2.3"
>>> round(a, 2)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: type str doesn't define __round__ method
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 think it. otherwise we would get
>>> a = "2.3"
>>> "{:.2f}".format(a)
Traceback (most recent call last):
File "<input>", line 1, in <module>
ValueError: Unknown format code 'f' for object of type 'str'
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.
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.
Let's maybe leave this like this for now as this PR is already quite large, but resolve this in another PR once this one is merged
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.
Agree, let's work on those improvements in separate smaller PR's. This one already got way to big :D
even though it is not working yet
Not related to the code, but I highly recommand you do only squash and merge your PR. You will have a nice commit history in the master branch without all your WIP and Fix commit of your branch. |
Makes sense. Especially since this PR doesn't include a nice commit log. 👍 |
You got it. You can allow only this method if you want in the settings of your repository. |
"{:.2f}".format( | ||
self.tahoma_device.active_states.get(CORE_RELATIVE_HUMIDITY_STATE) | ||
) | ||
"{:.2f}".format(states.get(CORE_RELATIVE_HUMIDITY_STATE)) |
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.
do we actually convert a float to string then to float?
Shall we merge? |
Exemple of water sensor
|
Added
Changed