feat: support for Z-Wave JS v13 #3799
Conversation
Pull Request Test Coverage Report for Build 9991427870Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
api/lib/ZwaveClient.ts
Outdated
| @@ -1802,7 +1798,8 @@ class ZwaveClient extends TypedEventEmitter<ZwaveClientEventCallbacks> { | |||
| this.logNode( | |||
| zwaveNode, | |||
| 'warn', | |||
| `Unable to add Node ${a.nodeId} to Group ${groupId} of ${sourceMsg}, association not allowed`, | |||
| // FIXME: We should explain the reason why it failed in human-readable form, see doc comments of AssociationCheckResult | |||
There was a problem hiding this comment.
do you mean on frontend side?
There was a problem hiding this comment.
Both logs and frontend IMO
There was a problem hiding this comment.
Check what i did and let me know if it's ok for you
There was a problem hiding this comment.
Although, if we do #3801, we don't have to show the error reason in the logs I think, because there shouldn't be one.
There was a problem hiding this comment.
there shouldn't doesn't mean there will not, also consider that api can be also called from driver function and MQTT so I would leave it there
api/lib/ZwaveClient.ts
Outdated
| const secure = strategy !== InclusionStrategy.Insecure | ||
| const message = `${secure ? 'Secure' : 'Non-secure'} inclusion started` | ||
| this._updateControllerStatus(message) | ||
| // FIXME: Should the frontend also accept the strategy instead of the boolean? | ||
| this.emit('event', EventSource.CONTROLLER, 'inclusion started', secure) |
There was a problem hiding this comment.
Frontend uses startInclusion and passes the mode:
zwave-js-ui/src/components/dialogs/DialogNodesManager.vue
Lines 995 to 998 in 80c1cd8
zwave-js-ui/src/components/dialogs/DialogNodesManager.vue
Lines 1149 to 1156 in 80c1cd8
There was a problem hiding this comment.
I meant the event that is emitted below the comment. Right now it gets passed secure, I wonder if it should now include the strategy instead.
There was a problem hiding this comment.
that event actually is only emittet to MQTT when Emit events flag is enabled in MQTT settings, nothing used by UI btw. I can fix that and use the inclusion strategy instead
api/lib/ZwaveClient.ts
Outdated
| @@ -1802,7 +1798,8 @@ class ZwaveClient extends TypedEventEmitter<ZwaveClientEventCallbacks> { | |||
| this.logNode( | |||
| zwaveNode, | |||
| 'warn', | |||
| `Unable to add Node ${a.nodeId} to Group ${groupId} of ${sourceMsg}, association not allowed`, | |||
| // FIXME: We should explain the reason why it failed in human-readable form, see doc comments of AssociationCheckResult | |||
There was a problem hiding this comment.
do you mean on frontend side?
api/lib/ZwaveClient.ts
Outdated
| const secure = strategy !== InclusionStrategy.Insecure | ||
| const message = `${secure ? 'Secure' : 'Non-secure'} inclusion started` | ||
| this._updateControllerStatus(message) | ||
| // FIXME: Should the frontend also accept the strategy instead of the boolean? | ||
| this.emit('event', EventSource.CONTROLLER, 'inclusion started', secure) |
There was a problem hiding this comment.
Frontend uses startInclusion and passes the mode:
zwave-js-ui/src/components/dialogs/DialogNodesManager.vue
Lines 995 to 998 in 80c1cd8
zwave-js-ui/src/components/dialogs/DialogNodesManager.vue
Lines 1149 to 1156 in 80c1cd8
This comment was marked as resolved.
This comment was marked as resolved.
TODO: