feat(knx): Enable KNX Sensor Setup with a Schema-Driven Approach #136293
Conversation
|
Hey there @Julius2342, @farmio, @marvin-w, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Note: This class was moved.
Only difference is self.entry.async_on_unload(self.entry.add_update_listener(async_update_entry)) which is now done in async_setup_entry().
Please avoid mixing big refactoring / code moving changes and new feature in PRs in the future. Having multiple smaller PRs makes it easier to review - especially if a reviewer is not familiar with the integration code.
There was a problem hiding this comment.
I agree with you. The change was necessary due to a cyclic dependency. Otherwise, it wouldn’t have been possible to access the config object in all required locations.
| prop: GroupAddressConfig.from_dict(data[prop]) | ||
| if isinstance(data[prop], dict) |
There was a problem hiding this comment.
A ConfigGroup can consist of more than just GroupAddressConfig instances, can't it?
|
|
||
| return { | ||
| "type": "group_address_config", | ||
| "properties": convert(value.build_schema()), |
There was a problem hiding this comment.
the resulting schema looks quite complex
{
"type": "group_address_config",
"properties": [
{
"type": "group_address",
"allow_none": false,
"allow_internal_address": true,
"name": "state",
"required": true
},
{
"type": "group_address_list",
"items": {
"type": "group_address",
"allow_none": false,
"allow_internal_address": true
},
"name": "passive",
"optional": true,
"default": []
},
{
"type": "dpt",
"properties": [
{
"name": "main",
"required": true
},
{
"allow_none": true,
"name": "sub",
"optional": true,
"default": null
}
],
"options": [
...
],
"name": "dpt",
"required": true
}
],
"name": "ga_sensor",
"required": true
},for a selector that is already working in frontend that expect something like (written out of the head - have not checked)
{
"type": "knx_ga_schema",
"write": false,
"write_required": false,
"state": true,
"state_required": true,
"passive": true,
"dpt": [
...
],
"name": "ga_sensor",
"required": true,
}Since frontend GA selectors are custom anyway, I don't think we have to overcomplicate these.
As resulting config we need not more than eg.
"ga_switch": {
"write": "1/1/45",
"state": "1/0/45",
"passive": []
"dpt": "5.001"
},There was a problem hiding this comment.
My idea was to design all objects in a uniform way, so that additional functions—like generating translation keys based on the schema's structure—can work without needing special handling for different types.
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
|
Here's the latest update: To ensure we provide all necessary translations according to the schema, I’ve added unit tests that confirm: All required translations exist, and no unnecessary translations are left dangling in the strings file. However, there’s a caveat: So, my question is: PS: I added a to-do list to the PR description to transparently track progress. |
|
I sent you some messages on Discord regarding translations. Imho the whole translation topic would best be handled separately in a follow-up PR. |
…anslations remain in sync
philippwaller
force-pushed
the
feat/knx2
branch
from
55ffe9a to
886bb44
Compare
Breaking change
Proposed change
Overview
This PR is a follow-up to PR #133979 and focuses on enabling the creation of a KNX sensor directly from the Home Assistant UI. It leverages and demonstrates a new, schema-driven approach to manage KNX device configurations more flexibly. Instead of maintaining extensive, hard-coded metadata in both the integration and the frontend, the goal is to define the data schema in the backend and expose it to the UI via voluptuous-serialize. This eliminates the need for frequent frontend adaptations and accelerates the transition from YAML to a modern UI-driven KNX integration.
Problem Statement
To deliver great user experience for managing KNX UI entities, we need a significant amount of metadata—such as required fields or pre-populated dropdown values. Currently, adding new KNX devices necessitates changes in both the integration and the frontend. This creates unnecessary complexity and slows down the shift toward a fully UI-based configuration model.
While additional endpoints could technically provide this metadata, they add considerable integration overhead. We need a more elegant, scalable way to define and share schema details between the integration and the UI.
Proposed Solution
Backend-Centric Schema Definition
Define the data schema (and relevant metadata) in the KNX integration itself.
Serialization for the Frontend
Use voluptuous-serialize to publish the schema to the KNX frontend. This allows dynamic UI rendering—such as automatic form generation, required-field indicators, dropdown options, and field grouping. (see get_entity_schemas_response.json)
UI Rendering from Schema
Let the frontend derive its structure from the provided schema, eliminating the need for repeated manual updates whenever new device types are introduced (as long as the UI supports the schema definitions).
Device Configuration Objects
Introduce a config object for each device type (e.g., SensorConfig) that can serialize and deserialize data from YAML, UI input, and a standardized internal format. By abstracting these data sources behind a single object, cosmetic changes to the UI schema—like moving a field from “basic” to “advanced”—won’t affect the core data structure.
Type of change
Additional information
Checklist
ruff format 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.To help with the load of incoming pull requests:
Open Tasks: