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.

Got your point. That means DPT is a mandatory field when it comes to a sensor, right?

The group address selector does return it this way. It is not yet tuned to support such many DPTs though.

I was also thinking of serializing all selectors (including valid dpts for each key - which are many for sensor 🙃) to pass it to the frontend (like https://github.com/home-assistant-libs/voluptuous-serialize) so the schema is only hardcoded in the backend, and doesn't have to be replicated in the frontend - it didn't happen yet though.

I believe it makes the most sense to handle mapping, exposing, and validation of DPTs directly within the xknx library. I've created a PR XKNX/xknx#1629 to address this, aiming to centralize the KNX-specific logic in one repository. What are your thoughts?

left a comment there as well. HA SensorEntity has different constraints than xknx Sensor.
I'd like to keep HA specifics in the HA knx integration and move the few things we still have in xknx out of there in the long run. Eg. ha_device_class should move into the integration, as well as unit of measurement so we can use HA enums and add unit translations etc. one day

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 😉

philippwaller · 2024-12-31T15:55:26Z

Thanks @farmio for your review! I’m currently on Christmas/New Year leave, but I’ll take a closer look at your proposals in the coming days.

I see the factory pattern as a good way to keep configuration and entity logic separate. Even if, down the road, only UI-based configuration will be supported, this approach helps maintain a clear distinction between the configuration store and the platform logic.

Entity objects would only need changes when their behavior actually evolves, which I’d expect to happen rarely—especially in something stable like the KNX integration.

The factory classes, on the other hand, act as a bridge, bundling the integration-specific business logic. They define how entities, devices, or other components are created from the configuration data specific to that integration.

That said, I’d expect the factory classes themselves to undergo more frequent adjustments until the migration from YAML-based configuration to UI-based configuration is fully completed.

I agree, let’s merge everything together once we have a clear overview of both the UI and backend changes.

Guten Rutsch!

farmio · 2025-01-08T18:18:03Z

As a side note: it may be useful to have a WS-api endpoint to read valid sensor types - in some form - from the integration. When we have this information in a store globally available in the knx-frontend (like project information), we can not only use for sensor creation dialog, but also match DPTs on the project panel to provide some kind of menu for creating sensors / binary sensors from there - given the DPT is supported.

philippwaller · 2025-01-22T22:49:17Z

Closed in favor of PR #136293.

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

philippwaller force-pushed the feat/knx branch from f7e98e4 to 2b2fc48 Compare January 3, 2025 14:17

philippwaller mentioned this pull request Jan 5, 2025

Add support for exposing and validating sensor-compatible DPT classes XKNX/xknx#1629

Closed

7 tasks

ehn mentioned this pull request Jan 21, 2025

Shutter / HVAC devices missing in KNX entities XKNX/knx-integration#17

Closed

philippwaller mentioned this pull request Jan 22, 2025

feat(knx): Enable KNX Sensor Setup with a Schema-Driven Approach #136293

Draft

25 tasks

philippwaller closed this Jan 22, 2025

github-actions bot locked and limited conversation to collaborators Jan 23, 2025

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

philippwaller Jan 4, 2025

farmio Jan 4, 2025

philippwaller Jan 5, 2025

farmio Jan 5, 2025 •

edited

Loading

home-assistant bot commented Dec 26, 2024

farmio Dec 26, 2024

farmio commented Dec 26, 2024

philippwaller commented Dec 31, 2024

farmio commented Jan 8, 2025

philippwaller commented Jan 22, 2025

	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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

farmio Jan 5, 2025 • edited Loading

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

philippwaller commented Dec 31, 2024

farmio commented Jan 8, 2025

philippwaller commented Jan 22, 2025

farmio Dec 26, 2024 •

edited

Loading

farmio Jan 5, 2025 •

edited

Loading