Consistent devLabel use: pass the model where it is in scope#2934
Conversation
af6e06f to
b8f05cb
Compare
|
I'm afraid this change falls short of the requirements. Please verify all instances within the statescontroller, where errors / messages concerning missing states, missing objects or similar occur. For all those cases, the given name of the device is not desired information, as the correct ID for the object is the key for finding them in the object tree. I only did a rough check - all messages of that case still called A. |
46c3408 to
7433833
Compare
|
Sorry for the inconvenience — I clearly read the requirement too narrowly the first time, and I appreciate you taking the time for another round on this. Now implemented: all messages in the statescontroller concerning missing, empty or orphaned states/objects print the raw object id instead of the label — the missing-state/value debug messages in Traffic-, converter- and event-level messages keep the label — there the device identity is the useful information, and the id stays part of the label anyway. If I've still missed messages you'd want switched to the id, just name them and I'll adjust right away. Also rebased onto current master (the |
|
can you please rebase onto the current master to resolve the conflict ? A |
Full review of all devLabel call sites (172 active, 5 in comment blocks) as requested in ioBroker#2933, split into the three cases: 1) The model is in scope (function parameter, mappedModel/mappedDevice, or a resolved entity): 111 sites now pass it as third parameter. The expression used is always the mapped model (the common.type key space, 'group' for groups) - never a raw modelID where a mapped model exists. 2) Calling devLabel makes no sense: messages concerning missing, empty or orphaned states/objects are object-tree diagnostics - the object id is the key for finding them there, so these print the raw id (14 sites, see below). Same for the deleteObj warning: devId there can be any object path (e.g. a state id from deleteOrphanedDeviceStates). 3) The model is not available at the call site (before/without entity resolution, admin messages, delayed actions): 35 sites deliberately stay id-only; the custom name and the cached object name still apply. The case-2 messages in the statescontroller (review feedback): the elevated-debug missing-state/value messages in publishFromState and publishToState (NOSTATES/NOVAL1/NOSTATE/ESTATE and 'No value published'), the disconnected-state cleanup messages in deleteOrphanedDeviceStates, and the delete-device debug in leaveDevice now print the raw object id instead of the device label. Two icon warnings (AddModelFromHerdsman, updateDev) printed a bare device name without any id at all - they now print the id instead, and the name lookup that only served that message is gone. Name resolution order in devLabel is now: local-config override (per-device wins over per-model; the per-model lookup is what needs the model parameter) -> cached object name (matches the admin display, including group names) -> model as last resort (no object yet, e.g. while pairing) -> 'unnamed (id)'. An empty-string model is treated as absent so it cannot suppress these fallbacks (publishFromState can be reached with model ''). Fixed on the way, all label-related: - publishPayload error path labelled via payload.device, which is undefined when the payload arrives as JSON string ('unknown device' in the log); now payloadObj.device. - removeDevFromGroup exception message printed the device label a second time where the group id belongs. - two group member read labels used member.id, which does not exist on the members the zigbee controller's getGroupMembersFromController returns ({ieee, model, epid, ...}); now member.ieee like the neighbouring labels. The devLabel methods on StatesController, ZigbeeController and BaseExtension remain exact (idOrIeee, model) pass-throughs to lib/deviceLabel.js. Addresses ioBroker#2933 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
7433833 to
217fa0f
Compare
What this does
Addresses #2933 — the requested review of every
devLabelcall site, split into the three cases from the issue. 172 active call sites (plus 5 in comment blocks) were read in their function context and classified individually; the full classification is reflected 1:1 in the diff:Case 1 — the model is in scope (111 sites): they now pass it as third parameter. The expression is always the mapped model (the
common.typekey space,'group'for groups,'Coordinator'for the coordinator) — taken from the enclosing function'smodelparameter,mappedModel/mappedDevice, orentity.mapped?.model. No site passes a rawmodelIDwhere a mapped model exists. In the receive path (onZigbeeEventand below) the in-scopemodelisentity.mapped?.model ?? device.modelID, which is exactly whatupdateDevwrites tocommon.type, so the label lookup stays in one key space.Case 2 — calling devLabel makes no sense (review round, now 14 sites + 2 name-only warnings): all messages concerning missing, empty or orphaned states/objects are object-tree diagnostics — the raw id is the key for finding them there, the device name is not desired information. These now print the id instead of the label:
publishFromState(NOSTATES, NOVAL1) andpublishToState(NOSTATE ×2, ESTATE, "No value published"); their sibling NOSTKEY identified the object neither by name nor by id and now carries the id as welldeleteOrphanedDeviceStates(keeping/deleting, both log and admin dialog output)leaveDevice(the identity was logged one line above by the leave event)deleteObjwarning (the original case-2 site):devIdthere can be any object path, e.g. a state idAddModelFromHerdsman,updateDev) printed a bare device name with no id at all — they now print the id, and the name lookup that only served the first message is goneTraffic-, converter- and event-level messages (incoming events, publishes, pings, OTA, queries) keep the label — there the device identity is the useful information, and the id stays part of the label anyway.
Case 3 — no model available at the call site (35 sites): deliberately left id-only: paths before/without entity resolution, admin message handlers, delayed actions, availability paths that only have the herdsman device. The per-device override and the cached object name still resolve there; only the last-resort model default is unavailable.
The
devLabelmethods onStatesController,ZigbeeControllerandBaseExtensionremain exact(idOrIeee, model)pass-throughs tolib/deviceLabel.js.Name resolution order
With the model now arriving at the resolver, the default chain in
devLabelis:model === 'group', and that string must not beat a cached "Kitchen"unnamed (ieee)unnamed (id)/unknown devicefor an invalid id — unchangedAn empty-string model is treated as absent (
publishFromStatecan be reached withmodel === ''for unsupported devices;''is not nullish and would otherwise suppress the fallbacks).Label bugs fixed on the way
publishPayloaderror path built the label frompayload.device—payloadmay be the raw JSON string, so the log saidunknown device. NowpayloadObj.device(with the model, which is resolved at that point).removeDevFromGroupexception message printed the device label a second time where the group id belongs.readMemberStatususedmember.id, which does not exist on the members the zigbee controller'sgetGroupMembersFromControllerreturns ({ieee, model, epid, deviceNetworkAddress, ep}) — nowmember.ieeelike the neighbouring labels in the same function.Verification
node --checkon all touched files, ESLint clean,npm testgreenlib/deviceLabel.jsplus the realStatesController.verifyDeviceName(custom-name precedence, per-model override with/without model, group name vs'group', empty-string model, ns-prefixed/bare/numeric ids, never-throw contract)insertedDeviceLeaveoverlap with Fix ignore_device_delete: TypeError on device leave, option lookup never matched #2935 is resolved on top of the merged version)🤖 Generated with Claude Code