Remove % sign from Vera Battery Levels#8069
Conversation
Vera devices are reporting battery levels as a sting by appending a percentage sign (%) on the end. To make the Vera component act like other Home Assistant components, let's remove the percentage sign from the battery report levels so that we only display the battery level. This may be a "breaking change" if people are relying on the Vera battery levels to be a string instead of an int. However, this will make the battery level reports compatible with everything else.
|
@philhawthorne, thanks for your PR! By analyzing the history of the files in this pull request, we identified @pavoni, @arjenfvellinga and @balloob to be potential reviewers. |
|
|
||
| if self.vera_device.has_battery: | ||
| attr[ATTR_BATTERY_LEVEL] = self.vera_device.battery_level + '%' | ||
| attr[ATTR_BATTERY_LEVEL] = self.vera_device.battery_level |
There was a problem hiding this comment.
I don't think that this has the desired effect. Python does not allow concatenation of a number and a string, so the battery level itself must be a number in string form. So the attribute is still a string?
There was a problem hiding this comment.
@balloob you might be right on that.
The original intention was for InfluxDB, which would handle the string to number conversion.
Should we wrap self.vera_device.battery_level around a float()?
There was a problem hiding this comment.
yes (or int if it's just a number?). And catch the TypeError in case the conversion fails. When failed, just not add that attribute (that way the data in that field stays of a consistent type)
5 participants
Description:
Vera devices are reporting battery levels as a sting by appending a
percentage sign (%) on the end.
To make the Vera component act like other Home Assistant components,
let's remove the percentage sign from the battery report levels so that
we only display the battery level.
This may be a "breaking change" if people are relying on the Vera
battery levels to be a string instead of an int. However, this will make
the battery level reports compatible with everything else.
Related issue (if applicable): See discussion in #7879
Pull request in home-assistant.github.io with documentation (if applicable): N/A
Checklist:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests pass