-
Notifications
You must be signed in to change notification settings - Fork 91
Fixes, Optimizations and Preparations #1 #2814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR consolidates various internal fixes, optimizations, and preparatory changes to the Matter.js implementation. The changes primarily focus on improving code reliability, proper resource handling, and fixing edge cases in commissioning and subscription management.
- Fixed subscription clearing logic to only remove subscriptions from the same peer when
keepSubscriptions=false - Added guard against double
Node.close()calls and proper async handling in controller behaviors - Moved fabric-sensitive data sanitation from
BehaviorstoServerEndpointInitializerfor better separation of concerns
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/protocol/src/protocol/MessageExchange.ts | Improved comment clarity about resubmission handling |
| packages/protocol/src/fabric/FabricAuthority.ts | Added TODO comment for fabric certificate validation |
| packages/protocol/src/certificate/CertificateAuthority.ts | Enhanced log message for credential loading |
| packages/node/src/node/server/ServerSubscription.ts | Added TODO comment about timing consideration |
| packages/node/src/node/server/ServerEndpointInitializer.ts | Moved fabric-scoped data limitation logic here from Behaviors |
| packages/node/src/node/server/InteractionServer.ts | Fixed subscription clearing to only affect same peer's sessions |
| packages/node/src/node/index.ts | Exported ClientNode class |
| packages/node/src/node/client/index.ts | Exported ClientNodeInteraction |
| packages/node/src/node/NodePhysicalProperties.ts | Optimized by using maybeStateOf and extracting status variable |
| packages/node/src/node/Node.ts | Added guard against double close() calls |
| packages/node/src/endpoint/properties/Endpoints.ts | Renamed parameter from 'number' to 'id' for better clarity |
| packages/node/src/endpoint/properties/EndpointInitializer.ts | Changed behaviorsInitialized return type to MaybePromise |
| packages/node/src/endpoint/properties/Behaviors.ts | Added string overloads for has() and versionOf() methods; moved fabric logic |
| packages/node/src/behaviors/group-key-management/GroupKeyManagementServer.ts | Removed unnecessary MaybePromise return type |
| packages/node/src/behaviors/general-diagnostics/GeneralDiagnosticsServer.ts | Removed unnecessary MaybePromise return type |
| packages/node/src/behaviors/general-commissioning/GeneralCommissioningServer.ts | Added check to only arm failsafe for incoming PASE sessions |
| packages/node/src/behaviors/basic-information/BasicInformationServer.ts | Removed unnecessary MaybePromise return type and blank line |
| packages/node/src/behaviors/access-control/AccessControlServer.ts | Removed unnecessary MaybePromise return type |
| packages/node/src/behavior/system/network/index.ts | Exported NetworkClient |
| packages/node/src/behavior/system/controller/ControllerBehavior.ts | Made nodeOnline async and added fabric label synchronization |
| packages/node/src/behavior/system/commissioning/index.ts | Exported RemoteDescriptor |
| packages/node/src/behavior/system/commissioning/RemoteDescriptor.ts | Fixed toLongForm to return the long object |
| packages/node/src/behavior/system/commissioning/CommissioningServer.ts | Moved FabricManager event registration to enterOnlineMode |
| packages/node/src/behavior/system/commissioning/CommissioningClient.ts | Simplified passcode validation; added removeFabric response validation; added commissioningFlowImpl option |
| packages/general/src/util/Cancelable.ts | Fixed spelling: "cancelation" to "cancellation" |
This PR is a "all and nothing PR" collecting several fixes and optimizations I run across while testing. Some highlights are:
Nothing is needed for changelog because most smaller internal optimizations or introduced by recent commits