Enable Terms and Conditions (TC) feature for AutoCommissioner #3
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
examples/all-clusters-app/all-clusters-common/all-clusters-app.zap
Outdated
Show resolved
Hide resolved
chrisdecenzo
reviewed
Aug 9, 2024
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-client
branch
2 times, most recently
from
2a862d5 to
697b9dc
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-server
branch
from
ba88272 to
3833a44
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-client
branch
from
697b9dc to
b9ab19f
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-server
branch
2 times, most recently
from
99dcfe9 to
2e647e6
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-client
branch
3 times, most recently
from
1162c88 to
da87ee8
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-server
branch
from
2e647e6 to
fed714e
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-client
branch
from
da87ee8 to
e444bd7
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-server
branch
2 times, most recently
from
95cc173 to
7a31eb1
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-client
branch
from
e444bd7 to
b8ea8c1
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-server
branch
2 times, most recently
from
4fd7f9e to
35d685e
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-client
branch
from
b8ea8c1 to
56fc7ab
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-server
branch
from
35d685e to
03c730d
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-client
branch
from
56fc7ab to
5051120
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-server
branch
from
03c730d to
3ccc0ec
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-client
branch
from
5051120 to
ccbfee5
Compare
swan-amazon
force-pushed
the
feature/enhanced-setup-flow-server
branch
2 times, most recently
from
f88845b to
3ccc0ec
Compare
* Decouple InitDataModelHandler from libCHIP * Use StartUp to init * Seperate namespace cleanup out * Mock function for linking * Restyled by clang-format * Add API comment * Fix mutiple defination conflicts * Address review comments * Restyled by whitespace * Seperate InitDataModel out * Revert "Seperate InitDataModel out" This reverts commit 5a8af59. * Do not directly manipulate the base class's Startup method * Address review comment * Restyled by whitespace * Adjust the init order * Restyled by whitespace * Update src/app/server/Server.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Update src/controller/CHIPDeviceControllerFactory.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Add TODO comment * documented/named InitDataModel to make it clear that this is a temporary hack for a test --------- Co-authored-by: Restyled.io <commits@restyled.io> Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
Co-authored-by: Andrei Litvin <andreilitvin@google.com>
* [ESP32]: Removed esp32-m5-with-rpc from the CI. * Add esp32 with rpc and ipv6only variation to CI * Add default sdkconfig.
…ect-chip#36868) * Map the return error from AppendUserLabel to RESOURCE_EXHAUSTED * Restyled by whitespace * Addressed the review comments * Update API comment to algin with implemantion --------- Co-authored-by: Restyled.io <commits@restyled.io>
…roject-chip#36878) This commit further increases the stack sizes when Pigweed logger is enabled. Signed-off-by: Łukasz Duda <lukasz.duda@nordicsemi.no>
* Fix Null Pointer Dereference in TCP Packet Handling * Fix handle zero messageSize in TCP packet processing * Add test for TCP MessageSize * Modify test * Restyled by clang-format * Modify the position of an if statement * Modify test --------- Co-authored-by: BoB13-Matter <--global> Co-authored-by: Restyled.io <commits@restyled.io>
* Inject event management into report engine * Restyled by whitespace * Restyled by clang-format * add event scheduler file * add missing includes * Rename it to EventReporter * Restyled by clang-format * Restyled by gn * Modify comments and fix typo * Fix some comments * Restyled by clang-format --------- Co-authored-by: Restyled.io <commits@restyled.io>
Set the app_state callback object in the Connection state to null when the CASE session object is being cleared, on top of setting the inner callback methods to null. This prevents the callback object from being accessed later, when the connection is getting closed(after the CASE session has been set up and the session object no longer exists).
Add CloseActiveConnections() call in TCPBase::Close(), which is called as part of Server::Shutdown(). Active connections should be closed as part of Server shutdown. This allows the TCPConnectionState to also close the associated TCPEndpoint object as part of this shutdown flow. Previously, the CloseActiveConnections() call was present in the TCPBase destructor alone. Add test for Connection Close() and checking for TCPEndPoint.
…rten the code/indent level on early return instead of if/else) (project-chip#36887) * Follow Up Fix PR project-chip#36596 * Remove unwanted comments * Restyled by autopep8 * Comment and space fixes --------- Co-authored-by: Restyled.io <commits@restyled.io>
…36874) * Fix CCTRL tests on CI - CCTRL cluster is on endpoint 1 * Fail CI tests if any test cases were skipped * Add compatibility flag to mobile-device-test.py * Comment out skipped tests * Change executable mode * Improve --test-case command line option * Disable skipped test cases on TC_SWTCH * Disable TC_LVL_2_3 on CI * Make TestBdxTransfer quiet * Verify testing support before running actual tests * Add missing definition
* TC-VALCC: Update wording on test steps Test: Tests are all run in repl CI. * You know what really helps? Adding all your changes before pushing smooth moves, freeman.
…6896) Unconditionally clear members that point to buffers, and handle them via the explicit code that copies the pointed-to data. The logic to work out whether any pointers need to be cleared was quite complicated. Use memmove() instead of memcpy() since src and dst may overlap / be identical. Always assign mNeedIcdRegistration when the kReadCommissioningInfo step finishes, instead of relying on SetCommissioningParameters to clear it.
…chip#36836) * Decouple ember functions from general commissioning cluster * Address review comments * Rename gAttrAccess * Remove new added log info * Flag SetTCAcknowledgements command * Revert "Flag SetTCAcknowledgements command" This reverts commit 90de8a1. * Add the original debug log back
The wait-stage is not required. The user input availability must be a pre-condition for starting the AutoCommissioner process. The wait stage was previously to support requesting user input after identifying the device VID/PID using a different channel than within the pairing payload.
Updated documentation for T&C-related commissioning arguments to better reflect their actual usage and purpose: - require-tc-acknowledgements: Clarified the impact on commissioning flow - tc-acknowledgements: Explained the bit-field usage for user acceptance - tc-acknowledgements-version: Added context about version tracking
…project-chip#36905) * remove provisionnal markings * zap regen
The Terms and Conditions acknowledgements parameter was incorrectly included in ClearExternalBufferDependentValues(). This parameter contains a fixed-size struct with two uint16_t values and does not reference any external buffers. The CommissioningParameters class appears to be designed for additive-only parameter setting without explicit clear/reset functionality, so removing this inappropriate clearing operation is the correct approach.
Move kConfigureTCAcknowledgments before kCleanup in the CommissioningStage enum to fix cirque test failures. The tests validate that commissioning stages do not exceed kCleanup, which was causing failures when T&C acknowledgements were positioned after it. While the original comment from 2 years ago suggested the enum order was fixed, testing reveals that the stages can be reordered. The cirque tests now pass with this corrected ordering, indicating that any previous constraints on enum ordering no longer apply.
|
The review was re-published to project-chip repo. project-chip#36863 |
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.
This commit enables the Terms and Conditions (TC) feature in the all-clusters-app example application. The changes ensure that the app can handle TC acknowledgements and enforce required terms and conditions during the commissioning process.
Changes include:
This enables the TC feature logic in the example app, building on the initial implementation.
Suggested Testing
Build chip-all-clusters-app with TC required:
chip_config_tc_required=true chip_config_tc_required_acknowledgements=1 chip_config_tc_required_acknowledgements_version=1gn gen --check --fail-on-unused-args --export-compile-commands --root=./examples/all-clusters-app/linux --args="chip_config_tc_required=true chip_config_tc_required_acknowledgements=1 chip_config_tc_required_acknowledgements_version=1 chip_config_network_layer_ble=false chip_enable_wifi=false is_debug=true" ./out/linux-all-clusters-tc-requiredBuild chip-all-clusters-app with TC explicitly not required:
chip_config_tc_required=falsegn gen --check --fail-on-unused-args --export-compile-commands --root=./examples/all-clusters-app/linux --args="chip_config_tc_required=false chip_config_network_layer_ble=false chip_enable_wifi=false is_debug=true" ./out/linux-all-clusters-tc-not-requiredBuild chip-all-clusters-app (legacy)
Build chip-tool
Run unit tests
gn gen --check --fail-on-unused-args --export-compile-commands --args='is_debug=true target_os="all"' out/unifiedBuild the projects
Run the chip-all-clusters-app, requiring TCs
rm /tmp/chip_* ./out/linux-all-clusters-tc-required/chip-all-clusters-app --version 0 --vendor-id 1 --product-id 257 --custom-flow 2 --capabilities 4 --discriminator 3840 --passcode 20202021 --secured-device-port 5540 --unsecured-commissioner-port 5550 --KVS /tmp/chip_kvs.bin --trace_file /tmp/chip_trace.log --trace_log 1 --trace_decode 1Commission the chip-all-clusters-app, and set the TCs
./out/linux-chip-tool/chip-tool pairing code 1 34970112332 --trace_decode 1 --require-tc-acknowledgements 1 --tc-acknowledgements 1 --tc-acknowledgements-version 1 --bypass-attestation-verifier trueRun the chip-all-clusters-app, requiring TCs
rm /tmp/chip_* ./out/linux-all-clusters-tc-not-required/chip-all-clusters-app --version 0 --vendor-id 1 --product-id 257 --custom-flow 2 --capabilities 4 --discriminator 3840 --passcode 20202021 --secured-device-port 5540 --unsecured-commissioner-port 5550 --KVS /tmp/chip_kvs.bin --trace_file /tmp/chip_trace.log --trace_log 1 --trace_decode 1Commission the chip-all-clusters-app, and skip setting TCs
./out/linux-chip-tool/chip-tool pairing code 1 34970112332 --trace_decode 1 --require-tc-acknowledgements 0 --bypass-attestation-verifier trueRun the chip-all-clusters-app, as previously would be done
rm /tmp/chip_* ./out/linux-all-clusters/chip-all-clusters-app --version 0 --vendor-id 1 --product-id 257 --custom-flow 2 --capabilities 4 --discriminator 3840 --passcode 20202021 --secured-device-port 5540 --unsecured-commissioner-port 5550 --KVS /tmp/chip_kvs.bin --trace_file /tmp/chip_trace.log --trace_log 1 --trace_decode 1Commission the chip-all-clusters-app, without any special flags
./out/linux-chip-tool/chip-tool pairing code 1 34970112332 --trace_decode 1 --bypass-attestation-verifier true