[SL-ONLY] MATTER-7023: Changes as per feedback on Alpha1 (#1165)

* Added conditional compilation for HTTP client feature under dual-stack settings and improved formatting for clarity.

* Update response handling to utilize mEndOfData for improved clarity in data state management.

* Refactor MQTT client header and implementation to improve clarity by moving the MQTT_USE_HOST_LWIP_TLS definition to the header file and removing it from the source file. This change enhances code organization and maintains consistent configuration management.

* Enhance documentation in matter_service.h to clarify the purpose of the initialization delay for Matter services, ensuring the network is fully established before service startup.

* Updating MQTT HEAP to 60K as per expectation

* Refactor HTTPS client configuration to use default certificate index for improved clarity and maintainability. Removed hardcoded certificate index in the demo example.

* Revert the RAM changes

* Move to AppTask instead of BaseApplicationTask

* Enhance MQTT client functionality by adding idle yield management and subscription callback for incoming messages. Updated the MQTT service thread to handle idle conditions more effectively, ensuring timely keepalive and message processing. Improved logging for MQTT operations to provide clearer feedback on session status.

* Replaced direct disconnection flag manipulation with a dedicated ProcessDisconnect function for better clarity and maintainability.

* Enhance MQTT client by adding comments to clarify idle yield timeout logic and its default value. Ensure that idle yield timeout does not exceed the keep-alive interval, improving connection stability and maintainability.

* Adding a TODO comment to revisit idle yield timeout calculations. This aims to streamline the connection keep-alive process by suggesting a more efficient invocation of the MQTTYield function.

* Improve error handling in MQTT disconnection process by logging failures in ProcessDisconnect during idle yield and yield operations. This enhances debugging capabilities and ensures better visibility of connection issues.

* [PATCH] Restyled by whitespace

Author: rosahay-silabs

examples/platform/silabs/BaseApplication.cpp

@@ -89,20 +89,6 @@
 #include "MatterAwsControl.h"
 #endif // SL_MATTER_ENABLE_AWS
 
-#if defined(SL_MATTER_ENABLE_SERVICES) && SL_MATTER_ENABLE_SERVICES
-
-#include <platform/silabs/services/matter_service.h>
-
-#if defined(SL_MATTER_ENABLE_HTTP_SERVICE) && SL_MATTER_ENABLE_HTTP_SERVICE
-#include "https_offload_example.h"
-#endif // SL_MATTER_ENABLE_HTTP_SERVICE
-
-#if defined(SL_MATTER_ENABLE_MQTT_SERVICE) && SL_MATTER_ENABLE_MQTT_SERVICE
-#include "mqtt_example.h"
-#endif // SL_MATTER_ENABLE_MQTT_SERVICE
-
-#endif // SL_MATTER_ENABLE_SERVICES
-
 #ifdef PERFORMANCE_TEST_ENABLED
 #include <performance_test_commands.h>
 #endif // PERFORMANCE_TEST_ENABLED
@@ -1030,22 +1016,6 @@ void InitMatterAwsHandler(System::Layer * systemLayer, void * appState)
 } // namespace
 #endif // SL_MATTER_ENABLE_AWS
 
-#if defined(SL_MATTER_ENABLE_SERVICES) && SL_MATTER_ENABLE_SERVICES
-namespace {
-void InitMatterServicesHandler(System::Layer * systemLayer, void * appState)
-{
-
-#if defined(SL_MATTER_ENABLE_MQTT_SERVICE) && SL_MATTER_ENABLE_MQTT_SERVICE
-    VerifyOrReturn(SL_STATUS_OK == mqtt_client_demo_start(), ChipLogError(AppServer, "mqtt_client_demo_start failed"));
-#endif // SL_MATTER_ENABLE_MQTT_SERVICE
-
-#if defined(SL_MATTER_ENABLE_HTTP_SERVICE) && SL_MATTER_ENABLE_HTTP_SERVICE
-    VerifyOrReturn(SL_STATUS_OK == https_client_demo_start(), ChipLogError(AppServer, "https_client_demo_start failed"));
-#endif // SL_MATTER_ENABLE_HTTP_SERVICE
-}
-} // namespace
-#endif // SL_MATTER_ENABLE_SERVICES
-
 void BaseApplication::OnPlatformEvent(const ChipDeviceEvent * event, intptr_t)
 {
     switch (event->Type)
@@ -1071,19 +1041,6 @@ void BaseApplication::OnPlatformEvent(const ChipDeviceEvent * event, intptr_t)
         }
 #endif // SL_MATTER_ENABLE_AWS
 
