feat(knx): enable sensor creation via API #133979

philippwaller · 2024-12-24T23:36:45Z

Breaking change

NONE

Proposed change

This PR introduces the necessary backend changes to allow KNX sensors to be created and managed via the websocket API/frontend. While no UI changes are contributed yet, this only lays the technical foundation.

In addition, I’d like to suggest using EntityDescription objects as a general approach moving forward. This allows us to separate the entity logic from the configuration mapping (how it’s set up via YAML or the UI). By keeping these concerns apart, we make the code easier to maintain, reduce duplication, and ensure consistent behavior across different configuration methods.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

I’d be happy to implement the necessary UI changes as well. However, due to time constraints, it will likely take around two weeks before I can start working on them.

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant · 2024-12-24T23:36:53Z

Hey there @Julius2342, @farmio, @marvin-w, mind taking a look at this pull request as it has been labeled with an integration (knx) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of knx can trigger bot actions by commenting:

@home-assistant close Closes the pull request.
@home-assistant rename Awesome new title Renames the pull request.
@home-assistant reopen Reopen the pull request.
@home-assistant unassign knx Removes the current integration label and assignees on the pull request, add the integration domain after the command.
@home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
@home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

farmio · 2024-12-25T22:13:43Z

homeassistant/components/knx/entity.py

-            self._attr_device_info = DeviceInfo(identifiers={(DOMAIN, device_info)})
+
+        # If no entity description is defined, fall back to the entity_config dictionary
+        if entity_config and not getattr(self, "entity_description", None):


Suggested change

if entity_config and not getattr(self, "entity_description", None):

if entity_config and not hasattr(self, "entity_description"):

When we start using EntityDescription, I think in a separate PR, we should just move all existing UI configs to EntityDescription and remove this check as well as the last 2 positional arguments of __init__() instead of using 2 different ways of configuration.

farmio · 2024-12-25T22:22:56Z

homeassistant/components/knx/sensor.py

+from .const import ATTR_SOURCE, DOMAIN, KNX_MODULE_KEY
+from .entity import KnxUiEntity, KnxUiEntityPlatformController, KnxYamlEntity
+from .light import CONF_GA_STATE, CONF_SYNC_STATE
 from .schema import SensorSchema
+from .storage.const import CONF_ALWAYS_CALLBACK, CONF_DEVICE_INFO, CONF_ENTITY
+from .storage.entity_store_schema import CONF_GA_SENSOR, CONF_VALUE_TYPE


Suggested change

from .const import ATTR_SOURCE, DOMAIN, KNX_MODULE_KEY

from .entity import KnxUiEntity, KnxUiEntityPlatformController, KnxYamlEntity

from .light import CONF_GA_STATE, CONF_SYNC_STATE

from .schema import SensorSchema

from .storage.const import CONF_ALWAYS_CALLBACK, CONF_DEVICE_INFO, CONF_ENTITY

from .storage.entity_store_schema import CONF_GA_SENSOR, CONF_VALUE_TYPE

from .const import ATTR_SOURCE, CONF_SYNC_STATE, DOMAIN, KNX_MODULE_KEY

from .entity import KnxUiEntity, KnxUiEntityPlatformController, KnxYamlEntity

from .schema import SensorSchema

from .storage.const import (

CONF_ALWAYS_CALLBACK,

CONF_DEVICE_INFO,

CONF_ENTITY,

CONF_GA_SENSOR,

CONF_GA_STATE,

CONF_VALUE_TYPE,

)

farmio · 2024-12-25T22:23:38Z

homeassistant/components/knx/sensor.py

@@ -49,10 +55,10 @@ class KNXSystemEntityDescription(SensorEntityDescription):
    entity_category: EntityCategory = EntityCategory.DIAGNOSTIC
    has_entity_name: bool = True
    should_poll: bool = True
-    value_fn: Callable[[KNXModule], StateType | datetime] = lambda knx: None
+    value_fn: Callable[[KNXModule], StateType | datetime | None] = lambda knx: None


Suggested change

value_fn: Callable[[KNXModule], StateType | datetime | None] = lambda knx: None

value_fn: Callable[[KNXModule], StateType | datetime] = lambda knx: None

StateType already includes None

farmio · 2024-12-25T22:48:00Z

homeassistant/components/knx/sensor.py


        self._attr_device_info = knx.interface_device.device_info
        self._attr_should_poll = description.should_poll
        self._attr_translation_key = description.key
        self._attr_unique_id = f"_{knx.entry.entry_id}_{description.key}"

    @property
-    def native_value(self) -> StateType | datetime:
+    def native_value(self) -> StateType | datetime | None:


Suggested change

def native_value(self) -> StateType | datetime | None:

def native_value(self) -> StateType | datetime:

StateType already includes None

farmio · 2024-12-25T23:12:47Z

homeassistant/components/knx/sensor.py

+
+@dataclass(frozen=True, kw_only=True)
+class KnxSensorDescription(SensorEntityDescription):
+    """Class describing KNX system sensor entities."""


Suggested change

"""Class describing KNX system sensor entities."""

"""Class describing KNX sensor entities."""

farmio · 2024-12-25T23:48:04Z

homeassistant/components/knx/sensor.py

+        _state_class: SensorStateClass | None = (
+            SensorStateClass(state_class) if state_class else None
+        )
+
+        _entity_category: EntityCategory | None = (
+            EntityCategory(entity_category) if entity_category else None
+        )
+
+        _device_class: SensorDeviceClass | None = (
+            SensorDeviceClass(device_class) if device_class else None
+        )
+
+        if not _device_class:
+            _device_class = try_parse_enum(SensorDeviceClass, device.ha_device_class())


YAML config is already coerced to those StrEnum members by voluptuous. UI aren't. I'd move these to the ui-specific branch and have arguments state_class: SensorStateClass | None instead of state_class: str | None here.

