From f9a219cecaf876127849fef049a45da8628ad2f2 Mon Sep 17 00:00:00 2001
From: Nivedita Sarkar <nivedita_sarkar@apple.com>
Date: Thu, 2 Feb 2023 15:02:07 -0800
Subject: [PATCH 1/6] Add a timer to track whether we have received BDX init
after a query image was successful
- Currently if an ota requester has a successful query image and an image it available but
if for any reason, the ota requester doesn't send BDX init, the provider will be stuck until
we reboot the resident. In order to have a fail safe, we are adding a timer that starts after
query image returns image available and waits for a BDX init to come. In case BDX init doesn't
come, it times out and resets state
- Add code to reset state if any API fails on the provider once we prepare for BDX transfer
- Stop polling when BDX transfer reset is called
- Return QueryImageResponse status busy instead of general failure if the sdk is busy and
gets a sexond query image so accessory can handle the error correctly and retry until the
sdk is done
- When we send a message, handle the case where if an error happens in sending the message
close the exchange context and reset state. If the message is a status report and it
succeeds, null out the exchange and reset state
---
.../CHIP/MTROTAProviderDelegateBridge.mm | 128 ++++++++++++++----
src/protocols/bdx/TransferFacilitator.cpp | 1 +
2 files changed, 101 insertions(+), 28 deletions(-)
diff --git a/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm b/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
index 8592a645982d89..65b338f36e0bc4 100644
--- a/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
+++ b/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
@@ -42,6 +42,14 @@
// TODO Expose a method onto the delegate to make that configurable.
constexpr uint32_t kMaxBdxBlockSize = 1024;
constexpr uint32_t kMaxBDXURILen = 256;
+
+// Since the BDX timeout is 5 minutes and we are starting this after query image is available and before the BDX init comes,
+// we just double the timeout to give enough time for the BDX init to come in a reasonable amount of time.
+constexpr System::Clock::Timeout kBdxInitReceivedTimeout = System::Clock::Seconds16(10 * 60);
+
+// Time in seconds after which the requestor should retry calling query image if busy status is receieved
+constexpr uint32_t kDelayedActionTime = 120;
+
constexpr System::Clock::Timeout kBdxTimeout = System::Clock::Seconds16(5 * 60); // OTA Spec mandates >= 5 minutes
constexpr System::Clock::Timeout kBdxPollIntervalMs = System::Clock::Milliseconds32(50);
constexpr bdx::TransferRole kBdxRole = bdx::TransferRole::kSender;
@@ -89,13 +97,12 @@ CHIP_ERROR Shutdown()
VerifyOrReturnError(mExchangeMgr != nullptr, CHIP_ERROR_INCORRECT_STATE);
mExchangeMgr->UnregisterUnsolicitedMessageHandlerForProtocol(Protocols::BDX::Id);
+ ResetState();
mExchangeMgr = nullptr;
mSystemLayer = nullptr;
mDelegateNotificationQueue = nil;
- ResetState();
-
return CHIP_NO_ERROR;
}
@@ -120,7 +127,39 @@ void SetDelegate(id<MTROTAProviderDelegate> delegate, dispatch_queue_t delegateN
}
}
+ void ResetState()
+ {
+ assertChipStackLockedByCurrentThread();
+ if (mSystemLayer) {
+ mSystemLayer->CancelTimer(HandleBdxInitReceivedTimeoutExpired, this);
+ }
+ // TODO: Check if this can be removed. It seems like we can close the exchange context and reset transfer regardless.
+ if (!mInitialized) {
+ return;
+ }
+ Responder::ResetTransfer();
+ ++mTransferGeneration;
+ mFabricIndex.ClearValue();
+ mNodeId.ClearValue();
+
+ if (mExchangeCtx != nullptr) {
+ mExchangeCtx->Close();
+ mExchangeCtx = nullptr;
+ }
+
+ mInitialized = false;
+ }
+
private:
+ /**
+ * Timer callback called when we don't receive a BDX init within a reasonable time after a successful QueryImage response.
+ */
+ static void HandleBdxInitReceivedTimeoutExpired(chip::System::Layer * systemLayer, void * state)
+ {
+ VerifyOrReturn(state != nullptr);
+ static_cast<BdxOTASender *>(state)->ResetState();
+ }
+
CHIP_ERROR OnMessageToSend(TransferSession::OutputEvent & event)
{
assertChipStackLockedByCurrentThread();
@@ -137,12 +176,41 @@ CHIP_ERROR OnMessageToSend(TransferSession::OutputEvent & event)
}
auto & msgTypeData = event.msgTypeData;
- return mExchangeCtx->SendMessage(msgTypeData.ProtocolId, msgTypeData.MessageType, std::move(event.MsgData), sendFlags);
+ // If there's an error sending the message, close the exchange and call ResetState.
+ // TODO: If we can remove the !mInitialized check in ResetState(), just calling ResetState() will suffice here.
+ CHIP_ERROR err
+ = mExchangeCtx->SendMessage(msgTypeData.ProtocolId, msgTypeData.MessageType, std::move(event.MsgData), sendFlags);
+ if (err != CHIP_NO_ERROR) {
+ mExchangeCtx->Close();
+ mExchangeCtx = nullptr;
+ ResetState();
+ }
+ // If we sent a status report and it was successful, set the Exchange context to null and call ResetState.
+ if (event.msgTypeData.HasMessageType(Protocols::SecureChannel::MsgType::StatusReport) && err == CHIP_NO_ERROR) {
+ mExchangeCtx = nullptr;
+ ResetState();
+ }
+ return err;
+ }
+
+ bdx::StatusCode GetBdxStatusCodeFromChipError(CHIP_ERROR err)
+ {
+ if (err == CHIP_ERROR_INCORRECT_STATE) {
+ return bdx::StatusCode::kUnexpectedMessage;
+ }
+ if (err == CHIP_ERROR_INVALID_ARGUMENT) {
+ return bdx::StatusCode::kBadMessageContents;
+ }
+ return bdx::StatusCode::kUnknown;
}
CHIP_ERROR OnTransferSessionBegin(TransferSession::OutputEvent & event)
{
assertChipStackLockedByCurrentThread();
+ // Once we receive the BDX init, cancel the BDX Init timeout and start the BDX session
+ if (mSystemLayer) {
+ mSystemLayer->CancelTimer(HandleBdxInitReceivedTimeoutExpired, this);
+ }
VerifyOrReturnError(mFabricIndex.HasValue(), CHIP_ERROR_INCORRECT_STATE);
VerifyOrReturnError(mNodeId.HasValue(), CHIP_ERROR_INCORRECT_STATE);
@@ -169,8 +237,9 @@ CHIP_ERROR OnTransferSessionBegin(TransferSession::OutputEvent & event)
}
if (error != nil) {
- LogErrorOnFailure([MTRError errorToCHIPErrorCode:error]);
- LogErrorOnFailure(mTransfer.AbortTransfer(bdx::StatusCode::kUnknown));
+ CHIP_ERROR err = [MTRError errorToCHIPErrorCode:error];
+ LogErrorOnFailure(err);
+ LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(err)));
return;
}
@@ -328,6 +397,9 @@ void HandleTransferSessionOutput(TransferSession::OutputEvent & event) override
switch (event.EventType) {
case TransferSession::OutputEventType::kInitReceived:
err = OnTransferSessionBegin(event);
+ if (err != CHIP_NO_ERROR) {
+ LogErrorOnFailure(mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(err)));
+ }
break;
case TransferSession::OutputEventType::kStatusReceived:
ChipLogError(BDX, "Got StatusReport %x", static_cast<uint16_t>(event.statusData.statusCode));
@@ -370,6 +442,10 @@ CHIP_ERROR ConfigureState(chip::FabricIndex fabricIndex, chip::NodeId nodeId)
ResetState();
}
+ // Start a timer to track whether we receive a BDX init after a successful query image in a reasonable amount of time
+ CHIP_ERROR err = mSystemLayer->StartTimer(kBdxInitReceivedTimeout, HandleBdxInitReceivedTimeoutExpired, this);
+ VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE);
+
mFabricIndex.SetValue(fabricIndex);
mNodeId.SetValue(nodeId);
@@ -378,27 +454,6 @@ CHIP_ERROR ConfigureState(chip::FabricIndex fabricIndex, chip::NodeId nodeId)
return CHIP_NO_ERROR;
}
- void ResetState()
- {
- assertChipStackLockedByCurrentThread();
-
- if (!mInitialized) {
- return;
- }
-
- Responder::ResetTransfer();
- ++mTransferGeneration;
- mFabricIndex.ClearValue();
- mNodeId.ClearValue();
-
- if (mExchangeCtx != nullptr) {
- mExchangeCtx->Close();
- mExchangeCtx = nullptr;
- }
-
- mInitialized = false;
- }
-
bool mInitialized = false;
Optional<FabricIndex> mFabricIndex;
Optional<NodeId> mNodeId;
@@ -574,11 +629,19 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
CHIP_ERROR err = gOtaSender.PrepareForTransfer(fabricIndex, nodeId);
if (CHIP_NO_ERROR != err) {
LogErrorOnFailure(err);
+ if (err == CHIP_ERROR_BUSY) {
+ Commands::QueryImageResponse::Type busyResponse;
+ busyResponse.status = static_cast<OTAQueryStatus>(MTROTASoftwareUpdateProviderOTAQueryStatusBusy);
+ busyResponse.delayedActionTime.SetValue(response.delayedActionTime.ValueOr(kDelayedActionTime));
+ handler->AddResponse(cachedCommandPath, busyResponse);
+ handle.Release();
+ return;
+ }
handler->AddStatus(cachedCommandPath, Protocols::InteractionModel::Status::Failure);
handle.Release();
+ gOtaSender.ResetState();
return;
}
-
auto targetNodeId
= handler->GetExchangeContext()->GetSessionHandle()->AsSecureSession()->GetLocalScopedNodeId();
@@ -589,6 +652,7 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
LogErrorOnFailure(err);
handler->AddStatus(cachedCommandPath, Protocols::InteractionModel::Status::Failure);
handle.Release();
+ gOtaSender.ResetState();
return;
}
@@ -597,9 +661,17 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
handle.Release();
return;
}
-
+ if (!isBDXProtocolSupported) {
+ Commands::QueryImageResponse::Type protocolNotSupportedResponse;
+ protocolNotSupportedResponse.status
+ = static_cast<OTAQueryStatus>(MTROTASoftwareUpdateProviderOTAQueryStatusBusy);
+ handler->AddResponse(cachedCommandPath, protocolNotSupportedResponse);
+ } else {
+ handler->AddResponse(cachedCommandPath, response);
+ }
handler->AddResponse(cachedCommandPath, response);
handle.Release();
+ gOtaSender.ResetState();
}
errorHandler:^(NSError *) {
diff --git a/src/protocols/bdx/TransferFacilitator.cpp b/src/protocols/bdx/TransferFacilitator.cpp
index 65ab653420c2cd..767d16313994e4 100644
--- a/src/protocols/bdx/TransferFacilitator.cpp
+++ b/src/protocols/bdx/TransferFacilitator.cpp
@@ -112,6 +112,7 @@ CHIP_ERROR Responder::PrepareForTransfer(System::Layer * layer, TransferRole rol
void Responder::ResetTransfer()
{
mTransfer.Reset();
+ mStopPolling = true;
}
CHIP_ERROR Initiator::InitiateTransfer(System::Layer * layer, TransferRole role, const TransferSession::TransferInitData & initData,
From fb41a58a81752078d78c0f19405296942eed9935 Mon Sep 17 00:00:00 2001
From: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com>
Date: Thu, 2 Feb 2023 15:48:06 -0800
Subject: [PATCH 2/6] Update
src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
---
src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm b/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
index 65b338f36e0bc4..402bd9e3302ce3 100644
--- a/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
+++ b/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
@@ -664,7 +664,7 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
if (!isBDXProtocolSupported) {
Commands::QueryImageResponse::Type protocolNotSupportedResponse;
protocolNotSupportedResponse.status
- = static_cast<OTAQueryStatus>(MTROTASoftwareUpdateProviderOTAQueryStatusBusy);
+ = static_cast<OTAQueryStatus>(MTROTASoftwareUpdateProviderOTAQueryStatusDownloadProtocolNotSupported);
handler->AddResponse(cachedCommandPath, protocolNotSupportedResponse);
} else {
handler->AddResponse(cachedCommandPath, response);
From 15c255e46617d2ec97ee591ff20ff61a00b69fda Mon Sep 17 00:00:00 2001
From: Nivi Sarkar <55898241+nivi-apple@users.noreply.github.com>
Date: Thu, 2 Feb 2023 15:48:21 -0800
Subject: [PATCH 3/6] Update
src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
---
src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm b/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
index 402bd9e3302ce3..34a9cce0c1a373 100644
--- a/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
+++ b/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
@@ -669,7 +669,6 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
} else {
handler->AddResponse(cachedCommandPath, response);
}
- handler->AddResponse(cachedCommandPath, response);
handle.Release();
gOtaSender.ResetState();
}
From 0e11ee92dac5ddb83a9f0909efba9756aee0761d Mon Sep 17 00:00:00 2001
From: Nivedita Sarkar <nivedita_sarkar@apple.com>
Date: Fri, 3 Feb 2023 11:45:04 -0800
Subject: [PATCH 4/6] Reset mStopPolling to false when we call
PrepareForTransfer to start polling again
- This covers the use case where we have an image available but the requestor rejects
the transfer for some reason. On the next query image the requestor is able to
continue with the BDX transfer. The provider state was not being reset for this scenario
due to the polling flag not being reset.
- Fixes #23573
---
src/protocols/bdx/TransferFacilitator.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/protocols/bdx/TransferFacilitator.cpp b/src/protocols/bdx/TransferFacilitator.cpp
index 767d16313994e4..3195bb4749ac36 100644
--- a/src/protocols/bdx/TransferFacilitator.cpp
+++ b/src/protocols/bdx/TransferFacilitator.cpp
@@ -102,6 +102,7 @@ CHIP_ERROR Responder::PrepareForTransfer(System::Layer * layer, TransferRole rol
mPollFreq = pollFreq;
mSystemLayer = layer;
+ mStopPolling = false;
ReturnErrorOnFailure(mTransfer.WaitForTransfer(role, xferControlOpts, maxBlockSize, timeout));
From fb92d595a6654cd96f1f9bfcfa526fcf121c2824 Mon Sep 17 00:00:00 2001
From: Nivedita Sarkar <nivedita_sarkar@apple.com>
Date: Fri, 3 Feb 2023 13:03:15 -0800
Subject: [PATCH 5/6] Addressed review comments
---
.../CHIP/MTROTAProviderDelegateBridge.mm | 149 +++++++++---------
1 file changed, 76 insertions(+), 73 deletions(-)
diff --git a/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm b/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
index 34a9cce0c1a373..3ac55cc9da5e67 100644
--- a/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
+++ b/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
@@ -48,7 +48,7 @@
constexpr System::Clock::Timeout kBdxInitReceivedTimeout = System::Clock::Seconds16(10 * 60);
// Time in seconds after which the requestor should retry calling query image if busy status is receieved
-constexpr uint32_t kDelayedActionTime = 120;
+constexpr uint32_t kDelayedActionTimeSeconds = 120;
constexpr System::Clock::Timeout kBdxTimeout = System::Clock::Seconds16(5 * 60); // OTA Spec mandates >= 5 minutes
constexpr System::Clock::Timeout kBdxPollIntervalMs = System::Clock::Milliseconds32(50);
@@ -184,9 +184,9 @@ CHIP_ERROR OnMessageToSend(TransferSession::OutputEvent & event)
mExchangeCtx->Close();
mExchangeCtx = nullptr;
ResetState();
- }
- // If we sent a status report and it was successful, set the Exchange context to null and call ResetState.
- if (event.msgTypeData.HasMessageType(Protocols::SecureChannel::MsgType::StatusReport) && err == CHIP_NO_ERROR) {
+ } else if (event.msgTypeData.HasMessageType(Protocols::SecureChannel::MsgType::StatusReport) {
+ // If the send was successful for a status report, since we are not expecting a response the exchange context is
+ // already closed. We need to null out the reference to avoid having a dangling pointer.
mExchangeCtx = nullptr;
ResetState();
}
@@ -444,6 +444,9 @@ CHIP_ERROR ConfigureState(chip::FabricIndex fabricIndex, chip::NodeId nodeId)
// Start a timer to track whether we receive a BDX init after a successful query image in a reasonable amount of time
CHIP_ERROR err = mSystemLayer->StartTimer(kBdxInitReceivedTimeout, HandleBdxInitReceivedTimeoutExpired, this);
+
+ // The caller of this method maps CHIP_ERROR to specific BDX Status Codes (see GetBdxStatusCodeFromChipError)
+ // and those are used by the BDX session to prepare the status report.
VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE);
mFabricIndex.SetValue(fabricIndex);
@@ -604,79 +607,79 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
__block CommandHandler::Handle handle(commandObj);
__block ConcreteCommandPath cachedCommandPath(commandPath.mEndpointId, commandPath.mClusterId, commandPath.mCommandId);
- auto completionHandler
- = ^(MTROTASoftwareUpdateProviderClusterQueryImageResponseParams * _Nullable data, NSError * _Nullable error) {
- [controller
- asyncDispatchToMatterQueue:^() {
- assertChipStackLockedByCurrentThread();
+ auto completionHandler = ^(
+ MTROTASoftwareUpdateProviderClusterQueryImageResponseParams * _Nullable data, NSError * _Nullable error) {
+ [controller
+ asyncDispatchToMatterQueue:^() {
+ assertChipStackLockedByCurrentThread();
- CommandHandler * handler = EnsureValidState(handle, cachedCommandPath, "QueryImage", data, error);
- VerifyOrReturn(handler != nullptr);
+ CommandHandler * handler = EnsureValidState(handle, cachedCommandPath, "QueryImage", data, error);
+ VerifyOrReturn(handler != nullptr);
- ChipLogDetail(Controller, "QueryImage: application responded with: %s",
- [[data description] cStringUsingEncoding:NSUTF8StringEncoding]);
+ ChipLogDetail(Controller, "QueryImage: application responded with: %s",
+ [[data description] cStringUsingEncoding:NSUTF8StringEncoding]);
- Commands::QueryImageResponse::Type response;
- ConvertFromQueryImageResponseParms(data, response);
-
- auto hasUpdate = [data.status isEqual:@(MTROtaSoftwareUpdateProviderOTAQueryStatusUpdateAvailable)];
- auto isBDXProtocolSupported = [commandParams.protocolsSupported
- containsObject:@(MTROtaSoftwareUpdateProviderOTADownloadProtocolBDXSynchronous)];
-
- if (hasUpdate && isBDXProtocolSupported) {
- auto fabricIndex = handler->GetSubjectDescriptor().fabricIndex;
- auto nodeId = handler->GetSubjectDescriptor().subject;
- CHIP_ERROR err = gOtaSender.PrepareForTransfer(fabricIndex, nodeId);
- if (CHIP_NO_ERROR != err) {
- LogErrorOnFailure(err);
- if (err == CHIP_ERROR_BUSY) {
- Commands::QueryImageResponse::Type busyResponse;
- busyResponse.status = static_cast<OTAQueryStatus>(MTROTASoftwareUpdateProviderOTAQueryStatusBusy);
- busyResponse.delayedActionTime.SetValue(response.delayedActionTime.ValueOr(kDelayedActionTime));
- handler->AddResponse(cachedCommandPath, busyResponse);
- handle.Release();
- return;
- }
- handler->AddStatus(cachedCommandPath, Protocols::InteractionModel::Status::Failure);
- handle.Release();
- gOtaSender.ResetState();
- return;
- }
- auto targetNodeId
- = handler->GetExchangeContext()->GetSessionHandle()->AsSecureSession()->GetLocalScopedNodeId();
-
- char uriBuffer[kMaxBDXURILen];
- MutableCharSpan uri(uriBuffer);
- err = bdx::MakeURI(targetNodeId.GetNodeId(), AsCharSpan(data.imageURI), uri);
- if (CHIP_NO_ERROR != err) {
- LogErrorOnFailure(err);
- handler->AddStatus(cachedCommandPath, Protocols::InteractionModel::Status::Failure);
- handle.Release();
- gOtaSender.ResetState();
- return;
- }
-
- response.imageURI.SetValue(uri);
- handler->AddResponse(cachedCommandPath, response);
- handle.Release();
- return;
- }
- if (!isBDXProtocolSupported) {
- Commands::QueryImageResponse::Type protocolNotSupportedResponse;
- protocolNotSupportedResponse.status
- = static_cast<OTAQueryStatus>(MTROTASoftwareUpdateProviderOTAQueryStatusDownloadProtocolNotSupported);
- handler->AddResponse(cachedCommandPath, protocolNotSupportedResponse);
- } else {
- handler->AddResponse(cachedCommandPath, response);
- }
- handle.Release();
- gOtaSender.ResetState();
- }
+ Commands::QueryImageResponse::Type response;
+ ConvertFromQueryImageResponseParms(data, response);
- errorHandler:^(NSError *) {
- // Not much we can do here
- }];
- };
+ auto hasUpdate = [data.status isEqual:@(MTROtaSoftwareUpdateProviderOTAQueryStatusUpdateAvailable)];
+ auto isBDXProtocolSupported = [commandParams.protocolsSupported
+ containsObject:@(MTROtaSoftwareUpdateProviderOTADownloadProtocolBDXSynchronous)];
+
+ if (hasUpdate && isBDXProtocolSupported) {
+ auto fabricIndex = handler->GetSubjectDescriptor().fabricIndex;
+ auto nodeId = handler->GetSubjectDescriptor().subject;
+ CHIP_ERROR err = gOtaSender.PrepareForTransfer(fabricIndex, nodeId);
+ if (CHIP_NO_ERROR != err) {
+ LogErrorOnFailure(err);
+ if (err == CHIP_ERROR_BUSY) {
+ Commands::QueryImageResponse::Type busyResponse;
+ busyResponse.status = static_cast<OTAQueryStatus>(MTROTASoftwareUpdateProviderOTAQueryStatusBusy);
+ busyResponse.delayedActionTime.SetValue(response.delayedActionTime.ValueOr(kDelayedActionTimeSeconds));
+ handler->AddResponse(cachedCommandPath, busyResponse);
+ handle.Release();
+ return;
+ }
+ handler->AddStatus(cachedCommandPath, Protocols::InteractionModel::Status::Failure);
+ handle.Release();
+ gOtaSender.ResetState();
+ return;
+ }
+ auto targetNodeId
+ = handler->GetExchangeContext()->GetSessionHandle()->AsSecureSession()->GetLocalScopedNodeId();
+
+ char uriBuffer[kMaxBDXURILen];
+ MutableCharSpan uri(uriBuffer);
+ err = bdx::MakeURI(targetNodeId.GetNodeId(), AsCharSpan(data.imageURI), uri);
+ if (CHIP_NO_ERROR != err) {
+ LogErrorOnFailure(err);
+ handler->AddStatus(cachedCommandPath, Protocols::InteractionModel::Status::Failure);
+ handle.Release();
+ gOtaSender.ResetState();
+ return;
+ }
+
+ response.imageURI.SetValue(uri);
+ handler->AddResponse(cachedCommandPath, response);
+ handle.Release();
+ return;
+ }
+ if (!isBDXProtocolSupported) {
+ Commands::QueryImageResponse::Type protocolNotSupportedResponse;
+ protocolNotSupportedResponse.status
+ = static_cast<OTAQueryStatus>(MTROTASoftwareUpdateProviderOTAQueryStatusDownloadProtocolNotSupported);
+ handler->AddResponse(cachedCommandPath, protocolNotSupportedResponse);
+ } else {
+ handler->AddResponse(cachedCommandPath, response);
+ }
+ handle.Release();
+ gOtaSender.ResetState();
+ }
+
+ errorHandler:^(NSError *) {
+ // Not much we can do here
+ }];
+ };
auto strongDelegate = mDelegate;
dispatch_async(mDelegateNotificationQueue, ^{
From db8dce154f0cbd6a376e8d2ad8585aa24c18fdc1 Mon Sep 17 00:00:00 2001
From: Nivedita Sarkar <nivedita_sarkar@apple.com>
Date: Fri, 3 Feb 2023 13:48:02 -0800
Subject: [PATCH 6/6] Fix the handling of the use case when hasUpdate is false
we need to send the response as is
- Log the error if the BDX timer fails to start
---
.../Framework/CHIP/MTROTAProviderDelegateBridge.mm | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm b/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
index 3ac55cc9da5e67..3cd88ab52454eb 100644
--- a/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
+++ b/src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
@@ -184,7 +184,7 @@ CHIP_ERROR OnMessageToSend(TransferSession::OutputEvent & event)
mExchangeCtx->Close();
mExchangeCtx = nullptr;
ResetState();
- } else if (event.msgTypeData.HasMessageType(Protocols::SecureChannel::MsgType::StatusReport) {
+ } else if (event.msgTypeData.HasMessageType(Protocols::SecureChannel::MsgType::StatusReport)) {
// If the send was successful for a status report, since we are not expecting a response the exchange context is
// already closed. We need to null out the reference to avoid having a dangling pointer.
mExchangeCtx = nullptr;
@@ -444,6 +444,7 @@ CHIP_ERROR ConfigureState(chip::FabricIndex fabricIndex, chip::NodeId nodeId)
// Start a timer to track whether we receive a BDX init after a successful query image in a reasonable amount of time
CHIP_ERROR err = mSystemLayer->StartTimer(kBdxInitReceivedTimeout, HandleBdxInitReceivedTimeoutExpired, this);
+ LogErrorOnFailure(err);
// The caller of this method maps CHIP_ERROR to specific BDX Status Codes (see GetBdxStatusCodeFromChipError)
// and those are used by the BDX session to prepare the status report.
@@ -664,13 +665,15 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
handle.Release();
return;
}
- if (!isBDXProtocolSupported) {
+ if (!hasUpdate) {
+ // Send whatever error response our delegate decided on.
+ handler->AddResponse(cachedCommandPath, response);
+ } else {
+ // We must have isBDXProtocolSupported false. Send the corresponding error status.
Commands::QueryImageResponse::Type protocolNotSupportedResponse;
protocolNotSupportedResponse.status
= static_cast<OTAQueryStatus>(MTROTASoftwareUpdateProviderOTAQueryStatusDownloadProtocolNotSupported);
handler->AddResponse(cachedCommandPath, protocolNotSupportedResponse);
- } else {
- handler->AddResponse(cachedCommandPath, response);
}
handle.Release();
gOtaSender.ResetState();