Skip to content

Commit

Permalink
Add a timer to track whether we have received BDX init after a query …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
nivi-apple committed Feb 1, 2023
1 parent 8dc9323 commit a957601
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 31 deletions.
3 changes: 3 additions & 0 deletions src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class MTROTAProviderDelegateBridge : public chip::app::Clusters::OTAProviderDele
// touches Matter objects.
void Shutdown();

void ResetState();

// ControllerShuttingDown must be called on the Matter work queue, since it
// touches Matter objects.
void ControllerShuttingDown(MTRDeviceController * controller);
Expand Down Expand Up @@ -65,6 +67,7 @@ class MTROTAProviderDelegateBridge : public chip::app::Clusters::OTAProviderDele
static void ConvertToNotifyUpdateAppliedParams(
const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::NotifyUpdateApplied::DecodableType & commandData,
MTROTASoftwareUpdateProviderClusterNotifyUpdateAppliedParams * commandParams);
static void handleBdxInitReceivedTimeoutExpired(chip::System::Layer * systemLayer, void * state);

_Nullable id<MTROTAProviderDelegate> mDelegate;
dispatch_queue_t mDelegateNotificationQueue;
Expand Down
94 changes: 63 additions & 31 deletions src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@
// 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);

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;
Expand Down Expand Up @@ -89,13 +94,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;
}

Expand All @@ -120,7 +124,40 @@ void SetDelegate(id<MTROTAProviderDelegate> delegate, dispatch_queue_t delegateN
}
}

void ResetState()
{
assertChipStackLockedByCurrentThread();
if (mSystemLayer) {
mSystemLayer->CancelTimer(handleBdxInitReceivedTimeoutExpired, this);
}

if (!mInitialized) {
return;
}
Responder::ResetTransfer();
++mTransferGeneration;
mFabricIndex.ClearValue();
mNodeId.ClearValue();

if (mExchangeCtx != nullptr) {
mExchangeCtx->Close();
mExchangeCtx = nullptr;
}

mInitialized = false;
}

private:
/**
* The callback when the BDX init received timer expires to track if a BDX init was received after a successful query image
* within a reasonable amount of time.
*/
static void handleBdxInitReceivedTimeoutExpired(chip::System::Layer * systemLayer, void * state)
{
VerifyOrReturn(state != nullptr);
static_cast<BdxOTASender *>(state)->ResetState();
}

CHIP_ERROR OnMessageToSend(TransferSession::OutputEvent & event)
{
assertChipStackLockedByCurrentThread();
Expand All @@ -143,7 +180,10 @@ CHIP_ERROR OnMessageToSend(TransferSession::OutputEvent & event)
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);
uint16_t fdl = 0;
Expand All @@ -165,6 +205,7 @@ CHIP_ERROR OnTransferSessionBegin(TransferSession::OutputEvent & event)

if (!mInitialized || mTransferGeneration != transferGeneration) {
// Callback for a stale transfer.
LogErrorOnFailure(mTransfer.AbortTransfer(bdx::StatusCode::kUnexpectedMessage));
return;
}

Expand Down Expand Up @@ -370,6 +411,9 @@ 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 reasonabale amount of time
mSystemLayer->StartTimer(kBdxInitReceivedTimeout, handleBdxInitReceivedTimeoutExpired, this);

mFabricIndex.SetValue(fabricIndex);
mNodeId.SetValue(nodeId);

Expand All @@ -378,27 +422,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;
Expand Down Expand Up @@ -574,11 +597,18 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
CHIP_ERROR err = gOtaSender.PrepareForTransfer(fabricIndex, nodeId);
if (CHIP_NO_ERROR != err) {
LogErrorOnFailure(err);
handler->AddStatus(cachedCommandPath, Protocols::InteractionModel::Status::Failure);
handle.Release();
return;
if (err == CHIP_ERROR_BUSY) {
response.status = static_cast<OTAQueryStatus>(MTROTASoftwareUpdateProviderOTAQueryStatusBusy);
handler->AddResponse(cachedCommandPath, response);
handle.Release();
return;
} else {
handler->AddStatus(cachedCommandPath, Protocols::InteractionModel::Status::Failure);
handle.Release();
gOtaSender.ResetState();
return;
}
}

auto targetNodeId
= handler->GetExchangeContext()->GetSessionHandle()->AsSecureSession()->GetLocalScopedNodeId();

Expand All @@ -589,6 +619,7 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
LogErrorOnFailure(err);
handler->AddStatus(cachedCommandPath, Protocols::InteractionModel::Status::Failure);
handle.Release();
gOtaSender.ResetState();
return;
}

Expand All @@ -600,11 +631,12 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath

handler->AddResponse(cachedCommandPath, response);
handle.Release();
gOtaSender.ResetState();
}

errorHandler:^(NSError *) {
// Not much we can do here
}];
errorHandler:^(NSError *) {
gOtaSender.ResetState();
}];
};

auto strongDelegate = mDelegate;
Expand Down
1 change: 1 addition & 0 deletions src/protocols/bdx/TransferFacilitator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit a957601

Please sign in to comment.