Summary
scanDeviceManually walks the object list by array index and treats any error as end-of-array. A
transient APDU timeout partway through therefore truncates the point list silently, and the truncated
list is then stored as if it were complete.
Found while investigating #49. Not the cause of that issue, but it directly undermines the fallback
path that #49's fix depends on.
Detail
bacnet_client.js:1288-1333:
that.client.readProperty(addressObject, {type: DEVICE, instance: deviceId}, OBJECT_LIST, readOptions,
(err, value) => {
if (err) {
resolve(discoveredPointList); // any error == "we're done"
}
if (value) {
discoveredPointList.push(value.values[0]);
index++;
send(index);
}
}
);
The scan starts at index 1 and never reads index 0 (the element count), so it has no idea how many
objects it is supposed to find. The only termination signal is an error — which is indistinguishable
from a dropped packet, a busy device, or a router hiccup.
Consequences:
- A timeout on object 12 of 48 yields 11 objects, resolved as success.
- The caller cannot tell truncation from completion.
getDevicePointListWithoutObjectList
(bacnet_client.js:1267) calls setPointsList(result) and setLastSeen(Date.now()) either way.
- A failure on the first index resolves
[]. setPointsList([]) iterates nothing and leaves the
existing [device] entry intact, so total failure looks identical to "no new points".
There is also no err/value else-branch: if a callback ever fires with neither, the promise never
settles.
Proposed fix
Read array index 0 first to get the element count, then read 1..count, and resolve only when
index > count. Anything short of count is a failure and should reject, not resolve.
Devices that reject the index-0 read can keep the current walk-until-error behaviour as a degraded
mode, but it should be logged as such rather than presented as a complete list.
Make the err branch return after resolving, and add an else-branch so a callback with neither
err nor value settles the promise instead of hanging.
Acceptance criteria
- A device that errors partway through the index scan produces a rejection or an explicitly-flagged
partial result, not a silently truncated point list stored as complete.
- A device that answers index 0 has exactly
count objects read, and the result is only accepted when
all count are present.
- A callback that fires with neither
err nor value settles the promise.
Summary
scanDeviceManuallywalks the object list by array index and treats any error as end-of-array. Atransient APDU timeout partway through therefore truncates the point list silently, and the truncated
list is then stored as if it were complete.
Found while investigating #49. Not the cause of that issue, but it directly undermines the fallback
path that #49's fix depends on.
Detail
bacnet_client.js:1288-1333:The scan starts at index 1 and never reads index 0 (the element count), so it has no idea how many
objects it is supposed to find. The only termination signal is an error — which is indistinguishable
from a dropped packet, a busy device, or a router hiccup.
Consequences:
getDevicePointListWithoutObjectList(
bacnet_client.js:1267) callssetPointsList(result)andsetLastSeen(Date.now())either way.[].setPointsList([])iterates nothing and leaves theexisting
[device]entry intact, so total failure looks identical to "no new points".There is also no
err/valueelse-branch: if a callback ever fires with neither, the promise neversettles.
Proposed fix
Read array index
0first to get the element count, then read1..count, and resolve only whenindex > count. Anything short ofcountis a failure and should reject, not resolve.Devices that reject the index-0 read can keep the current walk-until-error behaviour as a degraded
mode, but it should be logged as such rather than presented as a complete list.
Make the
errbranchreturnafter resolving, and add an else-branch so a callback with neithererrnorvaluesettles the promise instead of hanging.Acceptance criteria
partial result, not a silently truncated point list stored as complete.
countobjects read, and the result is only accepted whenall
countare present.errnorvaluesettles the promise.