Skip to content

Commit 017e8b5

Browse files
DiagnosticLogs: Add CHIP_ERROR Parameter to Reset and EndLogCollection Methods (project-chip#36775)
* BDX: update reset and endLogCollection to distinguish success and failure cases * diagnostic-logs-cluster: make changes backword compatible * diagnostic-logs-cluster: remove default parameter from Reset() method * diagnostic-logs-delegate: Add default implementation for EndLogCollection overload - Provides a default implementation for the EndLogCollection method with an additional error parameter. - This ensures backward compatibility, reduces the need for overriding in derived classes, and supports scenarios requiring context for log collection termination. * diagnostic-logs-delegate: Add default implementation for EndLogCollection - The one-argument EndLogCollection method is retained for backward compatibility. - The new two-argument overloaded EndLogCollection method will serve as the primary method to be implemented in delegates. * fix: remove unnecessary chip log statements * BDX: modify EndLogCollection parameter for internal log end requests
1 parent af336ec commit 017e8b5

File tree

5 files changed

+43
-14
lines changed

5 files changed

+43
-14
lines changed

examples/temperature-measurement-app/esp32/main/diagnostic-logs-provider-delegate-impl.cpp

+8-3
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ CHIP_ERROR LogProvider::GetLogForIntent(IntentEnum intent, MutableByteSpan & out
6565
err = CollectLog(sessionHandle, outBuffer, unusedOutIsEndOfLog);
6666
VerifyOrReturnError(CHIP_NO_ERROR == err, err, outBuffer.reduce_size(0));
6767

68-
err = EndLogCollection(sessionHandle);
68+
err = EndLogCollection(sessionHandle, err);
6969
VerifyOrReturnError(CHIP_NO_ERROR == err, err, outBuffer.reduce_size(0));
7070

7171
return CHIP_NO_ERROR;
@@ -276,8 +276,13 @@ CHIP_ERROR LogProvider::StartLogCollection(IntentEnum intent, LogSessionHandle &
276276
return CHIP_NO_ERROR;
277277
}
278278

279-
CHIP_ERROR LogProvider::EndLogCollection(LogSessionHandle sessionHandle)
279+
CHIP_ERROR LogProvider::EndLogCollection(LogSessionHandle sessionHandle, CHIP_ERROR error)
280280
{
281+
if (error != CHIP_NO_ERROR)
282+
{
283+
// Handle the error
284+
ChipLogProgress(DeviceLayer, "End log collection reason: %s", ErrorStr(error));
285+
}
281286
VerifyOrReturnValue(sessionHandle != kInvalidLogSessionHandle, CHIP_ERROR_INVALID_ARGUMENT);
282287
VerifyOrReturnValue(mSessionContextMap.count(sessionHandle), CHIP_ERROR_INVALID_ARGUMENT);
283288

@@ -288,7 +293,7 @@ CHIP_ERROR LogProvider::EndLogCollection(LogSessionHandle sessionHandle)
288293
Platform::MemoryFree(context);
289294
mSessionContextMap.erase(sessionHandle);
290295

291-
return CHIP_NO_ERROR;
296+
return error;
292297
}
293298

294299
CHIP_ERROR LogProvider::CollectLog(LogSessionHandle sessionHandle, MutableByteSpan & outBuffer, bool & outIsEndOfLog)

examples/temperature-measurement-app/esp32/main/include/diagnostic-logs-provider-delegate-impl.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ class LogProvider : public DiagnosticLogsProviderDelegate
4444
/////////// DiagnosticLogsProviderDelegate Interface /////////
4545
CHIP_ERROR StartLogCollection(IntentEnum intent, LogSessionHandle & outHandle, Optional<uint64_t> & outTimeStamp,
4646
Optional<uint64_t> & outTimeSinceBoot) override;
47-
CHIP_ERROR EndLogCollection(LogSessionHandle sessionHandle) override;
47+
CHIP_ERROR EndLogCollection(LogSessionHandle sessionHandle, CHIP_ERROR error) override;
4848
CHIP_ERROR CollectLog(LogSessionHandle sessionHandle, MutableByteSpan & outBuffer, bool & outIsEndOfLog) override;
4949
size_t GetSizeForIntent(IntentEnum intent) override;
5050
CHIP_ERROR GetLogForIntent(IntentEnum intent, MutableByteSpan & outBuffer, Optional<uint64_t> & outTimeStamp,

src/app/clusters/diagnostic-logs-server/BDXDiagnosticLogsProvider.cpp

+8-8
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ void BDXDiagnosticLogsProvider::OnMsgToSend(TransferSession::OutputEvent & event
162162
auto err =
163163
mBDXTransferExchangeCtx->SendMessage(msgTypeData.ProtocolId, msgTypeData.MessageType, std::move(event.MsgData), sendFlags);
164164

165-
VerifyOrDo(CHIP_NO_ERROR == err, Reset());
165+
VerifyOrDo(CHIP_NO_ERROR == err, Reset(err));
166166
}
167167

168168
void BDXDiagnosticLogsProvider::OnAcceptReceived()
@@ -191,7 +191,7 @@ void BDXDiagnosticLogsProvider::OnAckReceived()
191191
// If the buffer has empty space, end the log collection session.
192192
if (isEndOfLog)
193193
{
194-
mDelegate->EndLogCollection(mLogSessionHandle);
194+
mDelegate->EndLogCollection(mLogSessionHandle, CHIP_ERROR_INTERNAL);
195195
mLogSessionHandle = kInvalidLogSessionHandle;
196196
}
197197

@@ -213,7 +213,7 @@ void BDXDiagnosticLogsProvider::OnAckEOFReceived()
213213
{
214214
ChipLogProgress(BDX, "Diagnostic logs transfer: Success");
215215

216-
Reset();
216+
Reset(CHIP_NO_ERROR);
217217
}
218218

219219
void BDXDiagnosticLogsProvider::OnStatusReceived(TransferSession::OutputEvent & event)
@@ -223,21 +223,21 @@ void BDXDiagnosticLogsProvider::OnStatusReceived(TransferSession::OutputEvent &
223223
// If a failure StatusReport is received in response to the SendInit message, the Node SHALL send a RetrieveLogsResponse command
224224
// with a Status of Denied.
225225
VerifyOrDo(mIsAcceptReceived, SendCommandResponse(StatusEnum::kDenied));
226-
Reset();
226+
Reset(CHIP_ERROR_INCORRECT_STATE);
227227
}
228228

229229
void BDXDiagnosticLogsProvider::OnInternalError()
230230
{
231231
ChipLogError(BDX, "Internal Error");
232232
VerifyOrDo(mIsAcceptReceived, SendCommandResponse(StatusEnum::kDenied));
233-
Reset();
233+
Reset(CHIP_ERROR_INTERNAL);
234234
}
235235

236236
void BDXDiagnosticLogsProvider::OnTimeout()
237237
{
238238
ChipLogError(BDX, "Timeout");
239239
VerifyOrDo(mIsAcceptReceived, SendCommandResponse(StatusEnum::kDenied));
240-
Reset();
240+
Reset(CHIP_ERROR_TIMEOUT);
241241
}
242242

243243
void BDXDiagnosticLogsProvider::SendCommandResponse(StatusEnum status)
@@ -264,7 +264,7 @@ void BDXDiagnosticLogsProvider::SendCommandResponse(StatusEnum status)
264264
commandHandle->AddResponse(mRequestPath, response);
265265
}
266266

267-
void BDXDiagnosticLogsProvider::Reset()
267+
void BDXDiagnosticLogsProvider::Reset(CHIP_ERROR error)
268268
{
269269
assertChipStackLockedByCurrentThread();
270270

@@ -279,7 +279,7 @@ void BDXDiagnosticLogsProvider::Reset()
279279

280280
if (mDelegate != nullptr)
281281
{
282-
mDelegate->EndLogCollection(mLogSessionHandle);
282+
mDelegate->EndLogCollection(mLogSessionHandle, error);
283283
mDelegate = nullptr;
284284
}
285285

src/app/clusters/diagnostic-logs-server/BDXDiagnosticLogsProvider.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,10 @@ class BDXDiagnosticLogsProvider : public bdx::Initiator
7676
/**
7777
* This method is called to reset state. It resets the transfer, cleans up the
7878
* exchange and ends log collection.
79+
* @param[in] error A CHIP_ERROR value indicating the reason for resetting the state.
80+
* It is permissible to pass CHIP_NO_ERROR to indicate normal termination.
7981
*/
80-
void Reset();
82+
void Reset(CHIP_ERROR error);
8183

8284
Messaging::ExchangeContext * mBDXTransferExchangeCtx;
8385
DiagnosticLogsProviderDelegate * mDelegate;

src/app/clusters/diagnostic-logs-server/DiagnosticLogsProviderDelegate.h

+23-1
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,32 @@ class DiagnosticLogsProviderDelegate
5858
* This must be called if StartLogCollection happens successfully and a valid sessionHandle has been
5959
* returned from StartLogCollection.
6060
*
61+
* @note New implementations should override the two-argument version instead, as it is the primary
62+
* method invoked during log collection.
63+
*
64+
* @param[in] sessionHandle The unique handle for this log session returned from a call to StartLogCollection.
65+
* @return CHIP_ERROR_NOT_IMPLEMENTED by default unless overridden.
66+
*/
67+
virtual CHIP_ERROR EndLogCollection(LogSessionHandle sessionHandle) { return CHIP_ERROR_NOT_IMPLEMENTED; }
68+
69+
/**
70+
* Called to end log collection for the log session identified by sessionHandle.
71+
* This must be called if StartLogCollection happens successfully and a valid sessionHandle has been
72+
* returned from StartLogCollection.
73+
*
74+
* This overload of EndLogCollection provides additional context through the error parameter, which
75+
* can be used to indicate the reason for ending the log collection.
76+
*
6177
* @param[in] sessionHandle The unique handle for this log session returned from a call to StartLogCollection.
78+
* @param[in] error A CHIP_ERROR value indicating the reason for ending the log collection.
79+
* It is permissible to pass CHIP_NO_ERROR to indicate normal termination.
80+
* @return CHIP_ERROR_NOT_IMPLEMENTED by default unless overridden.
6281
*
6382
*/
64-
virtual CHIP_ERROR EndLogCollection(LogSessionHandle sessionHandle) = 0;
83+
virtual CHIP_ERROR EndLogCollection(LogSessionHandle sessionHandle, CHIP_ERROR error)
84+
{
85+
return EndLogCollection(sessionHandle);
86+
}
6587

6688
/**
6789
* Called to get the next chunk for the log session identified by sessionHandle.

0 commit comments

Comments
 (0)