Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@nivi-apple
nivi-apple committed Feb 24, 2023
1 parent 920c904 commit 43bba83
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 14 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
28 changes: 15 additions & 13 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);
VerifyOrReturnError(err == CHIP_NO_ERROR, 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 @@ -639,12 +637,13 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
auto isBDXProtocolSupported = [commandParams.protocolsSupported
containsObject:@(MTROtaSoftwareUpdateProviderOTADownloadProtocolBDXSynchronous)];

// 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
// 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) {
Expand All @@ -657,7 +656,7 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
}

Commands::QueryImageResponse::Type delegateResponse;
ConvertFromQueryImageResponseParms(data, delegateResponse);
ConvertFromQueryImageResponseParams(data, delegateResponse);

// If update is not available, return the delegate response
if (!hasUpdate) {
Expand All @@ -674,7 +673,8 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath

// 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");
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));
Expand All @@ -685,6 +685,8 @@ bool GetPeerNodeInfo(CommandHandler * commandHandler, const ConcreteCommandPath
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 timer
// while preparing for transfer if any failure occurs afterwards.
gOtaSender.ResetState();
return;
}
Expand Down Expand Up @@ -878,7 +880,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 43bba83

Please sign in to comment.