-#if defined(SL_MATTER_ENABLE_SERVICES) && SL_MATTER_ENABLE_SERVICES
-        if (event->InternetConnectivityChange.IPv4 == kConnectivity_Established
-#if defined(SL_MATTER_ENABLE_DUAL_STACK) && SL_MATTER_ENABLE_DUAL_STACK
-            || event->InternetConnectivityChange.IPv6 == kConnectivity_Established
-#endif // SL_MATTER_ENABLE_DUAL_STACK
-        )
-        {
-            ChipLogProgress(AppServer, "Scheduling Matter Services initialization");
-            TEMPORARY_RETURN_IGNORED chip::DeviceLayer::SystemLayer().StartTimer(
-                chip::System::Clock::Seconds32(kMatterServicesInitDelaySec), InitMatterServicesHandler, nullptr);
-        }
-#endif // SL_MATTER_ENABLE_SERVICES
-
 #if SL_MATTER_DISPLAY_ENABLED
         SilabsLCD::Screen_e screen;
         AppTask::GetLCD().GetScreen(screen);

examples/thermostat/silabs/include/AppTask.h

@@ -111,4 +111,9 @@ class AppTask : public BaseApplication
      *         could not be created, or an error propagated from InitSensor().
      */
     CHIP_ERROR InitThermostat();
+
+#if defined(SL_MATTER_ENABLE_SERVICES) && SL_MATTER_ENABLE_SERVICES
+    /** Schedule MQTT/HTTP service demos after internet connectivity is established. */
+    static void MatterServicesEventHandler(const chip::DeviceLayer::ChipDeviceEvent * event, intptr_t arg);
+#endif // SL_MATTER_ENABLE_SERVICES
 };

examples/thermostat/silabs/src/AppTask.cpp

@@ -43,6 +43,7 @@
 #include <platform/CHIPDeviceLayer.h>
 #include <platform/PlatformError.h>
 #include <platform/silabs/platformAbstraction/SilabsPlatform.h>
+#include <sl_cmsis_os2_common.h>
 #include <thermostat-delegate-impl.h>
 
 #if defined(SL_MATTER_USE_SI70XX_SENSOR) && SL_MATTER_USE_SI70XX_SENSOR
@@ -53,6 +54,18 @@
 #include "MatterAwsControl.h"
 #endif // SL_MATTER_ENABLE_AWS
 
+#if defined(SL_MATTER_ENABLE_SERVICES) && SL_MATTER_ENABLE_SERVICES
+#include <platform/silabs/services/matter_service.h>
+
+#if defined(SL_MATTER_ENABLE_HTTP_SERVICE) && SL_MATTER_ENABLE_HTTP_SERVICE
+#include "https_offload_example.h"
+#endif // SL_MATTER_ENABLE_HTTP_SERVICE
+
+#if defined(SL_MATTER_ENABLE_MQTT_SERVICE) && SL_MATTER_ENABLE_MQTT_SERVICE
+#include "mqtt_example.h"
+#endif // SL_MATTER_ENABLE_MQTT_SERVICE
+#endif // SL_MATTER_ENABLE_SERVICES
+
 #define APP_FUNCTION_BUTTON 0
 
 using namespace chip;
@@ -111,6 +124,15 @@ CHIP_ERROR AppTask::AppInit()
         appError(err);
     }
 
+#if defined(SL_MATTER_ENABLE_SERVICES) && SL_MATTER_ENABLE_SERVICES
+    err = PlatformMgr().AddEventHandler(MatterServicesEventHandler, 0);
+    if (err != CHIP_NO_ERROR)
+    {
+        ChipLogError(AppServer, "MatterServicesEventHandler register failed: %" CHIP_ERROR_FORMAT, err.Format());
+        appError(err);
+    }
+#endif // SL_MATTER_ENABLE_SERVICES
+
     return err;
 }
 
@@ -360,6 +382,48 @@ void AppTask::DMThermostatClusterInit(chip::EndpointId endpoint)
     SetDefaultDelegate(endpoint, &delegate);
 }
 
+#if defined(SL_MATTER_ENABLE_SERVICES) && SL_MATTER_ENABLE_SERVICES
+namespace {
+
+using chip::DeviceLayer::Silabs::kMatterServicesInitDelaySec;
+
+void InitMatterServicesHandler(System::Layer * /* systemLayer */, void * /* appState */)
+{
+#if defined(SL_MATTER_ENABLE_MQTT_SERVICE) && SL_MATTER_ENABLE_MQTT_SERVICE
+    VerifyOrReturn(SL_STATUS_OK == mqtt_client_demo_start(), ChipLogError(AppServer, "mqtt_client_demo_start failed"));
+#endif // SL_MATTER_ENABLE_MQTT_SERVICE
+
+#if defined(SL_MATTER_ENABLE_HTTP_SERVICE) && SL_MATTER_ENABLE_HTTP_SERVICE
+    VerifyOrReturn(SL_STATUS_OK == https_client_demo_start(), ChipLogError(AppServer, "https_client_demo_start failed"));
+#endif // SL_MATTER_ENABLE_HTTP_SERVICE
+}
+
+} // namespace
+
+void AppTask::MatterServicesEventHandler(const ChipDeviceEvent * event, intptr_t)
+{
+    VerifyOrReturn(event != nullptr);
+
+    if (event->Type != DeviceEventType::kInternetConnectivityChange)
+    {
+        return;
+    }
+
+    if (event->InternetConnectivityChange.IPv4 != kConnectivity_Established
+#if defined(SL_MATTER_ENABLE_DUAL_STACK) && SL_MATTER_ENABLE_DUAL_STACK
+        && event->InternetConnectivityChange.IPv6 != kConnectivity_Established
+#endif // SL_MATTER_ENABLE_DUAL_STACK
+    )
+    {
+        return;
+    }
+
+    ChipLogProgress(AppServer, "Scheduling Matter Services initialization");
+    TEMPORARY_RETURN_IGNORED SystemLayer().StartTimer(System::Clock::Seconds32(kMatterServicesInitDelaySec),
+                                                      InitMatterServicesHandler, nullptr);
+}
+#endif // SL_MATTER_ENABLE_SERVICES
+
 // emberAfThermostatClusterInitCallback — weak ZAP entry point. CRTP forwarder into AppTask.
 void emberAfThermostatClusterInitCallback(chip::EndpointId endpoint)
 {

examples/thermostat/silabs/src/https_offload_example.cpp

@@ -167,7 +167,6 @@ sl_status_t https_client_demo_start(void)
     VerifyOrReturnError(sizeof(kCaCertExample) > 0, SL_STATUS_FAIL, ChipLogError(DeviceLayer, "CA certificate is not set"));
 
     const HttpClientConfig config = {
-        .certificateIndex = 1,
         // Enable HTTPS client.
         .httpsEnable  = true,
         .username     = kHttpClientUser,

examples/thermostat/silabs/src/mqtt_example.cpp

@@ -57,8 +57,13 @@ void OnOperationDone(CHIP_ERROR result, void * /* context */)
     gOpDone   = true;
 }
 
-// Must yield: demo runs on the CHIP task (osPriorityHigh), which is above the
-// mqtt_client service thread (osPriorityAboveNormal). A tight spin deadlocks.
+void OnMqttMessage(const char * topic, ByteSpan payload, void * /* context */)
+{
+    ChipLogProgress(DeviceLayer, "MQTT demo message on %s: %.*s", topic != nullptr ? topic : "(null)",
+                    static_cast<int>(payload.size()), reinterpret_cast<const char *>(payload.data()));
+}
+
+// Must yield: demo may run above mqtt_client priority; osDelay lets the service thread run.
 CHIP_ERROR RunOperation(CHIP_ERROR queueResult)
 {
     if (queueResult != CHIP_NO_ERROR)
@@ -111,6 +116,8 @@ sl_status_t mqtt_client_demo_start(void)
         return SL_STATUS_FAIL;
     }
 
+    gMqttsClient.SetSubscriptionCallback(OnMqttMessage, nullptr);
+
     ChipLogProgress(DeviceLayer, "MQTT demo starting");
 
     gOpDone = false;
@@ -146,6 +153,6 @@ sl_status_t mqtt_client_demo_start(void)
         return SL_STATUS_FAIL;
     }
 
-    ChipLogProgress(DeviceLayer, "MQTT demo completed");
+    ChipLogProgress(DeviceLayer, "MQTT demo completed (auto-yield keeps session alive)");
     return SL_STATUS_OK;
 }

src/platform/silabs/services/http_client.cpp

@@ -449,8 +449,9 @@ sl_status_t HttpClient::GetResponseCallback(const sl_http_client_t * client, sl_
     sl_http_client_response_t * getResponse = static_cast<sl_http_client_response_t *>(data);
     self->mCallbackStatus                   = getResponse->status;
 
-    ChipLogProgress(DeviceLayer, "HTTPS GET response: status=0x%lX http_code=%u data_len=%u",
-                    static_cast<unsigned long>(getResponse->status), getResponse->http_response_code, getResponse->data_length);
+    ChipLogProgress(DeviceLayer, "HTTPS GET response: status=0x%lX http_code=%u data_len=%u end_of_data=%lu",
+                    static_cast<unsigned long>(getResponse->status), getResponse->http_response_code, getResponse->data_length,
+                    static_cast<unsigned long>(getResponse->end_of_data));
 
     if ((getResponse->status != SL_STATUS_OK) && (getResponse->status != SL_STATUS_IN_PROGRESS))
     {
@@ -469,6 +470,8 @@ sl_status_t HttpClient::GetResponseCallback(const sl_http_client_t * client, sl_
         return getResponse->status;
     }
 
+    self->mEndOfData = static_cast<EndOfData>(getResponse->end_of_data);
+
     if (getResponse->data_length)
     {
         if (self->mActiveResponseBuffer != nullptr)
@@ -490,7 +493,7 @@ sl_status_t HttpClient::GetResponseCallback(const sl_http_client_t * client, sl_
         }
     }
 
-    if (getResponse->end_of_data)
+    if (self->mEndOfData == EndOfData::EndOfData)
     {
         if (self->mActiveResponseBuffer != nullptr)
         {

src/platform/silabs/services/http_client.h

@@ -47,8 +47,8 @@ namespace Silabs {
  */
 struct HttpClientConfig
 {
-    uint8_t certificateIndex = 1;       ///< NWP TLS / HTTPS certificate index.
-    bool httpsEnable         = true;    ///< true = HTTPS, false = plain HTTP.
+    uint8_t certificateIndex = SL_HTTPS_CLIENT_DEFAULT_CERTIFICATE_INDEX; ///< NWP TLS / HTTPS certificate index.
+    bool httpsEnable         = true;                                      ///< true = HTTPS, false = plain HTTP.
     const char * username    = nullptr; ///< Optional HTTP basic-auth username; nullptr skips auth.
     const char * password    = nullptr; ///< Optional HTTP basic-auth password; nullptr skips auth.
 

src/platform/silabs/services/matter_service.h

@@ -24,12 +24,8 @@ namespace chip {
 namespace DeviceLayer {
 namespace Silabs {
 
-namespace {
-/**
- * @brief Delay in seconds before initializing the Matter services.
- */
+/** Delay before starting service demos after internet connectivity is established. */
 static constexpr uint32_t kMatterServicesInitDelaySec = 10;
-} // namespace
 
 /**
  * @brief Common lifecycle for Matter platform services (HTTP, MQTT, ...).

src/platform/silabs/services/mqtt_client.cpp

@@ -26,10 +26,6 @@
 #include <cstdlib>
 #include <cstring>
 
-#ifndef MQTT_USE_HOST_LWIP_TLS
-#define MQTT_USE_HOST_LWIP_TLS 0
-#endif
-
 namespace chip {
 namespace DeviceLayer {
 namespace Silabs {
@@ -154,7 +150,23 @@ void MqttClient::ServiceThread(void * arg)
     auto eventFlags = static_cast<osEventFlagsId_t>(self->mEventFlags);
     while (true)
     {
-        uint32_t events = osEventFlagsWait(eventFlags, kEventOperation | kEventStop, osFlagsWaitAny, osWaitForever);
+        // If the client is connected and not busy, wait for the idle yield timeout.
+        // Otherwise, wait forever for an event.
+        // This is to publish PINGREQ messages in the background to keep the connection alive.
+        // idleYieldTimeoutMs cannot be greater than keepAliveIntervalSec * 1000 so PINGREQ can fire in time.
+
+        // TODO: revist this logic, since most of the calculations are done in the MQTTYield function.
+        // so just invoking MQTTYield function here with (keepAliveIntervalSec *1000u - idleYieldTimeoutMs) value.
+        // should be enough to keep the connection alive.
+        const uint32_t waitMs = (self->mConnected && !self->mBusy) ? self->GetIdleYieldTimeoutMs() : osWaitForever;
+        uint32_t events       = osEventFlagsWait(eventFlags, kEventOperation | kEventStop, osFlagsWaitAny, waitMs);
+
+        if (events == osFlagsErrorTimeout)
+        {
+            self->IdleYield();
+            continue;
+        }
+
         if (events & osFlagsError)
         {
             ChipLogError(DeviceLayer, "MQTT service event wait failed: 0x%lx", static_cast<unsigned long>(events));
@@ -176,6 +188,42 @@ void MqttClient::ServiceThread(void * arg)
     osThreadTerminate(osThreadGetId());
 }
 
+uint32_t MqttClient::GetIdleYieldTimeoutMs()
+{
+    // If the idle yield timeout is not set, use the default value of 1000 ms.
+    // Though this is set in the constructor, it is possible to change the value later.
+    if (mConfig.idleYieldTimeoutMs == 0)
+    {
+        ChipLogError(DeviceLayer, "MQTT idleYieldTimeoutMs is 0 (ignored); using default 1000 ms");
+        mConfig.idleYieldTimeoutMs = 1000;
+    }
+
+    const uint32_t configuredMs = mConfig.idleYieldTimeoutMs;
+
+    // MQTTYield timeout_ms must be <= keepAliveInterval * 1000 so PINGREQ can fire in time.
+    VerifyOrReturnValue(mConfig.keepAliveIntervalSec > 0, configuredMs);
+
+    const uint32_t keepAliveMs = static_cast<uint32_t>(mConfig.keepAliveIntervalSec) * 1000u;
+    return (configuredMs < keepAliveMs) ? configuredMs : keepAliveMs;
+}
+
+void MqttClient::IdleYield()
+{
+    VerifyOrReturn(mConnected && !mBusy);
+
+    const uint32_t yieldMs = GetIdleYieldTimeoutMs();
+    const int status       = MQTTYield(&mClient, static_cast<int>(yieldMs));
+    if (status != SUCCESS)
+    {
+        ChipLogError(DeviceLayer, "MQTT idle Yield failed: %d (session marked disconnected)", status);
+        CHIP_ERROR err = ProcessDisconnect();
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(DeviceLayer, "MQTT idle Yield disconnect failed: %" CHIP_ERROR_FORMAT, err.Format());
+        }
+    }
+}
+
 CHIP_ERROR MqttClient::Start()
 {
     VerifyOrReturnError(mThreadId == nullptr, CHIP_NO_ERROR);
@@ -487,6 +535,11 @@ CHIP_ERROR MqttClient::ProcessYield()
     if (status != SUCCESS)
     {
         ChipLogError(DeviceLayer, "MQTT Yield failed: %d", status);
+        CHIP_ERROR err = ProcessDisconnect();
+        if (err != CHIP_NO_ERROR)
+        {
+            ChipLogError(DeviceLayer, "MQTT Yield disconnect failed: %" CHIP_ERROR_FORMAT, err.Format());
+        }
         return MapPahoStatus(status);
     }
     return CHIP_NO_ERROR;