Generic Zigbee 3.0 device driver

[derek-miller]

Summary

Re-baselines the repo so main is a clean control4-driver-template v0.9.16 render, and presents the full generic Zigbee 3.0 driver as this PR. The prior 54-commit history is preserved at the archive/pre-restructure tag.

• Generic device driver (feat: generic Zigbee 3.0 device driver): one driver for any joined Zigbee 3.0 device, with per-device support generated from zigbee2mqtt (zigbee-herdsman-converters). The generator maps z2m exposes to Control4 capability descriptors; the shared ZCL runtime and generic zigbee3_device driver resolve the joined model and build its sensors, actuators (relay/light/lock), buttons/remotes, settings, and alerts. Companion Light and Lock drivers present the actuator proxies.
• Real-time event binding (feat: real-time event binding via zbind): adds src/zigbee3/zbind.lua so unsolicited reports and commands (button presses, on/off state, lock operation events, measurements) are delivered live through the device's binding table, established over zserver's public API. The bind set is derived generically from each device's resolved capabilities (Device:bindTargets); IAS security sensors are excluded because they use Zone enrollment. Best-effort throughout: any failure falls back to the existing reads and poll.

Test plan

• make test (53 passing, including a 10-case zbind bootstrap/verify test)
• make build-nodocs (all three drivers squish and parse)
• Join a button/remote (e.g. Shelly zb button) and confirm presses fire Control4 events live via zbind
• Confirm a sensor (motion/contact) still reports and an actuator (light/relay/lock) reflects external state changes

[svc-finitelabs]

Reviewed at 3f2ddf6. Reproduced locally: make test gives 53 passed, 0 failed, matching the test plan. make build-nodocs squishes and parses all three drivers.

On the red check first: build is failing, but not on this PR. Steps 1 through 12 (Check formatting, Test, Build, Verify PDFs, Check for dirty tree) are all green; only Upload drivercentral artifacts and Upload oss artifacts failed, both with Failed to CreateArtifact: Artifact storage quota has been hit. That is an org-level storage condition, so it needs artifact cleanup rather than a code change, and it will re-red every run until the quota recalculates.

src/zigbee3/zbind.lua already carries everything that converged on zigbee3-kwikset#9: bindKey(srcEp, cluster), the relaxed (dstEp == nil or dstEp == gwEp) gate, and the hexLE(rec[5]) == coordEui destination check. No findings there, and cases [8] through [10] pin all three discriminators.

The findings below are in the new generic device layer. Ranked.

---

1. binding_keys.xml advertises 5137 devices; devices.lua can only distinguish 3890, so 1247 keys load another device's descriptor

gen-devices.mjs:174 keys the table by ModelIdentifier alone, last-wins. The comment calls it "a known v1 limitation", but the limitation is not that those devices are unsupported: it is that the driver claims them and then presents a different device.

Verified:

grep -c '<ModelIdentifier>TS0601</ModelIdentifier>' ... binding_keys.xml   -> 662
grep -c '^\s*\["TS0601"\]'                          ... devices.lua        -> 1
grep -c '<ModelIdentifier>'  -> 5137      grep -cE '^  \["'  -> 3890

The single surviving ["TS0601"] (devices.lua:122363) is name = "QA QAT42Z3B", a 3-gang relay. So an integrator who joins any of the other 661 TS0601 devices, a Moes radiator valve for instance, gets a binding-key match and then three phantom relays plus that relay's settings. There is no "unsupported device" path because the key matched.

The extractor already captures manufacturerName per identifier, which is exactly what binding_keys.xml uses to keep the 662 distinct. Two ways out: key devices.lua on the (manufacturerName, modelIdentifier) pair the way the XML already does, or, if that is v2 work, emit only unambiguous keys into binding_keys.xml so the ambiguous ones fall through unclaimed instead of mis-binding. The second is a small generator change and turns a wrong-device bug into a missing-device bug.

Related, same entry: all three of its relays carry endpoint = 1. 16 generated entries have two or more actuator channels sharing one endpoint, and the runtime keys relay bindings by endpoint, so those gangs collapse into a single binding.

2. One unknown attribute type silently truncates the rest of a report, and 50 settings write a single byte

zcl.lua:163 readValue returns nil for any type absent from TYPE_FMT, and decodeAttributeReport:203 does a bare break on that, with no log. A report is therefore decoded only up to the first unsupported type; every attribute after it is dropped and the device looks alive and quiet.

On the write side, encodeWriteAttributes:131 is string.pack(fmt or "b", r.value), so an unsupported type is packed as one byte.

TYPE_FMT covers 0x10, 0x18, 0x19, 0x20, 0x21, 0x23, 0x28, 0x29, 0x2b, 0x30, 0x39. The ztype histogram over the generated data:

2385 0x0030    348 0x0020    118 0x0010     67 0x0023     46 0x0021
  23 0x0022     15 0x0029     12 0x0018     10 0x0041     10 0x0025
   7 0x0039      7 0x0019      3 0x0042      3 0x001B      1 0x0009

That is 50 settings (0x22 uint24, 0x25 uint48, 0x41/0x42 strings, 0x1B map32, 0x09) whose ztype has no entry. Concretely, "Led color" (devices.lua:49330, uint24) offers Red 16711680, Blue 65280, Purple 16777215, OFF 0. Packed as one byte those become 0x00, 0x00, 0xFF, 0x00: four choices collapse to two, and the record is two bytes short of what the device expects, so it rejects the frame outright.

Worth noting the asymmetry: reads special-case 0x41/0x42 above the table (zcl.lua:164), writes do not.

3. self.dstEndpoint is re-latched from every inbound packet and is also the default write destination and part of the binding key

device.lua:1127 sets self.dstEndpoint = srcEndpoint on every packet. Device:send (:135) falls back to it, and Device:sendWrite (:581) calls writeAttributes with no endpoint, so it always does.

On a multi-endpoint device this means the destination of a config write is whichever endpoint last happened to speak. Press gang 3 on a 3-gang relay, the device reports OnOff from endpoint 3, dstEndpoint becomes 3, and the next "Power-on behavior" write lands on gang 3 rather than gang 1. Where the target cluster does not exist on the latched endpoint the write is never confirmed and the config retry just re-sends.

The same field feeds the persisted binding key at :218 and :232 (endpoint = spec.endpoint or self.dstEndpoint), and no measurement or contact spec in the generated data carries an endpoint. So the key depends on which packet arrived before resolve ran, while the reload path builds a fresh instance at dstEndpoint = 1. That is a route to a second binding id for the same sensor, orphaning the Composer connection.

Note :169, :180 and :198 all spell it spec.endpoint or self.dstEndpoint or 1 while :218 and :232 omit the or 1. It is not reachable today because the constructor defaults to 1, but the inconsistency reads like the or 1 was meant to be there.

Defaulting both the write path and the key path to 1 would close this without touching the latch.

4. ZCL "invalid" sentinels are converted into readings

decoders/standard.lua:39 maps PowerConfig 0x0021 through halfPct, so BatteryPercentageRemaining = 0xFF, which the spec defines as unknown, becomes 128%. Same shape for Temperature MeasuredValue = 0x8000 (invalid) through centi giving -327.68 C, and Humidity = 0xFFFF giving 655.4 %. These reach the Snapshot and the TEMPERATURE_VALUE bindings, and battery specifically is the one a sleepy device sends while it is still settling after a join.

5. Config.decode picks nondeterministically when two options share a raw value

config.lua:31-36 reverse-maps raw to label by iterating with pairs. Where two labels share a raw the winner varies per run, which I reproduced on {["true"]=170,["false"]=170}. Real instances include Aqara lumi.plug.aq1 power_outage_memory, auto_off, led_disabled_night and ZG-302ZL.auto_on, 14 definitions across 8 models.

The knock-on is worse than a wrong label: setConfigValue guards on tostring(display) == tostring(configState), so a display that flips between runs makes writes get silently dropped. Iterating def.options in declaration order fixes both.

6. Light: a fade to off and a color change while off both collapse to a bare ZCL Off

capabilities/light.lua:187 returns early on has_state and not state, before the level and transition code, so transition_length is discarded. The companion's setDeviceBrightness(0, rate) (drivers/zigbee3_light/driver.lua:865) always sets state = false for level 0.

Hold an Off button link: rampToBrightness(0, 3000) makes the bulb go dark instantly rather than over 3 s. Release at 1.5 s and brightnessRampStopped interpolates 100 + (0-100)*0.5 = 50 and calls setDeviceBrightness(50, 0), so the already-dark bulb jumps back to 50%. Emitting MoveToLevelWithOnOff(0, tt) when tt > 0 covers it.

Same early return eats color: sendColor (driver.lua:1077) sets state = isLogicallyOn() meaning "do not turn it on", the capability reads state = false as "turn off" and drops has_rgb/has_color_temperature, but rampToColor still fires LIGHT_COLOR_CHANGED. The proxy shows red, the bulb keeps its previous color.

Related and probably the reason none of this self-corrects: Light never calls sendConfigureReporting and never polls. grep -rn sendConfigureReporting src/ returns only lock.lua:87, and the lock also polls 1500 ms after a command (lock.lua:111). Turn a bulb on at the wall and LIGHT_V2 stays where it was.

---

Notes, not requests

• decoders/xiaomi.lua:54 has SIZE[0x2a] = 6. ZCL 0x2a is int24, 3 bytes; 6 is int48 (0x2d), which is absent from the table. Any Aqara struct carrying an int24 tag desyncs by 3 bytes and the remainder of the struct is lost. 0x2c through 0x2f are also missing, where parseStruct breaks and drops the rest.
• lpack 'b' signedness, flagging a contradiction rather than asserting a bug. The code assumes 'b' is signed: zcl.lua:63 says so, and both UNSIGNED8 tables plus u8() exist to add 256 back. But Control4's own vendor/drivers-common-public/module/websocket.lua:499 does sunpack(self.buf, "bbbb") then b1 * 0x100 + b2 to read a WebSocket frame length, which is only correct if 'b' is unsigned. Exactly one of those is right and I could not settle it off-controller. If 'b' is unsigned then TYPE_FMT[0x28] = "b" decodes int8 unsigned (a -10 C device temperature reads as 246) and the UNSIGNED8/u8 fixups are dead but harmless; if it is signed, everything is fine as written. One line on the dev controller settles it: log select(2, string.unpack(string.char(0xF6), "b", 1)), where 246 means unsigned and -10 means signed.
• make generate does not reproduce the committed artifacts. Running the full pipeline gives a byte-identical binding_keys.xml, but devices.lua comes back 81,620 lines different while being character-identical once whitespace is normalized (3,515,441 chars both ways). The committed copies are stylua-formatted and generate emits raw, so make generate on its own leaves Check formatting red, and the reformat churn buries the actual device delta, which is the thing tools/generator/README.md:47 gives as the reason for committing them. Running the formatter as the last step of generate would fix both.
• gen-compat.mjs:103 stamps new Date().toISOString() into SUPPORTED_DEVICES.md, so a regeneration on unchanged input always diffs.
• extract-z2m.mjs:21 imports zigbee-herdsman, which is not in tools/generator/package.json and resolves only through hoisting of a transitive dependency. The catch sets Zcl = null silently, and every standard-cluster attribute lookup then returns null, degrading roughly 9,000 settings to read-only without a hard error. The prepareDefinition guard at line 76 hard-fails in the equivalent situation, which is the safer shape. The Makefile also uses npm install rather than npm ci against a committed lockfile.

Checked and found fine

