Skip to content

Commit d29c3d1

Browse files
committed
refactor: unify HA Discovery retry state into a single map
HaDiscovery tracked TREEXML retry plumbing across two parallel Maps (_treeWatchdogs and _treeRetryState) keyed identically on networkId, with three near-duplicate cleanup helpers and a keepAttempts boolean toggle that smelled of squashed operations. Collapse to a single _treeRequestState Map whose value is { attempts, watchdogHandle, retryHandle }. The cleanup helpers reduce to one parameterised _clearTimer and a _clearTreeState entry-killer; the boolean parameter goes away. handleTreeStart's "successful response" path is now a single _clearTreeState call, which both releases timers and resets the attempts counter for any future failure. Drop the typeof === 'function' guards in BridgeInitializationService around haDiscovery.handleCommandError and haDiscovery.stop — both are part of the class contract and the bridge holds a locally constructed instance, so the guards traded clarity for hypothetical safety. The test mock is updated to match the contract. Behaviour unchanged; full suite (1172 tests) passes. Bumps to 1.8.3.
1 parent 7020fd9 commit d29c3d1

7 files changed

Lines changed: 68 additions & 60 deletions

File tree

homeassistant-addon/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ All notable changes to the C-Gate Web Bridge Home Assistant add-on will be docum
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [1.8.3] - 2026-05-04
9+
10+
### Refactor
11+
- **HA Discovery retry state**: collapsed the parallel `_treeWatchdogs` and `_treeRetryState` Maps in `HaDiscovery` into a single `_treeRequestState` Map (`networkId -> { attempts, watchdogHandle, retryHandle }`), eliminating duplicate lookups, an awkward `keepAttempts` boolean parameter, and three near-duplicate cleanup helpers. Behaviour is unchanged.
12+
- Removed unnecessary `typeof === 'function'` guards around `haDiscovery.handleCommandError` and `haDiscovery.stop` calls in `BridgeInitializationService`; the methods are part of the class contract and the bridge holds a locally constructed instance.
13+
814
## [1.8.2] - 2026-05-04
915

1016
### Fixed

