Skip to content

Commit

Permalink
Clean up the code that sends the query image response from the provid…
Browse files Browse the repository at this point in the history 
…er (#24933)

* Clean up the code that sends the query image response from the provider

* Update src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm

Co-authored-by: Boris Zbarsky <[email protected]>

* Update src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm

Co-authored-by: Boris Zbarsky <[email protected]>

* Update src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm

Co-authored-by: Boris Zbarsky <[email protected]>

* Update src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm

Co-authored-by: Boris Zbarsky <[email protected]>

* Address review comments

* Update src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm

Co-authored-by: Boris Zbarsky <[email protected]>

* Add comments for why we do not reset state when we return busy from Configure State

---------

Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
@nivi-apple @bzbarsky-apple
nivi-apple and bzbarsky-apple authored Feb 25, 2023
1 parent 95772ae commit 265d55c
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 50 deletions.
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ class MTROTAProviderDelegateBridge : public chip::app::Clusters::OTAProviderDele
static CHIP_ERROR ConvertToQueryImageParams(
const chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::QueryImage::DecodableType & commandData,
MTROTASoftwareUpdateProviderClusterQueryImageParams * commandParams);
static void ConvertFromQueryImageResponseParms(
static void ConvertFromQueryImageResponseParams(
const MTROTASoftwareUpdateProviderClusterQueryImageResponseParams * responseParams,
chip::app::Clusters::OtaSoftwareUpdateProvider::Commands::QueryImageResponse::Type & response);
static void ConvertToApplyUpdateRequestParams(
Expand Down
115 changes: 66 additions & 49 deletions src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,9 +461,7 @@ CHIP_ERROR ConfigureState(chip::FabricIndex fabricIndex, chip::NodeId nodeId)
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.
VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_ERROR_INCORRECT_STATE);
ReturnErrorOnFailure(err);

mFabricIndex.SetValue(fabricIndex);
mNodeId.SetValue(nodeId);
Expand Down Expand Up @@ -615,7 +613,7 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
auto * commandParams = [[MTROTASoftwareUpdateProviderClusterQueryImageParams alloc] init];
CHIP_ERROR err = ConvertToQueryImageParams(commandData, commandParams);
if (err != CHIP_NO_ERROR) {
commandObj->AddStatus(commandPath, Protocols::InteractionModel::Status::InvalidCommand);
commandObj->AddStatus(commandPath, StatusIB(err).mStatus);
return;
}

Expand All @@ -635,64 +633,83 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
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(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();
// The logic we are following here is if none of the protocols supported by the requestor are supported by us, we
// can't transfer the image even if we had an image available and we would return a Protocol Not Supported status.
// Assumption here is the requestor would send us a list of all the protocols it supports. If one/more of the
// protocols supported by the requestor are supported by us, we check if an image is not available due to various
// reasons - image not available, delegate reporting busy, we will respond with the status in the delegate response.
// If update is available, we try to prepare for transfer and build the uri in the response with a status of Image
// Available

// If the protocol requested is not supported, return status - Protocol Not Supported
if (!isBDXProtocolSupported) {
Commands::QueryImageResponse::Type response;
response.status
= static_cast<OTAQueryStatus>(MTROTASoftwareUpdateProviderOTAQueryStatusDownloadProtocolNotSupported);
handler->AddResponse(cachedCommandPath, response);
handle.Release();
return;
}

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);
Commands::QueryImageResponse::Type delegateResponse;
ConvertFromQueryImageResponseParams(data, delegateResponse);

// If update is not available, return the delegate response
if (!hasUpdate) {
handler->AddResponse(cachedCommandPath, delegateResponse);
handle.Release();
return;
}

// If there is an update available, try to prepare for a transfer.
auto fabricIndex = handler->GetSubjectDescriptor().fabricIndex;
auto nodeId = handler->GetSubjectDescriptor().subject;
CHIP_ERROR err = gOtaSender.PrepareForTransfer(fabricIndex, nodeId);
if (CHIP_NO_ERROR != err) {

// Handle busy error separately as we have a query image response status that maps to busy
if (err == CHIP_ERROR_BUSY) {
ChipLogError(
Controller, "Responding with Busy due to being in the middle of handling another BDX transfer");
Commands::QueryImageResponse::Type response;
response.status = static_cast<OTAQueryStatus>(MTROTASoftwareUpdateProviderOTAQueryStatusBusy);
response.delayedActionTime.SetValue(delegateResponse.delayedActionTime.ValueOr(kDelayedActionTimeSeconds));
handler->AddResponse(cachedCommandPath, response);
handle.Release();
gOtaSender.ResetState();
// We do not reset state when we get the busy error because that means we are locked in a BDX transfer
// session with another requestor when we get this query image request. We do not want to interrupt the
// ongoing transfer instead just respond to the second requestor with a busy status and a delayedActionTime
// in which the requestor can retry.
return;
}

response.imageURI.SetValue(uri);
handler->AddResponse(cachedCommandPath, response);
LogErrorOnFailure(err);
handler->AddStatus(cachedCommandPath, StatusIB(err).mStatus);
handle.Release();
// We need to reset state here to clean up any initialization we might have done including starting the BDX
// timeout timer while preparing for transfer if any failure occurs afterwards.
gOtaSender.ResetState();
return;
}
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);
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, StatusIB(err).mStatus);
handle.Release();
gOtaSender.ResetState();
return;
}
delegateResponse.imageURI.SetValue(uri);
handler->AddResponse(cachedCommandPath, delegateResponse);
handle.Release();
}

errorHandler:^(NSError *) {
// Not much we can do here
}];
Expand Down Expand Up @@ -867,7 +884,7 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
return CHIP_NO_ERROR;
}

void MTROTAProviderDelegateBridge::ConvertFromQueryImageResponseParms(
void MTROTAProviderDelegateBridge::ConvertFromQueryImageResponseParams(
const MTROTASoftwareUpdateProviderClusterQueryImageResponseParams * responseParams,
Commands::QueryImageResponse::Type & response)
{
Expand Down

0 comments on commit 265d55c

Please sign in to comment.