Skip SetCurrentHeapHighWatermark if GetCurrentHeapUsed is not impleme… (

#18130) * Skip SetCurrentHeapHighWatermark if GetCurrentHeapUsed is not implemented * Address review comment
project-chip · Oct 12, 2023 · 4c691ee · 4c691ee
1 parent 93d4d4a
commit 4c691ee
Show file tree

Hide file tree

Showing 6 changed files with 29 additions and 33 deletions.
diff --git a/src/app/clusters/software-diagnostics-server/software-diagnostics-server.cpp b/src/app/clusters/software-diagnostics-server/software-diagnostics-server.cpp
@@ -156,14 +156,13 @@ bool emberAfSoftwareDiagnosticsClusterResetWatermarksCallback(app::CommandHandle
 {
     EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS;
 
-    uint64_t currentHeapUsed;
-    CHIP_ERROR err = DeviceLayer::GetDiagnosticDataProvider().GetCurrentHeapUsed(currentHeapUsed);
-    VerifyOrExit(err == CHIP_NO_ERROR, status = EMBER_ZCL_STATUS_FAILURE);
-
-    err = DeviceLayer::GetDiagnosticDataProvider().SetCurrentHeapHighWatermark(currentHeapUsed);
-    VerifyOrExit(err == CHIP_NO_ERROR, status = EMBER_ZCL_STATUS_FAILURE);
+    // If implemented, the server SHALL set the value of the CurrentHeapHighWatermark attribute to the
+    // value of the CurrentHeapUsed.
+    if (DeviceLayer::GetDiagnosticDataProvider().ResetWatermarks() != CHIP_NO_ERROR)
+    {
+        status = EMBER_ZCL_STATUS_FAILURE;
+    }
 
-exit:
     emberAfSendImmediateDefaultResponse(status);
 
     return true;

diff --git a/src/include/platform/DiagnosticDataProvider.h b/src/include/platform/DiagnosticDataProvider.h
@@ -180,6 +180,7 @@ class DiagnosticDataProvider
     virtual CHIP_ERROR GetCurrentHeapUsed(uint64_t & currentHeapUsed);
     virtual CHIP_ERROR GetCurrentHeapHighWatermark(uint64_t & currentHeapHighWatermark);
     virtual CHIP_ERROR SetCurrentHeapHighWatermark(uint64_t heapHighWatermark);
+    virtual CHIP_ERROR ResetWatermarks();
 
     /*
      * Get the linked list of thread metrics of the current plaform. After usage, each caller of GetThreadMetrics
@@ -272,6 +273,11 @@ inline CHIP_ERROR DiagnosticDataProvider::SetCurrentHeapHighWatermark(uint64_t h
     return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
 }
 
+inline CHIP_ERROR DiagnosticDataProvider::ResetWatermarks()
+{
+    return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+}
+
 inline CHIP_ERROR DiagnosticDataProvider::GetThreadMetrics(ThreadMetrics ** threadMetricsOut)
 {
     return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;

diff --git a/src/platform/Darwin/DiagnosticDataProviderImpl.cpp b/src/platform/Darwin/DiagnosticDataProviderImpl.cpp
@@ -68,30 +68,12 @@ CHIP_ERROR DiagnosticDataProviderImpl::GetTotalOperationalHours(uint32_t & total
     return CHIP_ERROR_INVALID_TIME;
 }
 
-CHIP_ERROR DiagnosticDataProviderImpl::GetCurrentHeapFree(uint64_t & currentHeapFree)
+CHIP_ERROR DiagnosticDataProviderImpl::ResetWatermarks()
 {
-    // Overide with dummy value to pass CI
-    currentHeapFree = 0;
-    return CHIP_NO_ERROR;
-}
-
-CHIP_ERROR DiagnosticDataProviderImpl::GetCurrentHeapUsed(uint64_t & currentHeapUsed)
-{
-    // Overide with dummy value to pass CI
-    currentHeapUsed = 0;
-    return CHIP_NO_ERROR;
-}
+    // If implemented, the server SHALL set the value of the CurrentHeapHighWatermark attribute to the
+    // value of the CurrentHeapUsed.
+    // On Darwin, overide with non-op to pass CI.
 
-CHIP_ERROR DiagnosticDataProviderImpl::GetCurrentHeapHighWatermark(uint64_t & currentHeapHighWatermark)
-{
-    // Overide with dummy value to pass CI
-    currentHeapHighWatermark = 0;
-    return CHIP_NO_ERROR;
-}
-
-CHIP_ERROR DiagnosticDataProviderImpl::SetCurrentHeapHighWatermark(uint64_t heapHighWatermark)
-{
-    // Overide to pass CI
     return CHIP_NO_ERROR;
 }
 

diff --git a/src/platform/Darwin/DiagnosticDataProviderImpl.h b/src/platform/Darwin/DiagnosticDataProviderImpl.h
@@ -40,10 +40,7 @@ class DiagnosticDataProviderImpl : public DiagnosticDataProvider
     CHIP_ERROR GetTotalOperationalHours(uint32_t & totalOperationalHours) override;
 
     // ===== Methods that implement the DiagnosticDataProvider abstract interface.
-    CHIP_ERROR GetCurrentHeapFree(uint64_t & currentHeapFree) override;
-    CHIP_ERROR GetCurrentHeapUsed(uint64_t & currentHeapUsed) override;
-    CHIP_ERROR GetCurrentHeapHighWatermark(uint64_t & currentHeapHighWatermark) override;
-    CHIP_ERROR SetCurrentHeapHighWatermark(uint64_t heapHighWatermark) override;
+    CHIP_ERROR ResetWatermarks() override;
 };
 
 } // namespace DeviceLayer

diff --git a/src/platform/Linux/DiagnosticDataProviderImpl.cpp b/src/platform/Linux/DiagnosticDataProviderImpl.cpp
@@ -274,6 +274,17 @@ CHIP_ERROR DiagnosticDataProviderImpl::SetCurrentHeapHighWatermark(uint64_t heap
     return CHIP_NO_ERROR;
 }
 
+CHIP_ERROR DiagnosticDataProviderImpl::ResetWatermarks()
+{
+    // If implemented, the server SHALL set the value of the CurrentHeapHighWatermark attribute to the
+    // value of the CurrentHeapUsed.
+
+    // On Linux, the write operation is non-op since we always rely on the mallinfo system
+    // function to get the current heap memory.
+
+    return CHIP_NO_ERROR;
+}
+
 CHIP_ERROR DiagnosticDataProviderImpl::GetThreadMetrics(ThreadMetrics ** threadMetricsOut)
 {
     CHIP_ERROR err = CHIP_ERROR_READ_FAILED;

diff --git a/src/platform/Linux/DiagnosticDataProviderImpl.h b/src/platform/Linux/DiagnosticDataProviderImpl.h
@@ -44,6 +44,7 @@ class DiagnosticDataProviderImpl : public DiagnosticDataProvider
     CHIP_ERROR GetCurrentHeapHighWatermark(uint64_t & currentHeapHighWatermark) override;
     CHIP_ERROR GetThreadMetrics(ThreadMetrics ** threadMetricsOut) override;
     CHIP_ERROR SetCurrentHeapHighWatermark(uint64_t heapHighWatermark) override;
+    CHIP_ERROR ResetWatermarks() override;
     void ReleaseThreadMetrics(ThreadMetrics * threadMetrics) override;
 
     CHIP_ERROR GetRebootCount(uint16_t & rebootCount) override;