Skip to content

Fix KNX telegram history migration for data of 2026.3 and earlier#175396

Merged
farmio merged 2 commits into
home-assistant:devfrom
martinhoefling:fix-knx-telegram-migration-fallback
Jul 2, 2026
Merged

Fix KNX telegram history migration for data of 2026.3 and earlier#175396
farmio merged 2 commits into
home-assistant:devfrom
martinhoefling:fix-knx-telegram-migration-fallback

Conversation

@martinhoefling

@martinhoefling martinhoefling commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Breaking change

None

Proposed change

When migrating old KNX telegram data from JSON -> SQLite DB, fall back to default values. We introduced data_secure field in 2026.3 and this change guards against missing fields like this.

Type of change

Migration fix.

  • 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

Checklist

  • I understand the code I am submitting and can explain how it works.
  • 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][dev-checklist]
  • I have followed the [perfect PR recommendations][perfect-pr]
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.
  • Any generated code has been carefully reviewed for correctness and compliance with project standards.

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

  • Documentation added/updated for [www.home-assistant.io][docs-repository]

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

  • The [manifest file][manifest-docs] 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 diff between library versions and ideally a link to the changelog/release notes is added to the PR description.

To help with the load of incoming pull requests:

Copilot AI review requested due to automatic review settings July 2, 2026 19:57
@home-assistant

home-assistant Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

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 mark-draft Mark the pull request as draft.
  • @home-assistant ready-for-review Remove the draft status from 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 update-branch Update the pull request branch with the base branch.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component, problem in config, problem in device, feature-request) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component, problem in config, problem in device, feature-request) on the pull request.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a setup-blocking KeyError: 'data_secure' in the KNX integration (issue #175346) that occurs when migrating pre-existing telegram history from the legacy JSON store to the newer SQLite store. The data_secure field was introduced in 2026.3, so telegram history saved by older Home Assistant versions lacks that key (and potentially other newer fields). The fix makes dict_to_model tolerant of missing fields by falling back to sensible defaults, and adds a regression test.

Changes:

  • dict_to_model now uses t.get(...) with defaults for the newer/optional fields (value, payload, dpt_main, dpt_sub, source_name, destination_name, data_secure) instead of hard key access, so legacy entries migrate gracefully.
  • Core fields that have always existed (timestamp, source, destination, direction, telegramtype) still use direct key access.
  • Adds test_migrate_telegrams_json_missing_keys to verify that a legacy telegram missing the newer fields migrates with the expected default values.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
homeassistant/components/knx/telegrams.py Uses .get() with defaults for optional/newer telegram fields during model conversion to avoid KeyError on legacy JSON migration.
tests/components/knx/test_telegrams_migration.py Adds a regression test that migrates a legacy telegram missing newer fields and asserts default values are applied.

The change is minimal, targeted, and correct: dict_to_model is invoked both for live telegrams (which always contain every key via telegram_to_dict) and for migration (legacy dicts that may lack newer keys), and the .get() defaults do not alter the live path. The defaults align with the model typing and the reporter's suggested fix, and the new test covers the reported scenario. I found no blocking issues.

dpt_sub=t.get("dpt_sub"),
source_name=t.get("source_name", ""),
destination_name=t.get("destination_name", ""),
data_secure=t.get("data_secure", False),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only missing when migrating data from old HA instances. The other keys should always be there.
Since this is quite a hot path in our code (every telegram received will go throughdict_to_model() on reception) I think we should not be too wary with that.

As an alternative that only effects migration, not live data, we could do in migrate_telegrams()

stored_telegrams = [self.dict_to_model({"data_secure": False} | t) for t in json_data]
# or
stored_telegrams = []
for t in json_data:
    t.setdefault("data_secure": False)  # was added in 2026.3
    stored_telegrams.append(self.dict_to_model(t))

@farmio farmio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway, the fix will work and I don't know if there is any reasonable speed penalty for those get() calls. So fine for me, we have a test case and can always do speed improvements later.

@martinhoefling

Copy link
Copy Markdown
Contributor Author

I think you have a fair point, probably less for speed reasons but in the production code, it could mask other issues. All other migrations should be handeled by telegram store. Let's see if we can move the change.

@martinhoefling martinhoefling force-pushed the fix-knx-telegram-migration-fallback branch from 59cc9f0 to 3f9d28f Compare July 2, 2026 21:37
Copilot AI review requested due to automatic review settings July 2, 2026 21:40
@martinhoefling martinhoefling force-pushed the fix-knx-telegram-migration-fallback branch from 3f9d28f to 17a049d Compare July 2, 2026 21:40
@martinhoefling martinhoefling force-pushed the fix-knx-telegram-migration-fallback branch from 17a049d to 82c334b Compare July 2, 2026 21:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment on lines +342 to +343
# Legacy JSON data from older HA instances might miss fields added later
# (e.g., data_secure was added in 2026.3, value, payload, dpt_main/sub, names might also be missing)
Copilot AI review requested due to automatic review settings July 2, 2026 21:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment on lines +155 to +156
with pytest.raises(KeyError, match="dpt_main"):
telegrams_module.dict_to_model(incomplete_dict)
@martinhoefling martinhoefling force-pushed the fix-knx-telegram-migration-fallback branch from 82c334b to 9ed406d Compare July 2, 2026 21:52
Copilot AI review requested due to automatic review settings July 2, 2026 21:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@farmio farmio left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much! 👍

@farmio farmio changed the title Fix knx telegram migration fallback, fixes #175346 Fix KNX telegram history migration for data of 2026.3 and earlier Jul 2, 2026
@farmio farmio merged commit 3d01a6c into home-assistant:dev Jul 2, 2026
33 checks passed
@frenck frenck mentioned this pull request Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

KNX integration fails to set up with KeyError: 'data_secure' during telegram history migration

5 participants