Allow and ignore '-' characters in manual pairing codes. #22238
Merged
Conversation
This file contains 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
The spec does not mention these, but the brand guidelines apparently talk about using them. While ideally all SDK consumers would do the right thing and strip them out on input, we should probably also try for maximal interop and do the same stripping in the SDK itself.
pullapprove
bot
requested review from
anush-apple,
arkq,
Byungjoo-Lee,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
erjiaqing,
franck-apple,
gjc13,
harimau-qirex,
harsha-rajendran,
hawk248,
isiu-apple,
jelderton,
jepenven-silabs,
jmartinez-silabs,
jtung-apple,
kpschoedel,
lazarkov,
LuDuda,
mrjerryjohns,
msandstedt,
mspang,
robszewczyk and
saurabhst
|
@bzbarsky-apple if not in the spec, should we do this at all at this stage for 1.0? |
|
PR #22238: Size comparison from 50f9835 to e1991ed Increases (8 builds for cc13x2_26x2, cyw30739, linux, psoc6, telink)
Decreases (4 builds for cc13x2_26x2, esp32, psoc6)
Full report (43 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, telink)
|
@andy31415 That is a good question. If we don't, all SDK consumers probably need to. Doable, of course, but might not be as good for interop.... |
tcarmelveilleux
approved these changes
Aug 29, 2022
|
PR #22238: Size comparison from 50f9835 to 57b77f4 Increases (3 builds for mbed, nrfconnect)
Full report (3 builds for mbed, nrfconnect)
|
|
Postponed: it seems like applications can ignore - in manual pairing codes, there is no strict need for SDK to check and/or allow it. |
jmartinez-silabs
approved these changes
Aug 31, 2022
jmeg-sfy
approved these changes
Sep 1, 2022
andy31415
previously requested changes
Sep 7, 2022
|
This seems like we should support this for 1.0, given if we don't fix all clients need to adopt this work. |
|
@andy31415 Please reconsider as this is actually in the spec
This is in the Setup Code specifications, which is outside the "core spec" and under different review process, but still part of what must be met for certification of matter devices.
Please consult source of truth in Causeway: https://groups.csa-iot.org/wg/matter-wg/document/27120 |
andy31415
dismissed
their stale review
Dismiss for 1.0 acceptance: it seems SVE participants are hitting this, may as well fix it then.
isiu-apple
pushed a commit
to isiu-apple/connectedhomeip
that referenced
this pull request
Sep 16, 2022
…p#22238) * Allow and ignore '-' characters in manual pairing codes. The spec does not mention these, but the brand guidelines apparently talk about using them. While ideally all SDK consumers would do the right thing and strip them out on input, we should probably also try for maximal interop and do the same stripping in the SDK itself. * Add test per review comment.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The spec does not mention these, but the brand guidelines apparently talk about
using them. While ideally all SDK consumers would do the right thing and strip
them out on input, we should probably also try for maximal interop and do the
same stripping in the SDK itself.
Problem
Using something with dashes in it as a manual pairing code does not work.
Change overview
Strip out '-' when initializing the manual code parser.
Testing
Tried doing
chip-tool pairing codewith a code with severa dashes inserted and made sure it worked.