A sample Matter Controller Electron UI Application. #2223
Conversation
Based on * Electron * Vite * React * Bootstrap CSS & Icons * Typescript Currently, it is only able to commission a single device via BLE onto Thread and on a restart connect to it again. Even on Windows. Missing features: * Dealing with multiple devices * Showing connectivity status * Interactions (Read, Write, Invoke, ...) with the device and displaying results * WiFi commissioning * Handling errors * .... Some Mock UI is commented in app.tsx.
There was a problem hiding this comment.
Code Review
This pull request introduces a new sample Matter Controller Electron UI application. Review focuses on improving correctness and maintainability, addressing potential critical issues such as the application exiting automatically and high severity issues such as runtime errors due to lack of input validation.
packages/examples/src/electron-ui-controller/src/matter/ControllerNode.ts
Outdated
Show resolved
Hide resolved
| parseInt(matterLongDiscriminatorText), | ||
| parseInt(matterPasscodeText) |
There was a problem hiding this comment.
Using parseInt directly on user input without validation is risky. If the input is not a valid number, parseInt will return NaN, potentially causing issues in the native Matter.js code. Add validation to ensure the parsed values are numbers before calling the API.
| threadOperationalDatasetText, | ||
| matterLongDiscriminatorNumber, | ||
| matterPasscodeNumber | ||
| ).catch(error => console.log(error)); |
There was a problem hiding this comment.
This .catch block swallows any errors from the ControllerNode. The renderer process will not be notified of the failure, making it difficult to provide feedback to the user. It's better to let the promise reject so the error can be caught in the renderer process.
packages/examples/src/electron-ui-controller/src/matter/ControllerNode.ts
Outdated
Show resolved
Hide resolved
| // console.log("Found commissioned nodes:", Diagnostic.json(nodes)); | ||
| console.log("Found commissioned nodes:", nodes); | ||
|
|
||
| const nodeId = NodeId(environment.vars.number("nodeid") ?? nodes[0]); |
There was a problem hiding this comment.
Accessing nodes[0] without checking if the nodes array is empty can cause a crash if getCommissionedNodes() returns no nodes. Add a check for nodes.length > 0 before attempting to access an element.
Based on
Currently, it is only able to commission a single device via BLE onto Thread and on a restart connect to it again. Even on Windows.
Missing features:
Some Mock UI is commented in app.tsx.