Add Quality Scale to ElkM1 Integration #152371
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| rules: | ||
|
Member
There was a problem hiding this comment. How many entries wouldn't have a unique id at this point?
Member
There was a problem hiding this comment. def _keypad_changed(keypad: Element, changeset: dict[str, Any]) -> None:
if (keypress := changeset.get("last_keypress")) is None:
return
hass.bus.async_fire(
EVENT_ELKM1_KEYPAD_KEY_PRESSED,
{
ATTR_KEYPAD_NAME: keypad.name,
ATTR_KEYPAD_ID: keypad.index + 1,
ATTR_KEY_NAME: keypress[0],
ATTR_KEY: keypress[1],
},
)
for keypad in elk.keypads:
keypad.add_callback(_keypad_changed)This could become an event entity, making it more visible for the users
Member
There was a problem hiding this comment. @property
def code_format(self) -> CodeFormat | None:
"""Return the alarm code format."""
return CodeFormat.NUMBERCan be set to
Member
There was a problem hiding this comment. @property
def alarm_state(self) -> AlarmControlPanelState | None:
"""Return the state of the element."""
return self._stateInstead, set it to
Member
There was a problem hiding this comment.
Member
There was a problem hiding this comment. class ElkSensor(ElkAttachedEntity, SensorEntity):
"""Base representation of Elk-M1 sensor."""
_attr_native_value: str | None = None
async def async_counter_refresh(self) -> None:
"""Refresh the value of a counter from the panel."""
if not isinstance(self, ElkCounter):
raise HomeAssistantError("supported only on ElkM1 Counter sensors")
self._element.get()
async def async_counter_set(self, value: int | None = None) -> None:
"""Set the value of a counter on the panel."""
if not isinstance(self, ElkCounter):
raise HomeAssistantError("supported only on ElkM1 Counter sensors")
if value is not None:
self._element.set(value)
async def async_zone_bypass(self, code: int | None = None) -> None:
"""Bypass zone."""
if not isinstance(self, ElkZone):
raise HomeAssistantError("supported only on ElkM1 Zone sensors")
if code is not None:
self._element.bypass(code)
async def async_zone_trigger(self) -> None:
"""Trigger zone."""
if not isinstance(self, ElkZone):
raise HomeAssistantError("supported only on ElkM1 Zone sensors")
self._element.trigger()Wouldn't it make more sense to make this a button entity for each sensor? |
||
| # Bronze | ||
| action-setup: done | ||
| appropriate-polling: done | ||
| brands: done | ||
| common-modules: done | ||
| config-flow-test-coverage: done | ||
|
Member
There was a problem hiding this comment. I would recommend to put this in a common fixture patch(
"homeassistant.components.elkm1.async_setup_entry",
return_value=True,
) as mock_setup_entry,(and maybe the async_setup as well) |
||
| config-flow: done | ||
| dependency-transparency: done | ||
| docs-actions: done | ||
| docs-high-level-description: done | ||
| docs-installation-instructions: done | ||
| docs-removal-instructions: done | ||
| entity-event-setup: done | ||
| entity-unique-id: done | ||
| has-entity-name: done | ||
| runtime-data: done | ||
| test-before-configure: done | ||
| test-before-setup: done | ||
|
Member
There was a problem hiding this comment. try:
_included(conf[item]["include"], True, config[item]["included"])
_included(conf[item]["exclude"], False, config[item]["included"])
except (ValueError, vol.Invalid) as err:
_LOGGER.error("Config item: %s; %s", item, err)
return FalseInstead of returning |
||
| unique-config-entry: done | ||
|
|
||
| # Silver | ||
| action-exceptions: done | ||
|
Member
There was a problem hiding this comment. async def async_alarm_disarm(self, code: str | None = None) -> None:
"""Send disarm command."""
if code is not None:
self._element.disarm(int(code))All alarm control panel methods are like this, but what happens when code is None? Is that realistic? If so, we should handle that error. Also, what happens if |
||
| config-entry-unloading: done | ||
| docs-configuration-parameters: done | ||
| docs-installation-parameters: done | ||
| entity-unavailable: done | ||
| integration-owner: done | ||
| log-when-unavailable: todo | ||
| parallel-updates: todo | ||
| reauthentication-flow: todo | ||
| test-coverage: todo | ||
|
|
||
| # Gold | ||
| devices: done | ||
| diagnostics: todo | ||
| discovery-update-info: done | ||
| discovery: done | ||
| docs-data-update: todo | ||
| docs-examples: done | ||
| docs-known-limitations: done | ||
| docs-supported-devices: done | ||
| docs-supported-functions: done | ||
| docs-troubleshooting: done | ||
| docs-use-cases: todo | ||
| dynamic-devices: todo | ||
| entity-category: done | ||
| entity-device-class: done | ||
|
Member
There was a problem hiding this comment.
def _element_changed(self, element: Element, changeset: dict[str, Any]) -> None:
if self._elk.is_connected():
self._attr_native_value = "Paused" if self._elk.is_paused() else "Connected"
else:
self._attr_native_value = "Disconnected"This can be an enum device class, where all the values are snake_case (would be a breaking change), but with all the options added to |
||
| entity-disabled-by-default: todo | ||
| entity-translations: todo | ||
| exception-translations: todo | ||
| icon-translations: todo | ||
| reconfiguration-flow: done | ||
| repair-issues: todo | ||
| stale-devices: todo | ||
|
|
||
| # Platinum | ||
| async-dependency: done | ||
| inject-websession: | ||
| status: exempt | ||
| comment: Integration does not use HTTP/websession (uses elkm1_lib for Elk panel protocol). | ||
| strict-typing: done | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why do we still have the import flow?