From 4c691ee818adfa0ea1c0f28d2f19b441e02ae394 Mon Sep 17 00:00:00 2001 From: Yufeng Wang Date: Tue, 10 May 2022 09:28:58 -0700 Subject: [PATCH] =?UTF-8?q?Skip=20SetCurrentHeapHighWatermark=20if=20GetCu?= =?UTF-8?q?rrentHeapUsed=20is=20not=20impleme=E2=80=A6=20(#18130)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Skip SetCurrentHeapHighWatermark if GetCurrentHeapUsed is not implemented * Address review comment --- .../software-diagnostics-server.cpp | 13 +++++----- src/include/platform/DiagnosticDataProvider.h | 6 +++++ .../Darwin/DiagnosticDataProviderImpl.cpp | 26 +++---------------- .../Darwin/DiagnosticDataProviderImpl.h | 5 +--- .../Linux/DiagnosticDataProviderImpl.cpp | 11 ++++++++ .../Linux/DiagnosticDataProviderImpl.h | 1 + 6 files changed, 29 insertions(+), 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 index e73949f5b44870..5639622f99fe6d 100644 --- 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 index acd169ead0a6fe..fa4165f2bc0dc1 100644 --- 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 index 9b1085b03ca717..b45de23707b190 100644 --- 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 index 21b6dfe2c8672e..7031fa7bd2a847 100644 --- 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 index bc50595f8f00da..c97aeca2cb4833 100644 --- 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 index 8940cfacc0a5d7..d3797d63a9a175 100644 --- 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;