Skip to content

Commit

Permalink
Addressed review comments and removed per subject handling, for now
Browse files Browse the repository at this point in the history 
supporting only global user consent poliy.
  • Loading branch information
@shubhamdp
shubhamdp committed Feb 2, 2022
1 parent fa45a7f commit cce6329
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 131 deletions.
14 changes: 7 additions & 7 deletions examples/ota-provider-app/linux/main.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,15 +191,15 @@ bool HandleOptions(const char * aProgram, OptionSet * aOptions, int aIdentifier,
PrintArgError("%s: ERROR: NULL UserConsent parameter\n", aProgram);
retval = false;
}
else if (strcmp(aValue, "Granted") == 0)
else if (strcmp(aValue, "granted") == 0)
{
gUserConsentState = chip::ota::UserConsentState::kGranted;
}
else if (strcmp(aValue, "Denied") == 0)
else if (strcmp(aValue, "denied") == 0)
{
gUserConsentState = chip::ota::UserConsentState::kDenied;
}
else if (strcmp(aValue, "Deferred") == 0)
else if (strcmp(aValue, "deferred") == 0)
{
gUserConsentState = chip::ota::UserConsentState::kObtaining;
}
Expand Down Expand Up @@ -237,10 +237,10 @@ OptionSet cmdLineOptions = { HandleOptions, cmdLineOptionsDef, "PROGRAM OPTIONS"
" -d/--DelayedActionTimeSec <time>\n"
" Value in seconds for the DelayedActionTime in the Query Image Response\n"
" and Apply Update Response\n"
" -u/--UserConsent <Granted | Denied | Deferred>\n"
" Granted: Status value in QueryImageResponse is set to UpdateAvailable\n"
" Denied: Status value in QueryImageResponse is set to UpdateNotAvailable\n"
" Deferred: Status value in QueryImageResponse is set to Busy\n"
" -u/--UserConsent <granted | denied | deferred>\n"
" granted: Status value in QueryImageResponse is set to UpdateAvailable\n"
" denied: Status value in QueryImageResponse is set to UpdateNotAvailable\n"
" deferred: Status value in QueryImageResponse is set to Busy\n"
" -q/--QueryImageBehavior overrides this option\n" };

