Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DiagnosticLogs: Add CHIP_ERROR Parameter to Reset and EndLogCollection Methods #36775

Merged
merged 7 commits into from
Dec 23, 2024
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);
pimpalemahesh marked this conversation as resolved.
Show resolved Hide resolved
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
ChipLogError(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 @@ -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);
pimpalemahesh marked this conversation as resolved.
Show resolved Hide resolved

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)
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
{
return EndLogCollection(sessionHandle);
}

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