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.

Sign up for GitHub

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

Add a timer to track whether we have received BDX init after a query on Darwin #24777

Merged
merged 6 commits into from
Feb 3, 2023
Merged

Add a timer to track whether we have received BDX init after a query on Darwin #24777

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f9a219c
Add a timer to track whether we have received BDX init after a query …
nivi-apple Feb 2, 2023
fb41a58
Update src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
nivi-apple Feb 2, 2023
15c255e
Update src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
nivi-apple Feb 2, 2023
0e11ee9
Reset mStopPolling to false when we call PrepareForTransfer to start …
nivi-apple Feb 3, 2023
fb92d59
Addressed review comments
nivi-apple Feb 3, 2023
db8dce1
Fix the handling of the use case when hasUpdate is false we need to s…
nivi-apple Feb 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 100 additions & 29 deletions src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved

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 +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;
}

Expand All @@ -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();
Expand All @@ -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.
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
if (event.msgTypeData.HasMessageType(Protocols::SecureChannel::MsgType::StatusReport) && err == CHIP_NO_ERROR) {
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
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);
nivi-apple marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -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;
}

Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved

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

Expand All @@ -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;
Expand Down Expand Up @@ -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();

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

Expand All @@ -597,9 +661,16 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
handle.Release();
return;
}

handler->AddResponse(cachedCommandPath, response);
if (!isBDXProtocolSupported) {
Commands::QueryImageResponse::Type protocolNotSupportedResponse;
protocolNotSupportedResponse.status
= static_cast<OTAQueryStatus>(MTROTASoftwareUpdateProviderOTAQueryStatusDownloadProtocolNotSupported);
handler->AddResponse(cachedCommandPath, protocolNotSupportedResponse);
} else {
handler->AddResponse(cachedCommandPath, response);
}
nivi-apple marked this conversation as resolved.
Show resolved Hide resolved
handle.Release();
gOtaSender.ResetState();
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
}

errorHandler:^(NSError *) {
Expand Down
1 change: 1 addition & 0 deletions src/protocols/bdx/TransferFacilitator.cpp
View file
Open in desktop
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;
nivi-apple marked this conversation as resolved.
Show resolved Hide resolved
}

CHIP_ERROR Initiator::InitiateTransfer(System::Layer * layer, TransferRole role, const TransferSession::TransferInitData & initData,
Expand Down