feat(zwave-js): skip interview if S2 failed
Fixes #7011
This answer to the issue guided me to the solution.
Problem:
A node should not be interviewed when security S2 fails
Solution:
Adding InclusionStrategy in InclusionResult allows to check for low security of S2 strategy just before the interview.
a) The
InclusionResultis used by applications, and those applications pass in the inclusion strategy when starting the inclusion. So this would essentially pass unnecessary info back
Oh I see. Would moving the skip condition before the "node added" event would be better then ?
// Skip the interview if S2 failed
if (result.lowSecurity
&& opts.strategy == InclusionStrategy.Security_S2) return;
this.emit("node added", newNode, result);
b) It does not consider the interviews that happen when Z-Wave JS restarts, loads its network information from cache and some devices have not fully been interviewed.
Can you point me where this happen please ? I would like to look into it.
Would moving the skip condition before the "node added" event would be better then ?
Not really. The event is needed for applications. I think it would make more sense to store this as an attribute of the node that also gets persisted to the cache.
Can you point me where this happen please ? I would like to look into it.
See the initializeControllerAndNodes method in Driver.ts - pretty far at the end of that method.
Yes sorry @AlCalzone, I 100% forgot 🥲
If it's ok to put the skip there in the Driver.ts file, I can quickly go to a computer with git on it and update the PR.
/** This is called when a new node has been added to the network */
private onNodeAdded(node: ZWaveNode): void {
this.addNodeEventHandlers(node);
if (this._options.interview?.disableOnNodeAdded) return;
if (this._options.testingHooks?.skipNodeInterview) return;
if (node.failedS2) return; // <===== SKIP ADDED HERE
// Interview the node
// don't await the interview, because it may take a very long time
// if a node is asleep
void this.interviewNodeInternal(node);
}
EDIT: pushed it