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

Clean up the code that sends the query image response from the provider #24933

Merged
merged 8 commits into from
Feb 25, 2023
Merged

Clean up the code that sends the query image response from the provider #24933

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
be2f7fb
Clean up the code that sends the query image response from the provider
nivi-apple Feb 17, 2023
214b7bc
Update src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
nivi-apple Feb 24, 2023
e2a2f08
Update src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
nivi-apple Feb 24, 2023
8f980ea
Update src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
nivi-apple Feb 24, 2023
920c904
Update src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
nivi-apple Feb 24, 2023
43bba83
Address review comments
nivi-apple Feb 24, 2023
b23b0ed
Update src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.mm
nivi-apple Feb 24, 2023
b9faa4e
Add comments for why we do not reset state when we return busy from C…
nivi-apple Feb 24, 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
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/MTROTAProviderDelegateBridge.h
View file
Open in desktop
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
Open in desktop
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) {
nivi-apple marked this conversation as resolved.
Show resolved Hide resolved
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);
nivi-apple marked this conversation as resolved.
Show resolved Hide resolved
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();
bzbarsky-apple marked this conversation as resolved.
Show resolved Hide resolved
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