XKNX / XKNX/xknxtoolkit

[Detail Bug] KNX GUI: Invalid individual address entry silently desyncs UI/device cache from persisted project state

Open
#76 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
4
Forks
0
Avg merge
15h 38m
Merged PRs (30d)
37

Description

Detail Bug Report

https://app.detail.dev/org_62aa40f5-2c23-4914-a665-3bb2068af20e/bugs/bug_8ae42490-2851-4494-a2e0-d33b56088d14

Introduced in 82f9642b8cdf7a2a782e3c714bd18b654c9b7b30 by @kewde on Jun 5, 2026

Summary

  • Context: ProjectPlugin._handle_individual_address_change reconciles a user-entered individual-address (IA) change typed into the Configure panel with the persisted project state by calling ProjectService.set_device_individual_address.
  • Bug: The plugin mutates the cache-shared Device.individual_address to the new value before invoking the persistence layer; when the persistence layer rejects the address the error is swallowed (logged only, no _bump) and the local mutation is never rolled back.
  • Actual vs. expected: After a rejected IA change the GUI keeps showing — and downstream code keeps using — the rejected address; expected: the device's IA should revert to the value the project actually persisted.
  • Impact: The displayed device address silently desyncs from the project DB; subsequently, programming and restart operations drive the KNX bus with the bogus address, putting the bus and the project into inconsistent state.

Code with Bug

apps/knx-gui/src/knx_gui/plugins/project/plugin.py

def _handle_individual_address_change(
    self, device: "Device", new_address: str
) -> None:
    old_address = device.individual_address
    if old_address != new_address:
        device.individual_address = new_address                          # <-- BUG 🔴 mutates cache-shared Device before persistence
        self._api.project.set_device_individual_address(
            device.node_id, old_address, new_address                    # <-- BUG 🔴 caller has no signal when persistence rejects
        )

apps/knx-gui/src/knx_gui/plugins/project/service.py

def set_device_individual_address(
    self, node_id: int, old_address: str, new_address: str
) -> None:
    if self._pid is None or old_address == new_address:
        return
    try:
        self._svc.set_individual_address(self._pid, node_id, new_address)
    except (KeyError, ValueError) as e:
        self._log.warning(
            "could not set individual address", address=new_address, error=str(e)
        )
        return                       # <-- BUG 🔴 swallowed error: no return-value signal and no _bump()
    self._bump()

Explanation

  • device is the same object stored in ProjectService._devices_cache; _handle_individual_address_change mutates it in place.
  • When xknxmono rejects an IA whose (area, line) does not exist, ProjectService.set_device_individual_address catches KeyError/ValueError, logs, and returns without calling _bump().
  • Because _bump() is not called, the version-gated cache is not invalidated; subsequent reads keep returning the same mutated Device, so the rejected IA persists in-memory and in the Configure panel.
  • End-to-end repro against the real DB (demo.xknx) confirms DB remains unchanged (e.g. '1.1.4') while the in-memory/cache device shows the rejected value (e.g. '1.5.5').

Recommended Fix (optional, only if obvious)

  • Only update device.individual_address after persistence succeeds.
  • Make ProjectService.set_device_individual_address signal failure (re-raise or return False) so the plugin can avoid mutating (or can roll back) on rejection.

History

This bug was introduced in commit 82f9642. The "Rebuild the GUI project plugin on xknxmono.project" rewrite replaced the in-tree event-store with a thin adapter over xknxmono.project, and in doing so introduced both halves of the desync: it copied the existing "mutate device.individual_address before calling the persistence layer" pattern (carried over verbatim from the earlier 357f004 "make individual address editable" feature) and, for the first time, wrapped the new xknxmono set_individual_address call in a try/except (KeyError, ValueError) that logs and returns without calling _bump(). Because the rewrite also moved device reads behind a version-gated cache (_devices_cache + _bump()), a swallowed error now leaves the in-memory Device mutated with no cache invalidation, so the corrupted value is served on every subsequent read — the silent DB/cache desync the report describes.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with _handle_individual_address_change in apps/knx-gui/src/knx_gui/plugins/project/plugin.py and set_device_individual_address in apps/knx-gui/src/knx_gui/plugins/project/service.py. Reproduce a rejected individual address such as 1.5.5 against demo.xknx, then verify the GUI and cached Device retain the persisted address rather than the rejected value, while successful changes still persist.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, desktop
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.