Skip to content
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

Session Refactoring to bring logic more near to specs #590

Merged
merged 22 commits into from
Dec 22, 2023
Merged

Session Refactoring to bring logic more near to specs #590

merged 22 commits into from
Dec 22, 2023

Conversation

Apollon77
Copy link
Collaborator

Description for now short: see single commits ... I will update here b tomorrow and will also check if tests are missing and possible

This commit moves the MessageCounter class into an own file and implements the required variants as defined by specification. It also adds tests
As by specification the message counter should be handled by the respective session, so also move it there.
To detect duplicate messages the spec defined message Reception state which tracks messages and tests.
The message Reception state for the incoming messages is tracked by the sessions. So we add it there. The method "updateMessageCounter" throws an DuplicateMessageError in case of a duplicate.
The logic to find the next available session id should wrap and search free spots - in worst case it should close the oldest unused session.
Additionally this commit moves the session id finding to the respective place where it is needed to prevent taking session ids that are then not ued because of errors.
Unsecure Sessions are created by their initiator Node Id ... od 0 if not specified. With this change UnsecureSessions also get a close callback for cleanup reasons only.
... if close is not already in progress
@Apollon77 Apollon77 marked this pull request as ready for review December 20, 2023 12:17
Copy link
Collaborator

@lauckhart lauckhart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't compare super closely to the spec but it's got to be an improvement

packages/matter.js/src/MatterController.ts Outdated Show resolved Hide resolved
@mergify mergify bot merged commit ec1d465 into project-chip:main Dec 22, 2023
20 checks passed
@Apollon77 Apollon77 deleted the session-revamp branch December 27, 2023 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants