Skip to content

Commit a07a8e4

Browse files
gabc-cppgabc-cpp
committed
Issue 37140 - new getters and setters
1 parent 799930b commit a07a8e4

File tree

5 files changed

+144
-45
lines changed

5 files changed

+144
-45
lines changed

‎src/app/InteractionModelEngine.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -1761,8 +1761,8 @@ Protocols::InteractionModel::Status InteractionModelEngine::CheckCommandAccess(c
17611761
Protocols::InteractionModel::Status InteractionModelEngine::CheckCommandFlags(const DataModel::InvokeRequest & aRequest,
17621762
const DataModel::AcceptedCommandEntry & entry)
17631763
{
1764-
const bool commandNeedsTimedInvoke = entry.flags.Has(DataModel::CommandQualityFlags::kTimed);
1765-
const bool commandIsFabricScoped = entry.flags.Has(DataModel::CommandQualityFlags::kFabricScoped);
1764+
const bool commandNeedsTimedInvoke = entry.FlagsHas(DataModel::CommandQualityFlags::kTimed);
1765+
const bool commandIsFabricScoped = entry.FlagsHas(DataModel::CommandQualityFlags::kFabricScoped);
17661766

17671767
if (commandNeedsTimedInvoke && !aRequest.invokeFlags.Has(DataModel::InvokeFlags::kTimed))
17681768
{

‎src/app/WriteHandler.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ std::optional<bool> WriteHandler::IsListAttributePath(const ConcreteAttributePat
125125
return std::nullopt;
126126
}
127127

128-
return info->flags.Has(DataModel::AttributeQualityFlags::kListAttribute);
128+
return info->FlagsHas(DataModel::AttributeQualityFlags::kListAttribute);
129129
}
130130

131131
Status WriteHandler::HandleWriteRequestMessage(Messaging::ExchangeContext * apExchangeContext,
@@ -823,7 +823,7 @@ DataModel::ActionReturnStatus WriteHandler::CheckWriteAllowed(const Access::Subj
823823
}
824824

825825
// validate that timed write is enforced
826-
VerifyOrReturnValue(IsTimedWrite() || !attributeEntry->flags.Has(DataModel::AttributeQualityFlags::kTimed),
826+
VerifyOrReturnValue(IsTimedWrite() || !attributeEntry->FlagsHas(DataModel::AttributeQualityFlags::kTimed),
827827
Status::NeedsTimedInteraction);
828828

829829
return Status::Success;

‎src/app/data-model-provider/MetadataTypes.h

+124-25
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <access/Privilege.h>
2323
#include <lib/core/DataModelTypes.h>
2424
#include <lib/support/BitFlags.h>
25+
#include <assert.h>
2526

2627
namespace chip {
2728
namespace app {
@@ -84,48 +85,102 @@ enum class AttributeQualityFlags : uint32_t
8485
kTimed = 0x0040, // `T` quality on attributes (writes require timed interactions)
8586
};
8687

88+
89+
struct attribute_entry_mask_t {
90+
91+
// attribute quality flags
92+
//
93+
std::underlying_type_t<AttributeQualityFlags> flags : 7 ; // generally defaults to View if readable
94+
95+
// read/write access privilege variables
96+
//
97+
std::underlying_type_t<Access::Privilege> readPrivilege : 5 ; // generally defaults to View if readable
98+
std::underlying_type_t<Access::Privilege> writePrivilege : 5 ; // generally defaults to Operate if writable
99+
100+
};
101+
102+
103+
// Static ASSERT to check size of mask type.
104+
static_assert(sizeof(attribute_entry_mask_t) <= 4, "Size of attribute_entry_mask_t is not as expected.");
105+
106+
// zero is not a valid privilege, just a default initial value for privileges
107+
constexpr std::underlying_type_t<Access::Privilege> kNoPrivilege(0) ;
108+
109+
87110
struct AttributeEntry
88111
{
89112
AttributeId attributeId;
90-
BitFlags<AttributeQualityFlags> flags;
91-
92113

93114
// Constructor
94-
AttributeEntry() : readPrivilege(0), writePrivilege(0) {}
115+
constexpr AttributeEntry() : attributeId(0), mask{ 0, kNoPrivilege, kNoPrivilege } {}
95116

117+
96118
void SetReadPrivilege(Access::Privilege r)
97119
{
98-
readPrivilege = chip::to_underlying(r);
120+
// Static ASSERT to check size of readPrivilege type vs entry parameter.
121+
static_assert(sizeof(std::underlying_type_t<Access::Privilege>) >=
122+
sizeof(chip::to_underlying(r)),
123+
"Size of readPrivilege is not able to accomodate parameter (r).");
124+
125+
// ASSERT to validate entry parameter value.
126+
assert(kNoPrivilege != chip::to_underlying(r));
127+
128+
129+
mask.readPrivilege = chip::to_underlying(r);
99130
}
100131

101132
Access::Privilege GetReadPrivilege() const
102133
{
103-
return static_cast< Access::Privilege >(readPrivilege);
134+
return static_cast< Access::Privilege >(mask.readPrivilege);
104135
}
105136

106137
void SetWritePrivilege(Access::Privilege w)
107138
{
108-
writePrivilege = chip::to_underlying(w);
139+
// Static ASSERT to check size of writePrivilege type vs entry parameter.
140+
static_assert(sizeof(std::underlying_type_t<Access::Privilege>) >=
141+
sizeof(chip::to_underlying(w)),
142+
"Size of writePrivilege is not able to accomodate parameter (w).");
143+
144+
// ASSERT to validate entry parameter value.
145+
assert(kNoPrivilege != chip::to_underlying(w));
146+
147+
mask.writePrivilege = chip::to_underlying(w);
109148
}
110149

111150
Access::Privilege GetWritePrivilege() const
112151
{
113-
return static_cast< Access::Privilege >(writePrivilege);
152+
return static_cast< Access::Privilege >(mask.writePrivilege);
114153
}
115154

116-
bool readPrivilegeHasValue() {return readPrivilege;}
117-
bool writePrivilegeHasValue() {return writePrivilege;}
155+
156+
AttributeQualityFlags SetFlags(AttributeQualityFlags f)
157+
{
158+
return static_cast< AttributeQualityFlags >( mask.flags |= chip::to_underlying(f) ) ;
159+
}
160+
161+
162+
AttributeQualityFlags SetFlags(AttributeQualityFlags f, bool isSet)
163+
{
164+
return isSet ?
165+
SetFlags(f) :
166+
static_cast< AttributeQualityFlags >( mask.flags &= ~chip::to_underlying(f) );
167+
}
168+
169+
170+
constexpr bool FlagsHas(AttributeQualityFlags f) const { return (mask.flags & chip::to_underlying(f)) != 0; }
171+
172+
bool readPrivilegeHasValue() {return mask.readPrivilege;}
173+
bool writePrivilegeHasValue() {return mask.writePrivilege;}
118174

119175

120176
private:
121177

122-
// read/write access will be missing if read/write is NOT allowed
123-
//
124-
// NOTE: this should be compacted for size
125-
std::underlying_type_t<Access::Privilege> readPrivilege : 5 ; // generally defaults to View if readable
126-
std::underlying_type_t<Access::Privilege> writePrivilege : 5 ; // generally defaults to Operate if writable
178+
attribute_entry_mask_t mask;
179+
127180
};
128181

182+
183+
129184
// Bitmask values for different Command qualities.
130185
enum class CommandQualityFlags : uint32_t
131186
{
@@ -134,33 +189,77 @@ enum class CommandQualityFlags : uint32_t
134189
kLargeMessage = 0x0004, // `L` quality on commands
135190
};
136191

192+
193+
194+
struct accepted_command_entry_mask_t {
195+
196+
// command quality flags
197+
//
198+
std::underlying_type_t<CommandQualityFlags> flags : 3 ; // generally defaults to View if readable
199+
200+
// invoke privilege variable
201+
//
202+
std::underlying_type_t<Access::Privilege> invokePrivilege : 5 ;
203+
204+
};
205+
206+
207+
// Static ASSERT to check size of mask type.
208+
static_assert(sizeof(accepted_command_entry_mask_t) <= 4, "Size of accepted_command_entry_mask_t is not as expected.");
209+
210+
211+
137212
struct AcceptedCommandEntry
138213
{
139214
CommandId commandId;
140215

141-
// TODO: this can be more compact (use some flags for privilege)
142-
// to make this compact, add a compact enum and make flags/invokePrivilege getters (to still be type safe)
143-
BitFlags<CommandQualityFlags> flags;
144-
145216

146217
// Constructor
147-
AcceptedCommandEntry() :
148-
invokePrivilege(chip::to_underlying(Access::Privilege::kOperate)) {}
218+
constexpr AcceptedCommandEntry() : commandId(0),
219+
mask{ 0, chip::to_underlying(Access::Privilege::kOperate) } {}
149220

150-
221+
222+
void SetInvokePrivilege(Access::Privilege i)
223+
{
224+
// Static ASSERT to check size of invokePrivilege type vs entry parameter.
225+
static_assert(sizeof(std::underlying_type_t<Access::Privilege>) >=
226+
sizeof(chip::to_underlying(i)),
227+
"Size of invokePrivilege is not able to accomodate parameter (i).");
228+
229+
// ASSERT to validate entry parameter value.
230+
assert(kNoPrivilege != chip::to_underlying(i));
231+
232+
mask.invokePrivilege = chip::to_underlying(i);
233+
}
234+
235+
151236
Access::Privilege GetInvokePrivilege() const
152237
{
153-
return static_cast< Access::Privilege >(invokePrivilege);
238+
return static_cast< Access::Privilege >(mask.invokePrivilege);
154239
}
155240

156-
void SetInvokePrivilege(Access::Privilege i)
241+
242+
CommandQualityFlags SetFlags(CommandQualityFlags f)
243+
{
244+
return static_cast< CommandQualityFlags >( mask.flags |= chip::to_underlying(f) ) ;
245+
}
246+
247+
248+
CommandQualityFlags SetFlags(CommandQualityFlags f, bool isSet)
157249
{
158-
invokePrivilege = chip::to_underlying(i);
250+
return isSet ?
251+
SetFlags(f) :
252+
static_cast< CommandQualityFlags >( mask.flags &= ~chip::to_underlying(f) );
159253
}
254+
255+
256+
constexpr bool FlagsHas(CommandQualityFlags f) const { return (mask.flags & chip::to_underlying(f)) != 0; }
160257

161258

162259
private:
163-
std::underlying_type_t<Access::Privilege> invokePrivilege : 5 ;
260+
261+
accepted_command_entry_mask_t mask;
262+
164263
};
165264

166265
/// Represents a device type that resides on an endpoint

‎src/data-model-providers/codegen/CodegenDataModelProvider.cpp

+6-6
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ DataModel::AcceptedCommandEntry AcceptedCommandEntryFor(const ConcreteCommandPat
5959

6060
entry.commandId = path.mCommandId;
6161
entry.SetInvokePrivilege( RequiredPrivilege::ForInvokeCommand(path) );
62-
entry.flags.Set(DataModel::CommandQualityFlags::kTimed, CommandNeedsTimedInvoke(path.mClusterId, commandId));
63-
entry.flags.Set(DataModel::CommandQualityFlags::kFabricScoped, CommandIsFabricScoped(path.mClusterId, commandId));
64-
entry.flags.Set(DataModel::CommandQualityFlags::kLargeMessage, CommandHasLargePayload(path.mClusterId, commandId));
62+
entry.SetFlags(DataModel::CommandQualityFlags::kTimed, CommandNeedsTimedInvoke(path.mClusterId, commandId));
63+
entry.SetFlags(DataModel::CommandQualityFlags::kFabricScoped, CommandIsFabricScoped(path.mClusterId, commandId));
64+
entry.SetFlags(DataModel::CommandQualityFlags::kLargeMessage, CommandHasLargePayload(path.mClusterId, commandId));
6565

6666
return entry;
6767
}
@@ -105,8 +105,8 @@ DataModel::AttributeEntry AttributeEntryFrom(const ConcreteClusterPath & cluster
105105
entry.SetWritePrivilege(RequiredPrivilege::ForWriteAttribute(attributePath));
106106
}
107107

108-
entry.flags.Set(DataModel::AttributeQualityFlags::kListAttribute, (attribute.attributeType == ZCL_ARRAY_ATTRIBUTE_TYPE));
109-
entry.flags.Set(DataModel::AttributeQualityFlags::kTimed, attribute.MustUseTimedWrite());
108+
entry.SetFlags(DataModel::AttributeQualityFlags::kListAttribute, (attribute.attributeType == ZCL_ARRAY_ATTRIBUTE_TYPE));
109+
entry.SetFlags(DataModel::AttributeQualityFlags::kTimed, attribute.MustUseTimedWrite());
110110

111111
// NOTE: we do NOT provide additional info for:
112112
// - IsExternal/IsSingleton/IsAutomaticallyPersisted is not used by IM handling
@@ -295,7 +295,7 @@ CHIP_ERROR CodegenDataModelProvider::Attributes(const ConcreteClusterPath & path
295295
DataModel::AttributeEntry globalListEntry;
296296

297297
globalListEntry.SetReadPrivilege(Access::Privilege::kView);
298-
globalListEntry.flags.Set(DataModel::AttributeQualityFlags::kListAttribute);
298+
globalListEntry.SetFlags(DataModel::AttributeQualityFlags::kListAttribute);
299299

300300
for (auto & attribute : GlobalAttributesNotInMetadata)
301301
{

‎src/data-model-providers/codegen/tests/TestCodegenModelViaMocks.cpp

+10-10
Original file line numberDiff line numberDiff line change
@@ -1077,26 +1077,26 @@ TEST_F(TestCodegenModelViaMocks, IterateOverAttributes)
10771077
ASSERT_EQ(attributes.size(), 7u);
10781078

10791079
ASSERT_EQ(attributes[0].attributeId, ClusterRevision::Id);
1080-
ASSERT_FALSE(attributes[0].flags.Has(AttributeQualityFlags::kListAttribute));
1080+
ASSERT_FALSE(attributes[0].FlagsHas(AttributeQualityFlags::kListAttribute));
10811081

10821082
ASSERT_EQ(attributes[1].attributeId, FeatureMap::Id);
1083-
ASSERT_FALSE(attributes[1].flags.Has(AttributeQualityFlags::kListAttribute));
1083+
ASSERT_FALSE(attributes[1].FlagsHas(AttributeQualityFlags::kListAttribute));
10841084

10851085
ASSERT_EQ(attributes[2].attributeId, MockAttributeId(1));
1086-
ASSERT_FALSE(attributes[2].flags.Has(AttributeQualityFlags::kListAttribute));
1086+
ASSERT_FALSE(attributes[2].FlagsHas(AttributeQualityFlags::kListAttribute));
10871087

10881088
ASSERT_EQ(attributes[3].attributeId, MockAttributeId(2));
1089-
ASSERT_TRUE(attributes[3].flags.Has(AttributeQualityFlags::kListAttribute));
1089+
ASSERT_TRUE(attributes[3].FlagsHas(AttributeQualityFlags::kListAttribute));
10901090

10911091
// Ends with global list attributes
10921092
ASSERT_EQ(attributes[4].attributeId, GeneratedCommandList::Id);
1093-
ASSERT_TRUE(attributes[4].flags.Has(AttributeQualityFlags::kListAttribute));
1093+
ASSERT_TRUE(attributes[4].FlagsHas(AttributeQualityFlags::kListAttribute));
10941094

10951095
ASSERT_EQ(attributes[5].attributeId, AcceptedCommandList::Id);
1096-
ASSERT_TRUE(attributes[5].flags.Has(AttributeQualityFlags::kListAttribute));
1096+
ASSERT_TRUE(attributes[5].FlagsHas(AttributeQualityFlags::kListAttribute));
10971097

10981098
ASSERT_EQ(attributes[6].attributeId, AttributeList::Id);
1099-
ASSERT_TRUE(attributes[6].flags.Has(AttributeQualityFlags::kListAttribute));
1099+
ASSERT_TRUE(attributes[6].FlagsHas(AttributeQualityFlags::kListAttribute));
11001100
}
11011101

11021102
TEST_F(TestCodegenModelViaMocks, FindAttribute)
@@ -1118,22 +1118,22 @@ TEST_F(TestCodegenModelViaMocks, FindAttribute)
11181118
// valid info
11191119
std::optional<AttributeEntry> info = finder.Find(ConcreteAttributePath(kMockEndpoint1, MockClusterId(1), FeatureMap::Id));
11201120
ASSERT_TRUE(info.has_value());
1121-
EXPECT_FALSE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access)
1121+
EXPECT_FALSE(info->FlagsHas(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access)
11221122

11231123
// Mocks always set everything as R/W with administrative privileges
11241124
EXPECT_EQ(info->GetReadPrivilege(), chip::Access::Privilege::kAdminister); // NOLINT(bugprone-unchecked-optional-access)
11251125
EXPECT_EQ(info->GetWritePrivilege(), chip::Access::Privilege::kAdminister); // NOLINT(bugprone-unchecked-optional-access)
11261126

11271127
info = finder.Find(ConcreteAttributePath(kMockEndpoint2, MockClusterId(2), MockAttributeId(2)));
11281128
ASSERT_TRUE(info.has_value());
1129-
EXPECT_TRUE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access)
1129+
EXPECT_TRUE(info->FlagsHas(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access)
11301130
EXPECT_EQ(info->GetReadPrivilege(), chip::Access::Privilege::kAdminister); // NOLINT(bugprone-unchecked-optional-access)
11311131
EXPECT_EQ(info->GetWritePrivilege(), chip::Access::Privilege::kAdminister); // NOLINT(bugprone-unchecked-optional-access)
11321132

11331133
// test a read-only attribute, which will not have a write privilege
11341134
info = finder.Find(ConcreteAttributePath(kMockEndpoint3, MockClusterId(3), kReadOnlyAttributeId));
11351135
ASSERT_TRUE(info.has_value());
1136-
EXPECT_FALSE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access)
1136+
EXPECT_FALSE(info->FlagsHas(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access)
11371137
EXPECT_EQ(info->GetReadPrivilege(), chip::Access::Privilege::kAdminister); // NOLINT(bugprone-unchecked-optional-access)
11381138
EXPECT_FALSE(info->writePrivilegeHasValue()); // NOLINT(bugprone-unchecked-optional-access)
11391139
}

0 commit comments

Comments
 (0)