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

Implement users and credentials in the Door Lock Cluster #13789

Conversation

Morozov-5F
Copy link
Contributor

@Morozov-5F Morozov-5F commented Jan 20, 2022

Problem

Door Lock cluster needs implementation of users and credentials to function properly.

Change overview

This PR implement almost complete support of core Users & Credentials stuff from the door lock cluster -- it is now possible to create, modify and delete users and credentials using Matter IM. Also this PR includes reference (but not exactly the best) implementation of the app that utilizes the door lock server.

However, there are a couple of caveats:

  • Due to an issue (Android ZAP generator produces bad code when command has nullable structure as an argument #13787) in the Android codegen Clear Credential command does not support deletion of all the credentials. As a workaround user can clear all credentials for each type by providing 0xFFFE in the CredentialIndex field of Credential argument.
  • While new unit tests are included into the chip-tool testing suite they are not run in the GitHub CI -- I would like to do that as a separate PR once this one is merge. I believe that it will be too much if I include that feature too.

Testing

  • Created two YAML test suites to exercise various scenarios:
    • First one (DL_UsersAndCredentials) verifies users and credentials creation, modification and deletion with various parameters.
    • Second one (DL_LockUnlock) makes sure that lock operations such as Lock and Unlock work as expected (fail with incorrect PIN, pass with correct one).
  • Manual testing using chip-tool -- mostly used to read events because YAML tests does not support them yet.

- This is done to avoid fixing github actions for now.
- I would like to address the github stuff as a separate fix.
@Morozov-5F Morozov-5F force-pushed the feature/implement-users-and-credentials branch from 8e6ae2b to 2735775 Compare January 27, 2022 12:26
@github-actions
Copy link

github-actions bot commented Jan 27, 2022

PR #13789: Size comparison from 40cc421 to 2735775

Increases above 0.2%:

platform target config section 40cc421 2735775 change % change
esp32 all-clusters-app c3devkit (read only) 923298 933734 10436 1.1
(read/write) 1384986 1398586 13600 1.0
.flash.rodata 180936 194536 13600 7.5
.flash.text 923298 933734 10436 1.1
m5stack (read only) 972227 981283 9056 0.9
(read/write) 452016 465424 13408 3.0
.flash.rodata 210232 223640 13408 6.4
.flash.text 966843 975899 9056 0.9
linux chip-tool-ipv6only arm64 (read only) 6537388 6724780 187392 2.9
(read/write) 275153 278497 3344 1.2
.data.rel.ro 174424 175816 1392 0.8
.got 40472 42424 1952 4.8
.rodata 377140 387156 10016 2.7
.text 5598724 5767508 168784 3.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read/write) 2357384 2383520 26136 1.1
.text 1319984 1346120 26136 2.0
p6 all-clusters-app default (read/write) 2418240 2442416 24176 1.0
.text 1376504 1400680 24176 1.8
Increases (5 builds for esp32, linux, mbed, p6)
platform target config section 40cc421 2735775 change % change
esp32 all-clusters-app c3devkit (read only) 923298 933734 10436 1.1
(read/write) 1384986 1398586 13600 1.0
.dram0.bss 70880 70888 8 0.0
.flash.rodata 180936 194536 13600 7.5
.flash.text 923298 933734 10436 1.1
m5stack (read only) 972227 981283 9056 0.9
(read/write) 452016 465424 13408 3.0
.dram0.bss 75624 75632 8 0.0
.flash.rodata 210232 223640 13408 6.4
.flash.text 966843 975899 9056 0.9
linux chip-tool-ipv6only arm64 (read only) 6537388 6724780 187392 2.9
(read/write) 275153 278497 3344 1.2
.data.rel.ro 174424 175816 1392 0.8
.got 40472 42424 1952 4.8
.rodata 377140 387156 10016 2.7
.text 5598724 5767508 168784 3.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read/write) 2357384 2383520 26136 1.1
.bss 189588 189596 8 0.0
.text 1319984 1346120 26136 2.0
p6 all-clusters-app default (read/write) 2418240 2442416 24176 1.0
.bss 117932 117940 8 0.0
.text 1376504 1400680 24176 1.8
Decreases (4 builds for esp32, mbed, p6)
platform target config section 40cc421 2735775 change % change
esp32 all-clusters-app c3devkit .dram0.data 14252 14236 -16 -0.1
m5stack .dram0.data 34032 34024 -8 -0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release .data 5296 5288 -8 -0.2
p6 all-clusters-app default .data 2592 2584 -8 -0.3
Full report (33 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, qpg, telink)
platform target config section 40cc421 2735775 change % change
cyw30739 light cyw930739m2evb_01 (read/write) 580442 580442 0 0.0
.app_xip_area 485336 485336 0 0.0
.bss 77852 77852 0 0.0
.data 596 596 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock-app CYW30739 (read/write) 538446 538446 0 0.0
.app_xip_area 444884 444884 0 0.0
.bss 76348 76348 0 0.0
.data 560 560 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
efr32 lighting-app BRD4161A (read only) 843492 843492 0 0.0
(read/write) 127396 127396 0 0.0
.bss 125496 125496 0 0.0
.data 1900 1900 0 0.0
.text 843484 843484 0 0.0
BRD4161A+rpc (read only) 830864 830864 0 0.0
(read/write) 144056 144056 0 0.0
.bss 142056 142056 0 0.0
.data 2000 2000 0 0.0
.text 830856 830856 0 0.0
window-app BRD4161A (read only) 816108 816108 0 0.0
(read/write) 126052 126052 0 0.0
.bss 124196 124196 0 0.0
.data 1856 1856 0 0.0
.text 816100 816100 0 0.0
esp32 all-clusters-app c3devkit (read only) 923298 933734 10436 1.1
(read/write) 1384986 1398586 13600 1.0
.dram0.bss 70880 70888 8 0.0
.dram0.data 14252 14236 -16 -0.1
.flash.rodata 180936 194536 13600 7.5
.flash.text 923298 933734 10436 1.1
.iram0.text 62056 62056 0 0.0
m5stack (read only) 972227 981283 9056 0.9
(read/write) 452016 465424 13408 3.0
.dram0.bss 75624 75632 8 0.0
.dram0.data 34032 34024 -8 -0.0
.flash.rodata 210232 223640 13408 6.4
.flash.text 966843 975899 9056 0.9
.iram0.text 123399 123399 0 0.0
k32w light k32w061+release (read/write) 665224 665224 0 0.0
.bss 77628 77628 0 0.0
.data 1868 1868 0 0.0
.text 579928 579928 0 0.0
lock k32w061+release (read/write) 666192 666192 0 0.0
.bss 77892 77892 0 0.0
.data 1892 1892 0 0.0
.text 580608 580608 0 0.0
linux chip-tool-ipv6only arm64 (read only) 6537388 6724780 187392 2.9
(read/write) 275153 278497 3344 1.2
.bss 55377 55377 0 0.0
.data 1128 1128 0 0.0
.data.rel.ro 174424 175816 1392 0.8
.dynamic 560 560 0 0.0
.got 40472 42424 1952 4.8
.init 24 24 0 0.0
.init_array 200 200 0 0.0
.rodata 377140 387156 10016 2.7
.text 5598724 5767508 168784 3.0
thermostat-no-ble arm64 (read only) 2082804 2082804 0 0.0
(read/write) 151873 151873 0 0.0
.bss 69585 69585 0 0.0
.data 960 960 0 0.0
.data.rel.ro 74208 74208 0 0.0
.dynamic 560 560 0 0.0
.got 4144 4144 0 0.0
.init 24 24 0 0.0
.init_array 336 336 0 0.0
.rodata 131940 131940 0 0.0
.text 1733616 1733616 0 0.0
mbed all-clusters-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2357384 2383520 26136 1.1
.bss 189588 189596 8 0.0
.data 5296 5288 -8 -0.2
.text 1319984 1346120 26136 2.0
lighting-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2341088 2341088 0 0.0
.bss 181104 181104 0 0.0
.data 5584 5584 0 0.0
.text 1303688 1303688 0 0.0
lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2307840 2307840 0 0.0
.bss 181000 181000 0 0.0
.data 5568 5568 0 0.0
.text 1270440 1270440 0 0.0
pigweed-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 1139712 1139712 0 0.0
.bss 11756 11756 0 0.0
.data 4368 4368 0 0.0
.text 103096 103096 0 0.0
shell CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2294044 2294044 0 0.0
.bss 177732 177732 0 0.0
.data 5384 5384 0 0.0
.text 1256616 1256616 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 (read/write) 983707 983707 0 0.0
bss 120860 120860 0 0.0
rodata 116480 116480 0 0.0
text 668576 668576 0 0.0
nrf52840dk_nrf52840+rpc (read/write) 967571 967571 0 0.0
bss 117904 117904 0 0.0
rodata 108016 108016 0 0.0
text 663248 663248 0 0.0
nrf52840dongle_nrf52840 (read/write) 999827 999827 0 0.0
bss 122032 122032 0 0.0
rodata 115332 115332 0 0.0
text 674004 674004 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 892930 892930 0 0.0
bss 117648 117648 0 0.0
rodata 109780 109780 0 0.0
text 584756 584756 0 0.0
lock-app nrf52840dk_nrf52840 (read/write) 916323 916323 0 0.0
bss 119240 119240 0 0.0
rodata 105120 105120 0 0.0
text 614560 614560 0 0.0
nrf5340dk_nrf5340_cpuapp (read/write) 826358 826358 0 0.0
bss 116056 116056 0 0.0
rodata 98344 98344 0 0.0
text 531512 531512 0 0.0
pigweed-app nrf52840dk_nrf52840 (read/write) 541835 541835 0 0.0
bss 52588 52588 0 0.0
rodata 50104 50104 0 0.0
text 376940 376940 0 0.0
pump-app nrf52840dk_nrf52840 (read/write) 919187 919187 0 0.0
bss 118992 118992 0 0.0
rodata 105632 105632 0 0.0
text 617096 617096 0 0.0
pump-controller-app nrf52840dk_nrf52840 (read/write) 914371 914371 0 0.0
bss 119016 119016 0 0.0
rodata 104736 104736 0 0.0
text 613132 613132 0 0.0
shell nrf52840dk_nrf52840 (read/write) 798203 798203 0 0.0
bss 109776 109776 0 0.0
rodata 78288 78288 0 0.0
text 533640 533640 0 0.0
p6 all-clusters-app default (read/write) 2418240 2442416 24176 1.0
.bss 117932 117940 8 0.0
.data 2592 2584 -8 -0.3
.text 1376504 1400680 24176 1.8
light-app default (read/write) 2339064 2339064 0 0.0
.bss 105684 105684 0 0.0
.data 2408 2408 0 0.0
.text 1297328 1297328 0 0.0
lock-app default (read/write) 2304584 2304584 0 0.0
.bss 105428 105428 0 0.0
.data 2360 2360 0 0.0
.text 1262848 1262848 0 0.0
qpg lighting-app qpg6105+debug (read only) 572036 572036 0 0.0
(read/write) 146936 146936 0 0.0
.bss 89840 89840 0 0.0
.data 1060 1060 0 0.0
.text 566716 566716 0 0.0
lock-app qpg6105+debug (read only) 518164 518164 0 0.0
(read/write) 146940 146940 0 0.0
.bss 89312 89312 0 0.0
.data 992 992 0 0.0
.text 512844 512844 0 0.0
persistent-storage-app qpg6105+debug (read only) 107140 107140 0 0.0
(read/write) 146940 146940 0 0.0
.bss 38504 38504 0 0.0
.data 288 288 0 0.0
.text 101820 101820 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 848158 848158 0 0.0
bss 87640 87640 0 0.0
noinit 37160 37160 0 0.0
text 592932 592932 0 0.0

