Introduce base entity for ping #104197
Introduce base entity for ping #104197
Conversation
| """Initialize the Ping Binary sensor.""" | ||
| super().__init__(coordinator) | ||
|
|
||
| self._attr_name = config_entry.title |
There was a problem hiding this comment.
_attr_has_entity_name = True and _attr_name=None? Maybe need to check how this works with the device tracker entity
There was a problem hiding this comment.
But this will not rename the entities when renaming the config entry. It was suggested in #103743 (comment) not to use a "name" option in the config flow, instead use the config entry title.
There was a problem hiding this comment.
What @joostlek is proposing is to use the device name instead of just the entity name.
I don't know either if this works well on this case but we should strive for implementing _attr_has_entity_name on as many as possible to go for devices and its name in case of config flows.
There was a problem hiding this comment.
I fully agree and I'm fine with this change, it was just a note that the user then has to change the title of the config entry and the device name, but that is something the user already knows.
frenck
added
the
smash
| def is_connected(self) -> bool: | ||
| """Return true if ping returns is_alive.""" | ||
| return self.coordinator.data.is_alive | ||
| def state(self) -> str: |
There was a problem hiding this comment.
Overriding state isn't really according to our design intentions. I think this needs further discussion. I suggest we revert this PR for now.
There was a problem hiding this comment.
Didn't know that, I just saw that ibeacon and private_ble_device do it the same way (extend BaseTrackerEntity and override state) and thought it would be fine.
There was a problem hiding this comment.
The two device tracker entity classes we've documented are ScannerEntity and TrackerEntity. If those aren't sufficient to cover all cases we should make an architecture discussion and figure out what else is needed.
https://developers.home-assistant.io/docs/core/entity/device-tracker
Proposed change
Introduce a base entity for ping, also move the device tracker from
ScannerEntitytoBaseTrackerEntityso we can attach device infos.ScannerEntitytries to attach the entity to an existing device entry based on MAC, but this integration has no MAC.Type of change
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.To help with the load of incoming pull requests: