More tests #334
Merged
More tests #334
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
PR #334: Size comparison from d8463db to c2cbbf8 Increases above 0.2%:
Full report (8 builds for dimmable-light, onoff-light, onoff-light-bt, rs-matter-core, speaker)
|
…y everywhere from pase session to commissioning window
|
PR #334: Size comparison from e14f27a to 383637b Increases above 0.2%:
Full report (8 builds for dimmable-light, onoff-light, onoff-light-bt, rs-matter-core, speaker)
|
|
Also see #335 |
|
The code size is increased (by 2.5% in the worst case for thumbv6m) because the 3rd commit introduces new functionality (configuring extra mDNS parameters) and also the 4th commit introduces new functionality (option to timeout the Commissioning Window). |
gmarcosb
approved these changes
Oct 20, 2025
|
PR #334: Size comparison from e14f27a to bd9c3e2 Increases above 0.2%:
Full report (8 builds for dimmable-light, onoff-light, onoff-light-bt, rs-matter-core, speaker)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains the following commits:
1st commit
Simply enables the 7 tests below. Did not require any changes to
rs-matter2nd commit
Enables the
TestDiscoverytest.For this test, it was necessary to implement all optional mDNS payload:
While I could've just disabled the tests for the optional payload in the
chip_tests.picsfile, I thought it would be good to support this payload in general, for end users. The payload is now configurable through theBasicInfostructure.3rd commit
This is a follow-up to the 2nd commit, which was not strictly necessary but I did it anyway. It documents as well as does a cleanup on the
pairingmodule, which is all about computing the commissioning QR code.Some of the problems addressed with this commit:
QrorQrPayload)QrTextPayloadrenamed to justQrPayload. The purpose in life ofQrPayloadis to compute the QR textQrwhich is the final outcome of the QR computation; it takes (a) text and encodes it into a QR; all utility functions to consume the QR code as ASCII-art are now moved into this type and this type is independent fromQrPayloadin that while it is expected that it will be used to render precisely the text produced byQrPayload, that might not be necessarily the case4th commit
This commit introduces a real expiration of the Commissioning Window. Previously, the timeout parameter was not really used and therefore, the commissioning window stayed open until explicitly closed, or in case commissioning did fail - forever.
Furthermore, I've tried to document and tighten up a bit the code in the
caseandpakemodules.While I doubt this would be the latest cleanup in these modules (the contain one of the oldest piece of code in the codebase), it is now in a slightly better shape, in that a lot of freestanding functions are moved to their appropriate structures, and some structures like
Spake2pandCaseSessionare more private in that the SC layer doesn't need to know about those. It only needs to know aboutPakeandCasewhich handle the establishment of a PASE or CASE session, andSpake2pandCaseSessionare actually their internal implementation detail.But the most important renaming change is that what used to be called "PASE session" is now called "Commissioning Window" everywhere, because really that's what it is.
The notion of a PASE session is anyway overloaded with two additional meanings so also calling the Commissioning Window a "PASE session" was wrong IMO:
Sessionwhich was created as a result of a PASE/PAKE session negotiation/establishment. (Ditto - a CASE session might mean aSessionwhich was created as a result of a CASE session negotiation/establishment.)Sessionusing the PAKE negotiation process. Though I've taken care to always talk there about a PASE session establishment, as in the Matter Core Spec