HelpOptions helpOptions("ota-provider-app", "Usage: ota-provider-app [options]", "1.0");
Expand Down
82 changes: 7 additions & 75 deletions examples/ota-provider-app/ota-provider-common/DefaultUserConsentProvider.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,16 @@
namespace chip {
namespace ota {

DefaultUserConsentProvider::DefaultUserConsentProvider()
{
for (uint8_t i = 0; i < kMaxUserConsentEntries; i++)
{
mUserConsentEntries[i].isEntryValid = false;
}
}

void DefaultUserConsentProvider::LogUserConsentSubject(const UserConsentSubject & subject)
{
ChipLogDetail(SoftwareUpdate, "User consent request for:");
ChipLogDetail(SoftwareUpdate, ": FabricId: " ChipLogFormatX64, ChipLogValueX64(subject.fabricId));
ChipLogDetail(SoftwareUpdate, ": NodeId: " ChipLogFormatX64, ChipLogValueX64(subject.nodeId));
ChipLogDetail(SoftwareUpdate, ": EndpointId: %" PRIu16, subject.endpointId);
ChipLogDetail(SoftwareUpdate, ": VendorId: %" PRIu16, subject.vendorId);
ChipLogDetail(SoftwareUpdate, ": ProductId: %" PRIu16, subject.productId);
ChipLogDetail(SoftwareUpdate, ": CurrentVersion: %" PRIu32, subject.currentVersion);
ChipLogDetail(SoftwareUpdate, ": NewVersion: %" PRIu32, subject.targetVersion);
ChipLogDetail(SoftwareUpdate, ": FabricIndex: %" PRIu8, subject.fabricIndex);
ChipLogDetail(SoftwareUpdate, ": RequestorNodeId: " ChipLogFormatX64, ChipLogValueX64(subject.requestorNodeId));
ChipLogDetail(SoftwareUpdate, ": ProviderEndpointId: %" PRIu16, subject.providerEndpointId);
ChipLogDetail(SoftwareUpdate, ": RequestorVendorId: %" PRIu16, subject.requestorVendorId);
ChipLogDetail(SoftwareUpdate, ": RequestorProductId: %" PRIu16, subject.requestorProductId);
ChipLogDetail(SoftwareUpdate, ": RequestorCurrentVersion: %" PRIu32, subject.requestorCurrentVersion);
ChipLogDetail(SoftwareUpdate, ": RequestorTargetVersion: %" PRIu32, subject.requestorTargetVersion);
ChipLogDetail(SoftwareUpdate, ": Metadata:");
ChipLogByteSpan(SoftwareUpdate, subject.metadata);
}
Expand All @@ -52,68 +44,8 @@ UserConsentState DefaultUserConsentProvider::GetUserConsentState(const UserConse
return mGlobalConsentState;
}

for (uint8_t i = 0; i < kMaxUserConsentEntries; i++)
{
if (mUserConsentEntries[i].isEntryValid && mUserConsentEntries[i].subject == subject)
{
return mUserConsentEntries[i].state;
}
}
return UserConsentState::kGranted;
}

CHIP_ERROR DefaultUserConsentProvider::SetUserConsentState(const UserConsentSubject & subject, UserConsentState state)
{
for (uint8_t i = 0; i < kMaxUserConsentEntries; i++)
{
if (mUserConsentEntries[i].isEntryValid && mUserConsentEntries[i].subject == subject)
{
mUserConsentEntries[i].state = state;
return CHIP_NO_ERROR;
}
}

for (uint8_t i = 0; i < kMaxUserConsentEntries; i++)
{
if (mUserConsentEntries[i].isEntryValid == false)
{
mUserConsentEntries[i].subject = subject;
mUserConsentEntries[i].state = state;
mUserConsentEntries[i].isEntryValid = true;
return CHIP_NO_ERROR;
}
}

return CHIP_ERROR_NO_MEMORY;
}

CHIP_ERROR DefaultUserConsentProvider::GrantUserConsent(const UserConsentSubject & subject)
{
return SetUserConsentState(subject, UserConsentState::kGranted);
}

CHIP_ERROR DefaultUserConsentProvider::RevokeUserConsent(const UserConsentSubject & subject)
{
return SetUserConsentState(subject, UserConsentState::kDenied);
}

CHIP_ERROR DefaultUserConsentProvider::DeferUserConsent(const UserConsentSubject & subject)
{
return SetUserConsentState(subject, UserConsentState::kObtaining);
}

CHIP_ERROR DefaultUserConsentProvider::ClearUserConsentEntry(const UserConsentSubject & subject)
{
for (uint8_t i = 0; i < kMaxUserConsentEntries; i++)
{
if (mUserConsentEntries[i].isEntryValid && mUserConsentEntries[i].subject == subject)
{
mUserConsentEntries[i].isEntryValid = false;
return CHIP_NO_ERROR;
}
}
return CHIP_ERROR_NOT_FOUND;
}

} // namespace ota
} // namespace chip
25 changes: 1 addition & 24 deletions examples/ota-provider-app/ota-provider-common/DefaultUserConsentProvider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,25 +26,13 @@ namespace ota {
class DefaultUserConsentProvider : public UserConsentDelegate
{
public:
DefaultUserConsentProvider();
DefaultUserConsentProvider() = default;

~DefaultUserConsentProvider() = default;

// This method returns kGranted unless explicitly denied by the user by calling RevokeUserConsent()
UserConsentState GetUserConsentState(const UserConsentSubject & subject) override;

// Grant the user consent for the given subject for OTA updates
CHIP_ERROR GrantUserConsent(const UserConsentSubject & subject);

// Revoke the user consent for the given subject for OTA updates
CHIP_ERROR RevokeUserConsent(const UserConsentSubject & subject);

// Set the user consent as deferred for the given subject for OTA updates
CHIP_ERROR DeferUserConsent(const UserConsentSubject & subject);

// Mark the user consent entry as invalid
CHIP_ERROR ClearUserConsentEntry(const UserConsentSubject & subject);

// If this is set to true, all the user consent requests will be replied with global consent.
void SetGlobalUserConsentState(UserConsentState state)
{
Expand All @@ -63,21 +51,10 @@ class DefaultUserConsentProvider : public UserConsentDelegate
void ClearGlobalUserConsentState() { mUseGlobalConsent = false; }

private:
CHIP_ERROR SetUserConsentState(const UserConsentSubject & subject, UserConsentState state);

static constexpr uint8_t kMaxUserConsentEntries = 10;

bool mUseGlobalConsent = false;

UserConsentState mGlobalConsentState = UserConsentState::kGranted;

struct UserConsentEntry
{
bool isEntryValid;
UserConsentSubject subject;
UserConsentState state;
} mUserConsentEntries[kMaxUserConsentEntries];

void LogUserConsentSubject(const UserConsentSubject & subject);
};

Expand Down
23 changes: 13 additions & 10 deletions examples/ota-provider-app/ota-provider-common/OTAProviderExample.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,13 @@ UserConsentSubject OTAProviderExample::GetUserConsentSubject(const app::CommandH
const QueryImage::DecodableType & commandData, uint32_t targetVersion)
{
UserConsentSubject subject;
FabricIndex fabricIndex = commandObj->GetSubjectDescriptor().fabricIndex;
subject.fabricId = Server::GetInstance().GetFabricTable().FindFabricWithIndex(fabricIndex)->GetFabricId();
subject.nodeId = commandObj->GetSubjectDescriptor().subject;
subject.endpointId = commandPath.mEndpointId;
subject.vendorId = commandData.vendorId;
subject.productId = commandData.productId;
subject.currentVersion = commandData.softwareVersion;
subject.targetVersion = targetVersion;
subject.fabricIndex = commandObj->GetSubjectDescriptor().fabricIndex;
subject.requestorNodeId = commandObj->GetSubjectDescriptor().subject;
subject.providerEndpointId = commandPath.mEndpointId;
subject.requestorVendorId = commandData.vendorId;
subject.requestorProductId = commandData.productId;
subject.requestorCurrentVersion = commandData.softwareVersion;
subject.requestorTargetVersion = targetVersion;
if (commandData.metadataForProvider.HasValue())
{
subject.metadata = commandData.metadataForProvider.Value();
Expand Down Expand Up @@ -272,8 +271,12 @@ EmberAfStatus OTAProviderExample::HandleQueryImage(chip::app::CommandHandler * c
response.status = queryStatus;
response.delayedActionTime.Emplace(delayedActionTimeSec);
response.userConsentNeeded.Emplace(requestorCanConsent);
// Could also just not send metadataForRequestor at all.
response.metadataForRequestor.Emplace(chip::ByteSpan());

// For test coverage, sending empty metadata when (requestorNodeId % 2) == 0 and not sending otherwise.
if (commandObj->GetSubjectDescriptor().subject % 2 == 0)
{
response.metadataForRequestor.Emplace(chip::ByteSpan());
}

VerifyOrReturnError(commandObj->AddResponseData(commandPath, response) == CHIP_NO_ERROR, EMBER_ZCL_STATUS_FAILURE);
return EMBER_ZCL_STATUS_SUCCESS;
Expand Down
42 changes: 27 additions & 15 deletions examples/ota-provider-app/ota-provider-common/UserConsentDelegate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,24 +34,36 @@ enum UserConsentState
kUnknown,
};

/**
* @brief User consent subject contains the information of the OTA requestor
* that requires obtaining user consent for performing the OTA update.
*/
struct UserConsentSubject
{
FabricId fabricId;
NodeId nodeId;
EndpointId endpointId;
uint16_t vendorId;
uint16_t productId;
uint32_t currentVersion;
uint32_t targetVersion;
ByteSpan metadata;
// Fabric Index
FabricIndex fabricIndex;

bool operator==(const UserConsentSubject & other) const
{
return (fabricId == other.fabricId && nodeId == other.nodeId && endpointId == other.endpointId &&
vendorId == other.vendorId && productId == other.productId && currentVersion == other.currentVersion &&
targetVersion == other.targetVersion);
// Ignoring medatada comparison
}
// Node ID of the OTA Requestor
NodeId requestorNodeId;

// Endpoint of the OTA Provider
EndpointId providerEndpointId;

// Vendor ID of the OTA Requestor
uint16_t requestorVendorId;

// Product ID of the OTA Requestor
uint16_t requestorProductId;

// Current software version of the OTA Requestor
uint32_t requestorCurrentVersion;

// Target software version available for the OTA Requestor
uint32_t requestorTargetVersion;

// This data is not owned by UserConsentSubject and therefore any user of this field
// has to copy the data and own it if not immediately used from an argument having a UserConsentSubject
ByteSpan metadata;
};

class UserConsentDelegate
Expand Down

0 comments on commit cce6329

Please sign in to comment.