homeassistant-addon/config.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
name: "C-Gate Web Bridge"
2-
version: "1.8.2"
2+
version: "1.8.3"
33
slug: cgateweb
44
description: "Bridge between Clipsal C-Bus systems and MQTT/Home Assistant"
55
url: "https://github.com/dougrathbone/cgateweb"

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "cgateweb",
3-
"version": "1.8.2",
3+
"version": "1.8.3",
44
"description": "Node.js bridge connecting Clipsal C-Bus automation systems to MQTT for Home Assistant integration",
55
"keywords": [
66
"cbus",

src/bridgeInitializationService.js

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ class BridgeInitializationService {
309309
* that fail because C-Gate hasn't finished loading networks at startup.
310310
*/
311311
handleCommandError(code, statusData) {
312-
if (this.bridge.haDiscovery && typeof this.bridge.haDiscovery.handleCommandError === 'function') {
312+
if (this.bridge.haDiscovery) {
313313
this.bridge.haDiscovery.handleCommandError(code, statusData);
314314
}
315315

@@ -344,9 +344,7 @@ class BridgeInitializationService {
344344
this.bridge.labelLoader.unwatch();
345345

346346
if (this.bridge.haDiscovery) {
347-
if (typeof this.bridge.haDiscovery.stop === 'function') {
348-
this.bridge.haDiscovery.stop();
349-
}
347+
this.bridge.haDiscovery.stop();
350348
this.bridge.haDiscovery.removeAllListeners?.();
351349
this.bridge.haDiscovery = null;
352350
this.bridge.commandResponseProcessor.haDiscovery = null;

src/haDiscovery.js

Lines changed: 49 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,11 @@ class HaDiscovery {
7272

7373
// C-Gate accepts TCP connections on the command port before its project
7474
// networks are loaded. Initial TREEXML can therefore return 401 "Network
75-
// not found" until C-Gate finishes startup. These maps drive a
75+
// not found" until C-Gate finishes startup. This map drives a
7676
// per-network retry loop with exponential backoff so HA Discovery
7777
// recovers automatically without restarting the bridge.
78-
this._treeWatchdogs = new Map(); // networkId -> timeoutHandle
79-
this._treeRetryState = new Map(); // networkId -> { attempts, retryHandle }
78+
// networkId -> { attempts, watchdogHandle, retryHandle }
79+
this._treeRequestState = new Map();
8080
this._maxTreeRetryAttempts = 8;
8181
this._treeRetryInitialDelayMs = 2000;
8282
this._treeRetryMaxDelayMs = 60000;
@@ -157,9 +157,11 @@ class HaDiscovery {
157157

158158
queueTreeRequest(networkId) {
159159
const normalizedNetwork = String(networkId);
160+
const state = this._getOrCreateTreeState(normalizedNetwork);
160161

161-
// Cancel any pending retry timer — we're sending a fresh request now.
162-
this._cancelTreeRetry(normalizedNetwork, /* keepAttempts */ true);
162+
// Sending a fresh request: clear any pending retry but keep the
163+
// attempts counter so backoff continues if this attempt also fails.
164+
this._clearTimer(state, 'retryHandle');
163165

164166
this.logger.info(`Requesting TreeXML for network ${normalizedNetwork}...`);
165167

@@ -168,40 +170,45 @@ class HaDiscovery {
168170
this.pendingTreeNetworks.push(normalizedNetwork);
169171
}
170172

171-
this._armTreeWatchdog(normalizedNetwork);
172-
this._sendCommand(`${CGATE_CMD_TREEXML} ${normalizedNetwork}${NEWLINE}`);
173-
}
173+
this._clearTimer(state, 'watchdogHandle');
174+
state.watchdogHandle = this._setTimer(this._treeRequestTimeoutMs, () => {
175+
state.watchdogHandle = null;
176+
this._handleTreeRequestFailure(normalizedNetwork, 'no response within timeout');
177+
});
174178

175-
_armTreeWatchdog(networkId) {
176-
this._cancelTreeWatchdog(networkId);
177-
const handle = setTimeout(() => {
178-
this._treeWatchdogs.delete(networkId);
179-
this._handleTreeRequestFailure(networkId, 'no response within timeout');
180-
}, this._treeRequestTimeoutMs);
181-
if (typeof handle.unref === 'function') handle.unref();
182-
this._treeWatchdogs.set(networkId, handle);
179+
this._sendCommand(`${CGATE_CMD_TREEXML} ${normalizedNetwork}${NEWLINE}`);
183180
}
184181

185-
_cancelTreeWatchdog(networkId) {
186-
const handle = this._treeWatchdogs.get(networkId);
187-
if (handle) {
188-
clearTimeout(handle);
189-
this._treeWatchdogs.delete(networkId);
182+
_getOrCreateTreeState(networkId) {
183+
let state = this._treeRequestState.get(networkId);
184+
if (!state) {
185+
state = { attempts: 0, watchdogHandle: null, retryHandle: null };
186+
this._treeRequestState.set(networkId, state);
190187
}
188+
return state;
191189
}
192190

193-
_cancelTreeRetry(networkId, keepAttempts = false) {
194-
const state = this._treeRetryState.get(networkId);
191+
_clearTreeState(networkId) {
192+
const state = this._treeRequestState.get(networkId);
195193
if (!state) return;
196-
if (state.retryHandle) {
197-
clearTimeout(state.retryHandle);
198-
state.retryHandle = null;
199-
}
200-
if (!keepAttempts) {
201-
this._treeRetryState.delete(networkId);
194+
this._clearTimer(state, 'watchdogHandle');
195+
this._clearTimer(state, 'retryHandle');
196+
this._treeRequestState.delete(networkId);
197+
}
198+
199+
_clearTimer(state, key) {
200+
if (state[key]) {
201+
clearTimeout(state[key]);
202+
state[key] = null;
202203
}
203204
}
204205

206+
_setTimer(delayMs, fn) {
207+
const handle = setTimeout(fn, delayMs);
208+
if (typeof handle.unref === 'function') handle.unref();
209+
return handle;
210+
}
211+
205212
/**
206213
* Receives a 4xx/5xx C-Gate command error and, if it indicates that an
207214
* in-flight TreeXML request failed because the network isn't loaded yet,
@@ -220,12 +227,13 @@ class HaDiscovery {
220227
if (this.pendingTreeNetworks.length === 0) return;
221228

222229
const failedNetwork = this.pendingTreeNetworks.shift();
223-
this._cancelTreeWatchdog(failedNetwork);
230+
const state = this._treeRequestState.get(failedNetwork);
231+
if (state) this._clearTimer(state, 'watchdogHandle');
224232
this._handleTreeRequestFailure(failedNetwork, '401 Network not found');
225233
}
226234

227235
_handleTreeRequestFailure(networkId, reason) {
228-
const state = this._treeRetryState.get(networkId) || { attempts: 0, retryHandle: null };
236+
const state = this._getOrCreateTreeState(networkId);
229237
state.attempts += 1;
230238

231239
if (state.attempts > this._maxTreeRetryAttempts) {
@@ -234,7 +242,7 @@ class HaDiscovery {
234242
`Auto-discovery for this network is paused. ` +
235243
`Verify the network is configured and reachable in C-Gate, then restart the bridge or publish to cbus/write/${networkId}///gettree to retry.`
236244
);
237-
this._treeRetryState.delete(networkId);
245+
this._clearTreeState(networkId);
238246
return;
239247
}
240248

@@ -249,27 +257,20 @@ class HaDiscovery {
249257
`This typically means C-Gate is still loading networks at startup.`
250258
);
251259

252-
if (state.retryHandle) clearTimeout(state.retryHandle);
253-
state.retryHandle = setTimeout(() => {
260+
this._clearTimer(state, 'retryHandle');
261+
state.retryHandle = this._setTimer(delay, () => {
254262
state.retryHandle = null;
255263
this.queueTreeRequest(networkId);
256-
}, delay);
257-
if (typeof state.retryHandle.unref === 'function') state.retryHandle.unref();
258-
this._treeRetryState.set(networkId, state);
264+
});
259265
}
260266

261267
/**
262268
* Cancels all retry timers and watchdogs. Call on bridge shutdown.
263269
*/
264270
stop() {
265-
for (const handle of this._treeWatchdogs.values()) {
266-
clearTimeout(handle);
267-
}
268-
this._treeWatchdogs.clear();
269-
for (const state of this._treeRetryState.values()) {
270-
if (state.retryHandle) clearTimeout(state.retryHandle);
271+
for (const networkId of [...this._treeRequestState.keys()]) {
272+
this._clearTreeState(networkId);
271273
}
272-
this._treeRetryState.clear();
273274
}
274275

275276
handleTreeStart(_statusData) {
@@ -280,10 +281,10 @@ class HaDiscovery {
280281
const nextNetwork = this.pendingTreeNetworks.shift() || this.treeNetwork || 'unknown';
281282
const networkKey = String(nextNetwork);
282283

283-
// The request succeeded — cancel watchdog and any pending retry so a
284-
// late retry doesn't issue a redundant TREEXML.
285-
this._cancelTreeWatchdog(networkKey);
286-
this._cancelTreeRetry(networkKey);
284+
// The request succeeded — drop watchdog and any pending retry so a
285+
// late retry doesn't issue a redundant TREEXML, and reset the attempts
286+
// counter for any future failure on this network.
287+
this._clearTreeState(networkKey);
287288

288289
this.activeTreeSession = {
289290
network: networkKey,

tests/bridgeInitializationService.test.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ describe('BridgeInitializationService', () => {
5858
HaDiscovery.mockClear();
5959
HaDiscovery.mockImplementation(() => ({
6060
trigger: jest.fn(),
61-
updateLabels: jest.fn()
61+
updateLabels: jest.fn(),
62+
handleCommandError: jest.fn(),
63+
stop: jest.fn()
6264
}));
6365
});
6466

tests/haDiscovery.test.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,9 @@ describe('HaDiscovery', () => {
602602
const errMsg = 'Bad object or device ID: Network not found';
603603

604604
// 8 retries permitted; the 9th failure exhausts the budget.
605+
// runOnlyPendingTimers fires just the scheduled retry (which
606+
// arms a fresh watchdog) — the next handleCommandError cancels
607+
// that watchdog before it can fire and double-count attempts.
605608
for (let i = 1; i <= 8; i++) {
606609
haDiscovery.handleCommandError('401', errMsg);
607610
jest.runOnlyPendingTimers();
@@ -637,8 +640,7 @@ describe('HaDiscovery', () => {
637640
haDiscovery.handleTreeEnd('end');
638641

639642
// Watchdog and retry state should now be cleared.
640-
expect(haDiscovery._treeWatchdogs.size).toBe(0);
641-
expect(haDiscovery._treeRetryState.size).toBe(0);
643+
expect(haDiscovery._treeRequestState.size).toBe(0);
642644

643645
// Advance time well past any potential retry — no extra commands.
644646
const callsBefore = mockSendCommandFn.mock.calls.length;
@@ -666,11 +668,10 @@ describe('HaDiscovery', () => {
666668
it('stop() clears all retry timers and watchdogs', () => {
667669
haDiscovery.trigger();
668670
haDiscovery.handleCommandError('401', 'Bad object or device ID: Network not found');
669-
expect(haDiscovery._treeRetryState.size).toBe(1);
671+
expect(haDiscovery._treeRequestState.size).toBe(1);
670672

671673
haDiscovery.stop();
672-
expect(haDiscovery._treeWatchdogs.size).toBe(0);
673-
expect(haDiscovery._treeRetryState.size).toBe(0);
674+
expect(haDiscovery._treeRequestState.size).toBe(0);
674675

675676
const callsBefore = mockSendCommandFn.mock.calls.length;
676677
jest.advanceTimersByTime(120000);

0 commit comments

Comments
 (0)