@bzbarsky-apple bzbarsky-apple merged commit e49d9ca into project-chip:master Jan 27, 2022
selissia pushed a commit to selissia/connectedhomeip that referenced this pull request Jan 28, 2022
…p#13789)

* Make nextUserIndex nullable in Door Lock as it is required by spec.

* Initial implementation of Get/Set/Clear User command.

* Refactor User-related commands in Door Lock Cluster

* Minor tweaks in commands logging, add todos for events.

* Properly handle nullable command arguments in chip-tool

* Make more door lock command arguments nullable

* Add placeholder implementation for the set credential command

* Add missing argument to SetCredential command in Door Lock CLuster

* Add basic SetCredential handler for Door Lock cluster.

* Add YAML tests for Door Lock cluster (users/credentials)

* Adjust underlying data structures for sample Door Lock credentials manager.

* Move command handler to the door lock server class

* A touch of clean-up in the Door Lock Server.

* Add features bitmap to the Door Lock Cluster.

* Implement GetCredentialStatus command in Door Lock Cluster.

* Fix the door lock cluster feature checks.

* Small refactoring of GetCredentialStatus.

* Enable RFID attributes in the all-clusters app for now.

* Refactor SetCredential command.

* Replace unnecessary parameters in the user management of the door lock.

* Reorder door lock server functions.

