From 93e50c772b4258a3d253a24719c41855132ac345 Mon Sep 17 00:00:00 2001 From: Pradip De Date: Wed, 18 Dec 2024 23:37:25 -0800 Subject: [PATCH 1/9] Fix for Bug #36731. (#36880) Add CloseActiveConnections() call in TCPBase::Close(), which is called as part of Server::Shutdown(). Active connections should be closed as part of Server shutdown. This allows the TCPConnectionState to also close the associated TCPEndpoint object as part of this shutdown flow. Previously, the CloseActiveConnections() call was present in the TCPBase destructor alone. Add test for Connection Close() and checking for TCPEndPoint. --- src/transport/raw/ActiveTCPConnectionState.h | 5 ++++- src/transport/raw/TCP.cpp | 13 +++++------ src/transport/raw/tests/TestTCP.cpp | 23 ++++++++++++++++++++ 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/src/transport/raw/ActiveTCPConnectionState.h b/src/transport/raw/ActiveTCPConnectionState.h index 0f53d4479e9300..2176048cc25364 100644 --- a/src/transport/raw/ActiveTCPConnectionState.h +++ b/src/transport/raw/ActiveTCPConnectionState.h @@ -62,7 +62,10 @@ struct ActiveTCPConnectionState void Free() { - mEndPoint->Free(); + if (mEndPoint) + { + mEndPoint->Free(); + } mPeerAddr = PeerAddress::Uninitialized(); mEndPoint = nullptr; mReceived = nullptr; diff --git a/src/transport/raw/TCP.cpp b/src/transport/raw/TCP.cpp index f12ee7892ad2d0..9976874d8dc6f1 100644 --- a/src/transport/raw/TCP.cpp +++ b/src/transport/raw/TCP.cpp @@ -55,14 +55,8 @@ constexpr int kListenBacklogSize = 2; TCPBase::~TCPBase() { - if (mListenSocket != nullptr) - { - // endpoint is only non null if it is initialized and listening - mListenSocket->Free(); - mListenSocket = nullptr; - } - - CloseActiveConnections(); + // Call Close to free the listening socket and close all active connections. + Close(); } void TCPBase::CloseActiveConnections() @@ -125,6 +119,9 @@ void TCPBase::Close() mListenSocket->Free(); mListenSocket = nullptr; } + + CloseActiveConnections(); + mState = TCPState::kNotReady; } diff --git a/src/transport/raw/tests/TestTCP.cpp b/src/transport/raw/tests/TestTCP.cpp index 80d56707481c45..00eb0562c96172 100644 --- a/src/transport/raw/tests/TestTCP.cpp +++ b/src/transport/raw/tests/TestTCP.cpp @@ -610,6 +610,29 @@ TEST_F(TestTCP, HandleConnCloseCalledTest6) HandleConnCloseTest(addr); } +TEST_F(TestTCP, CheckTCPEndpointAfterCloseTest) +{ + TCPImpl tcp; + + IPAddress addr; + IPAddress::FromString("::1", addr); + + MockTransportMgrDelegate gMockTransportMgrDelegate(mIOContext); + gMockTransportMgrDelegate.InitializeMessageTest(tcp, addr); + gMockTransportMgrDelegate.ConnectTest(tcp, addr); + + Transport::PeerAddress lPeerAddress = Transport::PeerAddress::TCP(addr, gChipTCPPort); + void * state = TestAccess::FindActiveConnection(tcp, lPeerAddress); + ASSERT_NE(state, nullptr); + TCPEndPoint * lEndPoint = TestAccess::GetEndpoint(state); + ASSERT_NE(lEndPoint, nullptr); + + // Call Close and check the TCPEndpoint + tcp.Close(); + lEndPoint = TestAccess::GetEndpoint(state); + ASSERT_EQ(lEndPoint, nullptr); +} + TEST_F(TestTCP, CheckProcessReceivedBuffer) { TCPImpl tcp; From 2c73b25c6427ff3d2f01f75b083eaba765b09799 Mon Sep 17 00:00:00 2001 From: shripad621git <79364691+shripad621git@users.noreply.github.com> Date: Thu, 19 Dec 2024 13:17:35 +0530 Subject: [PATCH 2/9] [ESP32] Fixed the documentation to use the Scan Response Data (#36796) --- docs/platforms/esp32/ble_settings.md | 17 ++++++++++------- src/platform/ESP32/BLEManagerImpl.h | 2 +- src/platform/ESP32/nimble/BLEManagerImpl.cpp | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/docs/platforms/esp32/ble_settings.md b/docs/platforms/esp32/ble_settings.md index 36178ebece5d71..af6ba325d16c9e 100644 --- a/docs/platforms/esp32/ble_settings.md +++ b/docs/platforms/esp32/ble_settings.md @@ -13,13 +13,14 @@ advertising packets. ``` { - uint8_t scanResponse[31]; // 0x05, 0x09, a, b, c, d - scanResponse[0] = 0x05; - scanResponse[1] = 0x09; - scanResponse[2] = 0x61; - scanResponse[3] = 0x62; - scanResponse[4] = 0x63; - scanResponse[5] = 0x64; + + // Max length is 31 bytes + // Enter data in (length, type, value) format + // 0x05 - length of data + // 0x09 - Type (Complete Local Name) + // 0x61, 0x62, 0x63, 0x64 - Data (a,b,c,d) + uint8_t scanResponse[] = { 0x05, 0x09, 0x61, 0x62, 0x63, 0x64}; + chip::ByteSpan data(scanResponse); CHIP_ERROR err = chip::DeviceLayer::Internal::BLEMgrImpl().ConfigureScanResponseData(data); if (err != CHIP_NO_ERROR) @@ -27,6 +28,8 @@ advertising packets. ESP_LOGE(TAG, "Failed to configure scan response, err:%" CHIP_ERROR_FORMAT, err.Format()); } } + + ``` Note: Scan response should be configure before `InitServer`. diff --git a/src/platform/ESP32/BLEManagerImpl.h b/src/platform/ESP32/BLEManagerImpl.h index eeed125012b00f..2a489420a055a1 100644 --- a/src/platform/ESP32/BLEManagerImpl.h +++ b/src/platform/ESP32/BLEManagerImpl.h @@ -132,7 +132,6 @@ class BLEManagerImpl final : public BLEManager, #endif // CONFIG_ENABLE_ESP32_BLE_CONTROLLER { public: - uint8_t scanResponseBuffer[MAX_SCAN_RSP_DATA_LEN]; BLEManagerImpl() {} #ifdef CONFIG_ENABLE_ESP32_BLE_CONTROLLER CHIP_ERROR ConfigureBle(uint32_t aAdapterId, bool aIsCentral); @@ -146,6 +145,7 @@ class BLEManagerImpl final : public BLEManager, private: chip::Optional mScanResponse; + uint8_t scanResponseBuffer[MAX_SCAN_RSP_DATA_LEN]; // Allow the BLEManager interface class to delegate method calls to // the implementation methods provided by this class. diff --git a/src/platform/ESP32/nimble/BLEManagerImpl.cpp b/src/platform/ESP32/nimble/BLEManagerImpl.cpp index 176fd81ee9f2c9..0a2ddb176332b2 100644 --- a/src/platform/ESP32/nimble/BLEManagerImpl.cpp +++ b/src/platform/ESP32/nimble/BLEManagerImpl.cpp @@ -1118,7 +1118,7 @@ CHIP_ERROR BLEManagerImpl::ConfigureScanResponseData(ByteSpan data) return CHIP_ERROR_INVALID_ARGUMENT; } memcpy(scanResponseBuffer, data.data(), data.size()); - ByteSpan scanResponseSpan(scanResponseBuffer); + ByteSpan scanResponseSpan(scanResponseBuffer, data.size()); mScanResponse = chip::Optional(scanResponseSpan); return CHIP_NO_ERROR; } From 4aea78d109ac2c00c6065d7e91ccd11b1904d890 Mon Sep 17 00:00:00 2001 From: Vatsal Ghelani <152916324+vatsalghelani-csa@users.noreply.github.com> Date: Thu, 19 Dec 2024 02:53:33 -0500 Subject: [PATCH 3/9] Follow Up Fix PR #36596 - Remove indents for returns (shorten the code/indent level on early return instead of if/else) (#36887) * Follow Up Fix PR #36596 * Remove unwanted comments * Restyled by autopep8 * Comment and space fixes --------- Co-authored-by: Restyled.io --- .../chip/testing/spec_parsing.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/python_testing/matter_testing_infrastructure/chip/testing/spec_parsing.py b/src/python_testing/matter_testing_infrastructure/chip/testing/spec_parsing.py index 3607f515d3a9ec..8160dc7ba99f62 100644 --- a/src/python_testing/matter_testing_infrastructure/chip/testing/spec_parsing.py +++ b/src/python_testing/matter_testing_infrastructure/chip/testing/spec_parsing.py @@ -541,17 +541,17 @@ def get_data_model_directory(data_model_directory: Union[PrebuiltDataModelDirect """ Get the directory of the data model for a specific version and level from the installed package. - `data_model_directory` given as a path MUST be of type Traversable (often `pathlib.Path(somepathstring)`). If `data_model_directory` is given as a Traversable, it is returned directly WITHOUT using the data_model_level at all. """ - # If it's a prebuilt directory, build the path based on the version and data model level - if isinstance(data_model_directory, PrebuiltDataModelDirectory): - return pkg_resources.files(importlib.import_module('chip.testing')).joinpath( - 'data_model').joinpath(data_model_directory.dirname).joinpath(data_model_level.dirname) - else: + # Early return if data_model_directory is already a Traversable type + if not isinstance(data_model_directory, PrebuiltDataModelDirectory): return data_model_directory + # If it's a prebuilt directory, build the path based on the version and data model level + return pkg_resources.files(importlib.import_module('chip.testing')).joinpath( + 'data_model').joinpath(data_model_directory.dirname).joinpath(data_model_level.dirname) + def build_xml_clusters(data_model_directory: Union[PrebuiltDataModelDirectory, Traversable] = PrebuiltDataModelDirectory.k1_4) -> typing.Tuple[dict[int, dict], list]: """ @@ -559,7 +559,7 @@ def build_xml_clusters(data_model_directory: Union[PrebuiltDataModelDirectory, T This function supports both pre-built locations and full paths. `data_model_directory`` given as a path MUST be of type Traversable (often `pathlib.Path(somepathstring)`). - If data_model_directory is a Travesable, it is assumed to already contain `clusters` (i.e. be a directory + If data_model_directory is a Traversable, it is assumed to already contain `clusters` (i.e. be a directory with all XML files in it) """ @@ -606,6 +606,7 @@ def remove_problem(location: typing.Union[CommandPathLocation, FeaturePathLocati mask = clusters[descriptor_id].feature_map[code] clusters[descriptor_id].features[mask].conformance = optional() remove_problem(FeaturePathLocation(endpoint_id=0, cluster_id=descriptor_id, feature_code=code)) + action_id = Clusters.Actions.id for c in Clusters.ClusterObjects.ALL_ACCEPTED_COMMANDS[action_id]: clusters[action_id].accepted_commands[c].conformance = optional() From 4828e1607e5a895d94ea7f2fce9b823fcb5611b5 Mon Sep 17 00:00:00 2001 From: Wang Qixiang <43193572+wqx6@users.noreply.github.com> Date: Thu, 19 Dec 2024 15:55:21 +0800 Subject: [PATCH 4/9] esp32: simplify CMakeLists file for chip component (#36883) --- config/esp32/components/chip/CMakeLists.txt | 174 ++---------------- .../esp32/components/chip/idf_component.yml | 1 - 2 files changed, 12 insertions(+), 163 deletions(-) diff --git a/config/esp32/components/chip/CMakeLists.txt b/config/esp32/components/chip/CMakeLists.txt index 0047cfecc70a1c..571e02d9eaab49 100644 --- a/config/esp32/components/chip/CMakeLists.txt +++ b/config/esp32/components/chip/CMakeLists.txt @@ -31,11 +31,7 @@ endif() include(${CMAKE_CURRENT_LIST_DIR}/ota-image.cmake) -set(CHIP_REQUIRE_COMPONENTS esp_eth freertos lwip bt mbedtls fatfs app_update console openthread nvs_flash spi_flash) - -if(NOT "${IDF_TARGET}" STREQUAL "esp32h2") - list(APPEND CHIP_REQUIRE_COMPONENTS mdns) -endif() +set(CHIP_REQUIRE_COMPONENTS esp_eth freertos lwip bt mbedtls fatfs app_update console openthread nvs_flash spi_flash mdns) if (NOT CMAKE_BUILD_EARLY_EXPANSION) if (CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_DISABLE) @@ -420,170 +416,24 @@ target_include_directories(${COMPONENT_LIB} INTERFACE "${CHIP_ROOT}/config/esp32/${CONFIG_CHIP_EXTERNAL_PLATFORM_DIR}/../../" ) -idf_component_get_property(mbedtls_lib mbedtls COMPONENT_LIB) - -idf_build_get_property(idf_target IDF_TARGET) -set(target_name "${idf_target}") - -if(CONFIG_BT_ENABLED) - idf_component_get_property(bt_lib bt COMPONENT_LIB) - if((target_name STREQUAL "esp32h2") OR (target_name STREQUAL "esp32c2") OR (target_name STREQUAL "esp32c6")) - idf_component_get_property(bt_dir bt COMPONENT_DIR) - list(APPEND chip_libraries $) - if (EXISTS ${bt_dir}/controller/lib_${target_name}/${target_name}-bt-lib/libble_app.a) - list(APPEND chip_libraries "${bt_dir}/controller/lib_${target_name}/${target_name}-bt-lib/libble_app.a") - elseif(EXISTS ${bt_dir}/controller/lib_${target_name}/${target_name}-bt-lib/${target_name}/libble_app.a) - list(APPEND chip_libraries "${bt_dir}/controller/lib_${target_name}/${target_name}-bt-lib/${target_name}/libble_app.a") - else() - message(WARNING "There is no libble_app.a in the given path") - endif() - elseif(target_name STREQUAL "esp32p4") - list(APPEND chip_libraries $) - else() - list(APPEND chip_libraries $ -lbtdm_app) - endif() -endif() - -if (CONFIG_ENABLE_CHIP_SHELL) - idf_component_get_property(console_lib console COMPONENT_LIB) - list(APPEND chip_libraries $) -endif() - -if(CONFIG_OPENTHREAD_ENABLED) - idf_component_get_property(openthread_lib openthread COMPONENT_LIB) - list(APPEND chip_libraries $) - if (CONFIG_IEEE802154_ENABLED) - idf_component_get_property(ieee802154_lib ieee802154 COMPONENT_LIB) - list(APPEND chip_libraries $) - endif() -endif() - -if(NOT CONFIG_USE_MINIMAL_MDNS) - idf_build_get_property(build_components BUILD_COMPONENTS) - # For IDF v5.x, the mdns component was moved to idf_managed_components. - # We should use 'espressif__mdns' for 'idf_component_get_property'. - if("espressif__mdns" IN_LIST build_components) - idf_component_get_property(mdns_lib espressif__mdns COMPONENT_LIB) - list(APPEND chip_libraries $) - elseif("mdns" IN_LIST build_components) - idf_component_get_property(mdns_lib mdns COMPONENT_LIB) - list(APPEND chip_libraries $) - endif() -endif() - -if(CONFIG_OPENTHREAD_BORDER_ROUTER) - idf_component_get_property(rcp_update_lib espressif__esp_rcp_update COMPONENT_LIB) - list(APPEND chip_libraries $) - idf_component_get_property(serial_flasher_lib espressif__esp-serial-flasher COMPONENT_LIB) - list(APPEND chip_libraries $) -endif() - -if (CONFIG_ENABLE_ENCRYPTED_OTA) - idf_component_get_property(esp_encrypted_img_lib espressif__esp_encrypted_img COMPONENT_LIB) - list(APPEND chip_libraries $) -endif() - -# Let user set EXECUTABLE_COMPONENT_NAME and defaults to main if not specified -if (NOT EXECUTABLE_COMPONENT_NAME) - set(EXECUTABLE_COMPONENT_NAME "main") -endif() - -if (CONFIG_ENABLE_DELTA_OTA) - idf_component_get_property(esp_delta_ota_lib espressif__esp_delta_ota COMPONENT_LIB) - list(APPEND chip_libraries $) -endif() - -idf_component_get_property(main_lib ${EXECUTABLE_COMPONENT_NAME} COMPONENT_LIB) -list(APPEND chip_libraries $) - -if (CONFIG_SEC_CERT_DAC_PROVIDER) - idf_component_get_property(esp32_secure_cert_mgr_lib espressif__esp_secure_cert_mgr COMPONENT_LIB) - list(APPEND chip_libraries $) -endif() - -if (CONFIG_ENABLE_ESP_INSIGHTS_TRACE) - idf_build_get_property(build_components BUILD_COMPONENTS) - # esp_insights can be used as an independent component or through component manager so, - # We should check and add the right component. - if("espressif__esp_insights" IN_LIST build_components) - idf_component_get_property(esp_insights_lib espressif__esp_insights COMPONENT_LIB) - elseif("esp_insights" IN_LIST build_components) - idf_component_get_property(esp_insights_lib esp_insights COMPONENT_LIB) - endif() - - list(APPEND chip_libraries $) -endif() - -idf_component_get_property(lwip_lib lwip COMPONENT_LIB) -list(APPEND chip_libraries $) - -if ("${IDF_VERSION_MAJOR}.${IDF_VERSION_MINOR}" VERSION_GREATER_EQUAL "5.3" OR CONFIG_ESP32_WIFI_ENABLED) - idf_component_get_property(esp_wifi_lib esp_wifi COMPONENT_LIB) - list(APPEND chip_libraries $) -endif() - -if (CONFIG_ESP32_WIFI_ENABLED) - idf_component_get_property(esp_wifi_dir esp_wifi COMPONENT_DIR) - if (CONFIG_IDF_TARGET_ESP32C2) - set(blobs core net80211 pp) - else() - set(blobs core mesh net80211 pp) - endif() - - foreach(blob ${blobs}) - list(APPEND chip_libraries "${esp_wifi_dir}/lib/${target_name}/lib${blob}.a") - endforeach() - - idf_component_get_property(wpa_supplicant_lib wpa_supplicant COMPONENT_LIB) - list(APPEND chip_libraries $) -endif() - -if (CONFIG_ETH_ENABLED) - idf_component_get_property(esp_eth_lib esp_eth COMPONENT_LIB) - list(APPEND chip_libraries $) +set(matter_requires lwip freertos console bt) +idf_build_get_property(build_components BUILD_COMPONENTS) +if("espressif__mdns" IN_LIST build_components) + list(APPEND matter_requires espressif__mdns) +elseif("mdns" IN_LIST build_components) + list(APPEND matter_requires mdns) endif() -idf_component_get_property(esp_netif_lib esp_netif COMPONENT_LIB) -list(APPEND chip_libraries $) - -idf_component_get_property(esp_hw_support_lib esp_hw_support COMPONENT_LIB) -list(APPEND chip_libraries $) - -if (NOT CONFIG_IDF_TARGET_ESP32P4) -idf_component_get_property(esp_phy_lib esp_phy COMPONENT_LIB) -idf_component_get_property(esp_phy_dir esp_phy COMPONENT_DIR) -list(APPEND chip_libraries $) - -if (CONFIG_IDF_TARGET_ESP32) - set(phy_blobs phy rtc) -elseif (CONFIG_IDF_TARGET_ESP32S2) - set(phy_blobs phy) -else() - set(phy_blobs phy btbb) -endif() -foreach(phy_blob ${phy_blobs}) - list(APPEND chip_libraries "${esp_phy_dir}/lib/${target_name}/lib${phy_blob}.a") -endforeach() +if (CONFIG_OPENTHREAD_BORDER_ROUTER) + list(APPEND matter_requires espressif__esp_rcp_update) endif() -set(components_to_link esp_event hal esp_system soc efuse vfs driver freertos esp_timer) -if (NOT CONFIG_IDF_TARGET_ESP32P4) -list(APPEND components_to_link esp_coex) -endif() -idf_build_get_property(build_components BUILD_COMPONENTS) -foreach(component ${components_to_link}) - # Some of the components are not present in IDF v4.x - # So, Check if the component is in the list of build components - if("${component}" IN_LIST build_components) - idf_component_get_property(lib_name ${component} COMPONENT_LIB) - list(APPEND chip_libraries $) - endif() -endforeach() +add_prebuilt_library(matterlib "${CMAKE_CURRENT_BINARY_DIR}/lib/libCHIP.a" + REQUIRES ${matter_requires}) target_link_libraries(${COMPONENT_LIB} INTERFACE -Wl,--start-group ${chip_libraries} - $ $ - $ + matterlib -Wl,--end-group) # Make the component dependent on our CHIP build diff --git a/config/esp32/components/chip/idf_component.yml b/config/esp32/components/chip/idf_component.yml index 78d21af787c631..c11eef55d97129 100644 --- a/config/esp32/components/chip/idf_component.yml +++ b/config/esp32/components/chip/idf_component.yml @@ -4,7 +4,6 @@ dependencies: version: "^1.1.0" rules: - if: "idf_version >=5.0" - - if: "target != esp32h2" espressif/esp_secure_cert_mgr: version: "^2.5.0" From 204fb273dd3716bdbf9fa8c345d8cfd6952b09b8 Mon Sep 17 00:00:00 2001 From: cdj <45139296+DejinChen@users.noreply.github.com> Date: Thu, 19 Dec 2024 15:56:33 +0800 Subject: [PATCH 5/9] ESP32: Remove unused config (#36869) --- config/esp32/components/chip/CMakeLists.txt | 4 + config/esp32/components/chip/Kconfig | 179 ------------------ .../esp32/sdkconfig.defaults.esp32c6 | 1 - .../esp32/sdkconfig.defaults.esp32h2 | 1 - .../esp32/sdkconfig.defaults.esp32c6 | 1 - .../esp32/sdkconfig.defaults.esp32h2 | 1 - .../esp32/sdkconfig.defaults.esp32c6 | 1 - .../esp32/sdkconfig.defaults.esp32h2 | 1 - examples/lit-icd-app/esp32/sdkconfig.defaults | 1 - .../lock-app/esp32/sdkconfig.defaults.esp32c6 | 1 - src/platform/ESP32/CHIPDevicePlatformConfig.h | 3 - 11 files changed, 4 insertions(+), 190 deletions(-) diff --git a/config/esp32/components/chip/CMakeLists.txt b/config/esp32/components/chip/CMakeLists.txt index 571e02d9eaab49..51f75954aae84f 100644 --- a/config/esp32/components/chip/CMakeLists.txt +++ b/config/esp32/components/chip/CMakeLists.txt @@ -310,6 +310,10 @@ if (CONFIG_CHIP_DEVICE_ENABLE_DYNAMIC_SERVER) chip_gn_arg_append("chip_build_controller_dynamic_server" "true") endif() +if (CONFIG_ICD_MAX_NOTIFICATION_SUBSCRIBERS) + chip_gn_arg_append("icd_max_notification_subscribers" ${CONFIG_ICD_MAX_NOTIFICATION_SUBSCRIBERS}) +endif() + set(args_gn_input "${CMAKE_CURRENT_BINARY_DIR}/args.gn.in") file(GENERATE OUTPUT "${args_gn_input}" CONTENT "${chip_gn_args}") diff --git a/config/esp32/components/chip/Kconfig b/config/esp32/components/chip/Kconfig index 2d8127900eed69..f8885b77e9739f 100644 --- a/config/esp32/components/chip/Kconfig +++ b/config/esp32/components/chip/Kconfig @@ -47,15 +47,6 @@ menu "CHIP Core" Each fabric can provision the device with its unique operational credentials and manage its own access control lists. - config MAX_PEER_NODES - int "Max Peer Nodes" - range 0 65535 - default 16 - help - The maximum number of peer nodes that the local node can communicate with using - connectionless communication (e.g. UDP). This value sizes a table that tracks - communication state with peer nodes by their CHIP node id. - config MAX_UNSOLICITED_MESSAGE_HANDLERS int "Max Unsolicited Message Handlers" range 0 65535 @@ -223,28 +214,6 @@ menu "CHIP Core" CHIP generally needs one UDP EndPoint object for each local network interface, plus 2 additional EndPoints for general UDP communcation. - config MAX_CONNECTIONS - int "Max CHIP Connections" - range 0 65535 - default 8 - help - The maximum number of simultaneously active CHIP connections, either locally - or remotely initiated. This limit covers both CHIP TCP connections, and - CHIP-over-BLE (WoBLE) connections. - - config DEFAULT_INCOMING_CONNECTION_IDLE_TIMEOUT - int "Default Incoming Connection Idle Timeout (ms)" - range 0 1000000 - default 15000 - help - The maximum amount of time, in milliseconds, that an idle inbound - CHIP connection will be allowed to exist before being closed. - - This is a default value that can be overridden at runtime by the - application. - - A value of 0 disables automatic closing of idle connections. - config ENABLE_ROUTE_HOOK bool "Enable route hook" depends on LWIP_HOOK_IP6_ROUTE_DEFAULT && LWIP_HOOK_ND6_GET_GW_DEFAULT @@ -643,58 +612,6 @@ menu "CHIP Device Layer" endmenu - menu "WiFi AP Options" - - config ENABLE_WIFI_AP - depends on ESP_WIFI_SOFTAP_SUPPORT - bool "Enable CHIP WIFI AP" - default y - help - Enables WiFi AP for CHIP. - - config WIFI_AP_SSID_PREFIX - string "WiFi AP SSID Prefix" - default "MATTER-" - depends on ENABLE_WIFI_AP - help - A prefix string used in forming the WiFi soft-AP SSID. The remainder of the SSID - consists of the final two bytes of the device's primary WiFi MAC address in hex. - - config WIFI_AP_CHANNEL - int "WiFi AP Channel" - range 1 14 - default 1 - depends on ENABLE_WIFI_AP - help - The WiFi channel number to be used by the soft-AP. - - config WIFI_AP_MAX_STATIONS - int "WiFi AP Max Allowed Stations" - range 1 10 - default 4 - depends on ENABLE_WIFI_AP - help - The maximum number of stations allowed to connect to the soft-AP. - - config WIFI_AP_BEACON_INTERVAL - int "WiFi AP Beacon Interval (ms)" - range 100 60000 - default 100 - depends on ENABLE_WIFI_AP - help - The beacon interval (in milliseconds) for the WiFi soft-AP. - - config WIFI_AP_IDLE_TIMEOUT - int "WiFi AP Idle Timeout (ms)" - range 0 600000 - default 120000 - depends on ENABLE_WIFI_AP - help - The amount of time (in milliseconds) after which the CHIP platform will deactivate the soft-AP - if it has been idle. - - endmenu - menu "BLE Options" visible if BT_ENABLED @@ -853,85 +770,7 @@ menu "CHIP Device Layer" endmenu - menu "Time Sync Options" - - config ENABLE_SERVICE_DIRECTORY_TIME_SYNC - bool "Enable Service Directory Time Sync" - default y - help - Enables synchronizing the device real-time clock using information returned during - a CHIP service directory query. For any device that uses the CHIP service directory - to lookup a tunnel server, enabling this option will result in the real time clock being - synchronized every time the service tunnel is established. - - config ENABLE_CHIP_TIME_SERVICE_TIME_SYNC - bool "Enable Time Service Time Sync" - default n - help - Enables synchronizing the device's real time clock with a remote CHIP Time service - using the CHIP Time Sync protocol. - - config CHIP_TIME_SERVICE_ENDPOINT_ID - hex "CHIP Time Service Endpoint Id" - default 18B4300200000005 - depends on ENABLE_CHIP_TIME_SERVICE_TIME_SYNC - help - Specifies the service endpoint id of the CHIP Time Sync service to be used to synchronize time. - - config DEFAULT_TIME_SYNC_INTERVAL - int "Time Sync Interval (seconds)" - default 60 - depends on ENABLE_CHIP_TIME_SERVICE_TIME_SYNC - help - Specifies the minimum interval (in seconds) at which the device should synchronize its real time - clock with the configured CHIP Time Sync server. - - config TIME_SYNC_TIMEOUT - int "Time Sync Timeout (ms)" - default 10000 - depends on ENABLE_CHIP_TIME_SERVICE_TIME_SYNC - help - Specifies the maximum amount of time (in milliseconds) to wait for a response from a - CHIP Time Sync server. - - endmenu - - menu "Service Provisioning Options" - - config SERVICE_PROVISIONING_ENDPOINT_ID - hex "CHIP Service Provisioning Endpoint Id" - default 18B4300200000010 - help - Specifies the service endpoint id of the CHIP Service Provisioning service. When a device - undergoes service provisioning, this is the endpoint to which it will send its Pair Device - to Account request. - - config SERVICE_PROVISIONING_CONNECTIVITY_TIMEOUT - int "Service Provisioning Connectivity Timeout (ms)" - default 10000 - help - The maximum amount of time (in milliseconds) to wait for service connectivity during the device - service provisioning step. More specifically, this is the maximum amount of time the device will - wait for connectivity to be established with the service at the point where the device waiting - to send a Pair Device to Account request to the Service Provisioning service. - - config SERVICE_PROVISIONING_REQUEST_TIMEOUT - int "Service Provisioning Request Timeout (ms)" - default 10000 - help - Specifies the maximum amount of time (in milliseconds) to wait for a response from the Service - Provisioning service. - - endmenu - menu "Commissioning options" - config RENDEZVOUS_WAIT_FOR_COMMISSIONING_COMPLETE - int "Use full IP-based commissioning (expect cluster commands)" - default 0 - help - Setting this to y will cause the commissioner to send commissioning commands to the - various clusters after establishing a PASE session. - config ENABLE_ROTATING_DEVICE_ID depends on ENABLE_CHIPOBLE bool "Enable Rotating Device Identifier Support" @@ -1065,24 +904,6 @@ menu "CHIP Device Layer" The EnableKey in hex string format used by TestEventTrigger command in GeneralDiagnostics cluster. The length of the string should be 32. - config ENABLE_FIXED_TUNNEL_SERVER - bool "Use Fixed Tunnel Server" - default n - help - Forces the use of a service tunnel server at a fixed IP address and port. This - bypasses the need for a directory query to the service directory endpoint to - determine the tunnel server address. When enabled, this option allows devices - that haven't been service provisioned to establish a service tunnel. - - config TUNNEL_SERVER_ADDRESS - string "Tunnel Server Address" - default "" - depends on ENABLE_FIXED_TUNNEL_SERVER - help - The IP address and port of the server to which the device should establish a service tunnel. - The supplied address must be a dot-notation IP address--not a host name. The port number is - optional; if present it should be separated from the IP address with a colon (e.g. 192.168.1.100:5540). - endmenu menu "Network Telemetry Options" diff --git a/examples/all-clusters-app/esp32/sdkconfig.defaults.esp32c6 b/examples/all-clusters-app/esp32/sdkconfig.defaults.esp32c6 index 29a525f2a75994..399eb5901c5c45 100644 --- a/examples/all-clusters-app/esp32/sdkconfig.defaults.esp32c6 +++ b/examples/all-clusters-app/esp32/sdkconfig.defaults.esp32c6 @@ -53,7 +53,6 @@ CONFIG_FREERTOS_ENABLE_BACKWARD_COMPATIBILITY=y # Wi-Fi Settings CONFIG_ENABLE_WIFI_STATION=y -CONFIG_ENABLE_WIFI_AP=n # Enable this to avoid implicit declaration of function 'esp_send_assoc_resp' CONFIG_ESP_WIFI_SOFTAP_SUPPORT=y diff --git a/examples/all-clusters-app/esp32/sdkconfig.defaults.esp32h2 b/examples/all-clusters-app/esp32/sdkconfig.defaults.esp32h2 index f415104a665ec2..c1f43ab77d1207 100644 --- a/examples/all-clusters-app/esp32/sdkconfig.defaults.esp32h2 +++ b/examples/all-clusters-app/esp32/sdkconfig.defaults.esp32h2 @@ -64,7 +64,6 @@ CONFIG_FREERTOS_ENABLE_BACKWARD_COMPATIBILITY=y # Disable STA and AP for ESP32H2 CONFIG_ENABLE_WIFI_STATION=n -CONFIG_ENABLE_WIFI_AP=n # Enable this to avoid implicit declaration of function 'esp_send_assoc_resp' CONFIG_ESP_WIFI_SOFTAP_SUPPORT=y diff --git a/examples/energy-management-app/esp32/sdkconfig.defaults.esp32c6 b/examples/energy-management-app/esp32/sdkconfig.defaults.esp32c6 index 104777a72bb735..6127b910506ab4 100644 --- a/examples/energy-management-app/esp32/sdkconfig.defaults.esp32c6 +++ b/examples/energy-management-app/esp32/sdkconfig.defaults.esp32c6 @@ -53,7 +53,6 @@ CONFIG_FREERTOS_ENABLE_BACKWARD_COMPATIBILITY=y # Wi-Fi Settings CONFIG_ENABLE_WIFI_STATION=y -CONFIG_ENABLE_WIFI_AP=n # Enable this to avoid implicit declaration of function 'esp_send_assoc_resp' CONFIG_ESP_WIFI_SOFTAP_SUPPORT=y diff --git a/examples/energy-management-app/esp32/sdkconfig.defaults.esp32h2 b/examples/energy-management-app/esp32/sdkconfig.defaults.esp32h2 index 11d5991c353b20..eb280c060b2482 100644 --- a/examples/energy-management-app/esp32/sdkconfig.defaults.esp32h2 +++ b/examples/energy-management-app/esp32/sdkconfig.defaults.esp32h2 @@ -67,7 +67,6 @@ CONFIG_FREERTOS_ENABLE_BACKWARD_COMPATIBILITY=y # Disable STA and AP for ESP32H2 CONFIG_ENABLE_WIFI_STATION=n -CONFIG_ENABLE_WIFI_AP=n # Enable this to avoid implicit declaration of function 'esp_send_assoc_resp' CONFIG_ESP_WIFI_SOFTAP_SUPPORT=y diff --git a/examples/lighting-app/esp32/sdkconfig.defaults.esp32c6 b/examples/lighting-app/esp32/sdkconfig.defaults.esp32c6 index 161fdd52aef6d6..c8617158acb9c0 100644 --- a/examples/lighting-app/esp32/sdkconfig.defaults.esp32c6 +++ b/examples/lighting-app/esp32/sdkconfig.defaults.esp32c6 @@ -53,7 +53,6 @@ CONFIG_FREERTOS_ENABLE_BACKWARD_COMPATIBILITY=y # Wi-Fi Settings CONFIG_ENABLE_WIFI_STATION=y -CONFIG_ENABLE_WIFI_AP=n # Enable this to avoid implicit declaration of function 'esp_send_assoc_resp' CONFIG_ESP_WIFI_SOFTAP_SUPPORT=y diff --git a/examples/lighting-app/esp32/sdkconfig.defaults.esp32h2 b/examples/lighting-app/esp32/sdkconfig.defaults.esp32h2 index 11d5991c353b20..eb280c060b2482 100644 --- a/examples/lighting-app/esp32/sdkconfig.defaults.esp32h2 +++ b/examples/lighting-app/esp32/sdkconfig.defaults.esp32h2 @@ -67,7 +67,6 @@ CONFIG_FREERTOS_ENABLE_BACKWARD_COMPATIBILITY=y # Disable STA and AP for ESP32H2 CONFIG_ENABLE_WIFI_STATION=n -CONFIG_ENABLE_WIFI_AP=n # Enable this to avoid implicit declaration of function 'esp_send_assoc_resp' CONFIG_ESP_WIFI_SOFTAP_SUPPORT=y diff --git a/examples/lit-icd-app/esp32/sdkconfig.defaults b/examples/lit-icd-app/esp32/sdkconfig.defaults index 330b6c27699f0b..4ac165c475a260 100644 --- a/examples/lit-icd-app/esp32/sdkconfig.defaults +++ b/examples/lit-icd-app/esp32/sdkconfig.defaults @@ -51,7 +51,6 @@ CONFIG_PARTITION_TABLE_OFFSET=0xC000 # Disable STA and AP for ESP32H2 CONFIG_ENABLE_WIFI_STATION=n -CONFIG_ENABLE_WIFI_AP=n CONFIG_ESP_WIFI_SOFTAP_SUPPORT=n # Disable chip shell diff --git a/examples/lock-app/esp32/sdkconfig.defaults.esp32c6 b/examples/lock-app/esp32/sdkconfig.defaults.esp32c6 index 8e941f423d0a22..a36808e3a4bdc0 100644 --- a/examples/lock-app/esp32/sdkconfig.defaults.esp32c6 +++ b/examples/lock-app/esp32/sdkconfig.defaults.esp32c6 @@ -52,7 +52,6 @@ CONFIG_FREERTOS_ENABLE_BACKWARD_COMPATIBILITY=y # Wi-Fi Settings CONFIG_ENABLE_WIFI_STATION=y -CONFIG_ENABLE_WIFI_AP=n # Enable this to avoid implicit declaration of function 'esp_send_assoc_resp' CONFIG_ESP_WIFI_SOFTAP_SUPPORT=y diff --git a/src/platform/ESP32/CHIPDevicePlatformConfig.h b/src/platform/ESP32/CHIPDevicePlatformConfig.h index 81ee804035d4ab..62665f2022227b 100644 --- a/src/platform/ESP32/CHIPDevicePlatformConfig.h +++ b/src/platform/ESP32/CHIPDevicePlatformConfig.h @@ -121,9 +121,6 @@ #define CHIP_DEVICE_CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MAX CONFIG_BLE_SLOW_ADVERTISING_INTERVAL_MAX #define CHIP_DEVICE_CONFIG_CHIPOBLE_SINGLE_CONNECTION CONFIG_CHIPOBLE_SINGLE_CONNECTION #define CHIP_DEVICE_CONFIG_CHIPOBLE_ENABLE_ADVERTISING_AUTOSTART CONFIG_CHIPOBLE_ENABLE_ADVERTISING_AUTOSTART -#define CHIP_DEVICE_CONFIG_SERVICE_PROVISIONING_ENDPOINT_ID CONFIG_SERVICE_PROVISIONING_ENDPOINT_ID -#define CHIP_DEVICE_CONFIG_SERVICE_PROVISIONING_CONNECTIVITY_TIMEOUT CONFIG_SERVICE_PROVISIONING_CONNECTIVITY_TIMEOUT -#define CHIP_DEVICE_CONFIG_SERVICE_PROVISIONING_REQUEST_TIMEOUT CONFIG_SERVICE_PROVISIONING_REQUEST_TIMEOUT #define CHIP_DEVICE_CONFIG_ENABLE_TEST_SETUP_PARAMS CONFIG_ENABLE_TEST_SETUP_PARAMS #define CHIP_DEVICE_CONFIG_TEST_SERIAL_NUMBER CONFIG_USE_TEST_SERIAL_NUMBER From ddc48d92407fc2669565800a86e1a82e30ed98e0 Mon Sep 17 00:00:00 2001 From: Arkadiusz Bokowy Date: Thu, 19 Dec 2024 09:24:30 +0100 Subject: [PATCH 6/9] Fix CCTRL tests on CI - CCTRL cluster is on endpoint 1 (#36874) * Fix CCTRL tests on CI - CCTRL cluster is on endpoint 1 * Fail CI tests if any test cases were skipped * Add compatibility flag to mobile-device-test.py * Comment out skipped tests * Change executable mode * Improve --test-case command line option * Disable skipped test cases on TC_SWTCH * Disable TC_LVL_2_3 on CI * Make TestBdxTransfer quiet * Verify testing support before running actual tests * Add missing definition --- .github/workflows/tests.yaml | 32 ++++++++++--------- scripts/tests/run_python_test.py | 9 ++++-- .../test/test_scripts/mobile-device-test.py | 5 ++- src/python_testing/TC_CCTRL_2_1.py | 4 +-- src/python_testing/TC_CCTRL_2_2.py | 4 +-- src/python_testing/TC_CCTRL_2_3.py | 4 +-- .../TC_DeviceBasicComposition.py | 29 ++++++++--------- src/python_testing/TC_LVL_2_3.py | 1 + src/python_testing/TC_SWTCH.py | 28 +++++++--------- src/python_testing/TestBdxTransfer.py | 2 +- .../TestMatterTestingSupport.py | 3 ++ src/python_testing/execute_python_tests.py | 1 + .../chip/testing/matter_testing.py | 27 ++++++++-------- src/python_testing/test_metadata.yaml | 6 +++- 14 files changed, 85 insertions(+), 70 deletions(-) mode change 100644 => 100755 src/python_testing/execute_python_tests.py diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index ff8d4d9b37c881..df1cdb03745a99 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -520,24 +520,26 @@ jobs: echo "TRACE_TEST_JSON: out/trace_data/test-{SCRIPT_BASE_NAME}" >> /tmp/test_env.yaml echo "TRACE_TEST_PERFETTO: out/trace_data/test-{SCRIPT_BASE_NAME}" >> /tmp/test_env.yaml + - name: Verify Testing Support + run: | + scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/test_testing/test_IDM_10_4.py' + scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/test_testing/test_TC_ICDM_2_1.py' + scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/test_testing/test_TC_SC_7_1.py' + scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/test_testing/TestDecorators.py' + scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/TestChoiceConformanceSupport.py' + scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/TestConformanceSupport.py' + scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/TestConformanceTest.py' + scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/TestIdChecks.py' + scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/TestMatterTestingSupport.py' + scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/TestSpecParsingDeviceType.py' + scripts/run_in_python_env.sh out/venv 'python3 src/python_testing/TestSpecParsingSupport.py' + - name: Run Tests run: | mkdir -p out/trace_data - scripts/run_in_python_env.sh out/venv './scripts/tests/run_python_test.py --load-from-env /tmp/test_env.yaml --script src/controller/python/test/test_scripts/mobile-device-test.py' - scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/execute_python_tests.py --env-file /tmp/test_env.yaml --search-directory src/python_testing' - scripts/run_in_python_env.sh out/venv './scripts/tests/TestTimeSyncTrustedTimeSourceRunner.py --all-clusters out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app' - scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/TestIdChecks.py' - scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/TestSpecParsingDeviceType.py' - scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/TestConformanceSupport.py' - scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/TestConformanceTest.py' - scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/TestChoiceConformanceSupport.py' - scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/TestMatterTestingSupport.py' - scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/TestSpecParsingSupport.py' - scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/test_testing/test_TC_ICDM_2_1.py' - scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/test_testing/test_IDM_10_4.py' - scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/test_testing/test_TC_SC_7_1.py' - scripts/run_in_python_env.sh out/venv 'python3 ./src/python_testing/test_testing/TestDecorators.py' - + scripts/run_in_python_env.sh out/venv 'scripts/tests/run_python_test.py --load-from-env /tmp/test_env.yaml --script src/controller/python/test/test_scripts/mobile-device-test.py' + scripts/run_in_python_env.sh out/venv 'src/python_testing/execute_python_tests.py --env-file /tmp/test_env.yaml --search-directory src/python_testing' + scripts/run_in_python_env.sh out/venv 'scripts/tests/TestTimeSyncTrustedTimeSourceRunner.py --all-clusters out/linux-x64-all-clusters-ipv6only-no-ble-no-wifi-tsan-clang-test/chip-all-clusters-app' - name: Uploading core files uses: actions/upload-artifact@v4 diff --git a/scripts/tests/run_python_test.py b/scripts/tests/run_python_test.py index c67cbf7d75c93d..aa2aca5500996b 100755 --- a/scripts/tests/run_python_test.py +++ b/scripts/tests/run_python_test.py @@ -215,8 +215,13 @@ def main_impl(app: str, factory_reset: bool, factory_reset_app_only: bool, app_a app_process.p.stdin.close() app_pid = app_process.p.pid - script_command = [script, "--paa-trust-store-path", os.path.join(DEFAULT_CHIP_ROOT, MATTER_DEVELOPMENT_PAA_ROOT_CERTS), - '--log-format', '%(message)s', "--app-pid", str(app_pid)] + shlex.split(script_args) + script_command = [ + script, + "--fail-on-skipped", + "--paa-trust-store-path", os.path.join(DEFAULT_CHIP_ROOT, MATTER_DEVELOPMENT_PAA_ROOT_CERTS), + "--log-format", '%(message)s', + "--app-pid", str(app_pid), + ] + shlex.split(script_args) if script_gdb: # diff --git a/src/controller/python/test/test_scripts/mobile-device-test.py b/src/controller/python/test/test_scripts/mobile-device-test.py index cb032bc3ac744a..794a54a61719fe 100755 --- a/src/controller/python/test/test_scripts/mobile-device-test.py +++ b/src/controller/python/test/test_scripts/mobile-device-test.py @@ -266,8 +266,11 @@ def do_tests(controller_nodeid, device_nodeid, address, timeout, discriminator, type=int, default=0, help="The PID of the app against which the test is going to run") +@click.option('--fail-on-skipped', + is_flag=True, + help="Fail the test if any test cases are skipped") def run(controller_nodeid, device_nodeid, address, timeout, discriminator, setup_pin, enable_test, disable_test, log_level, - log_format, print_test_list, paa_trust_store_path, trace_to, app_pid): + log_format, print_test_list, paa_trust_store_path, trace_to, app_pid, fail_on_skipped): coloredlogs.install(level=log_level, fmt=log_format, logger=logger) if print_test_list: diff --git a/src/python_testing/TC_CCTRL_2_1.py b/src/python_testing/TC_CCTRL_2_1.py index b9cb02e3f0b543..289a572af81981 100644 --- a/src/python_testing/TC_CCTRL_2_1.py +++ b/src/python_testing/TC_CCTRL_2_1.py @@ -31,7 +31,7 @@ # --commissioning-method on-network # --discriminator 1234 # --passcode 20202021 -# --endpoint 0 +# --endpoint 1 # --trace-to json:${TRACE_TEST_JSON}.json # --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto # factory-reset: true @@ -46,7 +46,7 @@ # --commissioning-method on-network # --discriminator 1234 # --passcode 20202021 -# --endpoint 0 +# --endpoint 1 # --trace-to json:${TRACE_TEST_JSON}.json # --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto # factory-reset: true diff --git a/src/python_testing/TC_CCTRL_2_2.py b/src/python_testing/TC_CCTRL_2_2.py index 0e651246230c7a..cf7989184b9015 100644 --- a/src/python_testing/TC_CCTRL_2_2.py +++ b/src/python_testing/TC_CCTRL_2_2.py @@ -31,7 +31,7 @@ # --commissioning-method on-network # --discriminator 1234 # --passcode 20202021 -# --endpoint 0 +# --endpoint 1 # --string-arg th_server_app_path:${ALL_CLUSTERS_APP} # --trace-to json:${TRACE_TEST_JSON}.json # --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto @@ -47,7 +47,7 @@ # --commissioning-method on-network # --discriminator 1234 # --passcode 20202021 -# --endpoint 0 +# --endpoint 1 # --string-arg th_server_app_path:${ALL_CLUSTERS_APP} # --trace-to json:${TRACE_TEST_JSON}.json # --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto diff --git a/src/python_testing/TC_CCTRL_2_3.py b/src/python_testing/TC_CCTRL_2_3.py index 32eecc6369004b..cb17c4c3d7d5ba 100644 --- a/src/python_testing/TC_CCTRL_2_3.py +++ b/src/python_testing/TC_CCTRL_2_3.py @@ -31,7 +31,7 @@ # --commissioning-method on-network # --discriminator 1234 # --passcode 20202021 -# --endpoint 0 +# --endpoint 1 # --string-arg th_server_app_path:${ALL_CLUSTERS_APP} # --trace-to json:${TRACE_TEST_JSON}.json # --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto @@ -47,7 +47,7 @@ # --commissioning-method on-network # --discriminator 1234 # --passcode 20202021 -# --endpoint 0 +# --endpoint 1 # --string-arg th_server_app_path:${ALL_CLUSTERS_APP} # --trace-to json:${TRACE_TEST_JSON}.json # --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto diff --git a/src/python_testing/TC_DeviceBasicComposition.py b/src/python_testing/TC_DeviceBasicComposition.py index 161fa3f07c6aee..a6b4666301f22c 100644 --- a/src/python_testing/TC_DeviceBasicComposition.py +++ b/src/python_testing/TC_DeviceBasicComposition.py @@ -189,7 +189,6 @@ find_tag_list_problems, find_tree_roots, flat_list_ok, get_direct_children_of_root, parts_list_cycles, separate_endpoint_types) from chip.tlv import uint -from mobly import asserts def get_vendor_id(mei: int) -> int: @@ -692,25 +691,25 @@ def test_TC_IDM_11_1(self): if not success: self.fail_current_test("At least one attribute string was not valid UTF-8") - def test_all_event_strings_valid(self): - asserts.skip("TODO: Validate every string in the read events is valid UTF-8 and has no nulls") + # def test_all_event_strings_valid(self): + # asserts.skip("TODO: Validate every string in the read events is valid UTF-8 and has no nulls") - def test_all_schema_scalars(self): - asserts.skip("TODO: Validate all int/uint are in range of the schema (or null if nullable) for known attributes") + # def test_all_schema_scalars(self): + # asserts.skip("TODO: Validate all int/uint are in range of the schema (or null if nullable) for known attributes") - def test_all_commands_reported_are_executable(self): - asserts.skip("TODO: Validate all commands reported in AcceptedCommandList are actually executable") + # def test_all_commands_reported_are_executable(self): + # asserts.skip("TODO: Validate all commands reported in AcceptedCommandList are actually executable") - def test_dump_all_pics_for_all_endpoints(self): - asserts.skip("TODO: Make a test that generates the basic PICS list for each endpoint based on actually reported contents") + # def test_dump_all_pics_for_all_endpoints(self): + # asserts.skip("TODO: Make a test that generates the basic PICS list for each endpoint based on actually reported contents") - def test_all_schema_mandatory_elements_present(self): - asserts.skip( - "TODO: Make a test that ensures every known cluster has the mandatory elements present (commands, attributes) based on features") + # def test_all_schema_mandatory_elements_present(self): + # asserts.skip( + # "TODO: Make a test that ensures every known cluster has the mandatory elements present (commands, attributes) based on features") - def test_all_endpoints_have_valid_composition(self): - asserts.skip( - "TODO: Make a test that verifies each endpoint has valid set of device types, and that the device type conformance is respected for each") + # def test_all_endpoints_have_valid_composition(self): + # asserts.skip( + # "TODO: Make a test that verifies each endpoint has valid set of device types, and that the device type conformance is respected for each") def test_TC_SM_1_2(self): self.print_step(1, "Wildcard read of device - already done") diff --git a/src/python_testing/TC_LVL_2_3.py b/src/python_testing/TC_LVL_2_3.py index 9f223677796751..32137ce28153ce 100644 --- a/src/python_testing/TC_LVL_2_3.py +++ b/src/python_testing/TC_LVL_2_3.py @@ -18,6 +18,7 @@ # See https://github.com/project-chip/connectedhomeip/blob/master/docs/testing/python.md#defining-the-ci-test-arguments # for details about the block below. # +# FIXME: https://github.com/project-chip/connectedhomeip/issues/36885 # === BEGIN CI TEST ARGUMENTS === # test-runner-runs: # run1: diff --git a/src/python_testing/TC_SWTCH.py b/src/python_testing/TC_SWTCH.py index e59842b056d5bc..0c7e74c69a094e 100644 --- a/src/python_testing/TC_SWTCH.py +++ b/src/python_testing/TC_SWTCH.py @@ -17,12 +17,15 @@ # See https://github.com/project-chip/connectedhomeip/blob/master/docs/testing/python.md#defining-the-ci-test-arguments # for details about the block below. # +# TODO: https://github.com/project-chip/connectedhomeip/issues/36884 +# # === BEGIN CI TEST ARGUMENTS === # test-runner-runs: # run1: # app: ${ALL_CLUSTERS_APP} # app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json # script-args: > +# --test-case test_TC_SWTCH_2_2 # --endpoint 1 # --storage-path admin_storage.json # --commissioning-method on-network @@ -37,20 +40,9 @@ # app: ${ALL_CLUSTERS_APP} # app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json # script-args: > -# --endpoint 2 -# --storage-path admin_storage.json -# --commissioning-method on-network -# --discriminator 1234 -# --passcode 20202021 -# --trace-to json:${TRACE_TEST_JSON}.json -# --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto -# --PICS src/app/tests/suites/certification/ci-pics-values -# factory-reset: true -# quiet: true -# run3: -# app: ${ALL_CLUSTERS_APP} -# app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json -# script-args: > +# --test-case test_TC_SWTCH_2_3 +# --test-case test_TC_SWTCH_2_4 +# --test-case test_TC_SWTCH_2_6 # --endpoint 3 # --storage-path admin_storage.json # --commissioning-method on-network @@ -61,10 +53,13 @@ # --PICS src/app/tests/suites/certification/ci-pics-values # factory-reset: true # quiet: true -# run4: +# run3: # app: ${ALL_CLUSTERS_APP} # app-args: --discriminator 1234 --KVS kvs1 --trace-to json:${TRACE_APP}.json # script-args: > +# --test-case test_TC_SWTCH_2_3 +# --test-case test_TC_SWTCH_2_4 +# --test-case test_TC_SWTCH_2_5 # --endpoint 4 # --storage-path admin_storage.json # --commissioning-method on-network @@ -281,7 +276,8 @@ def _expect_no_events_for_cluster(self, event_queue: queue.Queue, endpoint_id: i elapsed = 0.0 time_remaining = timeout_sec - logging.info(f"Waiting {timeout_sec:.1f} seconds for no more events for cluster {expected_cluster} on endpoint {endpoint_id}") + logging.info(f"Waiting {timeout_sec:.1f} seconds for no more events for " + f"cluster {expected_cluster} on endpoint {endpoint_id}") while time_remaining > 0: try: item: EventReadResult = event_queue.get(block=True, timeout=time_remaining) diff --git a/src/python_testing/TestBdxTransfer.py b/src/python_testing/TestBdxTransfer.py index bd173103bf4c49..b0bac7213d122f 100644 --- a/src/python_testing/TestBdxTransfer.py +++ b/src/python_testing/TestBdxTransfer.py @@ -32,7 +32,7 @@ # --trace-to json:${TRACE_TEST_JSON}.json # --trace-to perfetto:${TRACE_TEST_PERFETTO}.perfetto # factory-reset: true -# quiet: false +# quiet: true # === END CI TEST ARGUMENTS === import asyncio diff --git a/src/python_testing/TestMatterTestingSupport.py b/src/python_testing/TestMatterTestingSupport.py index d39c869456e8f9..f111250481032d 100644 --- a/src/python_testing/TestMatterTestingSupport.py +++ b/src/python_testing/TestMatterTestingSupport.py @@ -640,6 +640,8 @@ def test_xml_pics(self): def test_parse_matter_test_args(self): args = [ + # Verify that it is possible to pass multiple test cases at once + "--tests", "TC_1", "TC_2", # Verify that values are appended to a single argument "--int-arg", "PIXIT.TEST.DEC:42", "--int-arg", "PIXIT.TEST.HEX:0x1234", @@ -650,6 +652,7 @@ def test_parse_matter_test_args(self): ] parsed = parse_matter_test_args(args) + asserts.assert_equal(parsed.tests, ["TC_1", "TC_2"]) asserts.assert_equal(parsed.global_test_params.get("PIXIT.TEST.DEC"), 42) asserts.assert_equal(parsed.global_test_params.get("PIXIT.TEST.HEX"), 0x1234) asserts.assert_equal(parsed.global_test_params.get("PIXIT.TEST.STR.MULTI.1"), "foo") diff --git a/src/python_testing/execute_python_tests.py b/src/python_testing/execute_python_tests.py old mode 100644 new mode 100755 index c85a02bc40952e..e0f962a99981e3 --- a/src/python_testing/execute_python_tests.py +++ b/src/python_testing/execute_python_tests.py @@ -1,3 +1,4 @@ +#!/usr/bin/env -S python3 -B # # Copyright (c) 2024 Project CHIP Authors # All rights reserved. diff --git a/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py b/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py index 0ad55369e106cb..ec43db186eae6d 100644 --- a/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py +++ b/src/python_testing/matter_testing_infrastructure/chip/testing/matter_testing.py @@ -628,6 +628,7 @@ class MatterTestConfig: timeout: typing.Union[int, None] = None endpoint: typing.Union[int, None] = 0 app_pid: int = 0 + fail_on_skipped_tests: bool = False commissioning_method: Optional[str] = None discriminators: List[int] = field(default_factory=list) @@ -1932,10 +1933,11 @@ def convert_args_to_matter_config(args: argparse.Namespace) -> MatterTestConfig: config.paa_trust_store_path = args.paa_trust_store_path config.ble_interface_id = args.ble_interface_id config.pics = {} if args.PICS is None else read_pics_from_file(args.PICS) - config.tests = [] if args.tests is None else args.tests + config.tests = list(chain.from_iterable(args.tests or [])) config.timeout = args.timeout # This can be none, we pull the default from the test if it's unspecified config.endpoint = args.endpoint # This can be None, the get_endpoint function allows the tests to supply a default config.app_pid = 0 if args.app_pid is None else args.app_pid + config.fail_on_skipped_tests = args.fail_on_skipped config.controller_node_id = args.controller_node_id config.trace_to = args.trace_to @@ -1962,13 +1964,10 @@ def parse_matter_test_args(argv: Optional[List[str]] = None) -> MatterTestConfig basic_group = parser.add_argument_group(title="Basic arguments", description="Overall test execution arguments") - basic_group.add_argument('--tests', - '--test_case', - action="store", - nargs='+', - type=str, - metavar='test_a test_b...', + basic_group.add_argument('--tests', '--test-case', action='append', nargs='+', type=str, metavar='test_NAME', help='A list of tests in the test class to execute.') + basic_group.add_argument('--fail-on-skipped', action="store_true", default=False, + help="Fail the test if any test cases are skipped") basic_group.add_argument('--trace-to', nargs="*", default=[], help="Where to trace (e.g perfetto, perfetto:path, json:log, json:path)") basic_group.add_argument('--storage-path', action="store", type=pathlib.Path, @@ -2056,17 +2055,17 @@ def parse_matter_test_args(argv: Optional[List[str]] = None) -> MatterTestConfig help='Path to chip-tool credentials file root') args_group = parser.add_argument_group(title="Config arguments", description="Test configuration global arguments set") - args_group.add_argument('--int-arg', nargs='*', action='append', type=int_named_arg, metavar="NAME:VALUE", + args_group.add_argument('--int-arg', nargs='+', action='append', type=int_named_arg, metavar="NAME:VALUE", help="Add a named test argument for an integer as hex or decimal (e.g. -2 or 0xFFFF_1234)") - args_group.add_argument('--bool-arg', nargs='*', action='append', type=bool_named_arg, metavar="NAME:VALUE", + args_group.add_argument('--bool-arg', nargs='+', action='append', type=bool_named_arg, metavar="NAME:VALUE", help="Add a named test argument for an boolean value (e.g. true/false or 0/1)") - args_group.add_argument('--float-arg', nargs='*', action='append', type=float_named_arg, metavar="NAME:VALUE", + args_group.add_argument('--float-arg', nargs='+', action='append', type=float_named_arg, metavar="NAME:VALUE", help="Add a named test argument for a floating point value (e.g. -2.1 or 6.022e23)") - args_group.add_argument('--string-arg', nargs='*', action='append', type=str_named_arg, metavar="NAME:VALUE", + args_group.add_argument('--string-arg', nargs='+', action='append', type=str_named_arg, metavar="NAME:VALUE", help="Add a named test argument for a string value") - args_group.add_argument('--json-arg', nargs='*', action='append', type=json_named_arg, metavar="NAME:VALUE", + args_group.add_argument('--json-arg', nargs='+', action='append', type=json_named_arg, metavar="NAME:VALUE", help="Add a named test argument for JSON stored as a list or dict") - args_group.add_argument('--hex-arg', nargs='*', action='append', type=bytes_as_hex_named_arg, metavar="NAME:VALUE", + args_group.add_argument('--hex-arg', nargs='+', action='append', type=bytes_as_hex_named_arg, metavar="NAME:VALUE", help="Add a named test argument for an octet string in hex (e.g. 0011cafe or 00:11:CA:FE)") if not argv: @@ -2510,6 +2509,8 @@ def run_tests_no_exit(test_class: MatterBaseTest, matter_test_config: MatterTest try: runner.run() ok = runner.results.is_all_pass and ok + if matter_test_config.fail_on_skipped_tests and runner.results.skipped: + ok = False except TimeoutError: ok = False except signals.TestAbortAll: diff --git a/src/python_testing/test_metadata.yaml b/src/python_testing/test_metadata.yaml index dcabdeffdadb1d..dd5f5e3af49b5f 100644 --- a/src/python_testing/test_metadata.yaml +++ b/src/python_testing/test_metadata.yaml @@ -9,6 +9,10 @@ not_automated: reason: src/python_testing/test_testing/test_TC_DGGEN_3_2.py is the Unit test of this test + - name: TC_LVL_2_3.py + reason: + The microwave-oven-app does not have level control cluster on endpoint + 1 - https://github.com/project-chip/connectedhomeip/issues/36885 - name: TC_EEVSE_Utils.py reason: Shared code for TC_EEVSE, not a standalone test - name: TC_EWATERHTRBase.py @@ -74,7 +78,7 @@ not_automated: - name: test_plan_table_generator.py reason: Code/Test not being used or not shared code for any other tests -# This is a list of slow tests (just arbitrarely picked around 20 seconds) +# This is a list of slow tests (just arbitrarily picked around 20 seconds) # used in some script reporting for "be patient" messages as well as potentially # to consider improving. May not be exhaustive slow_tests: From aac3e3f8810570e494e50e9aae10d078e4f4bb5f Mon Sep 17 00:00:00 2001 From: C Freeman Date: Thu, 19 Dec 2024 04:27:53 -0500 Subject: [PATCH 7/9] TC-VALCC: Update wording on test steps (#36860) * TC-VALCC: Update wording on test steps Test: Tests are all run in repl CI. * You know what really helps? Adding all your changes before pushing smooth moves, freeman. --- src/python_testing/TC_VALCC_3_1.py | 6 +++--- src/python_testing/TC_VALCC_3_2.py | 15 ++++++++------- src/python_testing/TC_VALCC_3_3.py | 6 +++--- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/python_testing/TC_VALCC_3_1.py b/src/python_testing/TC_VALCC_3_1.py index 295c312c37f0cb..14da923f40ad58 100644 --- a/src/python_testing/TC_VALCC_3_1.py +++ b/src/python_testing/TC_VALCC_3_1.py @@ -49,15 +49,15 @@ def desc_TC_VALCC_3_1(self) -> str: def steps_TC_VALCC_3_1(self) -> list[TestStep]: steps = [ - TestStep(1, "Commissioning, already done", is_commissioning=True), + TestStep(1, "Commission DUT if required", is_commissioning=True), TestStep(2, "Set up a subscription to all attributes on the DUT"), TestStep(3, "Send a close command to the DUT and wait until the CurrentState is closed", "DUT returns SUCCESS"), TestStep(4, "Send Open command", "DUT returns SUCCESS"), - TestStep(5, "Wait until TH receives and data report for TargetState set to NULL and an attribute report for CurrentState set to Open (ordering does not matter)", + TestStep(5, "Wait until TH receives the following reports (ordering does not matter): TargetState set to NULL, CurrentState set to Open", "Expected attribute reports are received"), TestStep(6, "Read CurrentState and TargetState attribute", "CurrentState is Open, TargetState is NULL"), TestStep(7, "Send Close command", "DUT returns SUCCESS"), - TestStep(8, "Wait until TH receives and data report for TargetState set to NULL and an attribute report for CurrentState set to Closed (ordering does not matter)", + TestStep(8, "Wait until TH receives the following reports (ordering does not matter): TargetState set to NULL, CurrentState set to Closed", "Expected attribute reports are received"), TestStep(9, "Read CurrentState and TargetState attribute", "CurrentState is Closed, TargetState is NULL"), ] diff --git a/src/python_testing/TC_VALCC_3_2.py b/src/python_testing/TC_VALCC_3_2.py index 5e22cda0ef39f0..9beeefe339a770 100644 --- a/src/python_testing/TC_VALCC_3_2.py +++ b/src/python_testing/TC_VALCC_3_2.py @@ -51,19 +51,20 @@ def desc_TC_VALCC_3_2(self) -> str: def steps_TC_VALCC_3_2(self) -> list[TestStep]: steps = [ - TestStep(1, "Commissioning, already done", is_commissioning=True), + TestStep(1, "Commission DUT if required", is_commissioning=True), TestStep(2, "Set up a subscription to all attributes on the DUT"), TestStep(3, "Send a close command to the DUT and wait until the CurrentState is closed", "DUT returns SUCCESS"), TestStep(4, "TH sends command Open command with TargetLevel field set to 100 and the remaining fields not populated.", "Verify DUT responds w/ status SUCCESS(0x00)."), - TestStep(5, "Wait until TH receives data reports for TargetState set to NULL, TargetLevel set to NULL, CurrentState set to Open and CurrentLevel set to 100 (ordering does not matter)", - "Expected attribute reports are received"), + TestStep(5, "Wait until TH receives the following reports (ordering does not matter): TargetState set to NULL, TargetLevel set to NULL, CurrentState set to Open, CurrentLevel set to 100", + "Expected reports are received"), TestStep(6, "Read CurrentState, CurrentLevel, TargetState and TargetLevel attributes", - "CurrentState is Open, CurrentLevel is 100, TargetState is NULL and TargetLevel is NULL"), + "CurrentState is Open, CurrentLevel is 100, TargetState is NULL, TargetLevel is NULL"), TestStep(7, "Send Close command", "DUT returns SUCCESS"), - TestStep(8, "Wait until TH receives and data report for TargetState set to NULL and an attribute report for CurrentState set to Closed (ordering does not matter)", - "Expected attribute reports are received"), - TestStep(9, "Read CurrentState and TargetState attribute", "CurrentState is Closed, TargetState is NULL"), + TestStep(8, "Wait until TH receives the following reports (ordering does not matter): TargetState set to NULL, TargetLevel set to NULL, CurrentState set to Closed, CurrentLevel set to 0", + "Expected reports are received"), + TestStep(9, "Read CurrentState, CurrentLevel, TargetState and TargetLevel attributes", + "CurrentState is Closed, CurrentLevel is 0, TargetState is NULL, TargetLevel is NULL"), ] return steps diff --git a/src/python_testing/TC_VALCC_3_3.py b/src/python_testing/TC_VALCC_3_3.py index 2d962c22cb651a..0b5cffa8896eb0 100644 --- a/src/python_testing/TC_VALCC_3_3.py +++ b/src/python_testing/TC_VALCC_3_3.py @@ -49,7 +49,7 @@ def desc_TC_VALCC_3_3(self) -> str: def steps_TC_VALCC_3_3(self) -> list[TestStep]: steps = [ - TestStep(1, "Commissioning, already done", is_commissioning=True), + TestStep(1, "Commission DUT if required", is_commissioning=True), TestStep(2, "Read AttributeList attribute", "Verify that the DUT response contains the AttributeList attribute."), TestStep(3, "If the DefaultOpenLevel is not supported, skip all remaining steps in this test"), TestStep(4, "TH reads from the DUT the DefaultOpenLevel attribute. Store the value as defaultOpenLevel."), @@ -57,12 +57,12 @@ def steps_TC_VALCC_3_3(self) -> list[TestStep]: TestStep(6, "Send a close command to the DUT and wait until the CurrentState is reported as closed", "DUT returns SUCCESS"), # TODO: this test should probably SET the default open attribute as well and un-set it at the end, so we're not testing against the default. TestStep(7, "Send Open command with no fields populated", "DUT returns SUCCESS"), - TestStep(8, "Wait until TH receives the following data reports (ordering not checked): TargetState set to NULL, TargetLevel set to NULL, CurrentState set to Open, CurrentLevel set to defaultOpenLevel", + TestStep(8, "Wait until TH receives the following reports (ordering does not matter): TargetState set to NULL, TargetLevel set to NULL, CurrentState set to Open, CurrentLevel set to defaultOpenLevel", "Expected attribute reports are received"), TestStep(9, "Read CurrentState and TargetState attribute", "CurrentState is Open, TargetState is NULL"), TestStep(10, "Read CurrentLevel and TargetLevel attribute", "CurrentLevel is defaultOpenLevel, TargetLevel is NULL"), TestStep(11, "Send Close command", "DUT returns SUCCESS"), - TestStep(12, "Wait until TH receives the following data reports (ordering not checked): TargetState set to NULL, TargetLevel set to NULL, CurrentState set to Closed, CurrentLevel set to 0", + TestStep(12, "Wait until TH receives the following reports (ordering does not matter): TargetState set to NULL, TargetLevel set to NULL, CurrentState set to Closed, CurrentLevel set to 0", "Expected attribute reports are received"), TestStep(13, "Read CurrentState and TargetState attribute", "CurrentState is Closed, TargetState is NULL"), TestStep(14, "Read CurrentLevel and TargetLevel attribute", "CurrentLevel is 0, TargetLevel is NULL"), From 9b4181938593419f0be071baffa355a3b1a882bf Mon Sep 17 00:00:00 2001 From: Karsten Sperling <113487422+ksperling-apple@users.noreply.github.com> Date: Fri, 20 Dec 2024 03:49:01 +1300 Subject: [PATCH 8/9] Simplify AutoCommissioner::SetCommissioningParameters (#36896) Unconditionally clear members that point to buffers, and handle them via the explicit code that copies the pointed-to data. The logic to work out whether any pointers need to be cleared was quite complicated. Use memmove() instead of memcpy() since src and dst may overlap / be identical. Always assign mNeedIcdRegistration when the kReadCommissioningInfo step finishes, instead of relying on SetCommissioningParameters to clear it. --- src/controller/AutoCommissioner.cpp | 84 ++++++----------------------- src/lib/support/Span.h | 9 ++-- 2 files changed, 22 insertions(+), 71 deletions(-) diff --git a/src/controller/AutoCommissioner.cpp b/src/controller/AutoCommissioner.cpp index a373b6b34af13f..5e9e37461dec1a 100644 --- a/src/controller/AutoCommissioner.cpp +++ b/src/controller/AutoCommissioner.cpp @@ -49,25 +49,6 @@ void AutoCommissioner::SetOperationalCredentialsDelegate(OperationalCredentialsD mOperationalCredentialsDelegate = operationalCredentialsDelegate; } -// Returns true if maybeUnsafeSpan is pointing to a buffer that we're not sure -// will live for long enough. knownSafeSpan, if it has a value, points to a -// buffer that we _are_ sure will live for long enough. -template -static bool IsUnsafeSpan(const Optional & maybeUnsafeSpan, const Optional & knownSafeSpan) -{ - if (!maybeUnsafeSpan.HasValue()) - { - return false; - } - - if (!knownSafeSpan.HasValue()) - { - return true; - } - - return maybeUnsafeSpan.Value().data() != knownSafeSpan.Value().data(); -} - CHIP_ERROR AutoCommissioner::VerifyICDRegistrationInfo(const CommissioningParameters & params) { ChipLogProgress(Controller, "Checking ICD registration parameters"); @@ -101,44 +82,16 @@ CHIP_ERROR AutoCommissioner::VerifyICDRegistrationInfo(const CommissioningParame CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParameters & params) { - // Make sure any members that point to buffers that we are not pointing to - // our own buffers are not going to dangle. We can skip this step if all - // the buffers pointers that we don't plan to re-point to our own buffers - // below are already pointing to the same things as our own buffer pointers - // (so that we know they have to be safe somehow). - // - // The checks are a bit painful, because Span does not have a usable - // operator==, and in any case, we want to compare for pointer equality, not - // data equality. - bool haveMaybeDanglingBufferPointers = - ((params.GetNOCChainGenerationParameters().HasValue() && - (!mParams.GetNOCChainGenerationParameters().HasValue() || - params.GetNOCChainGenerationParameters().Value().nocsrElements.data() != - mParams.GetNOCChainGenerationParameters().Value().nocsrElements.data() || - params.GetNOCChainGenerationParameters().Value().signature.data() != - mParams.GetNOCChainGenerationParameters().Value().signature.data())) || - IsUnsafeSpan(params.GetRootCert(), mParams.GetRootCert()) || IsUnsafeSpan(params.GetNoc(), mParams.GetNoc()) || - IsUnsafeSpan(params.GetIcac(), mParams.GetIcac()) || IsUnsafeSpan(params.GetIpk(), mParams.GetIpk()) || - IsUnsafeSpan(params.GetAttestationElements(), mParams.GetAttestationElements()) || - IsUnsafeSpan(params.GetAttestationSignature(), mParams.GetAttestationSignature()) || - IsUnsafeSpan(params.GetPAI(), mParams.GetPAI()) || IsUnsafeSpan(params.GetDAC(), mParams.GetDAC()) || - IsUnsafeSpan(params.GetTimeZone(), mParams.GetTimeZone()) || - IsUnsafeSpan(params.GetDSTOffsets(), mParams.GetDSTOffsets()) || - IsUnsafeSpan(params.GetICDSymmetricKey(), mParams.GetICDSymmetricKey()) || - (params.GetDefaultNTP().HasValue() && !params.GetDefaultNTP().Value().IsNull() && - params.GetDefaultNTP().Value().Value().data() != mDefaultNtp)); - + // Our logic below assumes that we can modify mParams without affecting params. + VerifyOrReturnError(¶ms != &mParams, CHIP_NO_ERROR); + + // Copy the whole struct (scalars and pointers), but clear any members that might point to + // external buffers. For those members we have to copy the data over into our own buffers below. + // Note that all of the copy operations use memmove() instead of memcpy(), because the caller + // may be passing a modified shallow copy of our CommissioningParmeters, i.e. where various spans + // already point into the buffers we're copying into, and memcpy() with overlapping buffers is UB. mParams = params; - - mNeedIcdRegistration = false; - - if (haveMaybeDanglingBufferPointers) - { - mParams.ClearExternalBufferDependentValues(); - } - - // For members of params that point to some sort of buffer, we have to copy - // the data over into our own buffers. + mParams.ClearExternalBufferDependentValues(); if (params.GetThreadOperationalDataset().HasValue()) { @@ -146,11 +99,9 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam if (dataset.size() > CommissioningParameters::kMaxThreadDatasetLen) { ChipLogError(Controller, "Thread operational data set is too large"); - // Make sure our buffer pointers don't dangle. - mParams.ClearExternalBufferDependentValues(); return CHIP_ERROR_INVALID_ARGUMENT; } - memcpy(mThreadOperationalDataset, dataset.data(), dataset.size()); + memmove(mThreadOperationalDataset, dataset.data(), dataset.size()); ChipLogProgress(Controller, "Setting thread operational dataset from parameters"); mParams.SetThreadOperationalDataset(ByteSpan(mThreadOperationalDataset, dataset.size())); } @@ -162,12 +113,10 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam creds.credentials.size() > CommissioningParameters::kMaxCredentialsLen) { ChipLogError(Controller, "Wifi credentials are too large"); - // Make sure our buffer pointers don't dangle. - mParams.ClearExternalBufferDependentValues(); return CHIP_ERROR_INVALID_ARGUMENT; } - memcpy(mSsid, creds.ssid.data(), creds.ssid.size()); - memcpy(mCredentials, creds.credentials.data(), creds.credentials.size()); + memmove(mSsid, creds.ssid.data(), creds.ssid.size()); + memmove(mCredentials, creds.credentials.data(), creds.credentials.size()); ChipLogProgress(Controller, "Setting wifi credentials from parameters"); mParams.SetWiFiCredentials( WiFiCredentials(ByteSpan(mSsid, creds.ssid.size()), ByteSpan(mCredentials, creds.credentials.size()))); @@ -184,8 +133,6 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam else { ChipLogError(Controller, "Country code is too large: %u", static_cast(code.size())); - // Make sure our buffer pointers don't dangle. - mParams.ClearExternalBufferDependentValues(); return CHIP_ERROR_INVALID_ARGUMENT; } } @@ -195,7 +142,7 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam { ChipLogProgress(Controller, "Setting attestation nonce from parameters"); VerifyOrReturnError(params.GetAttestationNonce().Value().size() == sizeof(mAttestationNonce), CHIP_ERROR_INVALID_ARGUMENT); - memcpy(mAttestationNonce, params.GetAttestationNonce().Value().data(), params.GetAttestationNonce().Value().size()); + memmove(mAttestationNonce, params.GetAttestationNonce().Value().data(), params.GetAttestationNonce().Value().size()); } else { @@ -208,7 +155,7 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam { ChipLogProgress(Controller, "Setting CSR nonce from parameters"); VerifyOrReturnError(params.GetCSRNonce().Value().size() == sizeof(mCSRNonce), CHIP_ERROR_INVALID_ARGUMENT); - memcpy(mCSRNonce, params.GetCSRNonce().Value().data(), params.GetCSRNonce().Value().size()); + memmove(mCSRNonce, params.GetCSRNonce().Value().data(), params.GetCSRNonce().Value().size()); } else { @@ -271,7 +218,7 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam ReturnErrorOnFailure(VerifyICDRegistrationInfo(params)); // The values must be valid now. - memcpy(mICDSymmetricKey, params.GetICDSymmetricKey().Value().data(), params.GetICDSymmetricKey().Value().size()); + memmove(mICDSymmetricKey, params.GetICDSymmetricKey().Value().data(), params.GetICDSymmetricKey().Value().size()); mParams.SetICDSymmetricKey(ByteSpan(mICDSymmetricKey)); mParams.SetICDCheckInNodeId(params.GetICDCheckInNodeId().Value()); mParams.SetICDMonitoredSubject(params.GetICDMonitoredSubject().Value()); @@ -787,6 +734,7 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio } } + mNeedIcdRegistration = false; if (mParams.GetICDRegistrationStrategy() != ICDRegistrationStrategy::kIgnore) { if (mDeviceCommissioningInfo.icd.isLIT && mDeviceCommissioningInfo.icd.checkInProtocolSupport) diff --git a/src/lib/support/Span.h b/src/lib/support/Span.h index bba719e69e779c..18b077659be047 100644 --- a/src/lib/support/Span.h +++ b/src/lib/support/Span.h @@ -374,7 +374,8 @@ inline CHIP_ERROR CopySpanToMutableSpan(ByteSpan span_to_copy, MutableByteSpan & { VerifyOrReturnError(out_buf.size() >= span_to_copy.size(), CHIP_ERROR_BUFFER_TOO_SMALL); - memcpy(out_buf.data(), span_to_copy.data(), span_to_copy.size()); + // There is no guarantee that span_to_copy and out_buf don't overlap, so use memmove() + memmove(out_buf.data(), span_to_copy.data(), span_to_copy.size()); out_buf.reduce_size(span_to_copy.size()); return CHIP_NO_ERROR; @@ -384,7 +385,8 @@ inline CHIP_ERROR CopyCharSpanToMutableCharSpan(CharSpan cspan_to_copy, MutableC { VerifyOrReturnError(out_buf.size() >= cspan_to_copy.size(), CHIP_ERROR_BUFFER_TOO_SMALL); - memcpy(out_buf.data(), cspan_to_copy.data(), cspan_to_copy.size()); + // There is no guarantee that cspan_to_copy and out_buf don't overlap, so use memmove() + memmove(out_buf.data(), cspan_to_copy.data(), cspan_to_copy.size()); out_buf.reduce_size(cspan_to_copy.size()); return CHIP_NO_ERROR; @@ -400,7 +402,8 @@ inline void CopyCharSpanToMutableCharSpanWithTruncation(CharSpan span_to_copy, M { size_t size_to_copy = std::min(span_to_copy.size(), out_span.size()); - memcpy(out_span.data(), span_to_copy.data(), size_to_copy); + // There is no guarantee that span_to_copy and out_buf don't overlap, so use memmove() + memmove(out_span.data(), span_to_copy.data(), size_to_copy); out_span.reduce_size(size_to_copy); } From b880c14474b2b069caa73bc2ac50eb4371e9c31d Mon Sep 17 00:00:00 2001 From: Junior Martinez <67972863+jmartinez-silabs@users.noreply.github.com> Date: Thu, 19 Dec 2024 10:51:07 -0500 Subject: [PATCH 9/9] Bump sdk versions in Silabs docker. Add slc_cli in env paths of vscode docker (#36888) --- integrations/docker/images/base/chip-build/version | 2 +- .../docker/images/stage-2/chip-build-efr32/Dockerfile | 8 ++++---- .../docker/images/vscode/chip-build-vscode/Dockerfile | 1 + 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/integrations/docker/images/base/chip-build/version b/integrations/docker/images/base/chip-build/version index 57a32402f63ff2..842d3bdfda0653 100644 --- a/integrations/docker/images/base/chip-build/version +++ b/integrations/docker/images/base/chip-build/version @@ -1 +1 @@ -94 : [Telink] Update Docker image (Zephyr update) +95 : [Silabs] Update Silabs sisdk to v2024.12.0 and wifisdk to 3.4.0 diff --git a/integrations/docker/images/stage-2/chip-build-efr32/Dockerfile b/integrations/docker/images/stage-2/chip-build-efr32/Dockerfile index 3d8c0ae774c782..eb67c80ff9a080 100644 --- a/integrations/docker/images/stage-2/chip-build-efr32/Dockerfile +++ b/integrations/docker/images/stage-2/chip-build-efr32/Dockerfile @@ -13,8 +13,8 @@ RUN set -x \ && : # last line -# Download Simplicity SDK v2024.6.2 (36e12f0) -RUN wget https://github.com/SiliconLabs/simplicity_sdk/releases/download/v2024.6.2/gecko-sdk.zip -O /tmp/simplicity_sdk.zip \ +# Download Simplicity SDK v2024.12.0 (8627f84) +RUN wget https://github.com/SiliconLabs/simplicity_sdk/releases/download/v2024.12.0/gecko-sdk.zip -O /tmp/simplicity_sdk.zip \ && unzip /tmp/simplicity_sdk.zip -d /tmp/simplicity_sdk \ && rm -rf /tmp/simplicity_sdk.zip \ # Deleting files that are not needed to save space @@ -35,8 +35,8 @@ RUN git clone --depth=1 --single-branch --branch=2.10.3 https://github.com/Silic rm -rf .git examples \ && : # last line -# Clone WiSeConnect SDK v3.3.3 (a6390dd) -RUN git clone --depth=1 --single-branch --branch=v3.3.3 https://github.com/SiliconLabs/wiseconnect.git /tmp/wifi_sdk && \ +# Clone WiSeConnect SDK v3.4.0 (9f6db89) +RUN git clone --depth=1 --single-branch --branch=v3.4.0 https://github.com/SiliconLabs/wiseconnect.git /tmp/wifi_sdk && \ cd /tmp/wifi_sdk && \ rm -rf .git examples components/device/stm32 \ && : # last line diff --git a/integrations/docker/images/vscode/chip-build-vscode/Dockerfile b/integrations/docker/images/vscode/chip-build-vscode/Dockerfile index b550bea7d90eac..a9a2c9d690e24a 100644 --- a/integrations/docker/images/vscode/chip-build-vscode/Dockerfile +++ b/integrations/docker/images/vscode/chip-build-vscode/Dockerfile @@ -113,6 +113,7 @@ ENV SILABS_BOARD=BRD4186C # Keep GSDK_ROOT name until rename transition to SISDK is completed ENV GSDK_ROOT=/opt/silabs/simplicity_sdk/ ENV SISDK_ROOT=/opt/silabs/simplicity_sdk/ +ENV PATH $PATH:/opt/silabs/slc_cli/ ENV WISECONNECT_SDK_ROOT=/opt/silabs/wiseconnect-wifi-bt-sdk/ ENV WIFI_SDK_ROOT=/opt/silabs/wifi_sdk ENV IDF_PATH=/opt/espressif/esp-idf/