Skip to content

Commit 1d28aeb

Browse files
committed
refactor: lift labelSnapshot to an instance property in haDiscovery
labelSnapshot was previously threaded as the last positional parameter through seven helper methods inside HaDiscovery: _supplementFromLabels, _processLightingGroups, _processEnableControlGroups, _createDiscovery, _createHvacDiscovery, _publishTriggerButton, _publishTriggerScene with at least 23 call sites that had to remember to pass it. The threading obscured the dependency graph (the snapshot was conceptually a property of the active discovery run, not an arbitrary helper input) and made each helper signature longer than its purpose warranted. Now _publishDiscoveryFromTree stashes the snapshot on this._labelSnapshot at the start of the run and clears it after the run finishes. Helpers read this._labelSnapshot via destructuring; their signatures drop the parameter. The snapshot still protects against concurrent updateLabels() mutation because the whole discovery run is synchronous and the property is captured once per invocation. Behaviour unchanged - all 98 haDiscovery tests plus 1239-test full suite still pass.
1 parent 8b927cb commit 1d28aeb

1 file changed

Lines changed: 30 additions & 23 deletions

File tree

src/haDiscovery.js

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,10 @@ class HaDiscovery {
534534

535535
// Snapshot label data references so a concurrent updateLabels() call
536536
// cannot swap them out mid-operation, preventing inconsistent reads.
537-
const labelSnapshot = {
537+
// Lives on the instance for the duration of this synchronous discovery
538+
// run; helper methods read this._labelSnapshot rather than receiving
539+
// it as a parameter on every call. Cleared at the end of the run.
540+
this._labelSnapshot = {
538541
labelMap: this.labelMap,
539542
typeOverrides: this.typeOverrides,
540543
entityIds: this.entityIds,
@@ -577,16 +580,16 @@ class HaDiscovery {
577580
for (const [appId, groupMap] of groupsByApp) {
578581
const groups = Array.from(groupMap.values());
579582
if (String(appId) === String(lightingAppId)) {
580-
this._processLightingGroups(networkId, appId, groups, labelSnapshot);
583+
this._processLightingGroups(networkId, appId, groups);
581584
} else {
582-
this._processEnableControlGroups(networkId, appId, groups, labelSnapshot);
585+
this._processEnableControlGroups(networkId, appId, groups);
583586
}
584587
}
585588

586589
// Supplement with labeled groups not found in TREEXML.
587590
// C-Gate's flat TREEXML format omits groups not assigned to specific units,
588591
// but labels.json may define groups that are valid and controllable.
589-
this._supplementFromLabels(networkId, lightingAppId, groupsByApp, labelSnapshot);
592+
this._supplementFromLabels(networkId, lightingAppId, groupsByApp);
590593

591594
// Clear any previously published discovery topics for this network that were
592595
// not republished in this run (device excluded or type changed since last run).
@@ -614,15 +617,19 @@ class HaDiscovery {
614617
const duration = Date.now() - startTime;
615618
const { custom, treexml, fallback } = this.labelStats;
616619
this.logger.info(`HA Discovery completed for network ${networkId}. Published ${this.discoveryCount} entities (took ${duration}ms). Labels: ${custom} custom, ${treexml} from TREEXML, ${fallback} fallback`);
620+
621+
// Clear snapshot so any later code that reaches for it without an
622+
// active discovery run fails loudly rather than reading stale data.
623+
this._labelSnapshot = null;
617624
}
618625

619626
/**
620627
* Create discovery entities for labeled groups not already found in TREEXML.
621628
* The flat TREEXML format may omit groups not assigned to specific units,
622629
* but they are still valid and controllable on the C-Bus network.
623630
*/
624-
_supplementFromLabels(networkId, lightingAppId, groupsByApp, labelSnapshot) {
625-
const { labelMap, exclude } = labelSnapshot;
631+
_supplementFromLabels(networkId, lightingAppId, groupsByApp) {
632+
const { labelMap, exclude } = this._labelSnapshot;
626633
if (!labelMap || labelMap.size === 0) return;
627634

628635
const prefix = `${networkId}/${lightingAppId}/`;
@@ -636,7 +643,7 @@ class HaDiscovery {
636643
if (existingIds.has(groupId)) continue;
637644
if (exclude.has(labelKey)) continue;
638645

639-
this._processLightingGroups(networkId, lightingAppId, [{ GroupAddress: groupId }], labelSnapshot);
646+
this._processLightingGroups(networkId, lightingAppId, [{ GroupAddress: groupId }]);
640647
supplementCount++;
641648
}
642649

@@ -645,8 +652,8 @@ class HaDiscovery {
645652
}
646653
}
647654

648-
_processLightingGroups(networkId, appId, groups, labelSnapshot) {
649-
const { labelMap, typeOverrides, entityIds, exclude, areas } = labelSnapshot;
655+
_processLightingGroups(networkId, appId, groups) {
656+
const { labelMap, typeOverrides, entityIds, exclude, areas } = this._labelSnapshot;
650657
const groupArray = Array.isArray(groups) ? groups : [groups];
651658

652659
groupArray.forEach(group => {
@@ -668,7 +675,7 @@ class HaDiscovery {
668675
const config = getDiscoveryConfig(typeOverride);
669676
if (config) {
670677
this.logger.debug(`Type override: ${labelKey} -> ${typeOverride}`);
671-
this._createDiscovery(networkId, appId, groupId, group.Label, config, labelSnapshot);
678+
this._createDiscovery(networkId, appId, groupId, group.Label, config);
672679
// Remove any stale retained light config for this group.
673680
// This covers the case where the type changes within the same session
674681
// (e.g. first run saw it as a light; this run sees the type override).
@@ -730,7 +737,7 @@ class HaDiscovery {
730737
});
731738
}
732739

733-
_processEnableControlGroups(networkId, appAddress, groups, labelSnapshot) {
740+
_processEnableControlGroups(networkId, appAddress, groups) {
734741
const groupArray = Array.isArray(groups) ? groups : [groups];
735742

736743
// Tilt app groups are not standalone entities — they enrich cover discovery only
@@ -753,9 +760,9 @@ class HaDiscovery {
753760
}
754761

755762
if (discoveryType === 'hvac') {
756-
this._createHvacDiscovery(networkId, appAddress, groupId, group.Label, labelSnapshot);
763+
this._createHvacDiscovery(networkId, appAddress, groupId, group.Label);
757764
} else {
758-
this._createDiscovery(networkId, appAddress, groupId, group.Label, getDiscoveryConfig(discoveryType), labelSnapshot);
765+
this._createDiscovery(networkId, appAddress, groupId, group.Label, getDiscoveryConfig(discoveryType));
759766
}
760767
});
761768
}
@@ -779,8 +786,8 @@ class HaDiscovery {
779786
*
780787
* @private
781788
*/
782-
_createHvacDiscovery(networkId, appId, groupId, groupLabel, labelSnapshot) {
783-
const { labelMap, entityIds, exclude, areas } = labelSnapshot;
789+
_createHvacDiscovery(networkId, appId, groupId, groupLabel) {
790+
const { labelMap, entityIds, exclude, areas } = this._labelSnapshot;
784791
const labelKey = `${networkId}/${appId}/${groupId}`;
785792

786793
if (exclude.has(labelKey)) {
@@ -853,8 +860,8 @@ class HaDiscovery {
853860
this.discoveryCount++;
854861
}
855862

856-
_createDiscovery(networkId, appId, groupId, groupLabel, config, labelSnapshot) {
857-
const { labelMap, entityIds, exclude, areas } = labelSnapshot;
863+
_createDiscovery(networkId, appId, groupId, groupLabel, config) {
864+
const { labelMap, entityIds, exclude, areas } = this._labelSnapshot;
858865
const labelKey = `${networkId}/${appId}/${groupId}`;
859866

860867
if (exclude.has(labelKey)) {
@@ -926,15 +933,15 @@ class HaDiscovery {
926933
// - a button entity so HA automations can fire the C-Bus trigger via the trigger topic
927934
// - a scene entity (when enabled) so HA scenes can activate the C-Bus scene via the switch topic
928935
if (config.isTrigger) {
929-
this._publishTriggerButton(networkId, appId, groupId, finalLabel, labelSnapshot);
936+
this._publishTriggerButton(networkId, appId, groupId, finalLabel);
930937
if (this.settings.ha_discovery_scene_enabled !== false) {
931-
this._publishTriggerScene(networkId, appId, groupId, finalLabel, labelSnapshot);
938+
this._publishTriggerScene(networkId, appId, groupId, finalLabel);
932939
}
933940
}
934941
}
935942

936-
_publishTriggerButton(networkId, appId, groupId, label, labelSnapshot) {
937-
const { entityIds } = labelSnapshot;
943+
_publishTriggerButton(networkId, appId, groupId, label) {
944+
const { entityIds } = this._labelSnapshot;
938945
const labelKey = `${networkId}/${appId}/${groupId}`;
939946
const uniqueId = `cgateweb_${networkId}_${appId}_${groupId}_btn`;
940947
const entityId = entityIds.get(labelKey);
@@ -966,8 +973,8 @@ class HaDiscovery {
966973
this.discoveryCount++;
967974
}
968975

969-
_publishTriggerScene(networkId, appId, groupId, label, labelSnapshot) {
970-
const { entityIds } = labelSnapshot;
976+
_publishTriggerScene(networkId, appId, groupId, label) {
977+
const { entityIds } = this._labelSnapshot;
971978
const labelKey = `${networkId}/${appId}/${groupId}`;
972979
const uniqueId = `cgateweb_${networkId}_${appId}_${groupId}_scene`;
973980
const entityId = entityIds.get(labelKey);

0 commit comments

Comments
 (0)