From c1d543182fccd0c37c03379602a86fddadff68b1 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 30 Jun 2022 20:58:20 -0400 Subject: [PATCH 1/8] Enable "are we on the right thread?" asserts on Darwin. (#20177) A few changes here: 1) Enable chip_stack_lock_tracking on darwin. 2) Implement _IsChipStackLockedByCurrentThread on Darwin in a way that makes sense with the actual "no locks" setup there (by checking that we are on the right dispatch queue). 3) Fix controller shutdown on Darwin to not happen from outside the Matter event loop while the event loop is running. Removing the dealloc method from the Darwin controller implementation is safe because: * The only place where controllers get allocated is MTRControllerFactory's createController. * At that callsite, if initWithFactory returns nil it guarantees that it has called cleanup. * After allocation + init, we add the controller to the factory's controller list. After that the controller cannot be deallocated until we remove it from that list, which only happens in controllerShuttingDown, which is always followed by a call to cleanup. --- .../Framework/CHIP/CHIPDeviceController.mm | 22 ++++++++++--------- .../CHIP/CHIPDeviceController_Internal.h | 8 +++++++ .../Framework/CHIP/MatterControllerFactory.mm | 7 ++++++ src/platform/BUILD.gn | 2 +- src/platform/Darwin/PlatformManagerImpl.cpp | 22 +++++++++++++++---- src/platform/Darwin/PlatformManagerImpl.h | 9 ++++++-- 6 files changed, 53 insertions(+), 17 deletions(-) diff --git a/src/darwin/Framework/CHIP/CHIPDeviceController.mm b/src/darwin/Framework/CHIP/CHIPDeviceController.mm index ba2e4309fb35ce..1521d7a4035f11 100644 --- a/src/darwin/Framework/CHIP/CHIPDeviceController.mm +++ b/src/darwin/Framework/CHIP/CHIPDeviceController.mm @@ -125,14 +125,21 @@ - (void)cleanupAfterStartup [self cleanup]; } +// Part of cleanupAfterStartup that has to interact with the Matter work queue +// in a very specific way that only MTRControllerFactory knows about. +- (void)shutDownCppController +{ + if (_cppCommissioner) { + _cppCommissioner->Shutdown(); + delete _cppCommissioner; + _cppCommissioner = nullptr; + } +} + // Clean up any members we might have allocated. - (void)cleanup { - if (self->_cppCommissioner) { - self->_cppCommissioner->Shutdown(); - delete self->_cppCommissioner; - self->_cppCommissioner = nullptr; - } + VerifyOrDie(_cppCommissioner == nullptr); [self clearDeviceAttestationDelegateBridge]; @@ -675,11 +682,6 @@ - (BOOL)checkForError:(CHIP_ERROR)errorCode logMsg:(NSString *)logMsg error:(NSE return YES; } -- (void)dealloc -{ - [self cleanup]; -} - - (BOOL)deviceBeingCommissionedOverBLE:(uint64_t)deviceId { CHIP_ERROR errorCode = CHIP_ERROR_INCORRECT_STATE; diff --git a/src/darwin/Framework/CHIP/CHIPDeviceController_Internal.h b/src/darwin/Framework/CHIP/CHIPDeviceController_Internal.h index 8419de759b82f6..3c4cd4f0627f4e 100644 --- a/src/darwin/Framework/CHIP/CHIPDeviceController_Internal.h +++ b/src/darwin/Framework/CHIP/CHIPDeviceController_Internal.h @@ -80,6 +80,14 @@ NS_ASSUME_NONNULL_BEGIN fabricIndex:(chip::FabricIndex)fabricIndex isRunning:(BOOL *)isRunning; +/** + * Shut down the underlying C++ controller. Must be called on the Matter work + * queue or after the Matter work queue has been shut down. + * + * Only MTRControllerFactory should be calling this. + */ +- (void)shutDownCppController; + @end NS_ASSUME_NONNULL_END diff --git a/src/darwin/Framework/CHIP/MatterControllerFactory.mm b/src/darwin/Framework/CHIP/MatterControllerFactory.mm index ba8618302b8b57..12db00bf231f9c 100644 --- a/src/darwin/Framework/CHIP/MatterControllerFactory.mm +++ b/src/darwin/Framework/CHIP/MatterControllerFactory.mm @@ -525,6 +525,13 @@ - (void)controllerShuttingDown:(CHIPDeviceController *)controller // shuts down, because shutdown of the last controller will tear // down most of the world. DeviceLayer::PlatformMgrImpl().StopEventLoopTask(); + + [controller shutDownCppController]; + } else { + // Do the controller shutdown on the Matter work queue. + dispatch_sync(_chipWorkQueue, ^{ + [controller shutDownCppController]; + }); } } diff --git a/src/platform/BUILD.gn b/src/platform/BUILD.gn index fdcc45d0424ac7..580bd138ae312b 100644 --- a/src/platform/BUILD.gn +++ b/src/platform/BUILD.gn @@ -73,7 +73,7 @@ if (chip_device_platform != "none" && chip_device_platform != "external") { if (chip_stack_lock_tracking == "auto") { if (chip_device_platform == "linux" || chip_device_platform == "tizen" || chip_device_platform == "android" || current_os == "freertos" || - chip_device_platform == "webos") { + chip_device_platform == "webos" || chip_device_platform == "darwin") { # TODO: should be fatal for development. Change once bugs are fixed chip_stack_lock_tracking = "fatal" } else { diff --git a/src/platform/Darwin/PlatformManagerImpl.cpp b/src/platform/Darwin/PlatformManagerImpl.cpp index 3c85bee76cbd62..1a80645a0192c1 100644 --- a/src/platform/Darwin/PlatformManagerImpl.cpp +++ b/src/platform/Darwin/PlatformManagerImpl.cpp @@ -69,9 +69,9 @@ CHIP_ERROR PlatformManagerImpl::_InitChipStack() CHIP_ERROR PlatformManagerImpl::_StartEventLoopTask() { - if (mIsWorkQueueRunning == false) + if (mIsWorkQueueSuspended) { - mIsWorkQueueRunning = true; + mIsWorkQueueSuspended = false; dispatch_resume(mWorkQueue); } @@ -80,9 +80,9 @@ CHIP_ERROR PlatformManagerImpl::_StartEventLoopTask() CHIP_ERROR PlatformManagerImpl::_StopEventLoopTask() { - if (mIsWorkQueueRunning == true) + if (!mIsWorkQueueSuspended && !mIsWorkQueueSuspensionPending) { - mIsWorkQueueRunning = false; + mIsWorkQueueSuspensionPending = true; if (dispatch_get_current_queue() != mWorkQueue) { // dispatch_sync is used in order to guarantee serialization of the caller with @@ -90,6 +90,9 @@ CHIP_ERROR PlatformManagerImpl::_StopEventLoopTask() dispatch_sync(mWorkQueue, ^{ dispatch_suspend(mWorkQueue); }); + + mIsWorkQueueSuspended = true; + mIsWorkQueueSuspensionPending = false; } else { @@ -99,6 +102,8 @@ CHIP_ERROR PlatformManagerImpl::_StopEventLoopTask() // that no more tasks will run on the queue. dispatch_async(mWorkQueue, ^{ dispatch_suspend(mWorkQueue); + mIsWorkQueueSuspended = true; + mIsWorkQueueSuspensionPending = false; dispatch_semaphore_signal(mRunLoopSem); }); } @@ -138,5 +143,14 @@ CHIP_ERROR PlatformManagerImpl::_PostEvent(const ChipDeviceEvent * event) return CHIP_NO_ERROR; } +#if CHIP_STACK_LOCK_TRACKING_ENABLED +bool PlatformManagerImpl::_IsChipStackLockedByCurrentThread() const +{ + // If we have no work queue, or it's suspended, then we assume our caller + // knows what they are doing in terms of their own concurrency. + return !mWorkQueue || mIsWorkQueueSuspended || dispatch_get_current_queue() == mWorkQueue; +}; +#endif + } // namespace DeviceLayer } // namespace chip diff --git a/src/platform/Darwin/PlatformManagerImpl.h b/src/platform/Darwin/PlatformManagerImpl.h index 72b0bd9d7103d2..e8b8decf2fcfb3 100644 --- a/src/platform/Darwin/PlatformManagerImpl.h +++ b/src/platform/Darwin/PlatformManagerImpl.h @@ -49,6 +49,7 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener { mWorkQueue = dispatch_queue_create(CHIP_CONTROLLER_QUEUE, DISPATCH_QUEUE_SERIAL); dispatch_suspend(mWorkQueue); + mIsWorkQueueSuspended = true; } return mWorkQueue; } @@ -71,7 +72,7 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener CHIP_ERROR _PostEvent(const ChipDeviceEvent * event); #if CHIP_STACK_LOCK_TRACKING_ENABLED - bool _IsChipStackLockedByCurrentThread() const { return false; }; + bool _IsChipStackLockedByCurrentThread() const; #endif // ===== Members for internal use by the following friends. @@ -88,7 +89,11 @@ class PlatformManagerImpl final : public PlatformManager, public Internal::Gener // Semaphore used to implement blocking behavior in _RunEventLoop. dispatch_semaphore_t mRunLoopSem; - bool mIsWorkQueueRunning = false; + bool mIsWorkQueueSuspended = false; + // TODO: mIsWorkQueueSuspensionPending might need to be an atomic and use + // atomic ops, if we're worried about calls to StopEventLoopTask() from + // multiple threads racing somehow... + bool mIsWorkQueueSuspensionPending = false; inline ImplClass * Impl() { return static_cast(this); } }; From f704185f1b191cfb337c1678db05f9bde1fe0719 Mon Sep 17 00:00:00 2001 From: Martin Turon Date: Thu, 30 Jun 2022 19:21:34 -0700 Subject: [PATCH 2/8] [vscode] Add file watcher excludes to fix CPU fan spin by cpptools. (#20193) On mac os especially, just opening projectchip in vscode would hold one CPU spinning at 98% by a cpptools process owned by vscode. This patch to settings.json excludes the file watcher from endless recursion via examples/**/third_party/connectedhomeip... --- .vscode/settings.json | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.vscode/settings.json b/.vscode/settings.json index ac8bf404d4ec08..a8e2b99d4e8a70 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -117,6 +117,13 @@ "random": "cpp", "thread": "cpp" }, + // Configure paths or glob patterns to exclude from file watching. + "files.watcherExclude": { + "**/.git/objects/**": true, + "**/.git/subtree-cache/**": true, + "out/": true, + "**/third_party/**": true + }, "files.eol": "\n", "editor.formatOnSave": true, "better-comments.tags": [ From 056d06b1d507af3ba6d3f64e2c8c82f53c6b1d35 Mon Sep 17 00:00:00 2001 From: jczhang777 <101778393+jczhang777@users.noreply.github.com> Date: Fri, 1 Jul 2022 11:02:35 +0800 Subject: [PATCH 3/8] =?UTF-8?q?[BL602]=20Update=20README.md,=20modify=20th?= =?UTF-8?q?e=20baudrate=20from=202000000=20to=20115200,=E2=80=A6=20(#20115?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [BL602] Update README.md, modify the baudrate from 2000000 to 115200, in order to adapt to macos screen and minicom * [BL602] Update README.md format --- .../lighting-app/bouffalolab/bl602/README.md | 45 +++++-------------- 1 file changed, 12 insertions(+), 33 deletions(-) diff --git a/examples/lighting-app/bouffalolab/bl602/README.md b/examples/lighting-app/bouffalolab/bl602/README.md index 2bc2e05a371abd..22873801a823c9 100644 --- a/examples/lighting-app/bouffalolab/bl602/README.md +++ b/examples/lighting-app/bouffalolab/bl602/README.md @@ -30,28 +30,7 @@ The steps in this document were validated on Ubuntu 18.04 and 20.04. source ./scripts/activate.sh ``` -## Build the image - -- Build the example application: - - ``` - connectedhomeip$ ./scripts/build/build_examples.py --target bl602-light build - ``` - - Generated files - - ``` - connectedhomeip/out/bl602-light/chip-bl602-lighting-example.bin - ``` - - To delete generated executable, libraries and object files use: - - ``` - cd ~/connectedhomeip/ - rm -rf out/ - ``` - -## Flash the board +## Build the image and flash the board - Build the [lighting-app](https://github.com/project-chip/connectedhomeip/tree/master/examples/lighting-app/bouffalolab/bl602) @@ -94,7 +73,7 @@ The steps in this document were validated on Ubuntu 18.04 and 20.04. `/dev/ttyACM0`: ``` -picocom -b 2000000 /dev/ttyACM0 +picocom -b 115200 /dev/ttyACM0 ``` 2.To reset the board, press the RESET button, and you will see the log in the @@ -113,16 +92,16 @@ remote device, as well as the network credentials to use. The command below uses the default values hard-coded into the debug versions of the BL602 lighting-app to commission it onto a Wi-Fi network: -``` -$ sudo ./chip-tool pairing ble-wifi 1 ${SSID} ${PASSWORD} 20202021 3840 - - Parameters: - 1. Discriminator: 3840 - 2. Setup-pin-code: 20202021 - 3. Node ID: 1 - 4. SSID : Wi-Fi SSID - 5. PASSWORD : Wi-Fi Password -``` + ``` + $ sudo ./chip-tool pairing ble-wifi 1 ${SSID} ${PASSWORD} 20202021 3840 + + Parameters: + 1. Discriminator: 3840 + 2. Setup-pin-code: 20202021 + 3. Node ID: 1 + 4. SSID : Wi-Fi SSID + 5. PASSWORD : Wi-Fi Password + ``` ### Cluster control From 7b3f41989657ee92c37317550945d85ccd6ab672 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Fri, 1 Jul 2022 05:03:30 +0200 Subject: [PATCH 4/8] Allow using no CIPD pigweed configuration when activating (#20105) * Allow to set pigweed config via environment Allow to specify custom pigweed configuration file via PW_CONFIG_FILE, e.g. PW_CONFIG_FILE=scripts/environment_no_cipd.json source scripts/bootstrap.sh This allows to use a custom pigweed config for scripts/activate.sh as well. * Disable rosetta in no_cipd config as well --- scripts/bootstrap.sh | 5 ++--- scripts/environment_no_cipd.json | 1 + scripts/no_cipd_bootstrap.sh | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) delete mode 120000 scripts/no_cipd_bootstrap.sh diff --git a/scripts/bootstrap.sh b/scripts/bootstrap.sh index 40f1620ac8f5d1..be3a2d721abd57 100644 --- a/scripts/bootstrap.sh +++ b/scripts/bootstrap.sh @@ -26,9 +26,8 @@ _bootstrap_or_activate() { local _CONFIG_FILE="scripts/environment.json" - if [ "$_BOOTSTRAP_NAME" = "no_cipd_bootstrap.sh" ]; then - _CONFIG_FILE="scripts/environment_no_cipd.json" - _BOOTSTRAP_NAME="bootstrap.sh" + if [ ! -z "$PW_CONFIG_FILE" ]; then + _CONFIG_FILE="$PW_CONFIG_FILE" fi if [ "$_BOOTSTRAP_NAME" = "bootstrap.sh" ] || diff --git a/scripts/environment_no_cipd.json b/scripts/environment_no_cipd.json index cc9fde6220e1c3..dbb3106f0f50b2 100644 --- a/scripts/environment_no_cipd.json +++ b/scripts/environment_no_cipd.json @@ -4,5 +4,6 @@ "gn_targets": [":python_packages.install"] }, "required_submodules": ["third_party/pigweed/repo"], + "rosetta": "never", "gni_file": "build_overrides/pigweed_environment.gni" } diff --git a/scripts/no_cipd_bootstrap.sh b/scripts/no_cipd_bootstrap.sh deleted file mode 120000 index eba538975a899f..00000000000000 --- a/scripts/no_cipd_bootstrap.sh +++ /dev/null @@ -1 +0,0 @@ -bootstrap.sh \ No newline at end of file From 8582b3bfd7bb2528b4f82a195c498781b3402fdd Mon Sep 17 00:00:00 2001 From: nxptest <68574485+nxptest@users.noreply.github.com> Date: Fri, 1 Jul 2022 11:04:38 +0800 Subject: [PATCH 5/8] [MW320] Added code to support tinycrypt. (#20103) * Fix the building error for mw320 platform Signed-off-by: Chin-Ran Lo * Add to support using tiny_crypt library Building command: $ gn gen out/debug --args='treat_warnings_as_errors=false mbedtls_root="//third_party/connectedhomeip/third_party/nxp/libs/mbedtls" mbedtls_repo="//third_party/connectedhomeip/third_party/nxp/libs/mbedtls" mbedtls_use_tinycrypt=true' $ ninja -C out/debug Signed-off-by: Chin-Ran Lo * Fix the issue which is because of API changing that MatterPostAttributeChangeCallback() won't be called after the attribute is modified Signed-off-by: Chin-Ran Lo * Fix a build / linking in using tiny_crypt Building command: $ gn gen out/debug --args='treat_warnings_as_errors=false mbedtls_repo="//third_party/connectedhomeip/third_party/nxp/libs/mbedtls" mbedtls_use_tinycrypt=true' Signed-off-by: Chin-Ran Lo * Update README.md * Restyled by gn * Restyled by prettier-markdown Co-authored-by: Chin-Ran Lo Co-authored-by: Restyled.io --- examples/all-clusters-app/nxp/mw320/README.md | 10 +++++++++ examples/all-clusters-app/nxp/mw320/main.cpp | 3 +-- src/platform/device.gni | 3 ++- third_party/nxp/mw320_sdk/BUILD.gn | 22 +++++++++++++++++++ third_party/nxp/mw320_sdk/mw320_sdk.gni | 1 + 5 files changed, 36 insertions(+), 3 deletions(-) diff --git a/examples/all-clusters-app/nxp/mw320/README.md b/examples/all-clusters-app/nxp/mw320/README.md index 14699b15a29a48..af1c09621398d8 100755 --- a/examples/all-clusters-app/nxp/mw320/README.md +++ b/examples/all-clusters-app/nxp/mw320/README.md @@ -49,6 +49,16 @@ Note: 2. "source third_party/connectedhomeip/scripts/activate.sh" can be omitted if your environment is already setup without issues. +Tinycrypt ECC operations: + +Note: This solution is temporary. + +In order to use the tinycrypt ecc operations, use the following build arguments: + +``` +$ gn gen out/debug --args='treat_warnings_as_errors=false mbedtls_repo="//third_party/connectedhomeip/third_party/nxp/libs/mbedtls" mbedtls_use_tinycrypt=true' +``` + ## Flashing diff --git a/examples/all-clusters-app/nxp/mw320/main.cpp b/examples/all-clusters-app/nxp/mw320/main.cpp index bf7f1fc454fd4a..52fbb40964aeff 100644 --- a/examples/all-clusters-app/nxp/mw320/main.cpp +++ b/examples/all-clusters-app/nxp/mw320/main.cpp @@ -1411,8 +1411,7 @@ static void OnSwitchAttributeChangeCallback(EndpointId endpointId, AttributeId a /* Callback to receive the cluster modification event */ -void MatterPostAttributeChangeCallback(const chip::app::ConcreteAttributePath & path, uint8_t mask, uint8_t type, uint16_t size, - uint8_t * value) +void MatterPostAttributeChangeCallback(const chip::app::ConcreteAttributePath & path, uint8_t type, uint16_t size, uint8_t * value) { PRINTF("==> MatterPostAttributeChangeCallback, cluster: %x, attr: %x, size: %d \r\n", path.mClusterId, path.mAttributeId, size); // path.mEndpointId, path.mClusterId, path.mAttributeId, mask, type, size, value diff --git a/src/platform/device.gni b/src/platform/device.gni index 9af7d49c0d3141..ec3de35d4dcaac 100755 --- a/src/platform/device.gni +++ b/src/platform/device.gni @@ -63,7 +63,8 @@ declare_args() { chip_device_platform == "mbed" || chip_device_platform == "tizen" || chip_device_platform == "android" || chip_device_platform == "ameba" || chip_device_platform == "webos" || chip_device_platform == "cc32xx" || - chip_device_platform == "bl602" || chip_device_platform == "bl602" + chip_device_platform == "bl602" || chip_device_platform == "bl602" || + chip_device_platform == "mw320" # Enable ble support. if (chip_device_platform == "fake") { diff --git a/third_party/nxp/mw320_sdk/BUILD.gn b/third_party/nxp/mw320_sdk/BUILD.gn index 4d36aed69d5c01..be6fb914b1c4a6 100644 --- a/third_party/nxp/mw320_sdk/BUILD.gn +++ b/third_party/nxp/mw320_sdk/BUILD.gn @@ -62,11 +62,33 @@ config("mbedtls_mw320_config") { # "MBEDTLS_ECP_SHORT_WEIERSTRASS_ENABLED", ] + if (mbedtls_use_tinycrypt) { + defines += [ + "MBEDTLS_USE_TINYCRYPT", + "MBEDTLS_OPTIMIZE_TINYCRYPT_ASM", + ] + } include_dirs = [ chip_root ] + + if (mbedtls_use_tinycrypt) { + include_dirs += [ "${mbedtls_repo}/repo/include/tinycrypt" ] + } } mbedtls_target("mbedtls") { + import("${mw320_sdk_build_root}/mw320_sdk.gni") + + if (mbedtls_use_tinycrypt) { + if (!defined(sources)) { + sources = [] + } + sources += [ + "${mbedtls_repo}/repo/tinycrypt/ecc.c", + "${mbedtls_repo}/repo/tinycrypt/ecc_dh.c", + "${mbedtls_repo}/repo/tinycrypt/ecc_dsa.c", + ] + } public_configs = [ ":mbedtls_mw320_config" ] public_deps = [ ":mw320_sdk" ] diff --git a/third_party/nxp/mw320_sdk/mw320_sdk.gni b/third_party/nxp/mw320_sdk/mw320_sdk.gni index 2139089e298a84..4e9de699485dcc 100644 --- a/third_party/nxp/mw320_sdk/mw320_sdk.gni +++ b/third_party/nxp/mw320_sdk/mw320_sdk.gni @@ -19,6 +19,7 @@ import("//build_overrides/mw320_sdk.gni") declare_args() { # Location of the mw320 SDK. mw320_sdk_root = "${chip_root}/third_party/nxp/mw320_sdk/repo" + mbedtls_use_tinycrypt = false } assert(mw320_sdk_root != "", "mw320_sdk_root must be specified") From b8a4670119d41ffad7c21ea96080a27c653f4f25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Kr=C3=B3lik?= <66667989+Damian-Nordic@users.noreply.github.com> Date: Fri, 1 Jul 2022 05:07:56 +0200 Subject: [PATCH 6/8] [nrfconnect] Support USR & PIN features in lock-app (#20042) Add functionality to store and validate user PIN codes in the Lock example. Signed-off-by: Damian Krolik --- examples/lock-app/nrfconnect/Kconfig | 12 ++ .../nrfconnect/main/BoltLockManager.cpp | 114 ++++++++++++++++++ .../lock-app/nrfconnect/main/ZclCallbacks.cpp | 61 +++++++--- .../nrfconnect/main/include/BoltLockManager.h | 33 +++++ .../door-lock-server/door-lock-server.cpp | 2 +- src/app/tests/suites/DL_LockUnlock.yaml | 2 +- .../chip-tool/zap-generated/test/Commands.h | 2 +- .../zap-generated/test/Commands.h | 8 +- 8 files changed, 211 insertions(+), 23 deletions(-) diff --git a/examples/lock-app/nrfconnect/Kconfig b/examples/lock-app/nrfconnect/Kconfig index ddfeff16127be7..e9dfd7de6eff89 100644 --- a/examples/lock-app/nrfconnect/Kconfig +++ b/examples/lock-app/nrfconnect/Kconfig @@ -15,6 +15,18 @@ # mainmenu "Matter nRF Connect Lock Example Application" +config LOCK_NUM_USERS + int "Maximum number of users supported by lock" + default 10 + +config LOCK_NUM_CREDENTIALS + int "Maximum number of credentials supported by lock" + default 20 + +config LOCK_NUM_CREDENTIALS_PER_USER + int "Maximum number of credentials per user supported by lock" + default 3 + config STATE_LEDS bool "Use LEDs to indicate the device state" default y diff --git a/examples/lock-app/nrfconnect/main/BoltLockManager.cpp b/examples/lock-app/nrfconnect/main/BoltLockManager.cpp index a220dae31ece86..0a860818b0e5ce 100644 --- a/examples/lock-app/nrfconnect/main/BoltLockManager.cpp +++ b/examples/lock-app/nrfconnect/main/BoltLockManager.cpp @@ -23,6 +23,8 @@ #include "AppEvent.h" #include "AppTask.h" +using namespace chip; + BoltLockManager BoltLockManager::sLock; void BoltLockManager::Init(StateChangeCallback callback) @@ -33,6 +35,118 @@ void BoltLockManager::Init(StateChangeCallback callback) k_timer_user_data_set(&mActuatorTimer, this); } +bool BoltLockManager::GetUser(uint16_t userIndex, EmberAfPluginDoorLockUserInfo & user) const +{ + // userIndex is guaranteed by the caller to be between 1 and CONFIG_LOCK_NUM_USERS + user = mUsers[userIndex - 1]; + + ChipLogProgress(Zcl, "Getting lock user %u: %s", static_cast(userIndex), + user.userStatus == DlUserStatus::kAvailable ? "available" : "occupied"); + + return true; +} + +bool BoltLockManager::SetUser(uint16_t userIndex, FabricIndex creator, FabricIndex modifier, const CharSpan & userName, + uint32_t uniqueId, DlUserStatus userStatus, DlUserType userType, DlCredentialRule credentialRule, + const DlCredential * credentials, size_t totalCredentials) +{ + // userIndex is guaranteed by the caller to be between 1 and CONFIG_LOCK_NUM_USERS + UserData & userData = mUserData[userIndex - 1]; + auto & user = mUsers[userIndex - 1]; + + VerifyOrReturnError(userName.size() <= DOOR_LOCK_MAX_USER_NAME_SIZE, false); + VerifyOrReturnError(totalCredentials <= CONFIG_LOCK_NUM_CREDENTIALS_PER_USER, false); + + Platform::CopyString(userData.mName, userName); + memcpy(userData.mCredentials, credentials, totalCredentials * sizeof(DlCredential)); + + user.userName = CharSpan(userData.mName, userName.size()); + user.credentials = Span(userData.mCredentials, totalCredentials); + user.userUniqueId = uniqueId; + user.userStatus = userStatus; + user.userType = userType; + user.credentialRule = credentialRule; + user.creationSource = DlAssetSource::kMatterIM; + user.createdBy = creator; + user.modificationSource = DlAssetSource::kMatterIM; + user.lastModifiedBy = modifier; + + ChipLogProgress(Zcl, "Setting lock user %u: %s", static_cast(userIndex), + userStatus == DlUserStatus::kAvailable ? "available" : "occupied"); + + return true; +} + +bool BoltLockManager::GetCredential(uint16_t credentialIndex, DlCredentialType credentialType, + EmberAfPluginDoorLockCredentialInfo & credential) const +{ + VerifyOrReturnError(credentialIndex > 0 && credentialIndex <= CONFIG_LOCK_NUM_CREDENTIALS, false); + + credential = mCredentials[credentialIndex - 1]; + + ChipLogProgress(Zcl, "Getting lock credential %u: %s", static_cast(credentialIndex), + credential.status == DlCredentialStatus::kAvailable ? "available" : "occupied"); + + return true; +} + +bool BoltLockManager::SetCredential(uint16_t credentialIndex, FabricIndex creator, FabricIndex modifier, + DlCredentialStatus credentialStatus, DlCredentialType credentialType, const ByteSpan & secret) +{ + VerifyOrReturnError(credentialIndex > 0 && credentialIndex <= CONFIG_LOCK_NUM_CREDENTIALS, false); + VerifyOrReturnError(secret.size() <= kMaxCredentialLength, false); + + CredentialData & credentialData = mCredentialData[credentialIndex - 1]; + auto & credential = mCredentials[credentialIndex - 1]; + + if (!secret.empty()) + { + memcpy(credentialData.mSecret.Alloc(secret.size()).Get(), secret.data(), secret.size()); + } + + credential.status = credentialStatus; + credential.credentialType = credentialType; + credential.credentialData = ByteSpan(credentialData.mSecret.Get(), secret.size()); + credential.creationSource = DlAssetSource::kMatterIM; + credential.createdBy = creator; + credential.modificationSource = DlAssetSource::kMatterIM; + credential.lastModifiedBy = modifier; + + ChipLogProgress(Zcl, "Setting lock credential %u: %s", static_cast(credentialIndex), + credential.status == DlCredentialStatus::kAvailable ? "available" : "occupied"); + + return true; +} + +bool BoltLockManager::ValidatePIN(const Optional & pinCode, DlOperationError & err) const +{ + // Optionality of the PIN code is validated by the caller, so assume it is OK not to provide the PIN code. + if (!pinCode.HasValue()) + { + return true; + } + + // Check the PIN code + for (const auto & credential : mCredentials) + { + if (credential.status == DlCredentialStatus::kAvailable || credential.credentialType != DlCredentialType::kPin) + { + continue; + } + + if (credential.credentialData.data_equal(pinCode.Value())) + { + ChipLogDetail(Zcl, "Valid lock PIN code provided"); + return true; + } + } + + ChipLogDetail(Zcl, "Invalid lock PIN code provided"); + err = DlOperationError::kInvalidCredential; + + return false; +} + void BoltLockManager::Lock(OperationSource source) { VerifyOrReturn(mState != State::kLockingCompleted); diff --git a/examples/lock-app/nrfconnect/main/ZclCallbacks.cpp b/examples/lock-app/nrfconnect/main/ZclCallbacks.cpp index b56d6aa8ee8888..ccb5526344f05c 100644 --- a/examples/lock-app/nrfconnect/main/ZclCallbacks.cpp +++ b/examples/lock-app/nrfconnect/main/ZclCallbacks.cpp @@ -49,34 +49,63 @@ void MatterPostAttributeChangeCallback(const chip::app::ConcreteAttributePath & } } -bool emberAfPluginDoorLockOnDoorLockCommand(chip::EndpointId endpointId, const Optional & pinCode, DlOperationError & err) +bool emberAfPluginDoorLockGetUser(EndpointId endpointId, uint16_t userIndex, EmberAfPluginDoorLockUserInfo & user) { - return true; + return BoltLockMgr().GetUser(userIndex, user); } -bool emberAfPluginDoorLockOnDoorUnlockCommand(chip::EndpointId endpointId, const Optional & pinCode, - DlOperationError & err) +bool emberAfPluginDoorLockSetUser(EndpointId endpointId, uint16_t userIndex, FabricIndex creator, FabricIndex modifier, + const CharSpan & userName, uint32_t uniqueId, DlUserStatus userStatus, DlUserType userType, + DlCredentialRule credentialRule, const DlCredential * credentials, size_t totalCredentials) { - return true; + return BoltLockMgr().SetUser(userIndex, creator, modifier, userName, uniqueId, userStatus, userType, credentialRule, + credentials, totalCredentials); +} + +bool emberAfPluginDoorLockGetCredential(EndpointId endpointId, uint16_t credentialIndex, DlCredentialType credentialType, + EmberAfPluginDoorLockCredentialInfo & credential) +{ + return BoltLockMgr().GetCredential(credentialIndex, credentialType, credential); +} + +bool emberAfPluginDoorLockSetCredential(EndpointId endpointId, uint16_t credentialIndex, FabricIndex creator, FabricIndex modifier, + DlCredentialStatus credentialStatus, DlCredentialType credentialType, + const ByteSpan & secret) +{ + return BoltLockMgr().SetCredential(credentialIndex, creator, modifier, credentialStatus, credentialType, secret); +} + +bool emberAfPluginDoorLockOnDoorLockCommand(EndpointId endpointId, const Optional & pinCode, DlOperationError & err) +{ + return BoltLockMgr().ValidatePIN(pinCode, err); +} + +bool emberAfPluginDoorLockOnDoorUnlockCommand(EndpointId endpointId, const Optional & pinCode, DlOperationError & err) +{ + return BoltLockMgr().ValidatePIN(pinCode, err); } void emberAfDoorLockClusterInitCallback(EndpointId endpoint) { DoorLockServer::Instance().InitServer(endpoint); - EmberAfStatus status = DoorLock::Attributes::LockType::Set(endpoint, DlLockType::kDeadBolt); - if (status != EMBER_ZCL_STATUS_SUCCESS) - { - LOG_ERR("Updating type %x", status); - } + const auto logOnFailure = [](EmberAfStatus status, const char * attributeName) { + if (status != EMBER_ZCL_STATUS_SUCCESS) + { + ChipLogError(Zcl, "Failed to set DoorLock %s: %x", attributeName, status); + } + }; + + logOnFailure(DoorLock::Attributes::LockType::Set(endpoint, DlLockType::kDeadBolt), "type"); + logOnFailure(DoorLock::Attributes::NumberOfTotalUsersSupported::Set(endpoint, CONFIG_LOCK_NUM_USERS), "number of users"); + logOnFailure(DoorLock::Attributes::NumberOfPINUsersSupported::Set(endpoint, CONFIG_LOCK_NUM_USERS), "number of PIN users"); + logOnFailure(DoorLock::Attributes::NumberOfRFIDUsersSupported::Set(endpoint, 0), "number of RFID users"); + logOnFailure(DoorLock::Attributes::NumberOfCredentialsSupportedPerUser::Set(endpoint, CONFIG_LOCK_NUM_CREDENTIALS_PER_USER), + "number of credentials per user"); - // Set FeatureMap to 0, default is: + // Set FeatureMap to (kUsersManagement|kPINCredentials), default is: // (kUsersManagement|kAccessSchedules|kRFIDCredentials|kPINCredentials) 0x113 - status = DoorLock::Attributes::FeatureMap::Set(endpoint, 0); - if (status != EMBER_ZCL_STATUS_SUCCESS) - { - LOG_ERR("Updating feature map %x", status); - } + logOnFailure(DoorLock::Attributes::FeatureMap::Set(endpoint, 0x101), "feature map"); GetAppTask().UpdateClusterState(BoltLockMgr().GetState(), BoltLockManager::OperationSource::kUnspecified); } diff --git a/examples/lock-app/nrfconnect/main/include/BoltLockManager.h b/examples/lock-app/nrfconnect/main/include/BoltLockManager.h index deeff6024de5a3..940346e176fa2f 100644 --- a/examples/lock-app/nrfconnect/main/include/BoltLockManager.h +++ b/examples/lock-app/nrfconnect/main/include/BoltLockManager.h @@ -19,7 +19,9 @@ #pragma once +#include #include +#include #include @@ -30,6 +32,8 @@ class AppEvent; class BoltLockManager { public: + static constexpr size_t kMaxCredentialLength = 128; + enum class State : uint8_t { kLockingInitiated = 0, @@ -38,6 +42,17 @@ class BoltLockManager kUnlockingCompleted, }; + struct UserData + { + char mName[DOOR_LOCK_USER_NAME_BUFFER_SIZE]; + DlCredential mCredentials[CONFIG_LOCK_NUM_CREDENTIALS_PER_USER]; + }; + + struct CredentialData + { + chip::Platform::ScopedMemoryBuffer mSecret; + }; + using OperationSource = chip::app::Clusters::DoorLock::DlOperationSource; using StateChangeCallback = void (*)(State, OperationSource); @@ -48,6 +63,18 @@ class BoltLockManager State GetState() const { return mState; } bool IsLocked() const { return mState == State::kLockingCompleted; } + bool GetUser(uint16_t userIndex, EmberAfPluginDoorLockUserInfo & user) const; + bool SetUser(uint16_t userIndex, chip::FabricIndex creator, chip::FabricIndex modifier, const chip::CharSpan & userName, + uint32_t uniqueId, DlUserStatus userStatus, DlUserType userType, DlCredentialRule credentialRule, + const DlCredential * credentials, size_t totalCredentials); + + bool GetCredential(uint16_t credentialIndex, DlCredentialType credentialType, + EmberAfPluginDoorLockCredentialInfo & credential) const; + bool SetCredential(uint16_t credentialIndex, chip::FabricIndex creator, chip::FabricIndex modifier, + DlCredentialStatus credentialStatus, DlCredentialType credentialType, const chip::ByteSpan & secret); + + bool ValidatePIN(const Optional & pinCode, DlOperationError & err) const; + void Lock(OperationSource source); void Unlock(OperationSource source); @@ -63,6 +90,12 @@ class BoltLockManager OperationSource mActuatorOperationSource = OperationSource::kButton; k_timer mActuatorTimer = {}; + UserData mUserData[CONFIG_LOCK_NUM_USERS]; + EmberAfPluginDoorLockUserInfo mUsers[CONFIG_LOCK_NUM_USERS] = {}; + + CredentialData mCredentialData[CONFIG_LOCK_NUM_CREDENTIALS]; + EmberAfPluginDoorLockCredentialInfo mCredentials[CONFIG_LOCK_NUM_CREDENTIALS] = {}; + static BoltLockManager sLock; }; diff --git a/src/app/clusters/door-lock-server/door-lock-server.cpp b/src/app/clusters/door-lock-server/door-lock-server.cpp index f95ab24231d39c..d2a00bab7ea9b9 100644 --- a/src/app/clusters/door-lock-server/door-lock-server.cpp +++ b/src/app/clusters/door-lock-server/door-lock-server.cpp @@ -1654,7 +1654,7 @@ EmberAfStatus DoorLockServer::createUser(chip::EndpointId endpointId, chip::Fabr return EMBER_ZCL_STATUS_FAILURE; } - const auto & newUserName = !userName.IsNull() ? userName.Value() : chip::CharSpan(""); + const auto & newUserName = !userName.IsNull() ? userName.Value() : chip::CharSpan::fromCharString(""); auto newUserUniqueId = userUniqueId.IsNull() ? 0xFFFFFFFF : userUniqueId.Value(); auto newUserStatus = userStatus.IsNull() ? DlUserStatus::kOccupiedEnabled : userStatus.Value(); auto newUserType = userType.IsNull() ? DlUserType::kUnrestrictedUser : userType.Value(); diff --git a/src/app/tests/suites/DL_LockUnlock.yaml b/src/app/tests/suites/DL_LockUnlock.yaml index 4ddaee939b4254..cacad862b725b4 100644 --- a/src/app/tests/suites/DL_LockUnlock.yaml +++ b/src/app/tests/suites/DL_LockUnlock.yaml @@ -120,7 +120,7 @@ tests: response: value: 2 - - label: "Try to unlock the door with valid PIN" + - label: "Try to lock the door with valid PIN" command: "LockDoor" timedInteractionTimeoutMs: 10000 arguments: diff --git a/zzz_generated/chip-tool/zap-generated/test/Commands.h b/zzz_generated/chip-tool/zap-generated/test/Commands.h index 8552bab3e3bb58..3506efe95327e7 100644 --- a/zzz_generated/chip-tool/zap-generated/test/Commands.h +++ b/zzz_generated/chip-tool/zap-generated/test/Commands.h @@ -56821,7 +56821,7 @@ class DL_LockUnlockSuite : public TestCommand chip::NullOptional); } case 12: { - LogStep(12, "Try to unlock the door with valid PIN"); + LogStep(12, "Try to lock the door with valid PIN"); ListFreer listFreer; chip::app::Clusters::DoorLock::Commands::LockDoor::Type value; value.pinCode.Emplace(); diff --git a/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h b/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h index 89d7d708a2f923..17f5842c402c73 100644 --- a/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h +++ b/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h @@ -93248,8 +93248,8 @@ class DL_LockUnlock : public TestCommandBridge { err = TestVerifyThatLockStateAttributeValueIsSetToUnlocked_11(); break; case 12: - ChipLogProgress(chipTool, " ***** Test Step 12 : Try to unlock the door with valid PIN\n"); - err = TestTryToUnlockTheDoorWithValidPin_12(); + ChipLogProgress(chipTool, " ***** Test Step 12 : Try to lock the door with valid PIN\n"); + err = TestTryToLockTheDoorWithValidPin_12(); break; case 13: ChipLogProgress(chipTool, " ***** Test Step 13 : Verify that lock state attribute value is set to Locked\n"); @@ -93599,7 +93599,7 @@ class DL_LockUnlock : public TestCommandBridge { return CHIP_NO_ERROR; } - CHIP_ERROR TestTryToUnlockTheDoorWithValidPin_12() + CHIP_ERROR TestTryToLockTheDoorWithValidPin_12() { CHIPDevice * device = GetDevice("alpha"); CHIPTestDoorLock * cluster = [[CHIPTestDoorLock alloc] initWithDevice:device endpoint:1 queue:mCallbackQueue]; @@ -93609,7 +93609,7 @@ class DL_LockUnlock : public TestCommandBridge { params.pinCode = [[NSData alloc] initWithBytes:"123456" length:6]; [cluster lockDoorWithParams:params completionHandler:^(NSError * _Nullable err) { - NSLog(@"Try to unlock the door with valid PIN Error: %@", err); + NSLog(@"Try to lock the door with valid PIN Error: %@", err); VerifyOrReturn(CheckValue("status", err ? err.code : 0, 0)); From 3b92283b7f83fb8ccad14ce1407d423c0ab249ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damian=20Kr=C3=B3lik?= <66667989+Damian-Nordic@users.noreply.github.com> Date: Fri, 1 Jul 2022 05:08:22 +0200 Subject: [PATCH 7/8] [door-lock] Validate OperatingMode attribute (#20046) Signed-off-by: Damian Krolik --- .../door-lock-server/door-lock-server.cpp | 34 +++++--- .../door-lock-server/door-lock-server.h | 4 +- src/app/tests/suites/DL_LockUnlock.yaml | 18 ++++ .../chip-tool/zap-generated/test/Commands.h | 38 +++++++- .../zap-generated/test/Commands.h | 87 ++++++++++++++++++- 5 files changed, 161 insertions(+), 20 deletions(-) diff --git a/src/app/clusters/door-lock-server/door-lock-server.cpp b/src/app/clusters/door-lock-server/door-lock-server.cpp index d2a00bab7ea9b9..9b2482adf6e1cd 100644 --- a/src/app/clusters/door-lock-server/door-lock-server.cpp +++ b/src/app/clusters/door-lock-server/door-lock-server.cpp @@ -3094,6 +3094,14 @@ void DoorLockServer::clearHolidaySchedule(chip::app::CommandHandler * commandObj emberAfSendImmediateDefaultResponse(EMBER_ZCL_STATUS_SUCCESS); } +bool DoorLockServer::RemoteOperationEnabled(chip::EndpointId endpointId) const +{ + DlOperatingMode mode; + + return GetAttribute(endpointId, Attributes::OperatingMode::Id, Attributes::OperatingMode::Get, mode) && + mode != DlOperatingMode::kPrivacy && mode != DlOperatingMode::kNoRemoteLockUnlock; +} + bool DoorLockServer::HandleRemoteLockOperation(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, DlLockOperationType opType, RemoteLockOpHandler opHandler, const Optional & pinCode) @@ -3107,19 +3115,21 @@ bool DoorLockServer::HandleRemoteLockOperation(chip::app::CommandHandler * comma uint16_t pinUserIdx = 0; uint16_t pinCredIdx = 0; bool credentialsOk = false; - bool operationOk = false; + bool success = false; + + VerifyOrExit(RemoteOperationEnabled(endpoint), reason = DlOperationError::kUnspecified); // appclusters.pdf 5.3.4.1: // When the PINCode field is provided an invalid PIN will count towards the WrongCodeEntryLimit and the - // UserCodeTemporaryDisableTime will be triggered if the WrongCodeEntryLimit is exceeded. The lock SHALL ignore any attempts to - // lock/unlock the door until the UserCodeTemporaryDisableTime expires. + // UserCodeTemporaryDisableTime will be triggered if the WrongCodeEntryLimit is exceeded. The lock SHALL ignore any attempts + // to lock/unlock the door until the UserCodeTemporaryDisableTime expires. // TODO: check whether UserCodeTemporaryDisableTime expired or not. if (pinCode.HasValue()) { // appclusters.pdf 5.3.4.1: - // If the PINCode field is provided, the door lock SHALL verify PINCode before granting access regardless of the value of - // RequirePINForRemoteOperation attribute. + // If the PINCode field is provided, the door lock SHALL verify PINCode before granting access regardless of the value + // of RequirePINForRemoteOperation attribute. VerifyOrExit(SupportsPIN(endpoint) && SupportsUSR(endpoint), emberAfDoorLockClusterPrintln( "PIN code is supplied while USR/PIN features are disabled. Exiting [endpoint=%d, lock_op=%d]", endpoint, @@ -3140,9 +3150,9 @@ bool DoorLockServer::HandleRemoteLockOperation(chip::app::CommandHandler * comma bool requirePin = false; // appclusters.pdf 5.3.4.1: - // If the RequirePINForRemoteOperation attribute is True then PINCode field SHALL be provided and the door lock SHALL NOT - // grant access if it is not provided. This attribute exists when COTA and PIN features are both enabled. Otherwise we - // assume PIN to be OK. + // If the RequirePINForRemoteOperation attribute is True then PINCode field SHALL be provided and the door lock SHALL + // NOT grant access if it is not provided. This attribute exists when COTA and PIN features are both enabled. Otherwise + // we assume PIN to be OK. if (SupportsCredentialsOTA(endpoint) && SupportsPIN(endpoint)) { auto status = Attributes::RequirePINforRemoteOperation::Get(endpoint, &requirePin); @@ -3161,15 +3171,13 @@ bool DoorLockServer::HandleRemoteLockOperation(chip::app::CommandHandler * comma }); // credentials check succeeded, try to lock/unlock door - operationOk = opHandler(endpoint, pinCode, reason); - VerifyOrExit(operationOk, /* reason is set by the above call */); + success = opHandler(endpoint, pinCode, reason); + VerifyOrExit(success, /* reason is set by the above call */); // door locked, set cluster attribute VerifyOrDie(SetLockState(endpoint, newLockState, DlOperationSource::kRemote)); exit: - bool success = credentialsOk && operationOk; - // Send command response emberAfSendImmediateDefaultResponse(success ? EMBER_ZCL_STATUS_SUCCESS : EMBER_ZCL_STATUS_FAILURE); @@ -3256,7 +3264,7 @@ void DoorLockServer::SendEvent(chip::EndpointId endpointId, T & event) template bool DoorLockServer::GetAttribute(chip::EndpointId endpointId, chip::AttributeId attributeId, - EmberAfStatus (*getFn)(chip::EndpointId endpointId, T * value), T & value) + EmberAfStatus (*getFn)(chip::EndpointId endpointId, T * value), T & value) const { EmberAfStatus status = getFn(endpointId, &value); bool success = (EMBER_ZCL_STATUS_SUCCESS == status); diff --git a/src/app/clusters/door-lock-server/door-lock-server.h b/src/app/clusters/door-lock-server/door-lock-server.h index efe8a0ace57ae3..ac0cce099e6a56 100644 --- a/src/app/clusters/door-lock-server/door-lock-server.h +++ b/src/app/clusters/door-lock-server/door-lock-server.h @@ -313,6 +313,8 @@ class DoorLockServer void clearHolidaySchedule(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, uint8_t holidayIndex); + bool RemoteOperationEnabled(chip::EndpointId endpointId) const; + /** * @brief Common handler for LockDoor, UnlockDoor, UnlockWithTimeout commands * @@ -378,7 +380,7 @@ class DoorLockServer */ template bool GetAttribute(chip::EndpointId endpointId, chip::AttributeId attributeId, - EmberAfStatus (*getFn)(chip::EndpointId endpointId, T * value), T & value); + EmberAfStatus (*getFn)(chip::EndpointId endpointId, T * value), T & value) const; /** * @brief Set generic attribute value diff --git a/src/app/tests/suites/DL_LockUnlock.yaml b/src/app/tests/suites/DL_LockUnlock.yaml index cacad862b725b4..08ff372ee6b45e 100644 --- a/src/app/tests/suites/DL_LockUnlock.yaml +++ b/src/app/tests/suites/DL_LockUnlock.yaml @@ -134,7 +134,25 @@ tests: response: value: 1 + - label: "Set OperatingMode to NoRemoteLockUnlock" + command: "writeAttribute" + attribute: "OperatingMode" + arguments: + value: 3 + + - label: "Try to unlock the door when OperatingMode is NoRemoteLockUnlock" + command: "LockDoor" + timedInteractionTimeoutMs: 10000 + response: + error: FAILURE + # Clean-up + - label: "Set OperatingMode to Normal" + command: "writeAttribute" + attribute: "OperatingMode" + arguments: + value: 0 + - label: "Clean the created credential" command: "ClearCredential" timedInteractionTimeoutMs: 10000 diff --git a/zzz_generated/chip-tool/zap-generated/test/Commands.h b/zzz_generated/chip-tool/zap-generated/test/Commands.h index 3506efe95327e7..c4bf101e02c23e 100644 --- a/zzz_generated/chip-tool/zap-generated/test/Commands.h +++ b/zzz_generated/chip-tool/zap-generated/test/Commands.h @@ -56578,7 +56578,7 @@ class DL_UsersAndCredentialsSuite : public TestCommand class DL_LockUnlockSuite : public TestCommand { public: - DL_LockUnlockSuite(CredentialIssuerCommands * credsIssuerConfig) : TestCommand("DL_LockUnlock", 15, credsIssuerConfig) + DL_LockUnlockSuite(CredentialIssuerCommands * credsIssuerConfig) : TestCommand("DL_LockUnlock", 18, credsIssuerConfig) { AddArgument("nodeId", 0, UINT64_MAX, &mNodeId); AddArgument("cluster", &mCluster); @@ -56704,6 +56704,15 @@ class DL_LockUnlockSuite : public TestCommand case 14: VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); break; + case 15: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), EMBER_ZCL_STATUS_FAILURE)); + break; + case 16: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); + break; + case 17: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); + break; default: LogErrorOnFailure(ContinueOnChipMainThread(CHIP_ERROR_INVALID_ARGUMENT)); } @@ -56837,7 +56846,32 @@ class DL_LockUnlockSuite : public TestCommand chip::NullOptional); } case 14: { - LogStep(14, "Clean the created credential"); + LogStep(14, "Set OperatingMode to NoRemoteLockUnlock"); + ListFreer listFreer; + chip::app::Clusters::DoorLock::DlOperatingMode value; + value = static_cast(3); + return WriteAttribute(kIdentityAlpha, GetEndpoint(1), DoorLock::Id, DoorLock::Attributes::OperatingMode::Id, value, + chip::NullOptional, chip::NullOptional); + } + case 15: { + LogStep(15, "Try to unlock the door when OperatingMode is NoRemoteLockUnlock"); + ListFreer listFreer; + chip::app::Clusters::DoorLock::Commands::LockDoor::Type value; + return SendCommand(kIdentityAlpha, GetEndpoint(1), DoorLock::Id, DoorLock::Commands::LockDoor::Id, value, + chip::Optional(10000), chip::NullOptional + + ); + } + case 16: { + LogStep(16, "Set OperatingMode to Normal"); + ListFreer listFreer; + chip::app::Clusters::DoorLock::DlOperatingMode value; + value = static_cast(0); + return WriteAttribute(kIdentityAlpha, GetEndpoint(1), DoorLock::Id, DoorLock::Attributes::OperatingMode::Id, value, + chip::NullOptional, chip::NullOptional); + } + case 17: { + LogStep(17, "Clean the created credential"); ListFreer listFreer; chip::app::Clusters::DoorLock::Commands::ClearCredential::Type value; value.credential.SetNonNull(); diff --git a/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h b/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h index 17f5842c402c73..d882221b974457 100644 --- a/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h +++ b/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h @@ -93256,8 +93256,20 @@ class DL_LockUnlock : public TestCommandBridge { err = TestVerifyThatLockStateAttributeValueIsSetToLocked_13(); break; case 14: - ChipLogProgress(chipTool, " ***** Test Step 14 : Clean the created credential\n"); - err = TestCleanTheCreatedCredential_14(); + ChipLogProgress(chipTool, " ***** Test Step 14 : Set OperatingMode to NoRemoteLockUnlock\n"); + err = TestSetOperatingModeToNoRemoteLockUnlock_14(); + break; + case 15: + ChipLogProgress(chipTool, " ***** Test Step 15 : Try to unlock the door when OperatingMode is NoRemoteLockUnlock\n"); + err = TestTryToUnlockTheDoorWhenOperatingModeIsNoRemoteLockUnlock_15(); + break; + case 16: + ChipLogProgress(chipTool, " ***** Test Step 16 : Set OperatingMode to Normal\n"); + err = TestSetOperatingModeToNormal_16(); + break; + case 17: + ChipLogProgress(chipTool, " ***** Test Step 17 : Clean the created credential\n"); + err = TestCleanTheCreatedCredential_17(); break; } @@ -93315,6 +93327,15 @@ class DL_LockUnlock : public TestCommandBridge { case 14: VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); break; + case 15: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), EMBER_ZCL_STATUS_FAILURE)); + break; + case 16: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); + break; + case 17: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); + break; } // Go on to the next test. @@ -93328,7 +93349,7 @@ class DL_LockUnlock : public TestCommandBridge { private: std::atomic_uint16_t mTestIndex; - const uint16_t mTestCount = 15; + const uint16_t mTestCount = 18; chip::Optional mNodeId; chip::Optional mCluster; @@ -93642,7 +93663,65 @@ class DL_LockUnlock : public TestCommandBridge { return CHIP_NO_ERROR; } - CHIP_ERROR TestCleanTheCreatedCredential_14() + CHIP_ERROR TestSetOperatingModeToNoRemoteLockUnlock_14() + { + CHIPDevice * device = GetDevice("alpha"); + CHIPTestDoorLock * cluster = [[CHIPTestDoorLock alloc] initWithDevice:device endpoint:1 queue:mCallbackQueue]; + VerifyOrReturnError(cluster != nil, CHIP_ERROR_INCORRECT_STATE); + + id operatingModeArgument; + operatingModeArgument = [NSNumber numberWithUnsignedChar:3U]; + [cluster writeAttributeOperatingModeWithValue:operatingModeArgument + completionHandler:^(NSError * _Nullable err) { + NSLog(@"Set OperatingMode to NoRemoteLockUnlock Error: %@", err); + + VerifyOrReturn(CheckValue("status", err ? err.code : 0, 0)); + + NextTest(); + }]; + + return CHIP_NO_ERROR; + } + + CHIP_ERROR TestTryToUnlockTheDoorWhenOperatingModeIsNoRemoteLockUnlock_15() + { + CHIPDevice * device = GetDevice("alpha"); + CHIPTestDoorLock * cluster = [[CHIPTestDoorLock alloc] initWithDevice:device endpoint:1 queue:mCallbackQueue]; + VerifyOrReturnError(cluster != nil, CHIP_ERROR_INCORRECT_STATE); + + __auto_type * params = [[CHIPDoorLockClusterLockDoorParams alloc] init]; + [cluster lockDoorWithParams:params + completionHandler:^(NSError * _Nullable err) { + NSLog(@"Try to unlock the door when OperatingMode is NoRemoteLockUnlock Error: %@", err); + + VerifyOrReturn(CheckValue("status", err ? err.code : 0, EMBER_ZCL_STATUS_FAILURE)); + NextTest(); + }]; + + return CHIP_NO_ERROR; + } + + CHIP_ERROR TestSetOperatingModeToNormal_16() + { + CHIPDevice * device = GetDevice("alpha"); + CHIPTestDoorLock * cluster = [[CHIPTestDoorLock alloc] initWithDevice:device endpoint:1 queue:mCallbackQueue]; + VerifyOrReturnError(cluster != nil, CHIP_ERROR_INCORRECT_STATE); + + id operatingModeArgument; + operatingModeArgument = [NSNumber numberWithUnsignedChar:0U]; + [cluster writeAttributeOperatingModeWithValue:operatingModeArgument + completionHandler:^(NSError * _Nullable err) { + NSLog(@"Set OperatingMode to Normal Error: %@", err); + + VerifyOrReturn(CheckValue("status", err ? err.code : 0, 0)); + + NextTest(); + }]; + + return CHIP_NO_ERROR; + } + + CHIP_ERROR TestCleanTheCreatedCredential_17() { CHIPDevice * device = GetDevice("alpha"); CHIPTestDoorLock * cluster = [[CHIPTestDoorLock alloc] initWithDevice:device endpoint:1 queue:mCallbackQueue]; From a295bec33a7bfbb2beec3c43abcbaecfb4397f75 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 30 Jun 2022 23:08:33 -0400 Subject: [PATCH 8/8] Make sure we don't use un-initialized reader when a command has no fields struct. (#20056) * Make sure we don't use un-initialized reader when a command has no fields struct. Fixes https://github.com/project-chip/connectedhomeip/issues/10501 * Address review comments. --- src/app/CommandHandler.cpp | 19 +++++- src/app/tests/TestCommandInteraction.cpp | 87 +++++++++++++++++------- src/lib/core/CHIPTLV.h | 4 +- 3 files changed, 82 insertions(+), 28 deletions(-) diff --git a/src/app/CommandHandler.cpp b/src/app/CommandHandler.cpp index 206cb44718bc6d..95c77f04949466 100644 --- a/src/app/CommandHandler.cpp +++ b/src/app/CommandHandler.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -244,6 +245,18 @@ CHIP_ERROR CommandHandler::SendCommandResponse() return CHIP_NO_ERROR; } +namespace { +// We use this when the sender did not actually provide a CommandFields struct, +// to avoid downstream consumers having to worry about cases when there is or is +// not a struct available. We use an empty struct with anonymous tag, since we +// can't use a context tag at top level, and consumers should not care about the +// tag here). +constexpr uint8_t sNoFields[] = { + CHIP_TLV_STRUCTURE(CHIP_TLV_TAG_ANONYMOUS), + CHIP_TLV_END_OF_CONTAINER, +}; +} // anonymous namespace + CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommandElement) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -308,7 +321,8 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand ChipLogDetail(DataManagement, "Received command without data for Endpoint=%u Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI, concretePath.mEndpointId, ChipLogValueMEI(concretePath.mClusterId), ChipLogValueMEI(concretePath.mCommandId)); - err = CHIP_NO_ERROR; + commandDataReader.Init(sNoFields); + err = commandDataReader.Next(); } if (CHIP_NO_ERROR == err) { @@ -365,7 +379,8 @@ CHIP_ERROR CommandHandler::ProcessGroupCommandDataIB(CommandDataIB::Parser & aCo ChipLogDetail(DataManagement, "Received command without data for Group=%u Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI, groupId, ChipLogValueMEI(clusterId), ChipLogValueMEI(commandId)); - err = CHIP_NO_ERROR; + commandDataReader.Init(sNoFields); + err = commandDataReader.Next(); } SuccessOrExit(err); diff --git a/src/app/tests/TestCommandInteraction.cpp b/src/app/tests/TestCommandInteraction.cpp index 50c219b58e61a7..6b7a089104229c 100644 --- a/src/app/tests/TestCommandInteraction.cpp +++ b/src/app/tests/TestCommandInteraction.cpp @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -57,10 +58,14 @@ bool isCommandDispatched = false; bool sendResponse = true; bool asyncCommand = false; +// Allow us to do test asserts from arbitrary places. +nlTestSuite * gSuite = nullptr; + constexpr EndpointId kTestEndpointId = 1; constexpr ClusterId kTestClusterId = 3; -constexpr CommandId kTestCommandId = 4; -constexpr CommandId kTestCommandIdCommandSpecificResponse = 5; +constexpr CommandId kTestCommandIdWithData = 4; +constexpr CommandId kTestCommandIdNoData = 5; +constexpr CommandId kTestCommandIdCommandSpecificResponse = 6; constexpr CommandId kTestNonExistCommandId = 0; } // namespace @@ -97,6 +102,36 @@ void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, chip ChipLogDetail(Controller, "Received Cluster Command: Endpoint=%x Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI, aCommandPath.mEndpointId, ChipLogValueMEI(aCommandPath.mClusterId), ChipLogValueMEI(aCommandPath.mCommandId)); + // Duplicate what our normal command-field-decode code does, in terms of + // checking for a struct and then entering it before getting the fields. + if (aReader.GetType() != TLV::kTLVType_Structure) + { + apCommandObj->AddStatus(aCommandPath, Protocols::InteractionModel::Status::InvalidAction); + return; + } + + TLV::TLVType outerContainerType; + CHIP_ERROR err = aReader.EnterContainer(outerContainerType); + NL_TEST_ASSERT(gSuite, err == CHIP_NO_ERROR); + + err = aReader.Next(); + if (aCommandPath.mCommandId == kTestCommandIdNoData) + { + NL_TEST_ASSERT(gSuite, err == CHIP_ERROR_END_OF_TLV); + } + else + { + NL_TEST_ASSERT(gSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, aReader.GetTag() == TLV::ContextTag(1)); + bool val; + err = aReader.Get(val); + NL_TEST_ASSERT(gSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(gSuite, val); + } + + err = aReader.ExitContainer(outerContainerType); + NL_TEST_ASSERT(gSuite, err == CHIP_NO_ERROR); + if (asyncCommand) { asyncCommandHandle = apCommandObj; @@ -105,7 +140,7 @@ void DispatchSingleClusterCommand(const ConcreteCommandPath & aCommandPath, chip if (sendResponse) { - if (aCommandPath.mCommandId == kTestCommandId) + if (aCommandPath.mCommandId == kTestCommandIdNoData || aCommandPath.mCommandId == kTestCommandIdWithData) { apCommandObj->AddStatus(aCommandPath, Protocols::InteractionModel::Status::Success); } @@ -200,16 +235,20 @@ class TestCommandInteraction } private: + // Generate an invoke request. If aCommandId is kTestCommandIdWithData, a + // payload will be included. Otherwise no payload will be included. static void GenerateInvokeRequest(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload, - bool aNeedCommandData, bool aIsTimedRequest, EndpointId aEndpointId = kTestEndpointId, - ClusterId aClusterId = kTestClusterId, CommandId aCommandId = kTestCommandId); + bool aIsTimedRequest, CommandId aCommandId, ClusterId aClusterId = kTestClusterId, + EndpointId aEndpointId = kTestEndpointId); + // Generate an invoke response. If aCommandId is kTestCommandIdWithData, a + // payload will be included. Otherwise no payload will be included. static void GenerateInvokeResponse(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload, - bool aNeedCommandData, EndpointId aEndpointId = kTestEndpointId, - ClusterId aClusterId = kTestClusterId, CommandId aCommandId = kTestCommandId); + CommandId aCommandId, ClusterId aClusterId = kTestClusterId, + EndpointId aEndpointId = kTestEndpointId); static void AddInvokeRequestData(nlTestSuite * apSuite, void * apContext, CommandSender * apCommandSender, - CommandId aCommandId = kTestCommandId); + CommandId aCommandId = kTestCommandIdWithData); static void AddInvokeResponseData(nlTestSuite * apSuite, void * apContext, CommandHandler * apCommandHandler, - bool aNeedStatusCode, CommandId aCommandId = kTestCommandId); + bool aNeedStatusCode, CommandId aCommandId = kTestCommandIdWithData); static void ValidateCommandHandlerWithSendCommand(nlTestSuite * apSuite, void * apContext, bool aNeedStatusCode); }; @@ -224,14 +263,14 @@ class TestExchangeDelegate : public Messaging::ExchangeDelegate void OnResponseTimeout(Messaging::ExchangeContext * ec) override {} }; -CommandPathParams MakeTestCommandPath(CommandId aCommandId = kTestCommandId) +CommandPathParams MakeTestCommandPath(CommandId aCommandId = kTestCommandIdWithData) { return CommandPathParams(kTestEndpointId, 0, kTestClusterId, aCommandId, (chip::app::CommandPathFlags::kEndpointIdValid)); } void TestCommandInteraction::GenerateInvokeRequest(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload, - bool aNeedCommandData, bool aIsTimedRequest, EndpointId aEndpointId, - ClusterId aClusterId, CommandId aCommandId) + bool aIsTimedRequest, CommandId aCommandId, ClusterId aClusterId, + EndpointId aEndpointId) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -255,7 +294,7 @@ void TestCommandInteraction::GenerateInvokeRequest(nlTestSuite * apSuite, void * commandPathBuilder.EndpointId(aEndpointId).ClusterId(aClusterId).CommandId(aCommandId).EndOfCommandPathIB(); NL_TEST_ASSERT(apSuite, commandPathBuilder.GetError() == CHIP_NO_ERROR); - if (aNeedCommandData) + if (aCommandId == kTestCommandIdWithData) { chip::TLV::TLVWriter * pWriter = commandDataIBBuilder.GetWriter(); chip::TLV::TLVType dummyType = chip::TLV::kTLVType_NotSpecified; @@ -284,8 +323,7 @@ void TestCommandInteraction::GenerateInvokeRequest(nlTestSuite * apSuite, void * } void TestCommandInteraction::GenerateInvokeResponse(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload, - bool aNeedCommandData, EndpointId aEndpointId, ClusterId aClusterId, - CommandId aCommandId) + CommandId aCommandId, ClusterId aClusterId, EndpointId aEndpointId) { CHIP_ERROR err = CHIP_NO_ERROR; @@ -312,7 +350,7 @@ void TestCommandInteraction::GenerateInvokeResponse(nlTestSuite * apSuite, void commandPathBuilder.EndpointId(aEndpointId).ClusterId(aClusterId).CommandId(aCommandId).EndOfCommandPathIB(); NL_TEST_ASSERT(apSuite, commandPathBuilder.GetError() == CHIP_NO_ERROR); - if (aNeedCommandData) + if (aCommandId == kTestCommandIdWithData) { chip::TLV::TLVWriter * pWriter = commandDataIBBuilder.GetWriter(); chip::TLV::TLVType dummyType = chip::TLV::kTLVType_NotSpecified; @@ -406,7 +444,7 @@ void TestCommandInteraction::TestCommandHandlerWithWrongState(nlTestSuite * apSu { TestContext & ctx = *static_cast(apContext); CHIP_ERROR err = CHIP_NO_ERROR; - ConcreteCommandPath path = { kTestEndpointId, kTestClusterId, kTestCommandId }; + ConcreteCommandPath path = { kTestEndpointId, kTestClusterId, kTestCommandIdNoData }; app::CommandHandler commandHandler(&mockCommandHandlerDelegate); @@ -435,7 +473,7 @@ void TestCommandInteraction::TestCommandSenderWithSendCommand(nlTestSuite * apSu ctx.DrainAndServiceIO(); - GenerateInvokeResponse(apSuite, apContext, buf, true /*aNeedCommandData*/); + GenerateInvokeResponse(apSuite, apContext, buf, kTestCommandIdWithData); err = commandSender.ProcessInvokeResponse(std::move(buf)); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); } @@ -444,7 +482,7 @@ void TestCommandInteraction::TestCommandHandlerWithSendEmptyCommand(nlTestSuite { TestContext & ctx = *static_cast(apContext); CHIP_ERROR err = CHIP_NO_ERROR; - ConcreteCommandPath path = { kTestEndpointId, kTestClusterId, kTestCommandId }; + ConcreteCommandPath path = { kTestEndpointId, kTestClusterId, kTestCommandIdNoData }; app::CommandHandler commandHandler(&mockCommandHandlerDelegate); System::PacketBufferHandle commandDatabuf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize); @@ -470,7 +508,7 @@ void TestCommandInteraction::TestCommandSenderWithProcessReceivedMsg(nlTestSuite System::PacketBufferHandle buf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize); - GenerateInvokeResponse(apSuite, apContext, buf, true /*aNeedCommandData*/); + GenerateInvokeResponse(apSuite, apContext, buf, kTestCommandIdWithData); err = commandSender.ProcessInvokeResponse(std::move(buf)); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); } @@ -636,7 +674,7 @@ void TestCommandInteraction::TestCommandHandlerWithProcessReceivedMsg(nlTestSuit TestExchangeDelegate delegate; commandHandler.mpExchangeCtx = ctx.NewExchangeToAlice(&delegate); - GenerateInvokeRequest(apSuite, apContext, commandDatabuf, true /*aNeedCommandData*/, /* aIsTimedRequest = */ false); + GenerateInvokeRequest(apSuite, apContext, commandDatabuf, /* aIsTimedRequest = */ false, kTestCommandIdWithData); err = commandHandler.ProcessInvokeRequest(std::move(commandDatabuf), false); ChipLogDetail(DataManagement, "###################################### %s", err.AsString()); @@ -650,8 +688,8 @@ void TestCommandInteraction::TestCommandHandlerWithProcessReceivedNotExistComman System::PacketBufferHandle commandDatabuf = System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize); // Use some invalid endpoint / cluster / command. - GenerateInvokeRequest(apSuite, apContext, commandDatabuf, false /*aNeedCommandData*/, /* aIsTimedRequest = */ false, - 0xDE /* endpoint */, 0xADBE /* cluster */, 0xEF /* command */); + GenerateInvokeRequest(apSuite, apContext, commandDatabuf, /* aIsTimedRequest = */ false, 0xEF /* command */, + 0xADBE /* cluster */, 0xDE /* endpoint */); // TODO: Need to find a way to get the response instead of only check if a function on key path is called. // We should not reach CommandDispatch if requested command does not exist. @@ -676,7 +714,7 @@ void TestCommandInteraction::TestCommandHandlerWithProcessReceivedEmptyDataMsg(n commandHandler.mpExchangeCtx = ctx.NewExchangeToAlice(&delegate); chip::isCommandDispatched = false; - GenerateInvokeRequest(apSuite, apContext, commandDatabuf, false /*aNeedCommandData*/, messageIsTimed); + GenerateInvokeRequest(apSuite, apContext, commandDatabuf, messageIsTimed, kTestCommandIdNoData); err = commandHandler.ProcessInvokeRequest(std::move(commandDatabuf), transactionIsTimed); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); NL_TEST_ASSERT(apSuite, chip::isCommandDispatched == (messageIsTimed == transactionIsTimed)); @@ -934,6 +972,7 @@ nlTestSuite sSuite = int TestCommandInteraction() { + chip::gSuite = &sSuite; return chip::ExecuteTestsWithContext(&sSuite); } diff --git a/src/lib/core/CHIPTLV.h b/src/lib/core/CHIPTLV.h index 8a96f45ce1069f..3ab437560273d5 100644 --- a/src/lib/core/CHIPTLV.h +++ b/src/lib/core/CHIPTLV.h @@ -56,12 +56,12 @@ namespace chip { namespace TLV { -inline uint8_t operator|(TLVElementType lhs, TLVTagControl rhs) +constexpr inline uint8_t operator|(TLVElementType lhs, TLVTagControl rhs) { return static_cast(lhs) | static_cast(rhs); } -inline uint8_t operator|(TLVTagControl lhs, TLVElementType rhs) +constexpr inline uint8_t operator|(TLVTagControl lhs, TLVElementType rhs) { return static_cast(lhs) | static_cast(rhs); }