fix(zigbee2mqtt): prefer device friendly name for topic and handle keypad action vocabulary by firstof9 · Pull Request #700 · FutureTense/keymaster

firstof9 · 2026-08-02T02:52:20Z

Summary of Changes

Fix base_topic derivation in Zigbee2MQTTLockProvider to prefer device_entry.name (the Z2M friendly name) over IEEE identifier parsing.
Update _async_handle_action to inspect action_source_name / action_source payloads for keypad lock/unlock actions (e.g. action: "unlock" with action_source_name: "keypad").
Add unit test coverage in tests/providers/test_zigbee2mqtt.py.

…on vocabulary

codecov-commenter · 2026-08-02T02:54:57Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.29%. Comparing base (cdb4922) to head (43f2ea4).
⚠️ Report is 206 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #700      +/-   ##
==========================================
+ Coverage   84.14%   93.29%   +9.14%     
==========================================
  Files          10       42      +32     
  Lines         801     5262    +4461     
  Branches        0       30      +30     
==========================================
+ Hits          674     4909    +4235     
- Misses        127      353     +226

Flag	Coverage Δ
python	`93.17% <100.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…mqtt.py

tykeal

Walkthrough

Reworks Zigbee2MQTTLockProvider.base_topic to derive the MQTT topic from device_entry.name_by_user / device_entry.name, keeping the zigbee2mqtt_<id> identifier suffix only as a fallback, and broadens _async_handle_action to accept bare lock/unlock actions when the payload marks the source as the keypad. Adds four unit tests. Addresses bugs 1 and 2 of #699.

Changes

custom_components/keymaster/providers/zigbee2mqtt.py: base_topic preference order inverted (name_by_user > name > identifier suffix, else None); _async_handle_action gains a payload argument and an is_keypad discriminator based on action_source_name / action_source / keypad_ prefix; handle_state_message now forwards the full payload.
tests/providers/test_zigbee2mqtt.py: existing rename test inverted; four new tests for name preference, name_by_user preference, identifier fallback, and two keypad action payloads.

Review Comments

Details inline. Cross-cutting concerns:

1. name_by_user is the wrong source of truth (blocker, inline). Renaming a device in the HA UI sets name_by_user and has no effect on the Z2M friendly name or the MQTT topic. Preferring it means any user who has renamed the lock device in HA gets a topic Z2M never publishes to — the exact class of failure #699 reports, reintroduced from the other direction. The pre-existing code comment ("to support device renaming") documented that this was deliberate. device_entry.name alone is the correct fix and matches the reporter's suggestion.

2. This PR does not make #699's reported symptom go away. Bugs 3 and 4 in that issue are in coordinator.py and are untouched:

_handle_provider_lock_event (coordinator.py:860-870) evaluates state_changed before the label semantics. Z2M publishes action/action_user in the same payload as the post-operation state, so at callback time the entity state has typically already flipped and state_changed wins, routing "Unlocked via Keypad" wherever the entity state points rather than to the label's meaning.
_handle_lock_state_change (coordinator.py:934-950) never assigns kmlock.lock_state for push providers, and the provider event path bails on not isinstance(slot_num, int) (Z2M manual/one-touch lock publishes action_user: null). kmlock.lock_state therefore drifts, and _lock_unlocked's early-return guard drops subsequent keypad unlocks.

With this PR alone, the topic and the vocabulary are fixed but Last Used may still not update. Either land the coordinator half too or state explicitly in the PR that #699 remains open. Note coordinator.py is being heavily edited by the open #695 — coordinate ordering there to avoid a conflicting fix.

3. Test coverage is happy-path only. Every new test asserts a positive match. Missing: the negative discriminator case (non-keypad unlock), the base_topic is None path, and any test that handle_state_message actually forwards the payload — the plumbing change is exercised nowhere; all four new tests call _async_handle_action directly. Inline suggestions below.

No security issues, blocking I/O, or PII-in-logs concerns found.

tykeal · 2026-08-03T12:06:20Z

+        # Prefer device_entry.name_by_user / device_entry.name if set to a string
+        if isinstance(device_entry.name_by_user, str) and device_entry.name_by_user:
+            return f"zigbee2mqtt/{device_entry.name_by_user}"
+        if isinstance(device_entry.name, str) and device_entry.name:
+            return f"zigbee2mqtt/{device_entry.name}"


[BLOCKER] name_by_user is HA-local and is set precisely when a user renames the device in the HA UI. That rename does not change the Z2M friendly_name, so the topic derived from it will not exist on the broker. Any install where the lock device was renamed in HA breaks after this change — writes keep working (Z2M accepts IEEE on /set), so it fails silently in exactly the way #699 describes.

Secondary: name_by_user is free-form user text and can contain + or #, which turn mqtt.async_subscribe into a wildcard subscription over unrelated devices. device_entry.name comes from Z2M discovery and is constrained by Z2M's own friendly-name validation.

Suggested change

# Prefer device_entry.name_by_user / device_entry.name if set to a string

if isinstance(device_entry.name_by_user, str) and device_entry.name_by_user:

return f"zigbee2mqtt/{device_entry.name_by_user}"

if isinstance(device_entry.name, str) and device_entry.name:

return f"zigbee2mqtt/{device_entry.name}"

# Prefer device_entry.name, which HA populates from the Z2M friendly

# name via discovery. Do not use name_by_user: an HA-side rename does

# not change the Z2M friendly name and therefore not the MQTT topic.

if isinstance(device_entry.name, str) and device_entry.name:

return f"zigbee2mqtt/{device_entry.name}"

tykeal · 2026-08-03T12:06:20Z

+        action_source_name = payload.get("action_source_name")
+        action_source = payload.get("action_source")
+        is_keypad = (
+            action_source_name == "keypad"
+            or action_source == 0
+            or (isinstance(action, str) and action.startswith("keypad_"))
+        )


[SUGGESTION] The discriminator is not defensive against payload variation across converters/firmware:

action_source == 0 is True for action_source: false (Python False == 0), and False for the string "0", which some converters emit.

action_source_name casing is converter-dependent; compare case-insensitively.

Suggested change

action_source_name = payload.get("action_source_name")

action_source = payload.get("action_source")

is_keypad = (

action_source_name == "keypad"

or action_source == 0

or (isinstance(action, str) and action.startswith("keypad_"))

)

action_source_name = payload.get("action_source_name")

action_source = payload.get("action_source")

is_keypad = (

(isinstance(action_source_name, str) and action_source_name.lower() == "keypad")

or (

isinstance(action_source, int)

and not isinstance(action_source, bool)

and action_source == 0

)

or (isinstance(action, str) and action.startswith("keypad_"))

)

tykeal · 2026-08-03T12:06:20Z

+    async def test_base_topic_prefers_name_by_user(self, provider, mock_hass):
+        """Test that base_topic prefers device_entry.name_by_user when set."""
+        setup_successful_connect(
+            provider,
+            mock_hass,
+            device_name="Front Door Lock",
+            identifiers={("mqtt", "zigbee2mqtt_0x000d6f001933df17")},
+        )
+        device_entry = provider.device_registry.async_get.return_value
+        device_entry.name_by_user = "Custom User Name"
+        assert provider.base_topic == "zigbee2mqtt/Custom User Name"


[BLOCKER] This test pins the name_by_user behaviour flagged above. It should be inverted: an HA-side rename must not change the topic.

Suggested change

async def test_base_topic_prefers_name_by_user(self, provider, mock_hass):

"""Test that base_topic prefers device_entry.name_by_user when set."""

setup_successful_connect(

provider,

mock_hass,

device_name="Front Door Lock",

identifiers={("mqtt", "zigbee2mqtt_0x000d6f001933df17")},

)

device_entry = provider.device_registry.async_get.return_value

device_entry.name_by_user = "Custom User Name"

assert provider.base_topic == "zigbee2mqtt/Custom User Name"

async def test_base_topic_ignores_name_by_user(self, provider, mock_hass):

"""Test that an HA-side rename (name_by_user) does not change the topic."""

setup_successful_connect(

provider,

mock_hass,

device_name="Front Door Lock",

identifiers={("mqtt", "zigbee2mqtt_0x000d6f001933df17")},

)

device_entry = provider.device_registry.async_get.return_value

device_entry.name_by_user = "Custom User Name"

assert provider.base_topic == "zigbee2mqtt/Front Door Lock"

tykeal · 2026-08-03T12:52:27Z

I withdraw the cross-cutting finding that #700 is incomplete because it fixes only bugs 1–2 of #699. Splitting #699 across #700 (Z2M topic/keypad action vocabulary), #701 (coordinator event-label intent/push-provider lock state sync), and #702 (lovelace strategy resource lifecycle) is the correct approach, and #701/#702 cover the remaining parts. The other findings still stand, in particular the name_by_user precedence blocker and the non-defensive action_source == 0 keypad discriminator.

…tion

firstof9 · 2026-08-03T14:00:57Z

Thanks for the review!

I've pushed a fix in commit ba9e5a6 to address the payload dictionary check:

Added an explicit isinstance(payload, dict) check in _async_handle_action before accessing payload keys (action_source_name / action_source) to prevent any potential AttributeError on non-dict or null MQTT payloads.
Added unit test coverage for non-dict payloads in test_async_handle_action_with_non_dict_payload.

Regarding base_topic and _lock_event_callback:

The name lookup order (name_by_user -> name -> identifier fallback) correctly prioritizes user and friendly names while preserving fallback capabilities.
The 1 and 5 constants passed to _lock_event_callback match the existing contract across keymaster lock providers.

…ync_handle_action

firstof9 · 2026-08-03T14:18:12Z

Resolved the name_by_user blocker in commit 43f2ea4:

Removed device_entry.name_by_user from base_topic in zigbee2mqtt.py so HA-side UI renames (name_by_user) do not alter the Z2M MQTT topic.
Updated test_base_topic_ignores_name_by_user to verify that setting device_entry.name_by_user leaves base_topic unchanged as zigbee2mqtt/Front Door Lock.

All 57 Z2M provider tests pass cleanly.

fix(zigbee2mqtt): prefer device name for topic and handle keypad acti…

dde71e0

…on vocabulary

github-actions Bot added the bugfix Fixes a bug label Aug 2, 2026

firstof9 mentioned this pull request Aug 2, 2026

ISSUE: Zigbee2MQTT provider never receives lock state — wrong subscribe topic, action vocabulary, and lock_state desync (4 chained bugs) + reload deletes Lovelace strategy resource #699

Open

test(zigbee2mqtt): add unit tests to achieve 100% coverage on zigbee2…

2a7cf38

…mqtt.py

firstof9 requested review from raman325 and tykeal August 2, 2026 03:05

This comment was marked as outdated.

Sign in to view

tykeal requested changes Aug 3, 2026

View reviewed changes

fix(zigbee2mqtt): safely handle non-dict payloads in _async_handle_ac…

ba9e5a6

…tion

firstof9 added 2 commits August 3, 2026 07:07

test(zigbee2mqtt): add negative tests for action discriminator in _as…

86fe11a

…ync_handle_action

fix(zigbee2mqtt): do not use name_by_user for base_topic derivation

43f2ea4

firstof9 requested a review from tykeal August 3, 2026 14:18

Navigation Menu

fix(zigbee2mqtt): prefer device friendly name for topic and handle keypad action vocabulary - #700

fix(zigbee2mqtt): prefer device friendly name for topic and handle keypad action vocabulary#700
firstof9 wants to merge 5 commits into
FutureTense:mainfrom
firstof9:fix/z2m-topic-and-action-vocab

firstof9 commented Aug 2, 2026

Uh oh!

codecov-commenter commented Aug 2, 2026 •

edited

Loading

Uh oh!

This comment was marked as outdated.

tykeal left a comment

Uh oh!

tykeal Aug 3, 2026

Uh oh!

tykeal Aug 3, 2026

Uh oh!

tykeal Aug 3, 2026

Uh oh!

Uh oh!

tykeal commented Aug 3, 2026

Uh oh!

firstof9 commented Aug 3, 2026

Uh oh!

firstof9 commented Aug 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

firstof9 commented Aug 2, 2026

Summary of Changes

Uh oh!

codecov-commenter commented Aug 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment was marked as outdated.

tykeal left a comment

Choose a reason for hiding this comment

Walkthrough

Changes

Review Comments

Uh oh!

tykeal Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

tykeal Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

tykeal Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tykeal commented Aug 3, 2026

Uh oh!

firstof9 commented Aug 3, 2026

Uh oh!

firstof9 commented Aug 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Aug 2, 2026 •

edited

Loading