Skip to content

Commit b9d4e64

Browse files
committed
Address review comment
1 parent ab3e520 commit b9d4e64

File tree

6 files changed

+34
-16
lines changed

6 files changed

+34
-16
lines changed

src/app/clusters/software-diagnostics-server/software-diagnostics-server.cpp

+4-16
Original file line numberDiff line numberDiff line change
@@ -156,23 +156,11 @@ bool emberAfSoftwareDiagnosticsClusterResetWatermarksCallback(app::CommandHandle
156156
{
157157
EmberAfStatus status = EMBER_ZCL_STATUS_SUCCESS;
158158

159-
uint64_t currentHeapUsed;
160-
CHIP_ERROR err = DeviceLayer::GetDiagnosticDataProvider().GetCurrentHeapUsed(currentHeapUsed);
161-
162-
// Skip SetCurrentHeapHighWatermark if GetCurrentHeapUsed is not implemented
163-
if (err != CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE && err != CHIP_ERROR_NOT_IMPLEMENTED)
159+
// If implemented, the server SHALL set the value of the CurrentHeapHighWatermark attribute to the
160+
// value of the CurrentHeapUsed.
161+
if (DeviceLayer::GetDiagnosticDataProvider().ResetWatermarks() != CHIP_NO_ERROR)
164162
{
165-
if (err == CHIP_NO_ERROR)
166-
{
167-
if (CHIP_NO_ERROR != DeviceLayer::GetDiagnosticDataProvider().SetCurrentHeapHighWatermark(currentHeapUsed))
168-
{
169-
status = EMBER_ZCL_STATUS_FAILURE;
170-
}
171-
}
172-
else
173-
{
174-
status = EMBER_ZCL_STATUS_FAILURE;
175-
}
163+
status = EMBER_ZCL_STATUS_FAILURE;
176164
}
177165

178166
emberAfSendImmediateDefaultResponse(status);

src/include/platform/DiagnosticDataProvider.h

+6
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ class DiagnosticDataProvider
180180
virtual CHIP_ERROR GetCurrentHeapUsed(uint64_t & currentHeapUsed);
181181
virtual CHIP_ERROR GetCurrentHeapHighWatermark(uint64_t & currentHeapHighWatermark);
182182
virtual CHIP_ERROR SetCurrentHeapHighWatermark(uint64_t heapHighWatermark);
183+
virtual CHIP_ERROR ResetWatermarks();
183184

184185
/*
185186
* 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
272273
return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
273274
}
274275

276+
inline CHIP_ERROR DiagnosticDataProvider::ResetWatermarks()
277+
{
278+
return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
279+
}
280+
275281
inline CHIP_ERROR DiagnosticDataProvider::GetThreadMetrics(ThreadMetrics ** threadMetricsOut)
276282
{
277283
return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;

src/platform/Darwin/DiagnosticDataProviderImpl.cpp

+9
Original file line numberDiff line numberDiff line change
@@ -68,5 +68,14 @@ CHIP_ERROR DiagnosticDataProviderImpl::GetTotalOperationalHours(uint32_t & total
6868
return CHIP_ERROR_INVALID_TIME;
6969
}
7070

71+
CHIP_ERROR DiagnosticDataProviderImpl::ResetWatermarks()
72+
{
73+
// If implemented, the server SHALL set the value of the CurrentHeapHighWatermark attribute to the
74+
// value of the CurrentHeapUsed.
75+
// On Darwin, overide with non-op to pass CI.
76+
77+
return CHIP_NO_ERROR;
78+
}
79+
7180
} // namespace DeviceLayer
7281
} // namespace chip

src/platform/Darwin/DiagnosticDataProviderImpl.h

+3
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ class DiagnosticDataProviderImpl : public DiagnosticDataProvider
3838
// ===== Methods that implement the PlatformManager abstract interface.
3939
CHIP_ERROR GetUpTime(uint64_t & upTime) override;
4040
CHIP_ERROR GetTotalOperationalHours(uint32_t & totalOperationalHours) override;
41+
42+
// ===== Methods that implement the DiagnosticDataProvider abstract interface.
43+
CHIP_ERROR ResetWatermarks() override;
4144
};
4245

4346
} // namespace DeviceLayer

src/platform/Linux/DiagnosticDataProviderImpl.cpp

+11
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,17 @@ CHIP_ERROR DiagnosticDataProviderImpl::SetCurrentHeapHighWatermark(uint64_t heap
274274
return CHIP_NO_ERROR;
275275
}
276276

277+
CHIP_ERROR DiagnosticDataProviderImpl::ResetWatermarks()
278+
{
279+
// If implemented, the server SHALL set the value of the CurrentHeapHighWatermark attribute to the
280+
// value of the CurrentHeapUsed.
281+
282+
// On Linux, the write operation is non-op since we always rely on the mallinfo system
283+
// function to get the current heap memory.
284+
285+
return CHIP_NO_ERROR;
286+
}
287+
277288
CHIP_ERROR DiagnosticDataProviderImpl::GetThreadMetrics(ThreadMetrics ** threadMetricsOut)
278289
{
279290
CHIP_ERROR err = CHIP_ERROR_READ_FAILED;

src/platform/Linux/DiagnosticDataProviderImpl.h

+1
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class DiagnosticDataProviderImpl : public DiagnosticDataProvider
4444
CHIP_ERROR GetCurrentHeapHighWatermark(uint64_t & currentHeapHighWatermark) override;
4545
CHIP_ERROR GetThreadMetrics(ThreadMetrics ** threadMetricsOut) override;
4646
CHIP_ERROR SetCurrentHeapHighWatermark(uint64_t heapHighWatermark) override;
47+
CHIP_ERROR ResetWatermarks() override;
4748
void ReleaseThreadMetrics(ThreadMetrics * threadMetrics) override;
4849

4950
CHIP_ERROR GetRebootCount(uint16_t & rebootCount) override;

0 commit comments

Comments
 (0)