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.

Sign up for GitHub

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

Jump to bottom

samples: matter: lock: Support all fields of LockOperation event #21078

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

samples: matter: lock: Support all fields of LockOperation event #21078

wants to merge 6 commits into from
+170 −97

Conversation

adigie
Copy link
Member

@adigie adigie commented Mar 20, 2025

  • Remove redundant call to BoltLockManager::[Lock|Unlock] from
    MatterPostAttributeChangeCallback.
  • Return ValidatePINResult containing user id and credential used for
    validation.
  • Add support for UserIndex, FabricIndex, SourceNode and
    Credentials fields.

@adigie adigie requested a review from a team as a code owner March 20, 2025 19:53
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Mar 20, 2025
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Mar 20, 2025 •
edited
Loading

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 7

Inputs:

Sources:

sdk-nrf: PR head: 4ca1f2930a6ce3c8932955640831d450888dfcde

more details

sdk-nrf:

PR head: 4ca1f2930a6ce3c8932955640831d450888dfcde
merge base: 55d0fe25b2e97dbf7db51c703243ded53a28d49a
target head (main): 762492f3d6d86efb602182ce86ce21a3be588260
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (7) 
samples
│  ├── matter
│  │  ├── lock
│  │  │  ├── src
│  │  │  │  ├── access
│  │  │  │  │  ├── access_manager.cpp
│  │  │  │  │  │ access_manager.h
│  │  │  │  ├── app_task.cpp
│  │  │  │  ├── app_task.h
│  │  │  │  ├── bolt_lock_manager.cpp
│  │  │  │  ├── bolt_lock_manager.h
│  │  │  │  │ zcl_callbacks.cpp

Outputs:

Toolchain

Version: 4ffa2202d5
Build docker image: docker-dtr.nordicsemi.no/sw-production/ncs-build:4ffa2202d5_e579f9fbe6

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain - Skipped: existing toolchain is used
  • ✅ Build twister
    • sdk-nrf test count: 97
  • ✅ Integration tests
    • ✅ test-fw-nrfconnect-chip
Disabled integration tests
    • desktop52_verification
    • doc-internal
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-boot
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_cloud
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_mosh
    • test-fw-nrfconnect-nrf-iot_positioning
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-proprietary_esb
    • test-fw-nrfconnect-ps
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-fw-nrfconnect-thread
    • test-fw-nrfconnect-zigbee
    • test-low-level
    • test-sdk-audio
    • test-sdk-dfu
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-pmic-samples
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

@adigie adigie force-pushed the lock-operation-event-fix branch 2 times, most recently from 73fc0e6 to 4868011 Compare March 20, 2025 21:02
ArekBalysNordic
ArekBalysNordic reviewed Mar 21, 2025
View reviewed changes
Copy link
Contributor

@ArekBalysNordic ArekBalysNordic left a comment

Choose a reason for hiding this comment

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

You've changed the door lock API, so I think at least a release or migration guide note should be added.

samples/matter/lock/src/access/access_manager.h
* @return true on success, false otherwise.
*/
bool ValidatePIN(const Optional<chip::ByteSpan> &pinCode, OperationErrorEnum &err);
bool ValidatePIN(const Optional<chip::ByteSpan> &pinCode, OperationErrorEnum &err,
Copy link
Contributor

Choose a reason for hiding this comment

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

@markaj-nordic just FYI, the doorlock access manager API has been changed.

@adigie adigie force-pushed the lock-operation-event-fix branch 2 times, most recently from 9d32699 to d02c1ab Compare March 21, 2025 09:25
adigie added 4 commits March 21, 2025 10:51
@adigie
samples: matter: lock: Format sources

Verified

This commit was signed with the committer’s verified signature.
kilfoyle David Kilfoyle
GPG key ID: 60ADC6C4C8ABD280
Verified
Learn about vigilant mode
4782d4a 
Format sources before changes.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@adigie
samples: matter: lock: Remove Lock/Unlock from callback

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
04dbb37 
Remove redundant call to `BoltLockManager::[Lock|Unlock]` from
MatterPostAttributeChangeCallback.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@adigie
samples: matter: lock: Add ValidatePINResult

Unverified

The email in this signature doesn’t match the committer email.
GPG key ID: 60ADC6C4C8ABD280
Learn about vigilant mode
9f4bc34 
Return `ValidatePINResult` containing user id and credential used for
validation.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@adigie
samples: matter: lock: Support all fields of LockOperation event

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
Loading
Loading status checks…
47b31c8 
Add support for `UserIndex`, `FabricIndex`, `SourceNode` and
`Credentials` fields.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@adigie adigie force-pushed the lock-operation-event-fix branch from d02c1ab to 47b31c8 Compare March 21, 2025 09:51
adigie added 2 commits March 21, 2025 14:37
@adigie
samples: matter: lock: Reduce code nesting

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
GPG key ID: B5690EEEBB952194
Verified
Learn about vigilant mode
fa2d1fd 
Address SonarQube warning regarding code nesting.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@adigie
samples: matter: lock: Extract lambda to separate function
Loading
Loading status checks…
4ca1f29 
Address SonarQube issue regarding too long lambda body.

Signed-off-by: Adrian Gielniewski <adrian.gielniewski@nordicsemi.no>
@adigie adigie requested a review from ArekBalysNordic March 21, 2025 13:37
@adigie
Copy link
Member Author

adigie commented Mar 21, 2025

Addressed SonarQube issues.

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
3.3% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@adigie adigie requested a review from a team March 21, 2025 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ArekBalysNordic ArekBalysNordic

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@adigie @NordicBuilder @ArekBalysNordic