From 3be848d2374bfedaccc286eff3d1a80acfcaac23 Mon Sep 17 00:00:00 2001 From: Shubham Patil Date: Tue, 30 Aug 2022 21:33:43 +0530 Subject: [PATCH 1/7] [ESP32] Remove the unused files for make build system (#22253) Since ESP-IDF V4.0, the default build system is based on CMake and will be removed in future idf releases. Also, none of the other components or examples uses them so removing component.mk --- config/esp32/components/chip/component.mk | 167 ------------------ .../components/esp32_mbedtls/component.mk | 1 - 2 files changed, 168 deletions(-) delete mode 100644 config/esp32/components/chip/component.mk delete mode 100644 config/esp32/components/esp32_mbedtls/component.mk diff --git a/config/esp32/components/chip/component.mk b/config/esp32/components/chip/component.mk deleted file mode 100644 index 61ab9100fa78e7..00000000000000 --- a/config/esp32/components/chip/component.mk +++ /dev/null @@ -1,167 +0,0 @@ -# -# Copyright (c) 2020 Project CHIP Authors -# Copyright (c) 2018 Nest Labs, Inc. -# All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# -# Description: -# Component makefile for building CHIP within the ESP32 ESP-IDF environment. -# - -# ================================================== -# General settings -# ================================================== - -SHELL = /bin/bash - -# CHIP source root directory -CHIP_ROOT ?= $(realpath $(COMPONENT_PATH)/../../../..) - -# Directory into which the CHIP build system will place its output. -OUTPUT_DIR := $(BUILD_DIR_BASE)/chip -REL_OUTPUT_DIR := $(shell perl -e 'use File::Spec; use Cwd; print File::Spec->abs2rel(Cwd::realpath($$ARGV[0]), Cwd::realpath($$ARGV[1])) . "\n"' $(OUTPUT_DIR) $(COMPONENT_PATH)) - -REL_CHIP_ROOT := $(shell perl -e 'use File::Spec; use Cwd; print File::Spec->abs2rel(Cwd::realpath($$ARGV[0]), Cwd::realpath($$ARGV[1])) . "\n"' $(CHIP_ROOT) $(COMPONENT_PATH)) - -COMPONENT_LIBRARIES := - - -# ================================================== -# Compilation flags specific to building CHIP -# ================================================== - -# Include directories to be searched when building CHIP. Make sure -# that anything starting with $(IDF_PATH) ends up being included with -# -isystem, not -I, so warnings in those headers don't cause the build -# to fail. -INCLUDES := $(OUTPUT_DIR)/src/include \ - $(OUTPUT_DIR)/src/include/platform/ESP32 \ - $(filter-out $(IDF_PATH)/%, $(COMPONENT_INCLUDES)) - -SYSTEM_INCLUDES := $(IDF_PATH)/components/lwip/lwip/src/include \ - $(IDF_PATH)/components/freertos/include/freertos/ \ - $(IDF_PATH)/components/mbedtls/mbedtls/include \ - $(filter $(IDF_PATH)/%, $(COMPONENT_INCLUDES)) - - -# Compiler flags for building CHIP -ALL_INCLUDES := $(addprefix -I,$(INCLUDES)) $(addprefix -isystem,$(SYSTEM_INCLUDES)) -CFLAGS += $(ALL_INCLUDES) -CPPFLAGS += $(ALL_INCLUDES) -CXXFLAGS += $(ALL_INCLUDES) - - -# ================================================== -# Configuration for the CHIP ESF-IDF Component -# ================================================== - -# Header directories to be included when building other components that use CHIP. -# Note that these must be relative to the component source directory. -# TODO Boot the CHIP_ROOT includedirs -COMPONENT_ADD_INCLUDEDIRS = project-config \ - $(REL_OUTPUT_DIR)/include \ - $(REL_CHIP_ROOT)/src/include/platform/ESP32 \ - $(REL_CHIP_ROOT)/src/include/ \ - $(REL_CHIP_ROOT)/src/lib \ - $(REL_CHIP_ROOT)/src/ \ - $(REL_CHIP_ROOT)/src/system \ - $(IDF_PATH)/components/mbedtls/mbedtls/include \ - $(REL_CHIP_ROOT)/src/app - -# Linker flags to be included when building other components that use CHIP. -COMPONENT_ADD_LDFLAGS = -L$(OUTPUT_DIR)/lib/ \ - -lCHIP - -ifdef CONFIG_ENABLE_CHIP_SHELL -COMPONENT_ADD_LDFLAGS += -lCHIPShell -endif - -ifdef CONFIG_ENABLE_PW_RPC -COMPONENT_ADD_LDFLAGS += -lPwRpc -endif - -COMPONENT_ADD_INCLUDEDIRS += $(REL_OUTPUT_DIR)/src/include \ - $(REL_CHIP_ROOT)/third_party/nlassert/repo/include \ - $(REL_OUTPUT_DIR)/gen/third_party/connectedhomeip/src/app/include \ - $(REL_OUTPUT_DIR)/gen/include \ - $(REL_CHIP_ROOT)/zzz_generated/app-common - -# Tell the ESP-IDF build system that the CHIP component defines its own build -# and clean targets. -COMPONENT_OWNBUILDTARGET := chip_build -COMPONENT_OWNCLEANTARGET := chip_clean - -is_debug ?= true - -# ================================================== -# Build Rules -# ================================================== - -$(OUTPUT_DIR) : - echo "MKDIR $@" - @mkdir -p "$@" - - -fix_cflags = $(filter-out -DHAVE_CONFIG_H,\ - $(filter-out -D,\ - $(filter-out IDF_VER%,\ - $(1) -D$(filter IDF_VER%,$(1))\ - ))) -CHIP_CFLAGS = $(call fix_cflags,$(CFLAGS) $(CPPFLAGS)) -CHIP_CXXFLAGS = $(call fix_cflags,$(CXXFLAGS) $(CPPFLAGS)) - -install-chip : $(OUTPUT_DIR) - echo "INSTALL CHIP..." - echo > $(OUTPUT_DIR)/args.gn - echo "import(\"//args.gni\")" >> $(OUTPUT_DIR)/args.gn - echo target_cflags_c = [$(foreach word,$(CHIP_CFLAGS),\"$(word)\",)] | sed -e 's/=\"/=\\"/g;s/\"\"/\\"\"/g;' >> $(OUTPUT_DIR)/args.gn - echo target_cflags_cc = [$(foreach word,$(CHIP_CXXFLAGS),\"$(word)\",)] | sed -e 's/=\"/=\\"/g;s/\"\"/\\"\"/g;' >> $(OUTPUT_DIR)/args.gn - echo esp32_ar = \"$(AR)\" >> $(OUTPUT_DIR)/args.gn - echo esp32_cc = \"$(CC)\" >> $(OUTPUT_DIR)/args.gn - echo esp32_cxx = \"$(CXX)\" >> $(OUTPUT_DIR)/args.gn - echo esp32_cpu = \"esp32\" >> $(OUTPUT_DIR)/args.gn -ifeq ($(is_debug),false) - @echo "is_debug = false" >> $(OUTPUT_DIR)/args.gn -endif - if [[ "$(CONFIG_ENABLE_PW_RPC)" = "y" ]]; then \ - echo "chip_build_pw_rpc_lib = true" >> $(OUTPUT_DIR)/args.gn ;\ - echo "chip_build_pw_trace_lib = true" >> $(OUTPUT_DIR)/args.gn ;\ - echo "remove_default_configs = [\"//third_party/connectedhomeip/third_party/pigweed/repo/pw_build:cpp17\"]" >> $(OUTPUT_DIR)/args.gn ;\ - echo "pw_log_BACKEND = \"//third_party/connectedhomeip/third_party/pigweed/repo/pw_log_basic\"" >> $(OUTPUT_DIR)/args.gn ;\ - echo "pw_assert_BACKEND = \"//third_party/connectedhomeip/third_party/pigweed/repo/pw_assert_log\"" >> $(OUTPUT_DIR)/args.gn ;\ - echo "pw_sys_io_BACKEND = \"//third_party/connectedhomeip/examples/platform/esp32/pw_sys_io:pw_sys_io_esp32\"" >> $(OUTPUT_DIR)/args.gn ;\ - echo "pw_trace_BACKEND = \"//third_party/connectedhomeip/third_party/pigweed/repo/pw_trace_tokenized\"" >> $(OUTPUT_DIR)/args.gn ;\ - echo "dir_pw_third_party_nanopb = \"//third_party/connectedhomeip/third_party/nanopb/repo\"" >>$(OUTPUT_DIR)/args.gn ;\ - fi - if [[ "$(CONFIG_ENABLE_CHIP_SHELL)" = "y" ]]; then \ - echo "chip_build_libshell = true" >> $(OUTPUT_DIR)/args.gn ;\ - fi - if [[ "$(CONFIG_USE_MINIMAL_MDNS)" = "n" ]]; then \ - echo "chip_mdns = platform" >> $(OUTPUT_DIR)/args.gn ;\ - fi - if [[ "$(CONFIG_DISABLE_IPV4)" = "y" ]]; then \ - echo "chip_inet_config_enable_ipv4 = false" >> $(OUTPUT_DIR)/args.gn ;\ - fi - echo "Written file $(OUTPUT_DIR)/args.gn" - cd $(CHIP_ROOT) && PW_ENVSETUP_QUIET=1 . scripts/activate.sh && cd $(COMPONENT_PATH) && gn gen --check --fail-on-unused-args $(OUTPUT_DIR) - cd $(COMPONENT_PATH); ninja $(subst 1,-v,$(filter 1,$(V))) -C $(OUTPUT_DIR) esp32 - - -chip_build : install-chip - echo "CHIP built and installed..." - cp -a ${OUTPUT_DIR}/lib/libCHIP.a ${OUTPUT_DIR}/libchip.a - -chip_clean: - echo "RM $(OUTPUT_DIR)" - rm -rf $(OUTPUT_DIR) diff --git a/config/esp32/components/esp32_mbedtls/component.mk b/config/esp32/components/esp32_mbedtls/component.mk deleted file mode 100644 index a6290b41f3a6c1..00000000000000 --- a/config/esp32/components/esp32_mbedtls/component.mk +++ /dev/null @@ -1 +0,0 @@ -COMPONENT_DEPENDS := chip From 132be6e370e7c59d27aecd8727f8eeb37d9b0f05 Mon Sep 17 00:00:00 2001 From: Timothy Maes Date: Tue, 30 Aug 2022 20:22:34 +0200 Subject: [PATCH 2/7] Re-enable bloat reporting for QPG builds (#22221) --- .github/workflows/examples-qpg.yaml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.github/workflows/examples-qpg.yaml b/.github/workflows/examples-qpg.yaml index d43855ab74c4a2..46d84ce5a538d9 100644 --- a/.github/workflows/examples-qpg.yaml +++ b/.github/workflows/examples-qpg.yaml @@ -81,6 +81,17 @@ jobs: run: | config/qpg/chip-gn/build.sh + - name: Prepare some bloat report from the previous builds + run: | + .environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \ + qpg qpg6105+debug lighting-app \ + out/qpg-light/chip-qpg6105-lighting-example.out \ + /tmp/bloat_reports/ + .environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \ + qpg qpg6105+debug lock-app \ + out/qpg-lock/chip-qpg6105-lock-example.out \ + /tmp/bloat_reports/ + - name: Uploading Size Reports uses: actions/upload-artifact@v2 if: ${{ !env.ACT }} From bb1ebbab66759e3d3d0b70852e7c0d601dfd471d Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 30 Aug 2022 14:25:15 -0400 Subject: [PATCH 3/7] Don't notify OnSessionEstablishmentError after OnSessionEstablished. (#22261) A PairingSession that has sent an OnSessionEstablished notification should not send an OnSessionEstablishmentError notification after that (e.g. if the session gets evicted). The session is established at that point, and the _establishment_ cannot hit an error. This is needed to allow PASE sessions to be sanely evicted (e.g. when we're done with them) without triggering spurious errors. --- src/protocols/secure_channel/CASESession.cpp | 4 ++-- src/protocols/secure_channel/PASESession.cpp | 6 +++--- .../secure_channel/PairingSession.cpp | 21 +++++++++++++++++-- src/protocols/secure_channel/PairingSession.h | 6 ++++++ .../SessionEstablishmentDelegate.h | 8 +++++-- .../secure_channel/tests/TestPASESession.cpp | 17 +++++++++++++++ 6 files changed, 53 insertions(+), 9 deletions(-) diff --git a/src/protocols/secure_channel/CASESession.cpp b/src/protocols/secure_channel/CASESession.cpp index 5f4c04752fe389..623265d160349a 100644 --- a/src/protocols/secure_channel/CASESession.cpp +++ b/src/protocols/secure_channel/CASESession.cpp @@ -131,7 +131,7 @@ void CASESession::OnSessionReleased() { Clear(); // Do this last in case the delegate frees us. - mDelegate->OnSessionEstablishmentError(CHIP_ERROR_CONNECTION_ABORTED); + NotifySessionEstablishmentError(CHIP_ERROR_CONNECTION_ABORTED); } void CASESession::Clear() @@ -294,7 +294,7 @@ void CASESession::AbortPendingEstablish(CHIP_ERROR err) { Clear(); // Do this last in case the delegate frees us. - mDelegate->OnSessionEstablishmentError(err); + NotifySessionEstablishmentError(err); } CHIP_ERROR CASESession::DeriveSecureSession(CryptoContext & session) const diff --git a/src/protocols/secure_channel/PASESession.cpp b/src/protocols/secure_channel/PASESession.cpp index 6c48957a14d275..05fc3b38915983 100644 --- a/src/protocols/secure_channel/PASESession.cpp +++ b/src/protocols/secure_channel/PASESession.cpp @@ -72,7 +72,7 @@ void PASESession::OnSessionReleased() { Clear(); // Do this last in case the delegate frees us. - mDelegate->OnSessionEstablishmentError(CHIP_ERROR_CONNECTION_ABORTED); + NotifySessionEstablishmentError(CHIP_ERROR_CONNECTION_ABORTED); } void PASESession::Finish() @@ -242,7 +242,7 @@ void PASESession::OnResponseTimeout(ExchangeContext * ec) DiscardExchange(); Clear(); // Do this last in case the delegate frees us. - mDelegate->OnSessionEstablishmentError(CHIP_ERROR_TIMEOUT); + NotifySessionEstablishmentError(CHIP_ERROR_TIMEOUT); } CHIP_ERROR PASESession::DeriveSecureSession(CryptoContext & session) const @@ -859,7 +859,7 @@ CHIP_ERROR PASESession::OnMessageReceived(ExchangeContext * exchange, const Payl Clear(); ChipLogError(SecureChannel, "Failed during PASE session setup: %" CHIP_ERROR_FORMAT, err.Format()); // Do this last in case the delegate frees us. - mDelegate->OnSessionEstablishmentError(err); + NotifySessionEstablishmentError(err); } return err; } diff --git a/src/protocols/secure_channel/PairingSession.cpp b/src/protocols/secure_channel/PairingSession.cpp index 8c1dee5968ed4a..67d7229b462709 100644 --- a/src/protocols/secure_channel/PairingSession.cpp +++ b/src/protocols/secure_channel/PairingSession.cpp @@ -63,11 +63,15 @@ void PairingSession::Finish() if (err == CHIP_NO_ERROR) { VerifyOrDie(mSecureSessionHolder); - mDelegate->OnSessionEstablished(mSecureSessionHolder.Get().Value()); + // Make sure to null out mDelegate so we don't send it any other + // notifications. + auto * delegate = mDelegate; + mDelegate = nullptr; + delegate->OnSessionEstablished(mSecureSessionHolder.Get().Value()); } else { - mDelegate->OnSessionEstablishmentError(err); + NotifySessionEstablishmentError(err); } } @@ -165,4 +169,17 @@ void PairingSession::Clear() mSessionManager = nullptr; } +void PairingSession::NotifySessionEstablishmentError(CHIP_ERROR error) +{ + if (mDelegate == nullptr) + { + // Already notified success or error. + return; + } + + auto * delegate = mDelegate; + mDelegate = nullptr; + delegate->OnSessionEstablishmentError(error); +} + } // namespace chip diff --git a/src/protocols/secure_channel/PairingSession.h b/src/protocols/secure_channel/PairingSession.h index f8a707aa3af2d1..e220346e11b4a6 100644 --- a/src/protocols/secure_channel/PairingSession.h +++ b/src/protocols/secure_channel/PairingSession.h @@ -192,6 +192,12 @@ class DLL_EXPORT PairingSession : public SessionDelegate // TODO: remove Clear, we should create a new instance instead reset the old instance. void Clear(); + /** + * Notify our delegate about a session establishment error, if we have not + * notified it of an error or success before. + */ + void NotifySessionEstablishmentError(CHIP_ERROR error); + protected: CryptoContext::SessionRole mRole; SessionHolderWithDelegate mSecureSessionHolder; diff --git a/src/protocols/secure_channel/SessionEstablishmentDelegate.h b/src/protocols/secure_channel/SessionEstablishmentDelegate.h index f14566dae5a092..30cc8cac59f76b 100644 --- a/src/protocols/secure_channel/SessionEstablishmentDelegate.h +++ b/src/protocols/secure_channel/SessionEstablishmentDelegate.h @@ -36,7 +36,9 @@ class DLL_EXPORT SessionEstablishmentDelegate { public: /** - * Called when session establishment fails with an error + * Called when session establishment fails with an error. This will be + * called at most once per session establishment and will not be called if + * OnSessionEstablished is called. */ virtual void OnSessionEstablishmentError(CHIP_ERROR error) {} @@ -46,7 +48,9 @@ class DLL_EXPORT SessionEstablishmentDelegate virtual void OnSessionEstablishmentStarted() {} /** - * Called when the new secure session has been established + * Called when the new secure session has been established. This is + * mututally exclusive with OnSessionEstablishmentError for a give session + * establishment. */ virtual void OnSessionEstablished(const SessionHandle & session) {} diff --git a/src/protocols/secure_channel/tests/TestPASESession.cpp b/src/protocols/secure_channel/tests/TestPASESession.cpp index 61830fa174b530..ab97624e39e473 100644 --- a/src/protocols/secure_channel/tests/TestPASESession.cpp +++ b/src/protocols/secure_channel/tests/TestPASESession.cpp @@ -290,7 +290,9 @@ void SecurePairingHandshakeTestCommon(nlTestSuite * inSuite, void * inContext, S // Let's make sure atleast number is >= than the minimum messages required to complete the // handshake. NL_TEST_ASSERT(inSuite, loopback.mSentMessageCount >= sTestPaseMessageCount); + NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingErrors == 0); NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingComplete == 1); + NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingErrors == 0); NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingComplete == 1); if (mrpCommissionerConfig.HasValue()) @@ -313,6 +315,21 @@ void SecurePairingHandshakeTestCommon(nlTestSuite * inSuite, void * inContext, S mrpAccessoryConfig.Value().mActiveRetransTimeout); } + // Now evict the PASE sessions. + auto session = pairingCommissioner.CopySecureSession(); + NL_TEST_ASSERT(inSuite, session.HasValue()); + session.Value()->AsSecureSession()->MarkForEviction(); + + session = pairingAccessory.CopySecureSession(); + NL_TEST_ASSERT(inSuite, session.HasValue()); + session.Value()->AsSecureSession()->MarkForEviction(); + + // And check that this did not result in any new notifications. + NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingErrors == 0); + NL_TEST_ASSERT(inSuite, delegateAccessory.mNumPairingComplete == 1); + NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingErrors == 0); + NL_TEST_ASSERT(inSuite, delegateCommissioner.mNumPairingComplete == 1); + loopback.SetLoopbackTransportDelegate(nullptr); } From 8a0aad177fc127b6fc4d02b3954b1312d902e8a5 Mon Sep 17 00:00:00 2001 From: adabreuti <76965454+adabreuti@users.noreply.github.com> Date: Tue, 30 Aug 2022 13:37:49 -0500 Subject: [PATCH 4/7] Add DevinfoInit to Relevant CC13xx apps (#22184) * Add DevinfoInit to CC13xx all clusters * Add Devinfo init to lock-app * Update style --- examples/all-clusters-app/cc13x2x7_26x2x7/BUILD.gn | 2 ++ examples/all-clusters-app/cc13x2x7_26x2x7/main/AppTask.cpp | 7 +++++++ examples/all-clusters-minimal-app/cc13x2x7_26x2x7/BUILD.gn | 2 ++ .../cc13x2x7_26x2x7/main/AppTask.cpp | 7 +++++++ examples/lock-app/cc13x2x7_26x2x7/BUILD.gn | 2 ++ examples/lock-app/cc13x2x7_26x2x7/main/AppTask.cpp | 7 +++++++ 6 files changed, 27 insertions(+) diff --git a/examples/all-clusters-app/cc13x2x7_26x2x7/BUILD.gn b/examples/all-clusters-app/cc13x2x7_26x2x7/BUILD.gn index 48f3ef5b7583f9..3194d47bb0f188 100644 --- a/examples/all-clusters-app/cc13x2x7_26x2x7/BUILD.gn +++ b/examples/all-clusters-app/cc13x2x7_26x2x7/BUILD.gn @@ -78,6 +78,7 @@ ti_simplelink_executable("all-clusters-app") { "${chip_root}/examples/all-clusters-app/all-clusters-common/src/binding-handler.cpp", "${chip_root}/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp", "${chip_root}/examples/all-clusters-app/all-clusters-common/src/static-supported-modes-manager.cpp", + "${chip_root}/examples/providers/DeviceInfoProviderImpl.cpp", "${project_dir}/main/AppTask.cpp", "${project_dir}/main/ClusterManager.cpp", "${project_dir}/main/Globals.cpp", @@ -102,6 +103,7 @@ ti_simplelink_executable("all-clusters-app") { "${project_dir}", "${project_dir}/main", "${chip_root}/examples/all-clusters-app/all-clusters-common/include", + "${chip_root}/examples/providers/", ] cflags = [ diff --git a/examples/all-clusters-app/cc13x2x7_26x2x7/main/AppTask.cpp b/examples/all-clusters-app/cc13x2x7_26x2x7/main/AppTask.cpp index b00e11a766a6da..09762791f4cf1a 100644 --- a/examples/all-clusters-app/cc13x2x7_26x2x7/main/AppTask.cpp +++ b/examples/all-clusters-app/cc13x2x7_26x2x7/main/AppTask.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #if CHIP_DEVICE_CONFIG_ENABLE_OTA_REQUESTOR @@ -64,6 +65,7 @@ static QueueHandle_t sAppEventQueue; static Button_Handle sAppLeftHandle; static Button_Handle sAppRightHandle; +static DeviceInfoProviderImpl sExampleDeviceInfoProvider; AppTask AppTask::sAppTask; @@ -243,6 +245,11 @@ int AppTask::Init() PLAT_LOG("Initialize Server"); static chip::CommonCaseDeviceServerInitParams initParams; (void) initParams.InitializeStaticResourcesBeforeServerInit(); + + // Initialize info provider + sExampleDeviceInfoProvider.SetStorageDelegate(initParams.persistentStorageDelegate); + SetDeviceInfoProvider(&sExampleDeviceInfoProvider); + chip::Server::GetInstance().Init(initParams); ConfigurationMgr().LogDeviceConfig(); diff --git a/examples/all-clusters-minimal-app/cc13x2x7_26x2x7/BUILD.gn b/examples/all-clusters-minimal-app/cc13x2x7_26x2x7/BUILD.gn index 9c870ec32fa84e..6c20ed346ec530 100644 --- a/examples/all-clusters-minimal-app/cc13x2x7_26x2x7/BUILD.gn +++ b/examples/all-clusters-minimal-app/cc13x2x7_26x2x7/BUILD.gn @@ -78,6 +78,7 @@ ti_simplelink_executable("all-clusters-minimal-app") { "${chip_root}/examples/all-clusters-app/all-clusters-common/src/binding-handler.cpp", "${chip_root}/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp", "${chip_root}/examples/all-clusters-app/all-clusters-common/src/static-supported-modes-manager.cpp", + "${chip_root}/examples/providers/DeviceInfoProviderImpl.cpp", "${project_dir}/main/AppTask.cpp", "${project_dir}/main/ClusterManager.cpp", "${project_dir}/main/Globals.cpp", @@ -102,6 +103,7 @@ ti_simplelink_executable("all-clusters-minimal-app") { "${project_dir}", "${project_dir}/main", "${chip_root}/examples/all-clusters-app/all-clusters-common/include", + "${chip_root}/examples/providers/", ] cflags = [ diff --git a/examples/all-clusters-minimal-app/cc13x2x7_26x2x7/main/AppTask.cpp b/examples/all-clusters-minimal-app/cc13x2x7_26x2x7/main/AppTask.cpp index b00e11a766a6da..09762791f4cf1a 100644 --- a/examples/all-clusters-minimal-app/cc13x2x7_26x2x7/main/AppTask.cpp +++ b/examples/all-clusters-minimal-app/cc13x2x7_26x2x7/main/AppTask.cpp @@ -28,6 +28,7 @@ #include #include +#include #include #if CHIP_DEVICE_CONFIG_ENABLE_OTA_REQUESTOR @@ -64,6 +65,7 @@ static QueueHandle_t sAppEventQueue; static Button_Handle sAppLeftHandle; static Button_Handle sAppRightHandle; +static DeviceInfoProviderImpl sExampleDeviceInfoProvider; AppTask AppTask::sAppTask; @@ -243,6 +245,11 @@ int AppTask::Init() PLAT_LOG("Initialize Server"); static chip::CommonCaseDeviceServerInitParams initParams; (void) initParams.InitializeStaticResourcesBeforeServerInit(); + + // Initialize info provider + sExampleDeviceInfoProvider.SetStorageDelegate(initParams.persistentStorageDelegate); + SetDeviceInfoProvider(&sExampleDeviceInfoProvider); + chip::Server::GetInstance().Init(initParams); ConfigurationMgr().LogDeviceConfig(); diff --git a/examples/lock-app/cc13x2x7_26x2x7/BUILD.gn b/examples/lock-app/cc13x2x7_26x2x7/BUILD.gn index 57792e12f69e0c..ba2b327339e516 100644 --- a/examples/lock-app/cc13x2x7_26x2x7/BUILD.gn +++ b/examples/lock-app/cc13x2x7_26x2x7/BUILD.gn @@ -74,6 +74,7 @@ ti_simplelink_executable("lock_app") { output_name = "chip-${ti_simplelink_board}-lock-example.out" sources = [ + "${chip_root}/examples/providers/DeviceInfoProviderImpl.cpp", "${project_dir}/main/AppTask.cpp", "${project_dir}/main/BoltLockManager.cpp", "${project_dir}/main/ZclCallbacks.cpp", @@ -96,6 +97,7 @@ ti_simplelink_executable("lock_app") { include_dirs = [ "${project_dir}", "${project_dir}/main", + "${chip_root}/examples/providers/", ] cflags = [ diff --git a/examples/lock-app/cc13x2x7_26x2x7/main/AppTask.cpp b/examples/lock-app/cc13x2x7_26x2x7/main/AppTask.cpp index a0d30130644e43..4854eb4c5af0d8 100644 --- a/examples/lock-app/cc13x2x7_26x2x7/main/AppTask.cpp +++ b/examples/lock-app/cc13x2x7_26x2x7/main/AppTask.cpp @@ -27,6 +27,7 @@ #include #include +#include #include #if CHIP_DEVICE_CONFIG_ENABLE_OTA_REQUESTOR @@ -63,6 +64,7 @@ static LED_Handle sAppRedHandle; static LED_Handle sAppGreenHandle; static Button_Handle sAppLeftHandle; static Button_Handle sAppRightHandle; +static DeviceInfoProviderImpl sExampleDeviceInfoProvider; AppTask AppTask::sAppTask; @@ -166,6 +168,11 @@ int AppTask::Init() PLAT_LOG("Initialize Server"); static chip::CommonCaseDeviceServerInitParams initParams; (void) initParams.InitializeStaticResourcesBeforeServerInit(); + + // Initialize info provider + sExampleDeviceInfoProvider.SetStorageDelegate(initParams.persistentStorageDelegate); + SetDeviceInfoProvider(&sExampleDeviceInfoProvider); + chip::Server::GetInstance().Init(initParams); // Initialize device attestation config From 70143fe152a5dc97d6544315daa018cfe5fc6e77 Mon Sep 17 00:00:00 2001 From: chrisdecenzo <61757564+chrisdecenzo@users.noreply.github.com> Date: Tue, 30 Aug 2022 11:38:45 -0700 Subject: [PATCH 5/7] Fixes related to PASE-only commissioning on Android (#22140) * Draft: fixes related to PASE-only commissioning on Android * fix build * avoid null pointer in cleanup --- src/controller/AutoCommissioner.cpp | 18 +++++++++++++ src/controller/CHIPDeviceController.cpp | 1 - src/controller/CommissioningDelegate.cpp | 4 +++ src/controller/CommissioningDelegate.h | 10 ++++++++ .../java/AndroidDeviceControllerWrapper.cpp | 3 ++- .../java/AndroidDeviceControllerWrapper.h | 3 ++- .../java/CHIPDeviceController-JNI.cpp | 15 ++++++++--- .../devicecontroller/ControllerParams.java | 25 +++++++++++++++++++ 8 files changed, 72 insertions(+), 7 deletions(-) diff --git a/src/controller/AutoCommissioner.cpp b/src/controller/AutoCommissioner.cpp index e9cc301f45b904..b7d7f0c70755f4 100644 --- a/src/controller/AutoCommissioner.cpp +++ b/src/controller/AutoCommissioner.cpp @@ -139,6 +139,12 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam } mParams.SetCSRNonce(ByteSpan(mCSRNonce, sizeof(mCSRNonce))); + if (params.GetSkipCommissioningComplete().HasValue()) + { + ChipLogProgress(Controller, "Setting PASE-only commissioning from parameters"); + mParams.SetSkipCommissioningComplete(params.GetSkipCommissioningComplete().Value()); + } + return CHIP_NO_ERROR; } @@ -252,6 +258,10 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio } else { + if (mParams.GetSkipCommissioningComplete().ValueOr(false)) + { + return CommissioningStage::kCleanup; + } return CommissioningStage::kFindOperational; } case CommissioningStage::kWiFiNetworkSetup: @@ -280,11 +290,19 @@ CommissioningStage AutoCommissioner::GetNextCommissioningStageInternal(Commissio { return CommissioningStage::kThreadNetworkEnable; } + else if (mParams.GetSkipCommissioningComplete().ValueOr(false)) + { + return CommissioningStage::kCleanup; + } else { return CommissioningStage::kFindOperational; } case CommissioningStage::kThreadNetworkEnable: + if (mParams.GetSkipCommissioningComplete().ValueOr(false)) + { + return CommissioningStage::kCleanup; + } return CommissioningStage::kFindOperational; case CommissioningStage::kFindOperational: return CommissioningStage::kSendComplete; diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 88bef7820efd71..0452835651aec8 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -1572,7 +1572,6 @@ void DeviceCommissioner::CommissioningStageComplete(CHIP_ERROR err, Commissionin { // Once this stage is complete, reset mDeviceBeingCommissioned - this will be reset when the delegate calls the next step. MATTER_TRACE_EVENT_SCOPE("CommissioningStageComplete", "DeviceCommissioner"); - if (mDeviceBeingCommissioned == nullptr) { // We are getting a stray callback (e.g. due to un-cancellable diff --git a/src/controller/CommissioningDelegate.cpp b/src/controller/CommissioningDelegate.cpp index 01d9c6ecd90f88..b6b83980130e98 100644 --- a/src/controller/CommissioningDelegate.cpp +++ b/src/controller/CommissioningDelegate.cpp @@ -113,6 +113,10 @@ const char * StageToString(CommissioningStage stage) return "Cleanup"; break; + case kNeedsNetworkCreds: + return "NeedsNetworkCreds"; + break; + default: return "???"; break; diff --git a/src/controller/CommissioningDelegate.h b/src/controller/CommissioningDelegate.h index cf1743c2d21bae..1e3fdef1276e44 100644 --- a/src/controller/CommissioningDelegate.h +++ b/src/controller/CommissioningDelegate.h @@ -378,6 +378,15 @@ class CommissioningParameters return *this; } + // Only perform the PASE steps of commissioning. + // Commissioning will be completed by another admin on the network. + Optional GetSkipCommissioningComplete() const { return mSkipCommissioningComplete; } + CommissioningParameters & SetSkipCommissioningComplete(bool skipCommissioningComplete) + { + mSkipCommissioningComplete = MakeOptional(skipCommissioningComplete); + return *this; + } + private: // Items that can be set by the commissioner Optional mFailsafeTimerSeconds; @@ -407,6 +416,7 @@ class CommissioningParameters nullptr; // Delegate to handle device attestation failures during commissioning Optional mAttemptWiFiNetworkScan; Optional mAttemptThreadNetworkScan; // This automatically gets set to false when a ThreadOperationalDataset is set + Optional mSkipCommissioningComplete; }; struct RequestedCertificate diff --git a/src/controller/java/AndroidDeviceControllerWrapper.cpp b/src/controller/java/AndroidDeviceControllerWrapper.cpp index b4a8c6714ec9eb..2be556d5a2bfd9 100644 --- a/src/controller/java/AndroidDeviceControllerWrapper.cpp +++ b/src/controller/java/AndroidDeviceControllerWrapper.cpp @@ -75,7 +75,7 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew( chip::Inet::EndPointManager * udpEndPointManager, AndroidOperationalCredentialsIssuerPtr opCredsIssuerPtr, jobject keypairDelegate, jbyteArray rootCertificate, jbyteArray intermediateCertificate, jbyteArray nodeOperationalCertificate, jbyteArray ipkEpochKey, uint16_t listenPort, uint16_t controllerVendorId, uint16_t failsafeTimerSeconds, - bool attemptNetworkScanWiFi, bool attemptNetworkScanThread, CHIP_ERROR * errInfoOnFailure) + bool attemptNetworkScanWiFi, bool attemptNetworkScanThread, bool skipCommissioningComplete, CHIP_ERROR * errInfoOnFailure) { if (errInfoOnFailure == nullptr) { @@ -156,6 +156,7 @@ AndroidDeviceControllerWrapper * AndroidDeviceControllerWrapper::AllocateNew( params.SetFailsafeTimerSeconds(failsafeTimerSeconds); params.SetAttemptWiFiNetworkScan(attemptNetworkScanWiFi); params.SetAttemptThreadNetworkScan(attemptNetworkScanThread); + params.SetSkipCommissioningComplete(skipCommissioningComplete); wrapper->UpdateCommissioningParameters(params); CHIP_ERROR err = wrapper->mGroupDataProvider.Init(); diff --git a/src/controller/java/AndroidDeviceControllerWrapper.h b/src/controller/java/AndroidDeviceControllerWrapper.h index 799cd374e32439..8c0697b024170a 100644 --- a/src/controller/java/AndroidDeviceControllerWrapper.h +++ b/src/controller/java/AndroidDeviceControllerWrapper.h @@ -138,6 +138,7 @@ class AndroidDeviceControllerWrapper : public chip::Controller::DevicePairingDel * @param[in] failsafeTimerSeconds the failsafe timer in seconds * @param[in] attemptNetworkScanWiFi whether to attempt a network scan when configuring the network for a WiFi device * @param[in] attemptNetworkScanThread whether to attempt a network scan when configuring the network for a Thread device + * @param[in] skipCommissioningComplete whether to skip the CASE commissioningComplete command during commissioning * @param[out] errInfoOnFailure a pointer to a CHIP_ERROR that will be populated if this method returns nullptr */ static AndroidDeviceControllerWrapper * @@ -148,7 +149,7 @@ class AndroidDeviceControllerWrapper : public chip::Controller::DevicePairingDel AndroidOperationalCredentialsIssuerPtr opCredsIssuer, jobject keypairDelegate, jbyteArray rootCertificate, jbyteArray intermediateCertificate, jbyteArray nodeOperationalCertificate, jbyteArray ipkEpochKey, uint16_t listenPort, uint16_t controllerVendorId, uint16_t failsafeTimerSeconds, bool attemptNetworkScanWiFi, - bool attemptNetworkScanThread, CHIP_ERROR * errInfoOnFailure); + bool attemptNetworkScanThread, bool skipCommissioningComplete, CHIP_ERROR * errInfoOnFailure); chip::Controller::AndroidOperationalCredentialsIssuer * GetAndroidOperationalCredentialsIssuer() { diff --git a/src/controller/java/CHIPDeviceController-JNI.cpp b/src/controller/java/CHIPDeviceController-JNI.cpp index 9a46972dd54d79..5f2b6fb0b19caf 100644 --- a/src/controller/java/CHIPDeviceController-JNI.cpp +++ b/src/controller/java/CHIPDeviceController-JNI.cpp @@ -285,6 +285,11 @@ JNI_METHOD(jlong, newDeviceController)(JNIEnv * env, jobject self, jobject contr &getAttemptNetworkScanThread); SuccessOrExit(err); + jmethodID getSkipCommissioningComplete; + err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getSkipCommissioningComplete", "()Z", + &getSkipCommissioningComplete); + SuccessOrExit(err); + jmethodID getKeypairDelegate; err = chip::JniReferences::GetInstance().FindMethod(env, controllerParams, "getKeypairDelegate", "()Lchip/devicecontroller/KeypairDelegate;", &getKeypairDelegate); @@ -324,6 +329,7 @@ JNI_METHOD(jlong, newDeviceController)(JNIEnv * env, jobject self, jobject contr uint16_t failsafeTimerSeconds = env->CallIntMethod(controllerParams, getFailsafeTimerSeconds); bool attemptNetworkScanWiFi = env->CallBooleanMethod(controllerParams, getAttemptNetworkScanWiFi); bool attemptNetworkScanThread = env->CallBooleanMethod(controllerParams, getAttemptNetworkScanThread); + bool skipCommissioningComplete = env->CallBooleanMethod(controllerParams, getSkipCommissioningComplete); uint64_t adminSubject = env->CallLongMethod(controllerParams, getAdminSubject); std::unique_ptr opCredsIssuer( @@ -332,7 +338,7 @@ JNI_METHOD(jlong, newDeviceController)(JNIEnv * env, jobject self, jobject contr sJVM, self, kLocalDeviceId, fabricId, chip::kUndefinedCATs, &DeviceLayer::SystemLayer(), DeviceLayer::TCPEndPointManager(), DeviceLayer::UDPEndPointManager(), std::move(opCredsIssuer), keypairDelegate, rootCertificate, intermediateCertificate, operationalCertificate, ipk, listenPort, controllerVendorId, - failsafeTimerSeconds, attemptNetworkScanWiFi, attemptNetworkScanThread, &err); + failsafeTimerSeconds, attemptNetworkScanWiFi, attemptNetworkScanThread, skipCommissioningComplete, &err); SuccessOrExit(err); if (adminSubject != kUndefinedNodeId) @@ -384,7 +390,7 @@ JNI_METHOD(void, commissionDevice) ChipLogProgress(Controller, "commissionDevice() called"); - CommissioningParameters commissioningParams = CommissioningParameters(); + CommissioningParameters commissioningParams = wrapper->GetCommissioningParameters(); if (networkCredentials != nullptr) { err = wrapper->ApplyNetworkCredentials(commissioningParams, networkCredentials); @@ -422,7 +428,7 @@ JNI_METHOD(void, pairDevice) #endif .SetPeerAddress(Transport::PeerAddress::BLE()); - CommissioningParameters commissioningParams = CommissioningParameters(); + CommissioningParameters commissioningParams = wrapper->GetCommissioningParameters(); wrapper->ApplyNetworkCredentials(commissioningParams, networkCredentials); if (csrNonce != nullptr) @@ -456,7 +462,8 @@ JNI_METHOD(void, pairDeviceWithAddress) .SetDiscriminator(discriminator) .SetSetupPINCode(pinCode) .SetPeerAddress(Transport::PeerAddress::UDP(const_cast(addrJniString.c_str()), port)); - CommissioningParameters commissioningParams = CommissioningParameters(); + + CommissioningParameters commissioningParams = wrapper->GetCommissioningParameters(); if (csrNonce != nullptr) { JniByteArray jniCsrNonce(env, csrNonce); diff --git a/src/controller/java/src/chip/devicecontroller/ControllerParams.java b/src/controller/java/src/chip/devicecontroller/ControllerParams.java index 612079bfa29361..e2e429a6ad083d 100644 --- a/src/controller/java/src/chip/devicecontroller/ControllerParams.java +++ b/src/controller/java/src/chip/devicecontroller/ControllerParams.java @@ -11,6 +11,7 @@ public final class ControllerParams { private final int failsafeTimerSeconds; private final boolean attemptNetworkScanWiFi; private final boolean attemptNetworkScanThread; + private final boolean skipCommissioningComplete; @Nullable private final KeypairDelegate keypairDelegate; @Nullable private final byte[] rootCertificate; @Nullable private final byte[] intermediateCertificate; @@ -28,6 +29,7 @@ private ControllerParams(Builder builder) { this.failsafeTimerSeconds = builder.failsafeTimerSeconds; this.attemptNetworkScanWiFi = builder.attemptNetworkScanWiFi; this.attemptNetworkScanThread = builder.attemptNetworkScanThread; + this.skipCommissioningComplete = builder.skipCommissioningComplete; this.keypairDelegate = builder.keypairDelegate; this.rootCertificate = builder.rootCertificate; this.intermediateCertificate = builder.intermediateCertificate; @@ -61,6 +63,10 @@ public boolean getAttemptNetworkScanThread() { return attemptNetworkScanThread; } + public boolean getSkipCommissioningComplete() { + return skipCommissioningComplete; + } + public KeypairDelegate getKeypairDelegate() { return keypairDelegate; } @@ -112,6 +118,7 @@ public static class Builder { private int failsafeTimerSeconds = 30; private boolean attemptNetworkScanWiFi = false; private boolean attemptNetworkScanThread = false; + private boolean skipCommissioningComplete = false; @Nullable private KeypairDelegate keypairDelegate = null; @Nullable private byte[] rootCertificate = null; @Nullable private byte[] intermediateCertificate = null; @@ -197,6 +204,24 @@ public Builder setAttemptNetworkScanThread(boolean attemptNetworkScanThread) { return this; } + /** + * Disable the CASE phase of commissioning when the CommissioningComplete command is sent by + * this ChipDeviceCommissioner. + * + *

Specifically, this sets SkipCommissioningComplete in the CommissioningParameters passed to + * the CommissioningDelegate. + * + *

A controller will set this to true when the CASE phase of commissioning is done by a + * separate process, for example, by a Hub on the network. + * + * @param skipCommissioningComplete + * @return + */ + public Builder setSkipCommissioningComplete(boolean skipCommissioningComplete) { + this.skipCommissioningComplete = skipCommissioningComplete; + return this; + } + public Builder setKeypairDelegate(KeypairDelegate keypairDelegate) { this.keypairDelegate = keypairDelegate; return this; From b444d25462331714bf633a0f3715124786d0bf74 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 30 Aug 2022 15:30:59 -0400 Subject: [PATCH 6/7] Ignore DeviceCommissioner::OnDeviceConnectionFailureFn for unexpected devices. (#22247) Just like we ignore DeviceCommissioner::OnDeviceConnectedFn if the device id does not match mDeviceBeingCommissioned, we should ignore OnDeviceConnectionFailureFn. Fixes https://github.com/project-chip/connectedhomeip/issues/22244 --- src/controller/CHIPDeviceController.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/controller/CHIPDeviceController.cpp b/src/controller/CHIPDeviceController.cpp index 0452835651aec8..0bf9ee911359d9 100644 --- a/src/controller/CHIPDeviceController.cpp +++ b/src/controller/CHIPDeviceController.cpp @@ -1653,6 +1653,13 @@ void DeviceCommissioner::OnDeviceConnectionFailureFn(void * context, const Scope error = CHIP_ERROR_INTERNAL; } + if (commissioner->mDeviceBeingCommissioned == nullptr || + commissioner->mDeviceBeingCommissioned->GetDeviceId() != peerId.GetNodeId()) + { + // Not the device we are trying to commission. + return; + } + if (commissioner->mCommissioningStage == CommissioningStage::kFindOperational && commissioner->mCommissioningDelegate != nullptr) { From d47c8f7a3ac84f2cc04e5411f064baeb9fe8c93f Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Tue, 30 Aug 2022 12:49:23 -0700 Subject: [PATCH 7/7] Add initial Messaging Layer unit test to test various negative paths (#22202) --- src/messaging/tests/BUILD.gn | 4 + src/messaging/tests/MessagingContext.h | 42 +++++ src/messaging/tests/TestMessagingLayer.cpp | 180 +++++++++++++++++++++ src/transport/raw/UDP.cpp | 6 + src/transport/tests/UDPTransportManager.h | 62 +++++++ 5 files changed, 294 insertions(+) create mode 100644 src/messaging/tests/TestMessagingLayer.cpp create mode 100644 src/transport/tests/UDPTransportManager.h diff --git a/src/messaging/tests/BUILD.gn b/src/messaging/tests/BUILD.gn index 4f2fa28bc25825..15c7dbdc02eb74 100644 --- a/src/messaging/tests/BUILD.gn +++ b/src/messaging/tests/BUILD.gn @@ -58,6 +58,10 @@ chip_test_suite("tests") { chip_device_platform != "nrfconnect") { test_sources += [ "TestExchangeHolder.cpp" ] } + + if (chip_device_platform == "linux") { + test_sources += [ "TestMessagingLayer.cpp" ] + } } cflags = [ "-Wconversion" ] diff --git a/src/messaging/tests/MessagingContext.h b/src/messaging/tests/MessagingContext.h index 5c885300fddac2..8d33c50b6176b9 100644 --- a/src/messaging/tests/MessagingContext.h +++ b/src/messaging/tests/MessagingContext.h @@ -27,6 +27,7 @@ #include #include #include +#include #include @@ -231,6 +232,47 @@ class LoopbackMessagingContext : public LoopbackTransportManager, public Messagi using LoopbackTransportManager::GetSystemLayer; }; +// UDPMessagingContext enriches MessagingContext with an UDP transport +class UDPMessagingContext : public UDPTransportManager, public MessagingContext +{ +public: + virtual ~UDPMessagingContext() {} + + /// Initialize the underlying layers. + virtual CHIP_ERROR Init() + { + ReturnErrorOnFailure(chip::Platform::MemoryInit()); + ReturnErrorOnFailure(UDPTransportManager::Init()); + ReturnErrorOnFailure(MessagingContext::Init(&GetTransportMgr(), &GetIOContext())); + return CHIP_NO_ERROR; + } + + // Shutdown all layers, finalize operations + virtual void Shutdown() + { + MessagingContext::Shutdown(); + UDPTransportManager::Shutdown(); + chip::Platform::MemoryShutdown(); + } + + // Init/Shutdown Helpers that can be used directly as the nlTestSuite + // initialize/finalize function. + static int Initialize(void * context) + { + auto * ctx = static_cast(context); + return ctx->Init() == CHIP_NO_ERROR ? SUCCESS : FAILURE; + } + + static int Finalize(void * context) + { + auto * ctx = static_cast(context); + ctx->Shutdown(); + return SUCCESS; + } + + using UDPTransportManager::GetSystemLayer; +}; + // Class that can be used to capture decrypted message traffic in tests using // MessagingContext. class MessageCapturer : public SessionMessageDelegate diff --git a/src/messaging/tests/TestMessagingLayer.cpp b/src/messaging/tests/TestMessagingLayer.cpp new file mode 100644 index 00000000000000..7e0285d6f27170 --- /dev/null +++ b/src/messaging/tests/TestMessagingLayer.cpp @@ -0,0 +1,180 @@ +/* + * + * Copyright (c) 2022 Project CHIP Authors + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file + * This file implements unit tests for the ExchangeManager implementation. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include +#include + +namespace { + +using namespace chip; +using namespace chip::Inet; +using namespace chip::Transport; +using namespace chip::Messaging; +using namespace chip::Protocols; +using namespace chip::System::Clock::Literals; + +using TestContext = Test::UDPMessagingContext; + +// The message timeout value in milliseconds. +constexpr System::Clock::Timeout kMessageTimeout = System::Clock::Milliseconds32(100); + +class MockAppDelegate : public UnsolicitedMessageHandler, public ExchangeDelegate +{ +public: + CHIP_ERROR OnUnsolicitedMessageReceived(const PayloadHeader & payloadHeader, ExchangeDelegate *& newDelegate) override + { + newDelegate = this; + return CHIP_NO_ERROR; + } + + CHIP_ERROR OnMessageReceived(ExchangeContext * ec, const PayloadHeader & payloadHeader, + System::PacketBufferHandle && buffer) override + { + IsOnMessageReceivedCalled = true; + return CHIP_NO_ERROR; + } + + void OnResponseTimeout(ExchangeContext * ec) override { IsOnResponseTimeoutCalled = true; } + + bool IsOnMessageReceivedCalled = false; + bool IsOnResponseTimeoutCalled = false; +}; + +/** + * Tests sending exchange message with Success: + * + * DUT = sender, PEER = remote device + * + * 1) DUT sends message w/o MRP to PEER + * - Confirm the message is sent successfully + * - Observe DUT response timeout with no response + */ +void CheckExchangeOutgoingMessagesSuccess(nlTestSuite * inSuite, void * inContext) +{ + TestContext & ctx = *reinterpret_cast(inContext); + + // create solicited exchange + MockAppDelegate mockSolicitedAppDelegate; + ExchangeContext * ec = ctx.NewExchangeToAlice(&mockSolicitedAppDelegate); + + NL_TEST_ASSERT(inSuite, ec != nullptr); + ec->SetResponseTimeout(kMessageTimeout); + + CHIP_ERROR err = ec->SendMessage(Echo::MsgType::EchoRequest, System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize), + SendFlags(SendMessageFlags::kExpectResponse).Set(SendMessageFlags::kNoAutoRequestAck)); + + // Wait for the initial message to fail (should take 330-413ms) + ctx.GetIOContext().DriveIOUntil(500_ms32, [&] { return mockSolicitedAppDelegate.IsOnMessageReceivedCalled; }); + + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, mockSolicitedAppDelegate.IsOnResponseTimeoutCalled); +} + +/** + * Tests sending exchange message with Failure: + * + * DUT = sender, PEER = remote device + * + * 1) DUT configured to drop the outgoing UDP packet + * 2) DUT sends message w/o MRP to PEER + * - Confirm the message is sent with failure + * - Confirm the DUT response timeout timer is cancelled + */ +void CheckExchangeOutgoingMessagesFail(nlTestSuite * inSuite, void * inContext) +{ + TestContext & ctx = *reinterpret_cast(inContext); + + // create solicited exchange + MockAppDelegate mockSolicitedAppDelegate; + ExchangeContext * ec = ctx.NewExchangeToAlice(&mockSolicitedAppDelegate); + + NL_TEST_ASSERT(inSuite, ec != nullptr); + ec->SetResponseTimeout(kMessageTimeout); + + chip::FaultInjection::GetManager().FailAtFault(chip::FaultInjection::kFault_DropOutgoingUDPMsg, 0, 1); + + CHIP_ERROR err = ec->SendMessage(Echo::MsgType::EchoRequest, System::PacketBufferHandle::New(System::PacketBuffer::kMaxSize), + SendFlags(SendMessageFlags::kExpectResponse).Set(SendMessageFlags::kNoAutoRequestAck)); + + // Wait for the initial message to fail (should take 330-413ms) + ctx.GetIOContext().DriveIOUntil(500_ms32, [&] { return mockSolicitedAppDelegate.IsOnMessageReceivedCalled; }); + + NL_TEST_ASSERT(inSuite, err != CHIP_NO_ERROR); + NL_TEST_ASSERT(inSuite, !mockSolicitedAppDelegate.IsOnResponseTimeoutCalled); + ec->Close(); +} + +// Test Suite + +/** + * Test Suite that lists all the test functions. + */ +// clang-format off +const nlTest sTests[] = +{ + NL_TEST_DEF("Test MessagingLayer::ExchangeOutgoingMessagesSuccess", CheckExchangeOutgoingMessagesSuccess), + NL_TEST_DEF("Test MessagingLayer::ExchangeOutgoingMessagesFail", CheckExchangeOutgoingMessagesFail), + + NL_TEST_SENTINEL() +}; +// clang-format on + +// clang-format off +nlTestSuite sSuite = +{ + "Test-CHIP-MessagingLayer", + &sTests[0], + TestContext::Initialize, + TestContext::Finalize +}; +// clang-format on + +} // namespace + +/** + * Main + */ +int TestMessagingLayer() +{ + return chip::ExecuteTestsWithContext(&sSuite); +} + +CHIP_REGISTER_TEST_SUITE(TestMessagingLayer); diff --git a/src/transport/raw/UDP.cpp b/src/transport/raw/UDP.cpp index 57b1d3c615bffc..3113e9797bfc72 100644 --- a/src/transport/raw/UDP.cpp +++ b/src/transport/raw/UDP.cpp @@ -23,6 +23,7 @@ */ #include +#include #include #include #include @@ -110,6 +111,9 @@ CHIP_ERROR UDP::SendMessage(const Transport::PeerAddress & address, System::Pack addrInfo.DestPort = address.GetPort(); addrInfo.Interface = address.GetInterface(); + // Drop the message and return. Free the buffer. + CHIP_FAULT_INJECT(FaultInjection::kFault_DropOutgoingUDPMsg, msgBuf = nullptr; return CHIP_ERROR_CONNECTION_ABORTED;); + return mUDPEndPoint->SendMsg(&addrInfo, std::move(msgBuf)); } @@ -119,6 +123,8 @@ void UDP::OnUdpReceive(Inet::UDPEndPoint * endPoint, System::PacketBufferHandle UDP * udp = reinterpret_cast(endPoint->mAppState); PeerAddress peerAddress = PeerAddress::UDP(pktInfo->SrcAddress, pktInfo->SrcPort, pktInfo->Interface); + CHIP_FAULT_INJECT(FaultInjection::kFault_DropIncomingUDPMsg, buffer = nullptr; return;); + udp->HandleMessageReceived(peerAddress, std::move(buffer)); if (err != CHIP_NO_ERROR) diff --git a/src/transport/tests/UDPTransportManager.h b/src/transport/tests/UDPTransportManager.h new file mode 100644 index 00000000000000..2a940d3264e834 --- /dev/null +++ b/src/transport/tests/UDPTransportManager.h @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2022 Project CHIP Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +#pragma once + +#include +#include +#include +#include +#include +#include + +namespace chip { +namespace Test { + +class UDPTransportManager +{ +public: + /// Initialize the underlying layers. + CHIP_ERROR Init() + { + ReturnErrorOnFailure(mIOContext.Init()); + ReturnErrorOnFailure(DeviceLayer::PlatformMgr().InitChipStack()); + ReturnErrorOnFailure(mTransportManager.Init(Transport::UdpListenParameters(DeviceLayer::UDPEndPointManager()) + .SetAddressType(Inet::IPAddressType::kIPv6) + .SetListenPort(CHIP_PORT + 1))); + + return CHIP_NO_ERROR; + } + + // Shutdown all layers, finalize operations + void Shutdown() + { + mTransportManager.Close(); + DeviceLayer::PlatformMgr().Shutdown(); + mIOContext.Shutdown(); + } + + System::Layer & GetSystemLayer() { return mIOContext.GetSystemLayer(); } + chip::Transport::UDP & GetUDP() { return mTransportManager.GetTransport().template GetImplAtIndex<0>(); } + TransportMgrBase & GetTransportMgr() { return mTransportManager; } + IOContext & GetIOContext() { return mIOContext; } + +private: + Test::IOContext mIOContext; + TransportMgr mTransportManager; +}; + +} // namespace Test +} // namespace chip