Binding-key generation is deterministic and reproduced byte-identically; no duplicate (mfg, modelID) pairs across 5137. Escaping is clean in both outputs: no unescaped &, no raw control characters, no literal null/undefined, no duplicate Lua keys among the 3890. Tuya 0xEF00 gets the endianness right, big-endian DP length against little-endian ZCL, with correct two's complement on the int32 datapoint, and parseDatapoints bounds-checks truncated payloads. int16 decoding is correctly signed. Level scaling round-trips exactly for all 100 Control4 values with no value mapping to 0 while on, and ramp units are correctly milliseconds to deciseconds with no 10x error. Mired and Kelvin conversions guard their divisors. All 16 generated channel names exist in Channels.DEFS and no model resolves to an empty capability set. bindTargets matches every button key shape in the data. No table.sort comparator violates a strict weak ordering. Frame decode errors are contained by the pcall at drivers/zigbee3_device/driver.lua:269, so malformed payloads log and drop the packet rather than taking the driver down.

---

Verdict is comment rather than approve. Findings 1 through 3 are the load-bearing ones and they share a shape: the driver reports success while doing something other than what was asked, which is the failure mode that is expensive to diagnose in the field. None of them are regressions, since there was no generic driver before this, and the architecture underneath looks right to me. Happy to re-review as soon as you have pushed.

[svc-finitelabs]

Re-reviewed at 8b9a2be. make test gives 377 passed, 0 failed across 10 suites, including the new test/test_device_table.lua (13). build is green on this head, so the artifact-quota condition from the last round is gone and the red check is no longer in the way.

Five of the six are closed, and I verified each against the generated artifacts rather than the diff alone.

1, model disambiguation: closed. Every (ManufacturerName, ModelIdentifier) pair in binding_keys.xml now resolves. Of 5137 pairs, 5093 hit an exact (mfg, model) entry and 37 more take the zigbeeModel-only default. TS0601 is a wrapper carrying all 662 manufacturers, which is the exact count the XML keeps distinct, so the 661 mis-descriptions are gone. 64 models became wrappers, 1262 manufacturer mappings total. Collapsing identical descriptors by behaviour signature before wrapping is the right call: it keeps the table to 4270 entries instead of one per identifier.

2, wide attribute types: closed. Every ztype present in the generated data is now encodable. The histogram over devices.lua is 0x09, 0x10, 0x18, 0x19, 0x1b, 0x20, 0x21, 0x22, 0x23, 0x25, 0x29, 0x30, 0x39, 0x41, 0x42, and INT_TYPE plus the string branch plus TYPE_FMT covers all fifteen with zero uncovered. The uint24 "Led color" case that packed four choices into two bytes now packs three. Skip-and-log on an unsupported write beats the old one-byte pack, and reading via string.byte sidesteps the lpack 'b' signedness question I flagged, which retires most of that note.

3, endpoint latch: closed. Sensor keys and config writes both pin to 1. Worth recording that this is safe for report routing: Device:emit looks capabilities up by channel name (device.lua:249), not endpoint, so pinning moves only the persisted binding key, which is what needed to be stable.

5, config decode determinism: closed. Declaration order via def.options, sorted fallback when absent.

Two things below. The first is the one I would not merge without.

---

A. Finding 4 is documented but never wired up, so the sentinels still reach the proxy

decoders/standard.lua:26-44 adds invalid to all five MEAS entries and the comment says a report carrying one is "dropped rather than transformed into a bogus reading". The only consumer never reads the field:

-- standard.lua:83
for attrId, def in pairs(map) do
  local a = attrs[attrId]
  if a and a.value ~= nil then                       -- :85
    device:emit(def.channel, def.transform(a.value)) -- :86, def.invalid unused
  end
end

grep -n invalid src/zigbee3/decoders/standard.lua returns only the table literals and the comment, and there is no filtering downstream either: Device:emit (device.lua:248) goes straight to cap:report. So BatteryPercentageRemaining = 0xFF is still 128%, Temperature = 0x8000 is still -327.68 C, Humidity = 0xFFFF is still 655.4%, and battery is still the one a sleepy device emits while settling after a join.

The sentinel values themselves are all correct for their declared types, so this is only the missing guard:

if a and a.value ~= nil and a.value ~= def.invalid then

Flagging it mainly because the commit message lists it as resolved and the diff looks like a fix, which makes it the easiest one to lose.

B. A scene fade to off still snaps dark

The button-hold path is fixed, but the same early-off shape survives on the scene path. capabilities/light.lua:250-253 sends a bare Off and discards tt whenever state=false arrives without a brightness field, and the companion produces exactly that: drivers/zigbee3_light/driver.lua:1570 only sets has_brightness when step.level > 0, while transition_length is set independently at :1559 for any rate > 0.

So a scene step with level = 0, rate = 3000 sends has_state=true, state=false, has_transition_length=true, transition_length=3000 and no brightness. tt computes to 30, the brightness branch is skipped, and line 253 sends a bare Off. The bulb goes dark instantly while the proxy animates a 3 s fade, since :1568 still primes brightnessRamp for step.level ~= currentBrightness. Same visible mismatch as the original finding, reached through Scenes instead of a held button.

The fix mirrors what the brightness branch already does: when tt > 0 and self.hasBrightness, send MoveToLevelWithOnOff(0, tt) rather than CMD_OFF.

---

Worth knowing, not blocking

• Gang collapse is now 86 entries and 284 lost gangs, up from 16 and 26. Not a regression in this change: keying by (mfg, model) surfaced variants that last-wins used to discard, and many of them are Tuya multi-gang switches that list every gang on endpoint 1. Device:buildChannel keys relays and lights by endpoint (device.lua:170, :181), so Tuya TS0601_switch with four relay specs all at endpoint = 1 builds exactly one relay, and TS0601_switch_5_gang also builds one. Every actuator spec does carry an endpoint, so the self.dstEndpoint fallback on those three branches is unreachable and harmless. This is generator-side work, not runtime, and it was a tail note last round rather than a ranked finding, so I am not asking for it here.
• Seven models are claimed with a wildcard manufacturer but resolve to nothing: TS0505B, TS0202, TS0011, TS0501B, TS0503B, Dimmer-Switch-ZB3.0, HK_DIM_A. Each is <ManufacturerName>*</ManufacturerName> in binding_keys.xml but a wrapper with no default, so an unlisted manufacturer gets claimed and then falls through to generic contact handling. That is the wrong-device to missing-device trade I asked for, so it is working as intended; noting it because TS0505B and TS0202 are high-volume parts and TS0505B maps only 15 manufacturers.
• An ambiguous model needs 0x0004 in the same frame as 0x0005, with no retry. resolve only fires on 0x0005 (device.lua:1186), so a Basic report carrying ModelIdentifier alone leaves a wrapper model unresolved, and self.manufacturer arriving in a later frame never re-triggers it. Device:start runs only at init and on transport bind (driver.lua:140, :292). It surfaces as "Waiting for device" rather than failing silently, which is why this is a note; TS0202 being both sleepy and one of the seven above is the combination to watch. Stashing the model and re-attempting when 0x0004 lands would close it.
• device.lua:1182 says "requestModel reads both together". There is no requestModel; it is Device:start at device.lua:1023.
• encodeWriteAttributes' string branch writes string.char(#s % 256), so a value longer than 255 bytes would emit a length byte that disagrees with the payload and corrupt the rest of the frame. No generated setting is anywhere near that, so this is latent only.
• Unchanged from last round and still only notes: decoders/xiaomi.lua:54 SIZE[0x2a] = 6 (int24 is 3), and make generate still emits unformatted output so it leaves Check formatting red.

---

Verdict is comment again, on A alone: it is listed as fixed, and a battery reading of 128% is exactly the kind of thing that looks like a device fault rather than a driver bug. B is a genuine miss on finding 6 but cosmetic in impact.

Both are small and local. Push them and I will approve on the next green head; I have already verified everything else on this commit, so the next pass is just those two files.

[svc-finitelabs]

Re-reviewed at c1567bc. build is green on this head and make test gives 381 passed, 0 failed across 10 suites. Per-suite tally is identical to 8b9a2be except test_device_table at 13 -> 17, so nothing was dropped to make room for the new checks.

Both blocking items are closed, and this commit went well past the two files I asked for. Verified everything against the generated artifacts rather than the diff.

A, sentinels now actually dropped: closed. The guard is wired at decoders/standard.lua:85, and the sentinel values match what the decoder hands back. ZCL.TYPES marks 0x29 signed, so readInt returns -32768 for a 0x8000 temperature, which is what the table declares; humidity and illuminance are 0x21 unsigned so 0xFFFF is right, and battery is 0x20 so 0xFF is right. None of the five sentinels is reachable as a legitimate reading (battery caps at 200 half-percent, and 655% humidity has no other meaning). I also walked the other decoders for the same channels: xiaomi's battery is derived from millivolts and clamped to 0-100, its illuminance is clamped, and tuya emits datapoint-encoded values, so no other path carries a ZCL sentinel past the new guard.

B, scene fade to off: closed. capabilities/light.lua:256 mirrors the brightness branch exactly, same cluster, same command, same string.pack("b<H", ...) layout. It does mean a scene fade to off leaves CurrentLevel at 0, which is the latch the tt == 0 path at :239 deliberately avoids with a bare Off, but that trade is already the accepted one for level = 0, tt > 0 two branches up, so this is consistent rather than new.

The generator rework holds up under a full sweep of the generated table.