Fix compiler errors

* Check credential data range

* Add documentation for application callbacks related to user management in Door Lock cluster.

* Use Span for username and credentials.

* Implement ability to modify credentials.

* Change ZAP files for door lock app to include all necessary features

* Don't use 'using namespace' in header files

* Move users/credential database implementation to the door-lock-app.

* Remove debug prints in the door lock cluster.

* Add more verbose logging to the door lock app.

* Make door-lock XML compliant to spec

* Add clear credential command

* Update door lock test suite to excercise ClearCredential command

* Add Lock User Change event to Door Lock Cluster

* Use credentials instead of hard-coded PIN when operating the lock

* Support programming PIN

* Refactor SetCredential command handler

* Temporary make credential parameter in clearCredential command nullable to avoid generator errors

* Bring back definition to make ESP32 builds work again

* Fix format strings and leave out clearCredential logic that is not longer supported

* Use appropriate format strings for logging in door lock cluster.

* Add implicit casts when adding uint16_t variables to make compiler happy

* Temporally ignore Door Lock tests when starting all test.
- This is done to avoid fixing github actions for now.
- I would like to address the github stuff as a separate fix.

* Fix logging types in the door lock app

* Fix styling issues

* Clear the associated credentials when clearing the user

* Make argument in ClearCredential command nullable again because recent changes improved the ZAP generator

* Update auto-generated files
@Morozov-5F Morozov-5F deleted the feature/implement-users-and-credentials branch January 30, 2022 08:42
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.

3 participants