DiagnosticLogs: Add CHIP_ERROR Parameter to Reset and EndLogCollection Methods

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

@@ -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;
@@ -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);
 
@@ -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)

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

@@ -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,

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

@@ -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()
@@ -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;
     }
 
@@ -213,7 +213,7 @@ void BDXDiagnosticLogsProvider::OnAckEOFReceived()
 {
     ChipLogProgress(BDX, "Diagnostic logs transfer: Success");
 
-    Reset();
+    Reset(CHIP_NO_ERROR);
 }
 
 void BDXDiagnosticLogsProvider::OnStatusReceived(TransferSession::OutputEvent & event)
@@ -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)
@@ -264,7 +264,7 @@ void BDXDiagnosticLogsProvider::SendCommandResponse(StatusEnum status)
     commandHandle->AddResponse(mRequestPath, response);
 }
 
-void BDXDiagnosticLogsProvider::Reset()
+void BDXDiagnosticLogsProvider::Reset(CHIP_ERROR error)
 {
     assertChipStackLockedByCurrentThread();
 
@@ -279,7 +279,7 @@ void BDXDiagnosticLogsProvider::Reset()
 
     if (mDelegate != nullptr)
     {
-        mDelegate->EndLogCollection(mLogSessionHandle);
+        mDelegate->EndLogCollection(mLogSessionHandle, error);
         mDelegate = nullptr;
     }
 

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

@@ -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;

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

@@ -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.