From cdb3ff6949f71ff174b776958670baf778b75074 Mon Sep 17 00:00:00 2001 From: Rav Panchalingam Date: Thu, 6 Aug 2026 13:02:19 +0800 Subject: [PATCH 1/2] fix: fall back to per-index object-list scan and flatten the discovery loop Devices that answer ReadProperty(OBJECT_NAME) but silently drop ReadProperty(OBJECT_LIST, ALL) were never discovered by the scheduled poll. The per-index scan that works on them already existed but was only reachable from the read node's right-click "Update points" menu, so the poll retried the same doomed read every cycle and the point list stayed at just the device object. Add BacnetClient.discoverPointList(device): try the single round-trip ALL read, and fall back to the per-index scan when it fails. A successful fallback latches manualDiscoveryMode so later polls skip the doomed ALL read instead of burning an apduTimeout every cycle. Success is judged by what the scan returned on this pass rather than by growth of the device's cumulative point list, so a healthy latched device keeps reporting success in steady state instead of logging a spurious error every poll. Both the scheduled loop and updatePointsForDevice now go through this one strategy. scanDevice no longer resets manualDiscoveryMode as a side effect, which would otherwise clobber the latch on the next poll. Also rewrite the queryDevices device iteration from recursion to a for loop. A missing return in the getProtocolSupported error path made each failing device traverse the remaining list twice, so m failing devices produced 2^m traversals, each costing an apduTimeout per unresponsive device. pollInProgress now resets in a finally, so an exception escaping the loop can no longer disable device polling until Node-RED restarts. Error logs use getDeviceAddress() instead of interpolating the address object, which rendered as [object Object] for MSTP devices. Fixes #53 Fixes #52 Refs #49 Claude-Session: https://claude.ai/code/session_015Yf3sg2PeMKH29s3owfyxM --- bacnet_client.js | 137 +++++++++++++++++++++++------------------------ 1 file changed, 67 insertions(+), 70 deletions(-) diff --git a/bacnet_client.js b/bacnet_client.js index 70fbccb..1d0501c 100644 --- a/bacnet_client.js +++ b/bacnet_client.js @@ -516,21 +516,11 @@ class BacnetClient extends EventEmitter { } } - try { - await this.getDevicePointList(device); - await this.buildJsonObject(device); - } catch (e) { - this.logOut(`Update points list error 2: ${this.getDeviceAddress(device)}`, e); - device.setManualDiscoveryMode(true); - - try { - await this.getDevicePointListWithoutObjectList(device); - await this.buildJsonObject(device); - } catch (e) { - await this.buildJsonObject(device); - this.logOut(`Update points list error 4: ${this.getDeviceAddress(device)}`, e); - } + const discoverySucceeded = await this.discoverPointList(device); + if (!discoverySucceeded) { + this.logOut(`Update points list error: ${this.getDeviceAddress(device)} - ${device.getDeviceId()}`); } + await this.buildJsonObject(device); return true; } catch (e) { @@ -629,69 +619,38 @@ class BacnetClient extends EventEmitter { async queryDevices() { let that = this; + that.pollInProgress = true; try { - that.pollInProgress = true; - - let index = 0; - await query(index); - - async function query(index) { - if (index < that.deviceList.length) { - let device = that.deviceList[index]; - if (typeof device == "object" && (device.getIsDumbMstpRouter() == false || device.getIsDumbMstpRouter() == undefined)) { - if (device.getIsProtocolServicesSet() == false) { - try { - let result = await that.getProtocolSupported(device); - let decodedValues = decodeBitArray(8, result.values[0].originalBitString.value); - device.setProtocolServicesSupported(decodedValues); - } catch (error) { - that.logOut("getProtocolSupported error: ", error); - index++; - await query(index); - } - } - try { - await that.updateDeviceName(device); - - if (device.getSegmentation() !== 3) { - try { - await that.getDevicePointList(device); - index++; - await query(index); - } catch (e) { - that.logOut(`getDevicePointList error: ${device.getAddress()}`, e); - - index++; - await query(index); - } - } else if (device.getSegmentation() == 3) { - try { - await that.getDevicePointListWithoutObjectList(device); - index++; - await query(index); - } catch (e) { - that.logOut(`getDevicePointList error: ${device.getAddress()}`, e); + for (let device of that.deviceList) { + if (typeof device != "object" || device.getIsDumbMstpRouter() == true) { + continue; + } - index++; - await query(index); - } - } - } catch (e) { - that.logOut("Error while querying devices: ", e); + if (device.getIsProtocolServicesSet() == false) { + try { + let result = await that.getProtocolSupported(device); + let decodedValues = decodeBitArray(8, result.values[0].originalBitString.value); + device.setProtocolServicesSupported(decodedValues); + } catch (error) { + that.logOut("getProtocolSupported error: ", error); + continue; + } + } + try { + await that.updateDeviceName(device); - index++; - await query(index); - } - } else { - index++; - await query(index); + const discoverySucceeded = await that.discoverPointList(device); + if (!discoverySucceeded) { + that.logOut(`getDevicePointList error: ${that.getDeviceAddress(device)} - ${device.getDeviceId()}`); } - } else if (index == that.deviceList.length) { - that.pollInProgress = false; + } catch (e) { + that.logOut("Error while querying devices: ", e); } } } catch (e) { that.logOut("Error while querying devices: ", e); + } finally { + that.pollInProgress = false; } } @@ -1252,7 +1211,6 @@ class BacnetClient extends EventEmitter { let that = this; return new Promise(async function (resolve, reject) { try { - device.setManualDiscoveryMode(false); let result = await that.scanDevice(device); device.setPointsList(result); device.setLastSeen(Date.now()); @@ -1264,6 +1222,45 @@ class BacnetClient extends EventEmitter { }); } + async discoverPointList(device) { + const preferIndexScan = device.getSegmentation() == 3 || device.getManualDiscoveryMode() == true; + + if (!preferIndexScan) { + try { + await this.getDevicePointList(device); + device.clearPointListRetryCount(); + return true; + } catch (e) { + device.incrementPointListRetryCount(); + this.logOut( + `OBJECT_LIST ALL read failed for ${this.getDeviceAddress(device)} - ${device.getDeviceId()} ` + + `(retry ${device.getPointListRetryCount()}); falling back to per-index scan`, + e + ); + } + } + + let scanned; + try { + scanned = await this.getDevicePointListWithoutObjectList(device); + } catch (e) { + this.logOut( + `getDevicePointListWithoutObjectList error: ${this.getDeviceAddress(device)} - ${device.getDeviceId()}`, + e + ); + return false; + } + const found = Array.isArray(scanned) ? scanned.length : 0; + + if (found > 0) { + device.setManualDiscoveryMode(true); + device.clearPointListRetryCount(); + return true; + } + + return false; + } + getDevicePointListWithoutObjectList(device) { let that = this; return new Promise(function (resolve, reject) { From 49a4879b467246848ac6bd17cd336e4ff0c33e05 Mon Sep 17 00:00:00 2001 From: Rav Panchalingam Date: Thu, 6 Aug 2026 23:16:53 +0800 Subject: [PATCH 2/2] fix: make discoverPointList the single reporter of point-list failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses both points from the Copilot review on PR #54. scanDevice and getDevicePointList each logged the same failure on the way out, so one timeout produced three lines — and getDevicePointList interpolated device.getAddress().toString(), which renders as [object Object] for MSTP devices whose address is an object. Both now just reject; discoverPointList is the only place a point-list discovery failure is reported, and it is the only frame that knows whether a fallback follows. Rename the queryDevices verdict log from "getDevicePointList error" to "Point list discovery failed (both object-list strategies)". The old string named a function the call site no longer invokes, and it could not distinguish which strategy failed. Also reject instead of swallowing in scanDevice's malformed-acknowledgement branch. It logged and left the promise unsettled, which would hang the caller rather than surfacing the bad response. Claude-Session: https://claude.ai/code/session_015Yf3sg2PeMKH29s3owfyxM --- bacnet_client.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/bacnet_client.js b/bacnet_client.js index 1d0501c..b89b60c 100644 --- a/bacnet_client.js +++ b/bacnet_client.js @@ -641,7 +641,9 @@ class BacnetClient extends EventEmitter { const discoverySucceeded = await that.discoverPointList(device); if (!discoverySucceeded) { - that.logOut(`getDevicePointList error: ${that.getDeviceAddress(device)} - ${device.getDeviceId()}`); + that.logOut( + `Point list discovery failed (both object-list strategies): ${that.getDeviceAddress(device)} - ${device.getDeviceId()}` + ); } } catch (e) { that.logOut("Error while querying devices: ", e); @@ -1216,7 +1218,7 @@ class BacnetClient extends EventEmitter { device.setLastSeen(Date.now()); resolve(result); } catch (e) { - that.logOut(`Error getting point list for ${device.getAddress().toString()} - ${device.getDeviceId()}: `, e); + // Logged by discoverPointList, which knows whether a fallback follows. reject(e); } }); @@ -1747,10 +1749,12 @@ class BacnetClient extends EventEmitter { try { resolve(result.values); } catch (e) { - that.logOut("Issue with getting device point list, see error: ", e); + // Malformed acknowledgement — reject rather than leaving the promise unsettled, + // which would hang the caller until its own timeout (if it has one). + reject(e); } } else { - that.logOut(`Error while fetching objects: ${err}`); + // discoverPointList is the single place point-list discovery failures are logged. reject(err); } });