• All 5137 binding keys resolve, up from 5130. Exact (mfg, model) hits stay at 5093; the wildcard path goes 37 to 44, which is precisely the seven models I listed last round (TS0505B, TS0202, TS0011, TS0501B, TS0503B, Dimmer-Switch-ZB3.0, HK_DIM_A). Zero unresolved.
• Duplicate byMfg keys go 15 to 0, and the manufacturer key sets are identical across both heads: 1247 unique pairs each, 0 lost and 0 gained. So the dedup changed which variant 8 contested pairs point at and dropped nothing. (Last round's 1262 was 1247 plus the 15 duplicate literals that Lua was silently collapsing anyway.)
• TS000F losing its default is correct, not collateral. It has 15 named keys in binding_keys.xml and no * key, so no device can join on it without a listed manufacturer. The old mfgs.length === 0 heuristic had given it a fallback nothing could reach; keying on the wildcard claim is what removed it.
• Richness picks the fuller descriptor on the 8 contested pairs, e.g. GLEDOPTO/GLEDOPTO moves from GL-C-009 (1 channel) to GL-C-007-2ID (3 channels). Worth stating the direction of that trade: a wrong guess now shows extra channels that never report, rather than hiding channels the device has.
• SIZE[0x2a] = 3 is right, and 0x2c through 0x2f at 5 through 8 match int40 through int64.

Notes, none blocking

The SKU path can't use any of the new defaults, and isn't covered by the new retry. drivers/zigbee3_device/driver.lua:311 calls device:resolve(strSKU) with no manufacturer argument, so for a wrapper model the stricter rule now returns early. It also never sets pendingModel, and the retry only runs from onPacket, so a model learned from the SKU is dropped rather than held. That is a regression against 8b9a2be, where a nil manufacturer took entry.default and 37 models resolved from the SKU alone; now 44 models cannot. It matters most for exactly the case the comment at :309 is written for ("no Basic round-trip needed"), which is the sleepy device that Device:start has to catch while awake, and TS0202 is both newly defaulted and sleepy. Setting self.pendingModel = strSKU and calling resolve(self.pendingModel, self.manufacturer) would keep the hold-out invariant while letting the ManufacturerName frame close it.

restoreModel conflates an empty manufacturer with an absent one. device.lua:956 maps a persisted empty ResolvedManufacturer back to nil. An empty string is truthy in Lua, so a device reporting an empty ManufacturerName does resolve through default at runtime, persists "", and then holds out on the next reload instead of restoring its channels. Narrow, since it needs a device that answers Basic with an empty string, but the round trip is lossy in a way the runtime path is not.

The fall-through case lost its warning. An unlisted manufacturer on a wrapper with no default used to hit log:warn("... using generic contact handling"); it now returns early at log:debug with wording that says "yet", which is the one arm of that branch where nothing further will arrive. That is the wrong-device-versus-missing-device fallthrough I flagged last round, so it is the case most worth leaving visible at warn level.

Unreachable variants go 12 to 28. Dead entries in the table rather than wrong resolutions, and expected from richness dedup plus the stricter default rule, since a loser with a single contested manufacturer now has nothing pointing at it. Table bloat only; noting it so the number isn't a surprise later.

Approving as promised: the two I gated on are fixed, and the generator work that rode along verifies clean.

[svc-finitelabs]

Approving at 9f57483. All three resolve-path notes from the last round are closed, and I checked each against the call graph rather than the diff.

Note 1, the SKU path (driver.lua:311), closed. resolveFromModel stashes pendingModel before delegating, so the retry at device.lua:1229 now fires for the online-status path when ManufacturerName lands. Worth stating explicitly: this does not restore the 37 immediate default resolutions from 8b9a2be, and that is correct. The SKU never carries a manufacturer, so an ambiguous model still has to wait for 0x0004; the difference is that it now resolves on arrival instead of being dropped. Enumerated the other call sites to confirm the fix is complete: OnZigbeeOnlineStatusChanged and require("zigbee3.device") both appear only in zigbee3_device, so zigbee3_light and zigbee3_lock have no parallel path needing the same treatment.

Note 2, restoreModel, closed, and the empty-string semantics hold up. Checked the idiom against all three inputs: absent record gives nil, non-string gives nil, and "" survives because it is truthy, which is the case that was being lost. The semantic argument checks out too. resolve persists manufacturer or "", and a variants entry can only resolve with a non-nil manufacturer, so a persisted "" on a shared model can only have come from a runtime ManufacturerName that was itself the empty string (truthy at device.lua:1225, so it latched entry.default). Passing "" back reproduces that resolution exactly. Previously it became nil and the restore held forever.

Note 3, the fall-through log, closed. The nil and known-but-unlisted arms are now separated, the "yet" wording is gone from the arm where nothing more is coming, and the unlisted arm is back to warn.

build green on the head, make test 381 passed across 10 suites.

Two non-blocking items, neither of which should hold this up:

restoreModel does not stash pendingModel, which is the same asymmetry this commit just closed for the SKU path. If the restore at device.lua:970 fails to resolve, the model is known but pendingModel stays nil, so a Basic frame carrying ManufacturerName alone hits if self.pendingModel then and does nothing. It self-heals once the ModelIdentifier frame arrives, and reachability is low for a new driver (every blob this code writes contains both keys, so the failure needs a persisted manufacturer that a regenerated devices.lua no longer lists). One line would make the two entry points consistent.

Device:resolve still has no test coverage, and that is where every finding in this PR has lived. 381 passed is identical to the count at c1567bc, so no cases were added. test_device_table.lua validates the generated table, not the resolution logic, and nothing exercises the orderings that keep moving: model-first, manufacturer-first, shared model with and without a wildcard default, and the persist and restore round-trip including the empty-manufacturer case. Two of the last three rounds of findings were regressions introduced by the previous round's fix in this exact function, and the suite was green for all of them. A table-driven block against the shim would lock in the invariants that keep shifting. Good follow-up ticket rather than a change to this PR.

Merging stays yours.

[svc-finitelabs]

Fourth round, 1fa14cc. Both non-blocking notes from the 9f57483 approval are closed, build is green on the head, and my prior approval was auto-dismissed by the push so this is a fresh verdict. Approving.

The restore stash is safe on the common path, not just present

restoreModel now sets self.pendingModel = v.value before calling resolve, unconditionally. Worth saying why that is fine rather than only that it matches resolveFromModel: pendingModel is never cleared on a successful resolve anywhere in this file (only at the re-pair reset, device.lua:1176), so a successful restore leaves it set, and the onPacket retry at :1232 then re-enters resolve on the next Basic frame. That re-entry is a no-op because self.resolved is sticky and guards line 862. So the stash costs nothing when restore succeeds, and buys the retry when it holds. That was the reachable case I raised.

The retry uses self.manufacturer from the incoming 0x0004 frame, not the persisted one, which is the right source: the whole scenario is a persisted manufacturer the regenerated table no longer lists.

Test tally is the evidence that nothing was displaced

CI: 399 passed across 11 suites, against 381 across 10 at 9f57483. The delta is exactly 18, which is exactly test_resolve.lua's own count, and every pre-existing suite's number is byte-for-byte unchanged (test_zbind 53, test_c4_shim 125, test_device_table 17, and so on). Nothing was modified or dropped to make room. Last round I noted that an unchanged tally across a fix commit was proof no cases were added; the same comparison run the other way is what proves this commit is purely additive.

The suite drives the real device.lua against the real generated devices.lua, stubbing only the five capability builders, and it discovers its fixtures by walking the table for a flat model, a wrapper without a default, and a wrapper with one, rather than hardcoding model strings. Given that devices.lua was regenerated twice during this PR, that is the right call: the test will not rot the next time the generator runs.

Non-blocking: the one behaviour this commit adds is the one the new suite does not cover

pendingModel is asserted only in [5], on the resolveFromModel path. restoreModel is called twice in [7] and neither call asserts the stash or exercises hold-then-retry, which is the entire reason the stash exists. So the commit that closes the coverage gap on this function adds an untested behaviour to it. That is the exact pattern the file was written to stop, and it is cheap to close because lib.values returns a singleton instance, the same one device.lua holds, so the test can seed the blob directly:

local values = require("lib.values") -- singleton; same instance device.lua uses

print("\n[8] Restore with a no-longer-listed manufacturer holds, then the retry closes it")
do
  values:update("ResolvedModel", wrapNoDefault)
  values:update("ResolvedManufacturer", "__unlisted__")
  local d = mk()
  d:restoreModel()
  check("holds on an unlisted persisted manufacturer", not d.resolved, tostring(d.resolved))
  check("stashed the model for the retry", d.pendingModel == wrapNoDefault, tostring(d.pendingModel))
  d.manufacturer = wnA
  d:resolve(d.pendingModel, d.manufacturer)
  check("retry resolves once ManufacturerName lands", d.resolved and d.manufacturer == wnA, tostring(d.resolved))
end

Scope note in the same spirit: [5] simulates the retry by calling d:resolve(d.pendingModel, d.manufacturer) by hand rather than driving onPacket, so the wiring at :1232 and :1230 is still verified only by reading. That is a reasonable line to draw, since reaching onPacket needs a decoded ZCL frame, but it means the suite covers resolve's selection logic rather than the retry plumbing, and both regressions in rounds two and three were in the selection logic. Worth knowing where the net stops.

Neither point blocks. Merging stays yours.

[svc-finitelabs]

Approving 21637f4. build is green on the head, make test is 399 passed across 11 suites.

Both changes verified against the call graph rather than the diff.

Last Action reload guard: correct, and for a subtler reason than the diff shows

The ordering the comment claims holds: values:restoreValues() is OnDriverInit (drivers/zigbee3_device/driver.lua:124), and discovered() is reached later from OnDriverLateInit via device:restoreModel() -> buildFromEntry -> buildButtons -> Button:discovered() (src/zigbee3/device.lua:949). So on reload the restored value is already in place when the seed used to run, and the unconditional update("Last Action", "", ...) did write "" back through C4:SetVariable. Real bug, real fix.

The part worth stating explicitly: Values:getValue returns the persisted entry table, not the value (src/lib/values.lua:241). So == nil is an existence test on the record, not a test that the value is empty, which is exactly what is wanted here. Had it been written against the value, a legitimately empty "" would reseed forever and the guard would read identically in the diff.

Also swept the siblings: Button is the only capability that seeds inside discovered(). Contact:report, Relay:report and Measurement all write from report paths, so none of them carried the same clobber. And Values:reset() clears persist outright (_saveValues(nil)), so the full-reset path still reseeds correctly.

1. Re-pair now leaves Last Action stale (non-blocking, but this commit introduces it)

Device:reset() (src/zigbee3/device.lua:1177, fired from OnNetworkBindingChanged on Zigbee unbind, drivers/zigbee3_device/driver.lua:336) clears ResolvedModel, ResolvedManufacturer, Snapshot and DeviceConfig, but not Last Action. Previously the unconditional seed scrubbed it when the next device resolved. Now the record survives, so the newly paired device reports the previous device's last action until its first press.

It self-heals on that first press, so this is transient rather than corruption. But it does not heal at all if the newly paired device has no buttons: buildButtons returns early on an empty map, so discovered() never runs and a non-button device is left exposing a stale Last Action. That case is structurally out of reach for any guard inside discovered(), which is why the fix belongs in Device:reset():

values:delete("ResolvedModel")
values:delete("ResolvedManufacturer")
values:delete("Snapshot")
values:delete("DeviceConfig")
values:delete("Last Action")

2. That fix collides with the tombstone semantics of the new guard

Worth pairing with the above rather than landing alone. Values:delete does not remove the record, it marks it deleted = true with value = nil (src/lib/values.lua:208), and _trimDeletedTail only drops tombstones whose index is greater than every active index (src/lib/values.lua:312). Last Action is created at discovery and sits below Last Seen, Snapshot, DeviceConfig and the measurement variables, so its tombstone will normally survive the trim.

A surviving tombstone is not nil, so getValue("Last Action") == nil is false and discovered() would never reseed. The variable itself was already removed by C4:DeleteVariable inside delete, and restoreValues would only bring it back on the next reload as a hidden placeholder (src/lib/values.lua:272). Net effect: adding the delete alone silently disables the seed.

Making the guard tombstone-aware covers both:

local existing = values:getValue("Last Action")
if existing == nil or existing.deleted then
  values:update("Last Action", "", "STRING")
end

Values:update rewrites the entry without the deleted key, so it clears the tombstone on the way through. Nothing calls values:delete("Last Action") today, so this is latent as shipped; it only bites when finding 1 gets fixed.

3. GetDriverFileName guard: right idiom, one silent arm (nit)

C4.GetDriverFileName and C4:GetDriverFileName() matches the existing precedent at src/lib/utils.lua:105 exactly, and the stale getSuiteDriverIds reference in the header comment is now correct with zero residue anywhere in the repo.

Pricing the reachability honestly: this function lives only in the hub driver, whose minimum_os_version is 4.2.0, and CheckMinimumVersion early-returns out of OnDriverLateInit before startUpdateChecks() is ever reached. So the nil arm is defensive only. The difference from the utils.lua precedent is that utils.lua documents what the fallback does ("every filename takes the C4Z_ROOT path, which is what this function did previously"), whereas here nil yields an empty id list, which makes Select(getHubInstanceIds(), 1) == C4:GetDeviceID() false forever. That silently disables leader election and the auto-updater with no log line, where the previous code would at least have failed loudly. A log:warn in the nil arm would keep it diagnosable. Note also that utils.lua serves the companions, whose floor is 3.3.0, so the precedent covers a lower floor than this call site needs.

4. Both behaviour changes ship untested

399 across 11 suites is identical to 1fa14cc, and every suite's individual count is unchanged (test_resolve still 18), so this commit adds zero cases. The reload clobber is precisely the kind of ordering bug a test pins, and Button is straightforward to drive: it is a plain module and lib.values is a singleton.

getHubInstanceIds is a file-local in driver.lua and not reachable from the harness, so that half is reasonably left to inspection. The seed guard is not. Suggested test/test_button_seed.lua:

-- Tests for the Last Action seed guard in src/zigbee3/capabilities/button.lua.
--
-- discovered() runs on first discovery AND on every reload (OnDriverLateInit ->
-- restoreModel -> buildButtons), by which point restoreValues() has already put
-- the persisted action back, so an unconditional seed clobbers it. These pin the
-- three states the guard has to tell apart: absent, restored, and cleared by a
-- re-pair.
--
-- The action map is empty on purpose: with no actions, discovered() registers no
-- events, so the seed guard is the only thing under test.
--
-- Run from the driver root:
--   make test

require("c4_shim")
local values = require("lib.values")
local Button = require("zigbee3.capabilities.button")

local pass, fail = 0, 0
local function check(name, ok, detail)
  if ok then
    pass = pass + 1
    print(string.format("  ok   %s", name))
  else
    fail = fail + 1
    print(string.format("  FAIL %s%s", name, detail and ("  -> " .. tostring(detail)) or ""))
  end
end

local function discover()
  Button:new({}, {}):discovered()
end

print("\n[1] First discovery seeds the variable")
do
  values:reset()
  discover()
  local v = values:getValue("Last Action")
  check("the record exists", v ~= nil)
  check("it seeds empty", v and v.value == "", v and v.value)
end

print("\n[2] A reload does not clobber the restored action")
do
  values:reset()
  discover()
  values:update("Last Action", "1_single", "STRING")
  discover() -- reload: restoreValues() already put the value back
  local v = values:getValue("Last Action")
  check("the action survives discovery", v and v.value == "1_single", v and v.value)
end

print("\n[3] A re-pair clears it, so the next device starts empty")
do
  values:reset()
  discover()
  values:update("Last Action", "1_single", "STRING")
  values:update("Last Seen", "2026-01-01 00:00:00", "STRING") -- keeps the tombstone off the tail
  values:delete("Last Action") -- Device:reset()
  discover()
  local v = values:getValue("Last Action")
  check("the new device seeds empty", v and v.value == "", v and v.value)
end

print(string.format("\n%d passed, %d failed\n", pass, fail))
os.exit(fail == 0 and 0 or 1)

[1] and [2] pass on this head. [3] fails on it, deliberately: it encodes findings 1 and 2 together, and the Last Seen line is what keeps _trimDeletedTail from papering over the tombstone by trimming it. It goes green once the delete is added to Device:reset() and the guard checks existing.deleted.

None of this is blocking. Merging stays yours.

[svc-finitelabs]

Round 6 review: 9a573e3, both round-5 notes implemented, APPROVE

build green on 9a573e3. Tree 024eec4 -> 5ef8863, real content: 4 files, +88 -2.

The pair landed, and I checked the mechanism rather than the wording

The guard now reads existing == nil or existing.deleted, and the comment's claim that
update() clears the tombstone is true for a reason worth pinning down, because it is the
one way this fix could have been subtly wrong. Values:update builds a fresh entry table
(values.lua:142-148) rather than mutating the existing one, so deleted is dropped rather
than carried. And it always reaches that branch on a tombstone: changed compares
existing.value (which delete sets to nil, values.lua:209) against the seeded "", so
nil ~= "" is true before any of the other four disjuncts are consulted. Had update mutated
in place, the guard would still have looked correct and the reseed would have been a no-op.

Device:reset() gaining values:delete("Last Action") closes the half a guard could not
reach: a re-paired device with no buttons never runs discovered() at all, so the fix had
to live at the state transition.

The test earns its place, verified by breaking it

make test is a glob over test/test_*.lua, so the new file is picked up with no registration.
Counterfactual: reverting only the guard to if existing == nil then and re-running turns [3]
red with 2 failures (reseeded despite the tombstone, reseeded value is empty), while [1]
and [2] stay green. So the suite discriminates this change specifically, and the
Higher Index Var line is doing its job of keeping _trimDeletedTail from trimming the
tombstone and passing the test for the wrong reason.

Per-suite tally 407 across 12 suites, up from 399 across 11. Every pre-existing suite is
unchanged, so the +8 is exactly the new file. Read the other direction: the Device:reset()
delete is simulated in [3] rather than exercised, and the getHubInstanceIds warn has no
case. Both are fair calls at this size, just worth naming rather than letting the green tally
imply otherwise.

getHubInstanceIds is behaviour-identical to the old (fileName and ...) or {} expression,
now observable. log is the file-local from driver.lua:33, so it is in scope. That was the ask.

One finding, non-blocking, and not this PR's to fix

Adding Last Action to reset() makes an existing gap reachable for a user-facing variable:
after re-pair -> driver reload -> new device discovered, the persisted record heals but the
C4 variable stays hidden forever. Traced with the shim:

1. first discovery                 value=[]       hidden=False persistDeleted=nil
3. after re-pair (reset)           value=ABSENT   hidden=-     persistDeleted=true
4. after reload (restoreValues)    value=[]       hidden=True  persistDeleted=true
5. after new device discovered     value=[]       hidden=True  persistDeleted=nil
6. after first press on new device value=[double] hidden=True  persistDeleted=nil

restoreValues re-creates a deleted entry as a hidden placeholder (values.lua:272) to hold
the ID slot, which sets Variables[name]. Your guard then fires correctly at step 5 and heals
persist, but update sees Variables[name] ~= nil and takes the SetVariable branch
(values.lua:163-166), and SetVariable cannot change the hidden or readonly attribute. So
programming can never attach to Last Action again on that device.

Two reasons this is not a change I would want in this PR:

• It is pre-existing. The same probe against the four deletes already in reset() returns
  ResolvedModel, ResolvedManufacturer, Snapshot and DeviceConfig all hidden=True.
  This commit adds a fifth variable to the gap, it does not create it.
• It is template code. src/lib/values.lua is byte-identical to
  control4-driver-template's copy and has exactly one commit in this repo
  (f6bc2e3, the v0.9.16 render), so fixing it here would be drift that the next
  copier update clobbers.

Worth a template issue: update could clear the placeholder with a
C4:DeleteVariable + AddVariable when the persisted entry was a tombstone. Last Action
raises the priority because it is the button programming variable, so hidden is the difference
between working and silently unattachable, where the other four are internal state.

APPROVE. Merging stays yours.

[svc-finitelabs]

Round 7, head 691fded, tree 10cb32a (a real content change from 5ef8863). build is green: 407 checks across 12 suites, identical to the last head, which is expected since no file under test/ moved in this diff.

Verified correct

• <controlmethod>zigbee</controlmethod> matches control4-zigbee3-kwikset/drivers/kwikset_zigbee3_lock/driver.xml line for line, same position directly after <control>lua_gen</control>. This follows the hardware-proven sibling rather than a guess.
• The timer id renames are complete. I grepped every SetTimer/CancelTimer in src/ and drivers/: MotionClear, LightPoll_, LockPoll_ and RelayPulse_ have no CancelTimer counterpart anywhere, and ConfigRetry, the only renamed id that does, was renamed on both sides (device.lua:676 and device.lua:687). No cancel left pointing at an old id.
• The leading space on unit suffixes is right, and it does not double up. src/lib/values.lua:30 already documents the convention as " °C", " %", so this brings the driver to the template's own spelling. Both consumers concatenate bare: values.lua:182 (strValue .. propertySuffix) and www/html/index.html:240 (esc(r.value) + '<small>' + esc(r.unit)). The settings rows that do insert their own separator, index.html:208 and :221, read x.unit, which comes from def.unit at device.lua:528, a different field this diff does not touch. So no 23 %.
• The docs ## Connections section matches the code. driver.xml carries exactly one static connection (6001, ZIGBEE, consumer), and the dynamic classes listed are precisely the ones in the getOrAddDynamicBinding call sites: ZIGBEE_LIGHT, ZIGBEE_LOCK, RELAY, CONTACT_SENSOR, plus measurement's *_VALUE.
• cascadeOnline(false) is placed safely. It runs before self.lights/self.locks are cleared, so it still has capabilities to walk, and Light:disconnected / Lock:disconnected both early-return on a nil bindingId and only re-send UPDATE_DISCONNECT. An OFFLINE report followed by an unbind double-fires harmlessly.

1. The new State seeds go stale on a re-pair, the way Last Action did

The seed guard is correct for the reload path, and copying the tombstone-aware form from button.lua was the right call. But Device:reset() grew values:delete("Last Action") one commit ago for exactly this reason, and the two new variables were not added alongside it. I drove the real Contact, Relay and Button capabilities plus the real Device:reset() under the shim:

after first discovery:      Motion State=false  Relay State=false  Last Action=
device A active:            Motion State=true   Relay State=true   Last Action=single
after Device:reset():       Motion State=true   Relay State=true   Last Action=<TOMBSTONE>
device B after discovery:   Motion State=true   Relay State=true   Last Action=

Last Action heals. The two new ones do not, and the guard is what makes it permanent: the record is live and not a tombstone, so the reseed is skipped. Nothing else heals it either. reset() clears self.snapshot and deletes Snapshot, so replay() has nothing to re-emit, and there is no read or poll of On/Off or of a contact state at discovery. A re-paired contact sensor sitting closed reports 1 until it is next physically opened, and Relay:report early-returns on an unchanged state.

To be fair about the blast radius: this is not a regression. Before this commit Contact:report already wrote Motion State and reset() already did not clear it, so the staleness predates the change. What is new is that this is the pass where it would naturally have been closed, one commit after the identical fix landed for Last Action.

On the fix: do not reach for values:delete here. That walks straight into the placeholder gap I flagged last round, and on these variables it actually bites. Same probe, delete route:

device A active            value=true       hidden=False
after delete               value=<TOMBSTONE> hidden=<not-registered>
after reload restore       value=<TOMBSTONE> hidden=True
after re-discovery         value=false       hidden=True

The value heals but the variable is hidden forever, because restoreValues re-creates a deleted entry as a hidden placeholder (values.lua:272) and SetVariable cannot unhide it. For Last Action that gap was worth accepting since it joined four variables already in it. Here it would defeat the stated purpose of the commit, which is that programming can attach to these.

Re-seeding in place instead avoids the tombstone entirely:

after reset                value=false      hidden=False
after reload restore       value=false      hidden=False
after re-discovery         value=false      hidden=False
device B first report      value=true       hidden=False

So in Device:reset(), in the same slot as the new cascadeOnline(false) call and before the capabilities are dropped:

if self.contactCap then
  values:update(self.contactCap.name .. " State", "0", "BOOL")
end
for _, cap in pairs(self.relays) do
  values:update(cap.name .. " State", "0", "BOOL")
end

The discovery guard then correctly skips, since the record is live and already holds the seed value.

2. Battery State is the third one, and got neither half

device.lua:821 writes Battery State as a user-facing string, with no seed at discovery and no clear in reset(). Same class as above: a re-paired device reports the previous device's Low until its own first IAS Zone battery bit arrives, and setBatteryLow early-returns on an unchanged value. Pre-existing and not part of this diff, but if the intent of this pass is that programming can attach before the first report, it is the one that got missed.

3. test_device_table.lua silently skips an assertion about one run in five

The suite total is not stable. Running make test repeatedly on this head gives 406 or 407, and the difference is one check in test_device_table.lua:

run 1: device_table=16  TOTAL=406
run 2: device_table=17  TOTAL=407

test_device_table.lua:157 picks its wrapper with for m, entry in pairs(DEVICES) do ... break end, so which model it lands on depends on hash order. Line 175 then does local m1, i1 = next(e.byMfg) and line 177 only runs if m1. Of the 44 wrappers that carry a default, 9 have an empty byMfg (ZGRC-KEY-013, HK-SL-RELAY-A, Mec Driver module, and others), so on those runs the check silently does not execute.

The check that skips is a listed manufacturer still resolves to its own variant, which is the only assertion guarding the fix from c1567bc: that a wildcard default must not shadow a manufacturer that is actually listed. A run that skips it still prints green. Worth pinning the selection to a wrapper that has both a default and a non-empty byMfg, or iterating sorted keys so the pick is at least reproducible.

What this diff is not covered by

Nothing under test/ changed, so per the per-suite tally: the two new seed guards have no case (test_button_seed.lua drives Button only), and cascadeOnline(false) in reset() has none either. All three findings above are non-blocking, so this is an approve; the reset() re-seed is the one I would take before merge.

[svc-finitelabs]

Approving at 27b27fb. Both round-7 findings land, and build is green. I verified each rather than reading them.

Re-seed on re-pair (device.lua:1183-1193) is correct. The names match the writers exactly: contact.lua:36,46 and relay.lua:72,90 both build self.name .. " State", so the reset writes the same variable the report path writes. The block sits before contactCap/relays are nil'd, and every lastState guard is defeated by the caps being rebuilt fresh, so the first report from the new device always fires. The Battery State guard is right too: false ~= nil still re-seeds, and setBatteryLow's low == self.batteryLow early return cannot swallow the first report because reset() nils batteryLow.

Drove the real Device:reset() under the shim across re-pair, reload, re-discovery and first report. Motion State stays hidden=False throughout and returns to 0 on each re-pair.

De-flake confirmed empirically. 20 consecutive test_device_table.lua runs gave 17 passed, 0 failed every time; 5 consecutive full make test gave 407 passed, 0 failed every time. Matches the commit message. Worth noting the check that now always runs is the only assertion guarding c1567bc.

One finding, non-blocking and pre-existing

Last Action is the last typed variable still on the delete path. Two lines below the new block, device.lua:1226 still does values:delete("Last Action"), which is exactly the hidden-placeholder gap the new comment cites as the reason not to delete.

Same probe, same run, both variables side by side:

                     Last Action          Motion State
discovery            False                False
after reset()        ABSENT               False  value=0
after reload         True   value=        False  value=0
re-discovery         True   value=        False  value=0
first new report     True   value=single  False  value=1

Last Action is stuck hidden=True permanently. The mechanism: restoreValues recreates a deleted record as C4:AddVariable(name, "", varType, true, true), and from then on Values:update can only reach the C4:SetVariable branch because Variables[name] exists, so nothing ever clears the hidden flag. The tombstone-aware seed added in 9a573e3 heals the persist record but not the C4 variable. That is why test_button_seed.lua [3] passes while the variable is invisible: it asserts on values:getValue, never on the hidden attribute.

It is scoped to this one name. The other four deletes in reset() (ResolvedModel, ResolvedManufacturer, Snapshot, DeviceConfig) are all written 2-arg with no varType, and a varType-nil update takes the C4:DeleteVariable branch, so their placeholders self-clear on the next write. Last Action is the only typed one.

The fix is the change this commit already made everywhere else:

values:update("Last Action", "", "STRING") -- so a re-paired device doesn't show the old one's last press

Probed that too: Last Action then reads False at every stage in the table above, is still empty after the re-pair (so the original intent holds, the new device does not inherit the old one's last press), and picks up the new device's first press. Suite stays 407 passed, 0 failed.

Pre-existing since 9a573e3, so not a blocker for this commit, but this is the pass that closes it.

Not covered

No test exercises the new reset re-seed. test_button_seed.lua is the natural home, and a case there asserting the hidden attribute rather than only the persist record would have caught the Last Action gap as well.

Measurement variables (measurement.lua:98) are also not re-seeded and keep the previous device's reading after a re-pair. They are NUMBER with no neutral default, so leaving them alone is defensible rather than an oversight, but it is the same class if you ever want it closed.

The de-flake drops the 9 default-only wrappers (of the 44 carrying a default) from fixture selection, since it now requires a non-empty byMfg. That is the right trade: the assertion that mattered runs every time, and those 9 differ only in having no listed manufacturer to lose to the wildcard.

[svc-finitelabs]

Round 13, reviewed at e7af4f0 and re-anchored onto 5432034 (the dual-unit tile cleanup landed while I was writing; it touches only index.html markup, so nothing below changes apart from line numbers). build was green on e7af4f0, which clears last round's dirty-tree gate, and all 21 test files pass locally. Every finding from the last round has a named fix commit, and I verified most of them are genuinely fixed with regression tests that fail against the pre-fix source (I reverted and re-ran to check, rather than trusting that a green suite means the test is load-bearing).

Not approving, because the round introduced or exposed several new correctness defects, two of which ship wrong bytes today.

Confirmed fixed

• Reconcile now judges by a fresh per-pass observed set (zbind.lua:919-923,963-964,974,1024). Your scenario walk-through holds: persist claims N, real table empty, walk now runs to completion and re-fires all N. Test [22] fails on 2fe9076^.
• sentKeys no longer promotes to confirmed across sessions (zbind.lua:155-157).
• Level Step is discrete (button.lua:121); all eight Level command ids classify correctly against the ZCL. New case [9] fails on the old classifier.
• Cog dedupe is per channel (device.lua:1096). Measured by executing the generated table: 283 entries that bind 0x0B04 go from 1 cog to one per reading.
• Reporting overrides survive a reload (device.lua:1776, :1677).
• The string.pack gap is closed properly: test_zcl.lua covers both the packInt branch (uint48 0x25, int24 0x2a) and the string.pack branch, and both are live in production.
• Both disclosure asks are addressed, and the Settings page now validates the address instead of falling back to a literal.

New findings

1. The stale-prune can unbind a device's only binding and report success. zbind.lua:1011 still gates on self.confirmed, which is the persisted claim (:124), while missing correctly moved to self.observed (:1024). The prune also runs before the partial-read early return at :1019. So: persist claims a target bound, the device's real table holds only a stale bind to a previous coordinator, the device dozes mid-walk. The prune fires Unbind_req on the one binding that exists, the reconcileMode and incomplete branch returns, and finish(true) reports it bound. The comment at :1008 states the invariant ("so a target is never left with no bind at all") that the code now breaks, which is the same shape as the bug 2fe9076 set out to fix: the fix moved one consumer of confirmed to observed and left its sibling behind. Test [20] pins only the safe case, where the real bind is observed. Suggest gating the prune on self.observed and moving it below the incomplete return.

2. xiaomi.lua:23 decodes int8 as unsigned. Your shim's own comment states the production target is lpack, not the 5.3 stdlib. Under lpack, b is OP_BYTE (unsigned char) and c is OP_CHAR (signed); the 5.3 convention is the opposite. FMT[0x28] = "b" with 0x28 deliberately excluded from UNSIGNED8 shows the intent is signed, and the v < 0 fixup at :81 is a no-op under lpack. An Aqara device-temperature of -10 C (wire 0xF6) decodes as 246. The shim defines b as signed, so the suite agrees with the driver and both are wrong together, which is exactly the failure mode a hand-written shim risks. zcl.lua's TYPE_FMT[0x28] has the same code but is shadowed by INT_TYPE in both readValue and the encoder, so it is dead rather than live.

3. xiaomi.lua:29-30 uses <L/<l for 32-bit fields. In lpack these are C long, which is 8 bytes on an LP64 build, and the file's own SIZE table says 4 (:47-48). On a 64-bit Director this over-reads and desyncs the remainder of every Xiaomi struct. <I/<i are 4 bytes on both ILP32 and LP64. On a 32-bit controller the two agree, so this may be invisible on one controller generation and not another.

Discriminator for 2 and 3, since both rest on which pack library Composer actually loads. One line on the dev controller:
print(string.unpack("\xF6", "b", 1)) and print(#string.pack("<L", 1)).
If those return 246 and 8, both findings stand as written and the shim needs b/c and L/I corrected to match. If they return -10 and 4, the runtime is 5.3-style, findings 2 and 3 are withdrawn, and the shim comment should be corrected instead. I have not run this; I have no Xiaomi device paired on the dev controller.

4. The reportable-change field is not clamped. zcl.lua:213-214 clamps min and max to 0xFFFF under a comment about truncation, but changeBytes is built above that block and never bounded, and setReporting (device.lua:1150) has no upper bound either. A change of 40000 on an int16 attribute packs to 9C 40, a deadband of -25536. The UI number input has min="0" and no max.

5. The width check does not cover values already persisted. device.lua:99 restores DesiredConfig with no revalidation and no state version, so a value committed by the pre-fix build stays out of range; emitConfig then sees drift and re-writes, and startConfigRetry resets configRetryAttempts to 0 on each report (:864), so the 45-attempt window re-arms rather than bounding the loop. This is a first release, so the exposure is your own dev and beta installs rather than the field, but a sanitize-on-load pass is cheap now and awkward later. setReporting also reaches commitConfig directly without the width check.

6. confirmReporting collapses a multi-record response to one byte (device.lua:1184). It cannot confirm the successful subset of a partial failure, so a spec-compliant response that reports only the refused record leaves the accepted ones pending and re-sending. And on a device that echoes a record per attribute including successes, byte 1 is 0, the uncorrelated fallback runs, and a refused record is confirmed alongside the rest, so the UI shows applied for a reading that will never update. Related: the early return at :1185 precedes self.reportSeq[seq] = nil at :1207, so every failed or unanswered request leaks a reportSeq entry, and nextSeq is mod 256.

7. The hold timeout is 2000 ms (button.lua:21). A real hold routinely runs longer, and remotes that send Move once at press and Stop at release will have the latch cleared mid-hold, re-firing the event the latch exists to suppress. The mechanics around it are sound (the vendor SetTimer cancels first, so no double-fire or leak). Note the test cannot catch the duration: ShimFireTimers fires regardless of delay, so any value passes.

8. Twelve residual Step -> *_hold seed mappings. Executing the generated table: of 3951 cluster-8 Step mappings, 3939 use brightness_step_* names and 12 map to 1_hold through 4_hold across three devices. With the classifier fixed, those three fire an event labelled Hold on every discrete tap.

9. The ztype width guard misses the wide types: 0x24-0x27 and 0x2c-0x2f, including the int48 case, plus 0x1a and 0x1c-0x1f. Live today is 0x25 (uint48) on 11 entries, and no shipping entry currently overflows, so this is latent; raising it only because 892d732's rationale was to guard the set against a regen.

10. The cog key omits endpoint (device.lua:1096). 34 generated entries list a reportable channel more than once, and the generator emits endpoint = nil for all of them, so spec.endpoint or 1 collapses them. On an 8-probe device, tuning Temperature configures probe 1 and leaves the rest at defaults.

Nits

• esc() is not a correct escape for a JS string inside an HTML attribute. It maps ' to ', which the HTML parser decodes back to ' before the JS parser sees the attribute, so it does not protect the six sinks at index.html:304,356,358,359,363,364. Not exploitable today: all 25 distinct channel values in the generated table match ^[%w_.-]+$, and channels are internal identifiers rather than device-supplied strings. Worth aligning anyway because control() at :272,276 already uses the correct data-key plus onSetting(this) pattern, so the safe form is two functions away.
• Coverage gaps, listed only so they are a choice rather than an oversight: the override-persistence fix is not pinned by any test (test_resolve.lua:31 stubs startElectrical out, and test_electrical.lua never mentions desiredConfig), so it would pass with the bug present; test_c4_shim.lua contains no string.pack or string.unpack assertions despite the name; and nothing covers multi-record or partial-failure ConfigureReporting responses.
• electricalReports now sources un-overridden defaults from REPORTABLE rather than ELEC, which moves power's bootstrap deadband from 1 W to 5 W and leaves ELEC's min/max/change dead for channeled records. Looks deliberate, flagging in case it is not.

[svc-finitelabs]

Round 14 review at 3ce6196. build is green, all 21 Lua test files pass (676 assertions). Nine commits land nine of the ten findings from the last round.

How I checked the fixes. Rather than trust a green suite, I reverted each commit's src/ hunks against the current head and re-ran the whole suite. Of the 8 commits that touch src/, 6 have a test that genuinely fails against the pre-fix source: 27b09fd8 (zbind [24]), 7b65a299 (xiaomi), 669a9468 (zcl, 2 assertions), 08aeafe2 (reporting), a7340b58 (reporting, crashes pre-fix), baa3bb01 (button seed). Two do not, both explainable and noted below.

The string.pack discriminator came back from the controller as lpack semantics (b unsigned, c signed8), which is the branch where I pre-committed to keeping findings 2 and 3 rather than withdrawing them. They stood, and 7b65a299 fixes the driver rather than papering over it: 0x28 now maps to c, the 32-bit fields move to fixed-width <I/<i, the dead v < 0 fixups are gone, and test_c4_shim.lua now pins the exact 0xF6 case that was wrong. That closes the loop properly.

---

Correction: I am withdrawing my cog-dedupe finding

Last round I wrote that the reporting dedupe key omits the endpoint, that 34 entries repeat a channel with endpoint = nil, and that consequently "an 8-probe device tunes probe 1 only." The first half is true and the conclusion does not follow. I did not check it properly before publishing it.

Loading generated/devices.lua and walking every entry.channels list (the exact value buildReporting receives at device.lua:1536): 85 groups repeat a reportable channel within one entry, and zero of them have distinct endpoints. Every duplicate is endpoint = nil, so all of them resolve to endpoint 1 and the dedupe collapses exact duplicates, which is correct. Separately, buildChannel handles the genuinely per-endpoint exposes (relay, light, lock) keyed by endpoint before the channel-name dedupe at device.lua:206, so multi-gang devices were never at risk. There is no live defect here.

While confirming that, one observation worth a glance, not a change request: some entries carry heavy duplication from the generator. Device 0x0210 lists 45 channel specs that are the same three channels (battery, temperature, humidity) repeated 15 times with identical names and nil endpoints. Everything downstream dedupes so it is harmless, but the resolve log line at device.lua:1525 reports "45 channels" for what is really 3.

---

One finding: the retry fix bounds the writes but not the status

a7340b58 is correct on the half it targets. A value the device keeps refusing no longer re-arms the window on every drift report, so the permanent write loop is gone. But the commit message names the symptom as "Driver Status stuck on Applying", and the status is still stuck, because nothing ever clears pendingConfig for a value the window gave up on.

emitConfig sets self.pendingConfig[key] = desired and persists it on every drift report (device.lua:835-836), unconditionally and above the bounded send. updateStatus derives the status purely from that table being non-empty (device.lua:1331-1339), with no notion of an elapsed window. Repro against the real Device with only the wire send stubbed:

window elapsed; simulating 5 further drift reports from the device
writes sent after window elapsed : 0   (the fix works: bounded)
pendingConfig still holds the key: true
Driver Status                    : Applying (tap sensor button): Sensitivity

So the installer is told to keep tapping the button forever for a write the driver has already stopped attempting. Two things make it stickier than a cosmetic wart:

• It is re-persisted on every drift report, so it survives a restart.
• On reload, device.lua:1686 re-arms a fresh window via startConfigRetry(), which resets configRetryAttempts to 0. For a permanently-refused value that means another 90 seconds of writes on every driver reload, indefinitely. The only exit is the installer running clearDesiredConfig.

sanitizeDesiredConfig closes the invalid-value path well, but not this one: the case that remains is a value that validates locally and the device still refuses (read-only or unsupported optional attribute), which is exactly the case the commit set out to bound.

Worth noting this is the same shape as the zbind issue from last round: the fix moved one consumer of the state and left its sibling. 27b09fd8 itself is a clean fix on that front, both consumers moved and the prune now requires positive evidence from the current walk.

---

Nits, no action needed to land

zcl.lua still carries the corrected table's uncorrected twin. TYPE_FMT keeps [0x28] = "b", [0x23] = "<L" and [0x2b] = "<l", which encode precisely the lpack misconception this round just fixed in xiaomi.lua. It is dead today and I verified that rather than assuming it: of the 11 TYPE_FMT keys, 10 are shadowed by INT_TYPE in both readValue and encodeWriteAttributes, and 669a9468 removed the last TYPE_FMT use from encodeConfigureReporting. Only 0x39 (float) is reachable. The trap is that adding a type to TYPE_FMT without adding it to INT_TYPE silently resurrects the bug. Same for the now-dead v < 0 fixups left at zcl.lua:113 and tuya.lua:18 after the matching one was removed from xiaomi.lua. Tuya itself is fine: it builds its int32 from bytes with its own two's-complement fixup and only uses u8 for genuinely unsigned reads.

Relatedly, the shim comment says l/L are 4 bytes while 7b65a299's message says they are 8 on a 64-bit Director. Nothing live depends on it (those two dead TYPE_FMT entries are the only l/L left in the driver), but the two statements cannot both be right.

The two commits with no failing counterfactual. c694b4ae's ZTYPE_RANGE rewrite is a genuine improvement and reads correctly across the whole 8..64-bit ladder, but no test fails when it is reverted, so the newly covered widths (0x24-0x27, 0x2c-0x2f, 0x1c-0x1f) are unpinned. The hold timeout in the same commit cannot be pinned at all: ShimFireTimers fires regardless of the delay, so no value there can ever fail a test, which is worth remembering before anyone edits that constant. I did verify the reasoning it rests on: the timer really is re-armed on every Move at button.lua:133, under an id SetTimer replaces.

3ce6196f also has no failing counterfactual, which is the correct result: reverting the removal changes nothing observable, which is what "these values were dead" should look like.

namedHold is data-shaped. The _hold_ substring test is well anchored. I loaded the generated table and checked all 138 strings containing "hold": every *_threshold correctly misses, and the 19 that match are real hold actions plus a few setting keys that never reach self.map. Note there is no hold_release action in the data today. If a regen ever introduces one it would classify as a hold and latch instead of unlatching, so this is a mapping that changes behavior on a data refresh.

Not approving on this round only because the stuck status is the named symptom of a fix in it, and the repro is a couple of lines. Everything else here is solid work.

[svc-finitelabs]

Round 15 on 7176cd36 (3ce6196 -> 576fa40 -> 7176cd36, tree 68ea782 -> b53cce4, real content). build green, 22 test files / 702 assertions pass, including the new test_multi_endpoint [1]-[6].

First: my round-14 withdrawal was wrong, and this commit is the proof

Last round I published a retraction of my own round-13 finding. I had said the generator emitted endpoint = nil on measurement specs so a multi-probe device tuned probe 1 only. I then "verified" it away by walking every entry.channels list, finding 85 duplicate channel groups with zero distinct endpoints, and concluding the dedupe was collapsing exact duplicates correctly.

The walk was accurate and the conclusion was backwards. Every duplicate group had endpoint = nil because that was the bug, not because the device had one endpoint. I treated the symptom as the disproof. 7176cd36's own message states it plainly: "The generator dropped the endpoint on measurement/contact specs, so a device with the same channel on several endpoints emitted identical specs that the runtime then collapsed to one."

The check I should have run is the one that separates the hypotheses: resolve each spec back to its z2m expose endpoint, rather than reading the already-flattened field whose flattening was the thing in question. Noting it because it is the second time in two days I have published a conclusion off a premise I verified and a consequence I did not.

The design here reads well: the bare key for the primary endpoint keeps every existing Snapshot blob, report: override, and value binding intact, and soleChannelCap lets a lone sensor absorb a report on any srcEndpoint. That fallback is load-bearing, see finding 2.

1. ZigDC loses all 18 electrical channels (regression)

The new if (endpoint == null) return null; guards in map-c4.mjs (numeric branch and contact branch) drop a capability whose z2m endpoint label does not resolve. One model pays for it. Loading both generated tables and diffing per model:

models old=4299 new=4299   removedModels=0 addedModels=0
channel-count regressions = 3, all in ZigDC:
  ZigDC: power   6 -> 0
  ZigDC: current 6 -> 0
  ZigDC: voltage 6 -> 0

ZigDC (xyzroe) goes from 20 channels to 2, keeping only temperature and humidity. It is the only model that loses anything, and it is a genuine loss: before this PR its six identical power specs collapsed to one working power reading, so the change takes it from partially-correct to absent.

resolveEndpoint returns nil only when epName is non-numeric and absent from ctx.endpoints, and captureEndpoints returns nil outright when the z2m definition has no endpoint() function. A 6-channel DC monitor that names its endpoints without exposing an endpoint() map hits exactly that hole. Dropping the cap is defensible as a policy, silently dropping the device's entire reason for existing is not. Options, your call: fall back to enumerating distinct label order (l1->1, l2->2) when ctx.endpoints is nil, or keep the drop and have the generator fail loudly with the model plus label so a regression like this cannot land unremarked.

2. A reporting override is now silently ignored on 18 models (regression)

buildReporting keys reportCluster[cluster][ep] off the spec's endpoint, but ensureReporting looks it up with byEp[b.srcEp], the bind's endpoint. Those disagree whenever a device reports a measurement on an endpoint other than the one the spec resolved to, and resolveEndpoint(null, ...) returns 1 for every unlabelled expose.

Across the regenerated table, of 3358 reportable specs whose cluster is actually bound, 3301 match and 57 do not, spanning 18 models:

L101Z-DBI, L101Z-SBI, L101Ze-SLM, ROB_200-070-0, S-ZB-1RE1-R251,
S-ZB-COV1-R251, ZG9032B, ZG9041A-2R, ZG9098A-Light, ZG9098A-LightWin,
ZG9098A-Win, ZG9098A-WinLight, ZG9098A-WinOnly, ZigDC, ZigUSB,
msh.ina226, msh.ina226m, msh.pzem

A/B on the same scenario (L101Z-DBI: temperature spec resolves to endpoint 1, Temperature bind is on srcEp 2), driving the real Device with only the wire send stubbed:

parent 3ce6196:  reportCluster[0x0402] = "temperature" (string)
                 sent -> ep=2 min=60 max=900 change=150
                 override -> min=60 max=900 change=150     HONORED

head   7176cd36: reportCluster[0x0402] = table
                 byEp[2] = nil
                 sent -> ep=2 min=30 max=3600 change=20
                 override -> min=60 max=900 change=150     DROPPED

The installer sets 60s/900s/1.5 degrees, the driver keeps sending the 30s/3600s/0.2 baseline, and nothing reports a failure. Worth stressing that readings themselves are fine on these models: soleChannelCap catches the endpoint-2 report and routes it to the lone cap. It is only the override arm that regressed, which is why the suite stays green.

The report: def for these already carries write.endpoint = spec.endpoint, so the ConfigureReporting a user override sends goes to endpoint 1 while the device reports on 2. That half is pre-existing, but it means the fix is probably not "look up by srcEp" alone: for a single-endpoint channel, both sides want to agree on the bind's endpoint.

3. Contact endpoint stamping is generated but never consumed

gen-devices.mjs now stamps endpoint and an endpoint-labeled name on contact-family specs, and the runtime commit message says "Measurement/Contact capability files were already endpoint-aware, so two endpoints get distinct variables + value bindings for free". True of contact.lua, not true at the device level: buildChannel still has

if def.expose == "contact" then
  if self.contactCap then
    return -- one contact sensor per device
  end

which fires before any per-endpoint key is used. Feeding the regenerated Presence entry (occupancy on endpoints 1 through 10) to the real buildChannel:

specs fed = 10, caps built = 1
cap key=occupancy  name=Sensor
emitContact(true) -> caps that received it = 1

emitContact(detected) also takes no endpoint, so there is no path to route a second one even if it were built. Nine of the ten specs are inert. This is not a regression (that entry produced one cap before too), it is the generator half of a fix landing without the runtime half, so the data now describes a capability the driver cannot express. Either finish it for contacts or leave contact specs unstamped so the table does not claim more than the runtime delivers.

Separately, the contact branch never passes spec.name into Contact:new, which is why the cap is Sensor rather than Motion 1. That is pre-existing, but this commit is now actively computing a name for that field.

4. Nit: 4 models get a spurious endpoint label

chanCount in gen-devices.mjs counts exposes per channel, not distinct endpoints per channel, so two exposes that resolve to the same endpoint trip the multi path:

TKA105, tagv1, URC4450BC0-X-R, H34450BA00-00007:  occupancy x2, both ep=1
  old: name="Motion"     new: name="Motion L1"

Single-probe motion sensors now carry a label implying a sibling that does not exist. Counting distinct resolved endpoints rather than exposes fixes it, and would also stop these four from entering the multi path at all. 82 of the 86 groups are genuinely multi-endpoint, so the blast radius is small.

Checked and dropped rather than published

• standard.lua:192 passes endpoint to device:emit while the function signature names the parameter srcEndpoint. Looked like an undeclared-global nil. It is not: local endpoint = srcEndpoint or 1 is the first line of the function.
• out[key:gsub("^report:", "")] in uiReporting. gsub returns two values, but a table index expression is adjusted to exactly one, so the count does not leak into the key.
• parseChanKey against channel names: no channel in channels.lua contains @, so the separator stays unambiguous, and a legacy bare key parses to endpoint 1, which is the intended migration.
• 9 multi-endpoint groups have a minimum endpoint above 1 (EM, WCM-300Z, L101Ze-DLM, Soil Pro), so they have no bare key at all. I expected an old bare-key Snapshot to replay into a spurious third cap, but replay runs after resolve and those models are new to multi-endpoint status this commit, so there is no pre-existing blob with that shape to migrate.
• tuya.lua and xiaomi.lua still call device:emit without an endpoint. For a Tuya DP device that is correct, DPs are not endpoint-addressed, and soleChannelCap covers the single-cap case.
• setReporting/resetReporting correctly changed to REPORTABLE[def.channel] rather than indexing by the composite key. Both were needed and both landed.

Verdict

Commenting rather than approving on findings 1 and 2, both of which are regressions against 3ce6196 that the green suite does not catch. The core design is right and finding 1 is a one-model data hole, so this is close.

[svc-finitelabs]

Round 16 on 9e3f4ad (build green). Both regressions I raised in round 15 are fixed, and I verified each one by running it rather than reading the diff. Approving. Two non-blocking notes at the bottom.

Round-15 regressions: both closed

ZigDC channel loss. Parsed the generated table at three commits and diffed the per-model channel multiset, so the comparison is on structure rather than on a 240k-line text diff:

3ce6196 (pre-regression) : current x6, power x6, voltage x6, humidity, temperature
7176cd3 (regression)     : humidity, temperature
9e3f4ad (this head)      : current x6, power x6, voltage x6, humidity, temperature

Byte-for-byte back to the pre-regression baseline. The resolveEndpoint fallback to 1 is the right call over dropping: a collapsed reading beats no reading.

Override endpoint mismatch. Built every generated entry through the real buildReporting with its own binds, then checked each bound reportable spec for a reportCluster[cluster][srcEp] hit:

bound reportable specs                   = 3020
  endpoint mismatch                      = 33
  closed by the sole-reading fallback    = 33
  still unmatched (cluster has >1 reading) = 0

Every mismatch is on a cluster carrying exactly one reading, so the soleChannelCap-style fallback closes all of them with nothing left over. Correcting my own figure from last round: I reported "57 of 3358", this scan says 33 of 3020. The earlier number double-counted; 33 is the one to trust, and either way the remainder is now zero.

Spurious L1 labels. Gone on all four affected entries (TKA105, tagv1, URC4450BC0-X-R, H34450BA00-00007), which now read Motion with no phantom sibling.

Multi-endpoint contacts (8e52d3a): verified working

A/B of 7176cd3 against this head, driving the real generated Presence entry through the real Standard decoder with OccupancySensing reports on endpoints 1 through 10:

	parent	head
contact caps built	1 (Sensor)	10 (Motion 1 .. Motion 10)
routing	all zones collapse to one	ep N lands on zone N, no crosstalk
binding keys	contact/contact_1	contact/contact_1 .. contact_10, distinct displayNames
snapshot	contact=false	contacts keyed 1..10
uiReadings rows	1	10

Also checked the legacy blob migration for both polarities (contact=true and contact=false land on contacts["1"], legacy key dropped, cap state correct), and that MotionClear_<ep> matches the existing RelayPulse_/LightPoll_/LockPoll_ convention with nothing left cancelling the old bare MotionClear. Full Lua suite is green locally, 712 assertions across 22 files.

Note 1: "Seven multi-zone occupancy models benefit" is one model

Counting contact-expose channels by distinct endpoint across every entry (variants expanded):

models with contacts on >1 ENDPOINT = 1 :: Presence(10)

29 entries have more than one contact spec, but 28 put every spec on endpoint 1, so they are untouched by the change. Only Presence has zones on separate endpoints. Not a code problem, but that sentence is the changelog.

Note 2: the same-endpoint half is still open (pre-existing, not caused here)

Keying contacts by endpoint alone means two different binary sensors on the same endpoint still collapse into one capability. That is 28 entries:

3321-S           WINNER=contact     DROPPED=occupancy
multi / multiv4  WINNER=contact     DROPPED=occupancy
KEYPAD001        WINNER=occupancy   DROPPED=contact
MultiSensor      WINNER=contact     DROPPED=water
DCH-B112         WINNER=contact     DROPPED=vibration

It is a shared capability, not just a dropped one. standard.lua routes both IASZone and OccupancySensing into emitContact(..., endpoint) with the same endpoint, so on 3321-S both clusters drive the single Contact cap:

3321-S  contact caps built : ep1=Contact(contact)
        after IASZone alarm=1   : Contact=true
        after Occupancy clear=0 : Contact=false   <-- door-open reset by the motion sensor

I ran the same script against the parent and it behaves identically (only the name differs, Sensor vs Contact), so this commit did not cause it and this is not a blocker. Flagging it because the commit message reads as if the collapse problem is now closed, and it is closed only along the endpoint axis.

Not a one-line fix, either: the runtime cannot route by channel because the decoders never say which binary sensor fired. standard.lua passes only an endpoint, and tuya.lua's route() calls device:emitContact(dp.value == ...) with neither channel nor endpoint. That needs a decoder change, so a follow-up issue rather than more scope here.

One thing I expected to be worse and is not: I checked TS0601#147 ("Tuya TS0601_smoke_co") thinking a carbon monoxide alarm would flip the Smoke variable. It cannot. Its DP map has a single contact datapoint (dp 18) and the co channel has no DP at all, so that one is an inert channel entry rather than a mislabelled alarm.

Nit

uiReadings sorts by label string, so ten zones render Motion 1, Motion 10, Motion 2, Motion 3, .... Zero-padding the generated name or sorting on the endpoint would fix it.

[svc-finitelabs]

Round 17: APPROVE

60dfc6b is a clean follow-up to the two non-blocking notes from the round-16 approval. build is green on this head and the full suite passes locally (0 failures across every test_*.lua). Nothing here changes decode, bind, or reporting behavior; the Lua change is confined to display ordering.

Verified the sort fix by running it, not reading it

Built a 10-zone presence device (one contact per endpoint, Motion 1..Motion 10) plus temperature on endpoints 1, 2 and 10, then called uiReadings() at both commits.

At 9e3f4ad:

Motion 1, Motion 10, Motion 2, Motion 3, ... Motion 9,
Temperature, Temperature L10, Temperature L2

At 60dfc6b:

Motion 1, Motion 2, Motion 3, ... Motion 9, Motion 10,
Temperature, Temperature L2, Temperature L10

So the reported symptom reproduces on the parent and is closed here, and the L2/L10 pair confirms it fixes the measurement labels too, not just the zone labels.

The fix is applied everywhere it needs to be, and nowhere it does not

naturalKey only lands in uiReadings. I checked the other label-ordered surfaces to make sure this is not a half-applied fix:

• uiSettings sorts def.label but explicitly skips write.kind == "report", and the per-endpoint labels only exist on report-kind defs, so it never sees them.
• configChoices does sort report keys (they reach supportedConfig via the replay path). It emits nothing for them, though, because buildReporting builds report defs with no options and the loop is ipairs(def.options or {}). No user-visible list, so no ordering to fix.
• uiBindings sorts on numeric endpoint first, then cluster name, so it was never affected.
• actionChoices sorts action labels, which are not per-endpoint.

That leaves uiReadings as the only surface that renders these labels in order, which matches the change.

The 9-digit pad is comfortably sized for the real data

%09d is a minimum width, so a digit run longer than 9 would pad short and could misorder against another long run. I measured rather than assumed: across all 8002 channel spec entries in the generated table, 7530 carry a name, 178 contain digits, and the longest digit run is 2 (Temperature L15). Zero runs over 9, and zero cases where two distinct channel names collapse to the same natural key. The table does hold 727 name strings with longer runs, but those are all model-level description strings and never become a reading label, so they never reach this comparator.

Two non-blocking notes

1. Nothing pins the ordering. test_multi_endpoint [4] is the natural home for it, but it indexes uiReadings() rows into a byChannel map, which discards order by construction, and no test greps for sort or ordering at all. Deleting naturalKey today leaves the suite green. One assertion on the label sequence in that block would pin the exact regression this commit just fixed.

2. I cannot find the follow-up the commit message says was filed. The message states the remaining round-16 item, two different binary sensors on the same endpoint sharing one capability, "is filed as a follow-up." The newest ticket in the tracker is DRV-102 from yesterday and none of the open items match, and this repo has no GitHub issues. If it went somewhere I cannot see, ignore this; otherwise the commit message is currently promising a record that does not exist. Same shape as the count in 8e52d3a: the commit message is the changelog.

Changelog

The new Added bullet is accurate and correctly conditional. It describes what happens when a device reports the same reading on more than one endpoint rather than claiming a blanket improvement, which is the right framing given that only one model in the table actually exposes distinct endpoints for a repeated contact channel. README.md carries the identical rendered text, so the generated-docs check stays clean.

[svc-finitelabs]

Round 18. build is green on 008e1c7 (checked before this verdict) and the Lua suite is 0 FAIL across 22 files locally.

The 3321-S fix itself works. I drove the real resolve("3321-S") against the real generated entry: it sets multiContactEp[1], builds contact@1 and occupancy@1 with distinct binding keys, and an IASZone open followed by an OccupancySensing clear leaves the door true and the motion false. The round 16 follow-up is genuinely closed.

Not approving this round. The no-cluster path in emitContact lost its per-endpoint fallback, and that silently drops reports on two reachable paths.

Blocking: a Tuya multi-cap device stops reporting entirely

The commit message states that Tuya/Xiaomi devices "stay single-endpoint via the lone-contact fallback." There is a counterexample in the generated table: Excellux variant 3 (ZG-104PLV, manufacturer PIRIV01) exposes occupancy@1 plus vibration@1 and carries a tuya DP map with two contact = true datapoints (dp 1 and dp 3).

contactCluster maps those two channels to different clusters, so multiContactEp[1] fires and two caps get built. But tuya.lua:88 calls device:emitContact(dp.value == ...) with a single argument: no endpoint, no cluster. In emitContact that leaves cap nil, soleContactCap() returns nil at two caps, and the buildChannel({ channel = "occupancy", endpoint = 1 }) fallback collides with the existing occupancy@1 and returns nil. The report is discarded.

A/B, same script, only the checkout differs:

PARENT 60dfc6b  caps=1   contacts[1]            lastState=true    delivered: true
HEAD   008e1c7  caps=2   contacts[occupancy@1]  lastState=nil
                         contacts[vibration@1]  lastState=nil     delivered: false

Pre-fix this device conflated motion and vibration onto one cap. Post-fix it reports nothing at all. The cluster split is meaningless here anyway: neither channel rides IASZone or OccupancySensing, both ride the Tuya cluster.

Also dropped: the snapshot upgrade path, on all nine affected entries

replay() sends the legacy numeric key through that same no-cluster path (device.lua:1808). Pre-upgrade these endpoints held one collapsed cap keyed contacts[1], so the persisted key is "1"; post-upgrade there are two caps and nothing claims it.

PARENT 60dfc6b  contacts[1] lastState=true                              restored: true
HEAD   008e1c7  contacts[contact@1]=nil  contacts[occupancy@1]=nil      restored: false

Every existing install of the nine entries this PR fixes loses its restored contact state on the first reload after upgrade. Since contact sensors report on change, a door left open reads unknown until someone physically moves it. The stale "1" key is also never cleared, so it is re-attempted and re-dropped on every boot.

Root cause, and a suggested fix

Both are the same line. The old routing was self.contacts[endpoint] or self:soleContactCap(); the new one skips the endpoint tier entirely once the cluster route misses. Restoring that tier closes both without touching the cluster routing, but it has to be deterministic, since pairs(self.contacts) has no defined order and the old semantics were "the first channel built at that endpoint". Recording it at build time gives that for free:

-- buildChannel, alongside the contactByCluster write
self.contactByEndpoint[endpoint] = self.contactByEndpoint[endpoint] or cap

-- emitContact
local cap = cluster and self.contactByCluster[cluster] and self.contactByCluster[cluster][endpoint]
cap = cap or self.contactByEndpoint[endpoint] or self:soleContactCap()

That reproduces the parent's behavior for every no-cluster caller while leaving [10] and [11] untouched, since both supply a cluster and both routes hit. Skipping the split for entries that carry a tuya map would also close the first one, but not the snapshot path.

Test coverage

[10] sets d.multiContactEp = { [1] = true } by hand, so resolve()'s computation of it is not exercised anywhere. More to the point, no test drives emitContact without a cluster on a device that has two caps, which is exactly the hole both bugs sit in. A case built from the real Excellux / PIRIV01 entry would have caught the first; one that seeds snapshot.contacts = { ["1"] = true } before replay() would have caught the second.

Verified and dropped

• xiaomi.lua:140 is the other no-cluster caller, but none of the nine multi-cluster entries route through it, so it is not reachable today.
• The other 20 entries with two or more contact channels all collapse to a single cap (same cluster, same endpoint), so soleContactCap() still answers them and both paths above stay safe there.
• I suspected some occupancy channels actually report over IASZone, which would send motion to the door cap. Zero entries declare occupancy with a 0x0500 bind and no 0x0406, so the name-to-cluster mapping in contactCluster holds for every standard-ZCL entry in the table.
• local cl is declared twice in the same block in buildChannel. Harmless shadowing, and check-lua is stylua only, so it is not a CI risk.

[svc-finitelabs]

Round 19. Verified 69e2491 (tree a688f28, a real content change on 008e1c7). build is green. Both round-18 regressions are closed, and the Tuya generalization checks out. One new issue on the path this commit added, so COMMENT rather than approve.

Both round-18 paths verified closed

Drove the real resolve() against the real generated table, not the tests' hand-set fields.

Tuya no-cluster path, model Excellux / mfg PIRIV01:

resolved = Excellux ZG-104PLV,  tuya map = true,  multiContactEp[1] = true
one cap built: occupancy@1  (bindKey contact_occupancy_1)
emitContact(true, 1)  ->  occupancy@1 lastState=true, snapshot written

Previously both caps missed and the report was silently dropped. Closed.

Round 16's fix still holds on 3321-S: IAS open followed by an OCC clear leaves contact@1 = true, occupancy@1 = false.

The Tuya collapse is safe, and narrower than it reads

if self.tuya collapses every Tuya endpoint, so I enumerated the population rather than trusting the scope. 4851 entries, 251 with a tuya map, 10 carrying more than one contact channel on a single endpoint:

• HOBEIAN ZG-102ZM (ZG-102ZM, AY02SZ, TS0601 v261): vibration + contact
• Excellux ZG-104PLV (Excellux v3): occupancy + vibration
• Excellux ZG-102MV (Excellux v4): contact + vibration
• Tuya TS0601_smoke_co (v147): smoke + co
• Tuya MIR-HE200-TY (v157) and Pinjia PJ3201A (v255): occupancy + occupancy
• Tuya DCR-RQJ (v231): gas + co
• Tuya JKD-816COM-Z (v289): co + gas

None of the 10 declares a standard contact bind (0x0500 or 0x0406) on that endpoint, so no collapsed cap loses an addressable route. 9 of the 10 pair channels that already share a cluster, so they collapsed before this commit too. Exactly one entry changes cap count: Excellux v3, occupancy (0x0406) plus vibration (0x0500). The generalization is sound, and today its blast radius is one device.

Blocking: the legacy-key restore is half a migration, and it loses

replay() now restores the legacy numeric slot, but nothing retires it, and both key families are consumed in one pairs() loop whose order is undefined. On an install predating the split, the first post-upgrade report writes the canonical key while the numeric key stays frozen at its pre-upgrade value. The two then fight at every reload.

Test [14] passes because its blob holds only ["1"]. Give it the second key an upgraded install actually has:

d.snapshot.contacts = { ["1"] = true, ["contact@1"] = false } -- legacy frozen open, current closed

and the frozen value wins:

after replay, first cap lastState = true   (want false)
legacy key still in snapshot = true

It is order dependent rather than always wrong, which is what makes it worth fixing instead of tolerating:

{1, contact@1}               -> contact@1 then 1     legacy wins
{1, occupancy@1}             -> occupancy@1 then 1   legacy wins
{1, contact@1, occupancy@1}  -> 1 first              canonical wins
{2, contact@2}               -> 2 first              canonical wins

The two-key shape is exactly what an upgraded single-sensor install has, and it loses. Nothing ever clears ["1"], so this recurs on every reload. Contact sensors report on change, so a door that does not move reads at its stale value indefinitely. That is worse than the round-18 behaviour it replaces: dropping the restore left the cap unknown until the next report and self-healed, whereas this reasserts a wrong value permanently.

Same shape as the snapshot.contact to contacts["1"] migration at device.lua:161, which also never clears its source. That one is harmless only because the old key is never read back again.

Minimal fix, legacy first so a canonical entry written since the upgrade wins:

for slot, detected in pairs(self.snapshot.contacts or {}) do
  if tostring(slot):match("^%d+$") then
    self:emitContact(detected, tonumber(slot)) -- legacy endpoint-keyed blob
  end
end
for slot, detected in pairs(self.snapshot.contacts or {}) do
  if not tostring(slot):match("^%d+$") then
    local ch, ep = parseChanKey(slot)
    self:emitContact(detected, ep, contactCluster(ch))
  end
end

Applied locally, the probe above flips to false and test_multi_endpoint stays 49/0. Retiring ["1"] once its value has been re-persisted canonically would finish the job, but the ordering alone makes the outcome correct.

Pre-committed so this does not cost another round trip: either the ordering fix above, or deleting the legacy restore outright if pre-release blobs are out of scope (main carries no driver, so the only installs are dev controllers). Either one closes this and I approve.

Non-blocking

multiContactEp is computed from distinct clusters before buildChannel runs, so it does not know about the Tuya collapse. Excellux v3 therefore ends up with a single cap but a channel-qualified binding key contact_occupancy_1, where the stated policy is a bare contact_1 for a single sensor. No functional harm, since the dynamic binding is created either way. Clearing multiContactEp[ep] for a Tuya endpoint, or deriving the key from the built cap count, would match the comment.

[svc-finitelabs]

Approving. The round 19 blocking finding is closed, and I verified the fix on the real path rather than only through the new test.

The replay ordering fix is correct

Two-pass replay (legacy numeric slots first, canonical channel@endpoint second) makes the outcome independent of pairs() order, which is what the single pass could not guarantee. Verified against the real code at endpoints 1, 2 and 3: the canonical value wins in every case. Reverting just this hunk flips endpoints 1 and 2 to the stale legacy value, so the change is load bearing and not incidental.

I also checked the sibling loop directly above it, since it consumes self.snapshot.channels in one pass over the same mixed key shapes. It is safe, and for a structural reason: chanKey collapses endpoint 1 to the bare channel name, so temperature is the canonical endpoint 1 key and temperature@1 is never written. Contacts were the odd one out because contactKey always qualifies, which is exactly what created two competing spellings of the same slot. No second instance of this bug class.

The multiContactEp gate is right, and narrower than it looks

Gating on self.tuya is consistent with buildChannel: that collapse is device level and enforced per endpoint via contactByEndpoint, so a Tuya device never builds more than one contact cap on an endpoint. Qualifying its key was describing a collision that cannot occur.

Since this is a global branch, I enumerated it against the generated table rather than trusting the scope claim. Of 4851 entries, 251 carry a tuya map, and exactly one changes its binding key: Excellux / PIRIV01 variant 3 (ZG-104PLV), the device that motivated the change. The other 8 endpoints with more than one distinct contact cluster are all non Tuya and keep their qualified keys. Confirmed end to end through the real resolve(): that device now builds one cap with the bare key contact_1, which matches the stated policy for a single binary sensor, and the round 19 no-cluster fallback still lands. 3321-S still routes an IAS open and an occupancy clear independently on qualified keys, so the round 16 fix is intact.

Non-blocking: test [15] passes on the broken implementation about two thirds of the time

This does not affect shipped behavior, so it is not blocking, but the regression it is meant to lock in is not actually locked in.

LuaJIT randomizes string hashes per process, so pairs() order over { ["1"], ["contact@1"] } varies run to run. I reverted the fix and ran test_multi_endpoint 30 times: the suite came back fully green in 20 of 30 runs. The test catches the regression in only 10. Table construction matters too, which is why it looks stable in isolation: building the same two keys by assignment instead of by constructor produces the opposite order and fails where the test passes. If someone later collapses the two loops back into one, this test is a coin flip rather than a guard.

Pinning the adversarial order makes it deterministic. Verified: 30 of 30 failures against the single-pass version, 30 of 30 passes against this branch.

-- pairs() order is undefined and LuaJIT randomizes string hashes per process, so a
-- plain two-key blob exercises the legacy-last order only about a third of the time.
-- Pin the adversarial order (legacy numeric slot LAST) for the replay call.
local function withLegacyLast(fn)
  local realpairs = pairs
  _G.pairs = function(t)
    local keys = {}
    for k in realpairs(t) do
      keys[#keys + 1] = k
    end
    table.sort(keys, function(a, b)
      local an = tostring(a):match("^%d+$") ~= nil
      local bn = tostring(b):match("^%d+$") ~= nil
      if an ~= bn then
        return not an -- numeric (legacy) keys last
      end
      return tostring(a) < tostring(b)
    end)
    local i = 0
    return function()
      i = i + 1
      if keys[i] ~= nil then
        return keys[i], t[keys[i]]
      end
    end
  end
  local ok, err = pcall(fn)
  _G.pairs = realpairs
  if not ok then
    error(err)
  end
end

Then in [15], replace the bare d:replay() with:

  withLegacyLast(function()
    d:replay()
  end)

and the check name becomes "canonical value wins even when the legacy slot replays last".

Worth doing in this PR or a follow up, your call. build is green on 47f6b03 and the full suite passes locally (test_multi_endpoint 50/0).

[svc-finitelabs]

Approving. Test-only change, and it is the deterministic guard from the last round.

Verified the catch rate, not just that it passes. Reverting only the two-pass replay() hunk in src/zigbee3/device.lua and running test_multi_endpoint.lua 30 times:

• pinned order (this push): 0/30 pass on the broken code, 30/30 on the fixed code
• the previous version of [15]: 10/30 on the broken code

So [15] went from a coin flip to a real guard. Full suite still green locally (22 test files), and build is green on this head.

The pin is faithful to production, not just convenient. withLegacyLast collects keys up front, so it cannot observe keys added mid-traversal the way a live pairs() walk might. That is not a divergence here: the only write to snapshot.contacts is in emitContact, behind if not self.replaying, so nothing is inserted while replay() is iterating. The pinned walk sees exactly the key set production sees.

Two details I checked rather than assumed:

• The override reaches the code under test. Nothing in src/ or drivers/ captures pairs as a local, so replacing _G.pairs actually takes effect inside replay().
• pairs is restored on failure. The pcall plus re-raise means a failing assertion inside the block cannot leave the sorted iterator installed for the rest of the file.

The comparator is a consistent ordering (numeric-looking keys last, then lexicographic), so table.sort will not reject it, and two distinct keys sharing a tostring compare false in both directions rather than inconsistently.