farmio · 2024-12-25T23:50:27Z

homeassistant/components/knx/sensor.py

+        _device_info: DeviceInfo | None = (
+            DeviceInfo(identifiers={(DOMAIN, device_info)}) if device_info else None
+        )


YAML doesn't support devices, so move to UI-specific branch and set default in arguments here device_info: DeviceInfo | None = None

farmio · 2024-12-25T23:55:42Z

homeassistant/components/knx/storage/const.py

+CONF_STATE_CLASS: Final = "state_class"
+CONF_DEVICE_CLASS: Final = "device_class"


state_class can be imported from homeassistant.components.sensor.const and device_class from homeassistant.const

farmio · 2024-12-26T00:03:12Z

homeassistant/components/knx/storage/entity_store_schema.py

+        vol.Optional(CONF_STATE_CLASS, default=None): vol.Maybe(str),
+        vol.Optional(CONF_DEVICE_CLASS, default=None): vol.Maybe(str),


state_class and device_class are sensor platform specific - so should not be added to BASE_ENTITY_SCHEMA and should be validated by their respective schema instead of str.

farmio · 2024-12-26T00:12:35Z

homeassistant/components/knx/storage/entity_store_schema.py

+            vol.Required(CONF_GA_SENSOR): GASelector(state_required=True),
+            vol.Required(CONF_VALUE_TYPE): str,


Suggested change

vol.Required(CONF_GA_SENSOR): GASelector(state_required=True),

vol.Required(CONF_VALUE_TYPE): str,

vol.Required(CONF_GA_SENSOR): GASelector(

write=False,

state_required=True,

dpt=[],

),

The interesting thing for UI configurable sensor entities will be how to pass and validate available dpt from xknx.dpt up to the UI and back.
value_type can be omitted then.

home-assistant · 2024-12-26T00:20:36Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

farmio · 2024-12-26T00:24:05Z

homeassistant/components/knx/sensor.py

+    device_info: DeviceInfo | None = None
+
+
+class KnxSensorDescriptionFactory:


since this only holds @staticmethod, it can be either omitted and moved to plain module functions, or if you prefer, moved to methods or classmethods of KnxSensorDescription

farmio · 2024-12-26T16:44:48Z

Hi 👋!
Thank you very much for your contribution!

In addition, I’d like to suggest using EntityDescription objects as a general approach moving forward. (...) ensure consistent behavior across different configuration methods.

Just a small remark: I don't think it is necessary to keep YAML and UI entity behaviour 100% consistent. If there are features for UI entities that aren't possible or allowed for YAML then that is fine for me (eg. HA devices).

I’d be happy to implement the necessary UI changes as well. However, due to time constraints, it will likely take around two weeks before I can start working on them.

Would it make sense to build both, the backend and frontend, before merging into HA Core?
If you need any info, help or would like to exchange ideas, feel free to reach out. I'm available on Discord (HA or xknx servers @_farmio) or somewhere on xknx Github 😉

feat(knx): enable sensor creation via API

ad6a883

philippwaller requested review from farmio and marvin-w as code owners December 24, 2024 23:36

home-assistant bot added cla-signed has-tests integration: knx new-feature labels Dec 24, 2024

home-assistant bot assigned farmio and marvin-w Dec 24, 2024

home-assistant bot added the Quality Scale: No score label Dec 24, 2024

fix(knx): correct failing test cases

2b2fc48

farmio requested changes Dec 26, 2024

View reviewed changes

home-assistant bot marked this pull request as draft December 26, 2024 00:20

farmio reviewed Dec 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(knx): enable sensor creation via API #133979

feat(knx): enable sensor creation via API #133979

philippwaller commented Dec 24, 2024

home-assistant bot commented Dec 24, 2024

farmio Dec 25, 2024

farmio Dec 25, 2024

farmio Dec 25, 2024

farmio Dec 25, 2024

farmio Dec 25, 2024

farmio Dec 25, 2024

farmio Dec 25, 2024

farmio Dec 25, 2024

farmio Dec 26, 2024 •

edited

Loading

farmio Dec 26, 2024

home-assistant bot commented Dec 26, 2024

farmio Dec 26, 2024

farmio commented Dec 26, 2024

	if entity_config and not getattr(self, "entity_description", None):
	if entity_config and not hasattr(self, "entity_description"):

	value_fn: Callable[[KNXModule], StateType \| datetime \| None] = lambda knx: None
	value_fn: Callable[[KNXModule], StateType \| datetime] = lambda knx: None

	def native_value(self) -> StateType \| datetime \| None:
	def native_value(self) -> StateType \| datetime:

	"""Class describing KNX system sensor entities."""
	"""Class describing KNX sensor entities."""

		CONF_STATE_CLASS: Final = "state_class"
		CONF_DEVICE_CLASS: Final = "device_class"

		vol.Optional(CONF_STATE_CLASS, default=None): vol.Maybe(str),
		vol.Optional(CONF_DEVICE_CLASS, default=None): vol.Maybe(str),

		vol.Required(CONF_GA_SENSOR): GASelector(state_required=True),
		vol.Required(CONF_VALUE_TYPE): str,

		device_info: DeviceInfo \| None = None


		class KnxSensorDescriptionFactory:

feat(knx): enable sensor creation via API #133979

Are you sure you want to change the base?

feat(knx): enable sensor creation via API #133979

Conversation

philippwaller commented Dec 24, 2024

Breaking change

Proposed change

Type of change

Additional information

Checklist

home-assistant bot commented Dec 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

farmio Dec 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

home-assistant bot commented Dec 26, 2024

Choose a reason for hiding this comment

farmio commented Dec 26, 2024

farmio Dec 26, 2024 •

edited

Loading