-
-
Notifications
You must be signed in to change notification settings - Fork 29k
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
Include charging state for powerwall #33432
Include charging state for powerwall #33432
Conversation
Just a small question that serves to trigger the brain: Why make this an attribute and not a binary sensor? It makes it easier to automate and display. |
homekit wants it as an attribute: |
@frenck better to link to this line https://github.com/home-assistant/core/blob/dev/homeassistant/components/homekit/accessories.py#L187 |
And Homekit is having that code because that's how we decided it once upon a time in the iOS app… 😆 I think that it's better served as a binary sensor. |
There isn't a way to have the linked battery track the sensor charge level and the binary sensor charging state currently so I think I'm kind stuck with this unless I'm missing something |
In the frontend we do: state -> entity registry -> device registry -> all other entities for this device -> filter for binary_sensor with device class == X. |
Homekit doesn't implement linking a binary sensor and sensor for the
|
e8cdfa5
to
bd198db
Compare
Users shouldn't set up linking, this is something that homekit should discover itself by seeing what the device supports. We should add a helper functions to device registry entry to help with this |
I agree, its should show up if there a proper device entry. I'm thinking implementation looks something like:
|
Yeah that sounds good. |
@balloob https://github.com/home-assistant/core/blob/dev/homeassistant/components/homekit/accessories.py#L105 |
ent_reg = await entity_registry.async_get_registry(hass)
dev_reg = await device_registry.async_get_registry(hass)
entry = ent_reg.async_get(entity_id)
if entry is None or entry.device_id is None:
return
entries = entity_registry.async_entries_for_device(ent_reg, entry.device_id)
for entry in entries:
if entry.domain == "binary_sensor" and entry.device_class == X:
return entry
return None |
I'll move this to incoming in our project task board. Ping here when the architecture issue is merged and we can continue review here. |
bd198db
to
2476f8e
Compare
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.
Looks good!
Thanks @MartinHjelmare Will merge after I have a day of sun to revalidate this. |
After the sun went down, the sensor stayed off. The margin of error seems to be set right to avoid false on readings. |
Proposed change
This allows homekit to show the charging state in the
homekit app when the powerwall is exported as a linked
battery sensor.
Type of change
Example entry for
configuration.yaml
:# Example configuration.yaml
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale: