Skip to content

Commit

Permalink
DiagnosticLogs: Add CHIP_ERROR Parameter to Reset and EndLogCollectio…
Browse files Browse the repository at this point in the history
…n Methods (#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
  • Loading branch information
pimpalemahesh authored Dec 23, 2024
1 parent af336ec commit 017e8b5
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ CHIP_ERROR LogProvider::GetLogForIntent(IntentEnum intent, MutableByteSpan & out
err = CollectLog(sessionHandle, outBuffer, unusedOutIsEndOfLog);
VerifyOrReturnError(CHIP_NO_ERROR == err, err, outBuffer.reduce_size(0));

err = EndLogCollection(sessionHandle);
err = EndLogCollection(sessionHandle, err);
VerifyOrReturnError(CHIP_NO_ERROR == err, err, outBuffer.reduce_size(0));

return CHIP_NO_ERROR;
Expand Down Expand Up @@ -276,8 +276,13 @@ CHIP_ERROR LogProvider::StartLogCollection(IntentEnum intent, LogSessionHandle &
return CHIP_NO_ERROR;
}

CHIP_ERROR LogProvider::EndLogCollection(LogSessionHandle sessionHandle)
CHIP_ERROR LogProvider::EndLogCollection(LogSessionHandle sessionHandle, CHIP_ERROR error)
{
if (error != CHIP_NO_ERROR)
{
// Handle the error
ChipLogProgress(DeviceLayer, "End log collection reason: %s", ErrorStr(error));
}
VerifyOrReturnValue(sessionHandle != kInvalidLogSessionHandle, CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnValue(mSessionContextMap.count(sessionHandle), CHIP_ERROR_INVALID_ARGUMENT);

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

return CHIP_NO_ERROR;
return error;
}

CHIP_ERROR LogProvider::CollectLog(LogSessionHandle sessionHandle, MutableByteSpan & outBuffer, bool & outIsEndOfLog)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class LogProvider : public DiagnosticLogsProviderDelegate
/////////// DiagnosticLogsProviderDelegate Interface /////////
CHIP_ERROR StartLogCollection(IntentEnum intent, LogSessionHandle & outHandle, Optional<uint64_t> & outTimeStamp,
Optional<uint64_t> & outTimeSinceBoot) override;
CHIP_ERROR EndLogCollection(LogSessionHandle sessionHandle) override;
CHIP_ERROR EndLogCollection(LogSessionHandle sessionHandle, CHIP_ERROR error) override;
CHIP_ERROR CollectLog(LogSessionHandle sessionHandle, MutableByteSpan & outBuffer, bool & outIsEndOfLog) override;
size_t GetSizeForIntent(IntentEnum intent) override;
CHIP_ERROR GetLogForIntent(IntentEnum intent, MutableByteSpan & outBuffer, Optional<uint64_t> & outTimeStamp,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ void BDXDiagnosticLogsProvider::OnMsgToSend(TransferSession::OutputEvent & event
auto err =
mBDXTransferExchangeCtx->SendMessage(msgTypeData.ProtocolId, msgTypeData.MessageType, std::move(event.MsgData), sendFlags);

VerifyOrDo(CHIP_NO_ERROR == err, Reset());
VerifyOrDo(CHIP_NO_ERROR == err, Reset(err));
}

void BDXDiagnosticLogsProvider::OnAcceptReceived()
Expand Down Expand Up @@ -191,7 +191,7 @@ void BDXDiagnosticLogsProvider::OnAckReceived()
// If the buffer has empty space, end the log collection session.
if (isEndOfLog)
{
mDelegate->EndLogCollection(mLogSessionHandle);
mDelegate->EndLogCollection(mLogSessionHandle, CHIP_ERROR_INTERNAL);
mLogSessionHandle = kInvalidLogSessionHandle;
}

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

Reset();
Reset(CHIP_NO_ERROR);
}

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

void BDXDiagnosticLogsProvider::OnInternalError()
{
ChipLogError(BDX, "Internal Error");
VerifyOrDo(mIsAcceptReceived, SendCommandResponse(StatusEnum::kDenied));
Reset();
Reset(CHIP_ERROR_INTERNAL);
}

void BDXDiagnosticLogsProvider::OnTimeout()
{
ChipLogError(BDX, "Timeout");
VerifyOrDo(mIsAcceptReceived, SendCommandResponse(StatusEnum::kDenied));
Reset();
Reset(CHIP_ERROR_TIMEOUT);
}

void BDXDiagnosticLogsProvider::SendCommandResponse(StatusEnum status)
Expand All @@ -264,7 +264,7 @@ void BDXDiagnosticLogsProvider::SendCommandResponse(StatusEnum status)
commandHandle->AddResponse(mRequestPath, response);
}

void BDXDiagnosticLogsProvider::Reset()
void BDXDiagnosticLogsProvider::Reset(CHIP_ERROR error)
{
assertChipStackLockedByCurrentThread();

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

if (mDelegate != nullptr)
{
mDelegate->EndLogCollection(mLogSessionHandle);
mDelegate->EndLogCollection(mLogSessionHandle, error);
mDelegate = nullptr;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,10 @@ class BDXDiagnosticLogsProvider : public bdx::Initiator
/**
* This method is called to reset state. It resets the transfer, cleans up the
* exchange and ends log collection.
* @param[in] error A CHIP_ERROR value indicating the reason for resetting the state.
* It is permissible to pass CHIP_NO_ERROR to indicate normal termination.
*/
void Reset();
void Reset(CHIP_ERROR error);

Messaging::ExchangeContext * mBDXTransferExchangeCtx;
DiagnosticLogsProviderDelegate * mDelegate;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,32 @@ class DiagnosticLogsProviderDelegate
* This must be called if StartLogCollection happens successfully and a valid sessionHandle has been
* returned from StartLogCollection.
*
* @note New implementations should override the two-argument version instead, as it is the primary
* method invoked during log collection.
*
* @param[in] sessionHandle The unique handle for this log session returned from a call to StartLogCollection.
* @return CHIP_ERROR_NOT_IMPLEMENTED by default unless overridden.
*/
virtual CHIP_ERROR EndLogCollection(LogSessionHandle sessionHandle) { return CHIP_ERROR_NOT_IMPLEMENTED; }

/**
* Called to end log collection for the log session identified by sessionHandle.
* This must be called if StartLogCollection happens successfully and a valid sessionHandle has been
* returned from StartLogCollection.
*
* This overload of EndLogCollection provides additional context through the error parameter, which
* can be used to indicate the reason for ending the log collection.
*
* @param[in] sessionHandle The unique handle for this log session returned from a call to StartLogCollection.
* @param[in] error A CHIP_ERROR value indicating the reason for ending the log collection.
* It is permissible to pass CHIP_NO_ERROR to indicate normal termination.
* @return CHIP_ERROR_NOT_IMPLEMENTED by default unless overridden.
*
*/
virtual CHIP_ERROR EndLogCollection(LogSessionHandle sessionHandle) = 0;
virtual CHIP_ERROR EndLogCollection(LogSessionHandle sessionHandle, CHIP_ERROR error)
{
return EndLogCollection(sessionHandle);
}

/**
* Called to get the next chunk for the log session identified by sessionHandle.
Expand Down

0 comments on commit 017e8b5

Please sign in to comment.