Skip to content

Commit f83d67b

Browse files
tcarmelveilleuxrestyled-commits
andauthoredJul 10, 2024
Introduce a building block usable for all Q attributes (project-chip#34266)
* Introduce a building block usable for all Q attributes - Q quality requires marking attributes as dirty for the purposes of reporting only when certain conditions arise. - This PR introduces a building block attribute value wrapper compatible with any nullable or non-nullable numerical attribute, which allows applying the necessary policies, and all complex policies that currently exist in the Matter spec (e.g. any CountdownTime, CurrentLevel, etc). Testing done: - Added unit tests. Integration in existing clusters will follow in a different PR. * Reword examples * Restyled by clang-format * Address review comments * Restyled by clang-format * Restyled by gn * Address more review comments * Restyled by clang-format * Add clang-tidy exclusion due to clang-tidy bug See llvm/llvm-project#97426 * Apply review comments * Restyled by clang-format * Fix Darwin clang-tidy crash by adding an override --------- Co-authored-by: Restyled.io <commits@restyled.io>
1 parent 76a7b0b commit f83d67b

File tree

6 files changed

+565
-7
lines changed

6 files changed

+565
-7
lines changed
 

‎.github/workflows/build.yaml

+7-7
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,14 @@ on:
2626
run-codeql:
2727
required: false
2828
type: boolean
29-
29+
3030
concurrency:
3131
group: ${{ github.ref }}-${{ github.workflow }}-${{ (github.event_name == 'pull_request' && github.event.number) || (github.event_name == 'workflow_dispatch' && github.run_number) || github.sha }}
3232
cancel-in-progress: true
3333

3434
env:
3535
CHIP_NO_LOG_TIMESTAMPS: true
36-
36+
3737
jobs:
3838
build_linux_gcc_debug:
3939
name: Build on Linux (gcc_debug)
@@ -210,7 +210,7 @@ jobs:
210210
./scripts/run_in_build_env.sh \
211211
"./scripts/run-clang-tidy-on-compile-commands.py \
212212
--compile-database out/sanitizers/compile_commands.json \
213-
--file-exclude-regex '/(repo|zzz_generated|lwip/standalone)/|-ReadImpl|-InvokeSubscribeImpl|CodegenDataModel_Write' \
213+
--file-exclude-regex '/(repo|zzz_generated|lwip/standalone)/|-ReadImpl|-InvokeSubscribeImpl|CodegenDataModel_Write|QuieterReporting' \
214214
check \
215215
"
216216
- name: Clean output
@@ -243,7 +243,7 @@ jobs:
243243
run: |
244244
rm -rf ./zzz_pregenerated
245245
mv scripts/codegen.py.renamed scripts/codegen.py
246-
mv scripts/tools/zap/generate.py.renamed scripts/tools/zap/generate.py
246+
mv scripts/tools/zap/generate.py.renamed scripts/tools/zap/generate.py
247247
- name: Run fake linux tests with build_examples
248248
run: |
249249
./scripts/run_in_build_env.sh \
@@ -253,7 +253,7 @@ jobs:
253253
uses: ./.github/actions/perform-codeql-analysis
254254
with:
255255
language: cpp
256-
256+
257257
- name: Uploading core files
258258
uses: actions/upload-artifact@v4
259259
if: ${{ failure() && !env.ACT }}
@@ -430,7 +430,7 @@ jobs:
430430
./scripts/run_in_build_env.sh \
431431
"./scripts/run-clang-tidy-on-compile-commands.py \
432432
--compile-database out/default/compile_commands.json \
433-
--file-exclude-regex '/(repo|zzz_generated|lwip/standalone)/|CodegenDataModel_Write' \
433+
--file-exclude-regex '/(repo|zzz_generated|lwip/standalone)/|CodegenDataModel_Write|QuieterReporting' \
434434
check \
435435
"
436436
- name: Uploading diagnostic logs
@@ -445,7 +445,7 @@ jobs:
445445
uses: ./.github/actions/perform-codeql-analysis
446446
with:
447447
language: cpp
448-
448+
449449
# TODO Log Upload https://github.com/project-chip/connectedhomeip/issues/2227
450450
# TODO https://github.com/project-chip/connectedhomeip/issues/1512
451451

‎src/BUILD.gn

+1
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ if (chip_build_tests) {
5151
deps = []
5252
tests = [
5353
"${chip_root}/src/app/data-model/tests",
54+
"${chip_root}/src/app/cluster-building-blocks/tests",
5455
"${chip_root}/src/app/data-model-interface/tests",
5556
"${chip_root}/src/access/tests",
5657
"${chip_root}/src/crypto/tests",
+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
# Copyright (c) 2024 Project CHIP Authors
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
import("//build_overrides/chip.gni")
15+
16+
source_set("cluster-building-blocks") {
17+
sources = [ "QuieterReporting.h" ]
18+
19+
public_deps = [
20+
"${chip_root}/src/app/data-model:nullable",
21+
"${chip_root}/src/lib/support:support",
22+
"${chip_root}/src/system",
23+
]
24+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,241 @@
1+
/*
2+
*
3+
* Copyright (c) 2024 Project CHIP Authors
4+
* All rights reserved.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
#pragma once
20+
21+
#include <functional>
22+
#include <stdbool.h>
23+
#include <type_traits>
24+
25+
#include <app/data-model/Nullable.h>
26+
#include <lib/support/BitFlags.h>
27+
#include <system/SystemClock.h>
28+
29+
namespace chip {
30+
namespace app {
31+
32+
enum class QuieterReportingPolicyEnum
33+
{
34+
kMarkDirtyOnChangeToFromZero = (1u << 0),
35+
kMarkDirtyOnDecrement = (1u << 1),
36+
kMarkDirtyOnIncrement = (1u << 2),
37+
};
38+
39+
enum class AttributeDirtyState
40+
{
41+
kNoReportNeeded = 0,
42+
kMustReport = 1,
43+
};
44+
45+
using QuieterReportingPolicyFlags = BitFlags<QuieterReportingPolicyEnum>;
46+
47+
namespace detail {
48+
49+
using Timestamp = System::Clock::Milliseconds64;
50+
template <typename T>
51+
using Nullable = DataModel::Nullable<T>;
52+
53+
/**
54+
* This class helps track reporting state of an attribute to properly keep track of whether
55+
* it needs to be marked as dirty or not for purposes of reporting using
56+
* "7.7.9 Quieter Reporting Quality" (Q quality)
57+
*
58+
* The class can be configured via `policy()` to have some/all of the common reasons
59+
* for reporting (e.g. increment only, decrement only, change to/from zero).
60+
*
61+
* Changes of null to non-null or non-null to null are always considered dirty.
62+
*
63+
* It is possible to force mark the attribute as dirty (see `ForceDirty()`) such as
64+
* for conditions like "When there is any increase or decrease in the estimated time
65+
* remaining that was due to progressing insight of the server's control logic".
66+
*
67+
* Class maintains a `current value` and a timestamped `dirty` state. The `SetValue()`
68+
* method must be used to update the `current value` and will return AttributeDirtyState::kMustReport
69+
* if the attribute should be marked dirty/
70+
*
71+
* - `SetValue()` has internal rules for null/non-null changes and policy-based rules
72+
* - `SetValue()` with a `SufficientChangePredicate` uses the internal rules in addition to
73+
* the predicate to determine dirty state
74+
*
75+
* See [QuieterReportingPolicyEnum] for policy flags on when a value is considered dirty
76+
* beyond non/non-null changes.
77+
*
78+
* Common quieter reporting usecases that can be supported by this class are:
79+
* - If attribute has changed due to a change in the X or Y attributes
80+
* - Use SufficientChangePredicate version
81+
* - When it changes from 0 to any other value and vice versa
82+
* - Use `kMarkDirtyOnChangeToFromZero` internal policy.
83+
* - When it changes from null to any other value and vice versa
84+
* - Built-in rule.
85+
* - When it increases
86+
* - Use `kMarkDirtyOnIncrement` internal policy.
87+
* - When it decreases
88+
* - Use `kMarkDirtyOnDecrement` internal policy.
89+
* - When there is any increase or decrease in the estimated time remaining that was
90+
* due to progressing insight of the server's control logic
91+
* - Use SufficientChangePredicate version with an always-true predicate.
92+
* - When it changes at a rate significantly different from one unit per second.
93+
* - Use SufficientChangePredicate version.
94+
* Example usage in-situ:
95+
*
96+
* Class has:
97+
* QuieterReportingAttribute<uint8_t> mAttrib;
98+
*
99+
* Code at time of setting new value has:
100+
*
101+
* uint8_t newValue = driver.GetNewValue();
102+
* auto now = SystemClock().GetMonotonicTimestamp();
103+
* if (mAttrib.SetValue(newValue, now) == AttributeDirtyState::kMustReport)
104+
* {
105+
* MatterReportingAttributeChangeCallback(path_for_attribute);
106+
* }
107+
*
108+
* @tparam T - the type of underlying numerical value that will be held by the class.
109+
*/
110+
template <typename T, std::enable_if_t<std::is_arithmetic<T>::value, bool> = true>
111+
class QuieterReportingAttribute
112+
{
113+
public:
114+
explicit QuieterReportingAttribute(const Nullable<T> & initialValue) : mValue(initialValue), mLastDirtyValue(initialValue) {}
115+
116+
struct SufficientChangePredicateCandidate
117+
{
118+
// Timestamp of last time attribute was marked dirty.
119+
Timestamp lastDirtyTimestamp;
120+
// New (`now`) timestamp passed in `SetValue()`.
121+
Timestamp nowTimestamp;
122+
// Value last marked as dirty.
123+
const Nullable<T> & lastDirtyValue;
124+
// New value passed in `SetValue()`, to compare against lastDirtyValue for sufficient change if needed.
125+
const Nullable<T> & newValue;
126+
};
127+
128+
using SufficientChangePredicate = std::function<bool(const SufficientChangePredicateCandidate &)>;
129+
130+
/**
131+
* @brief Factory to generate a functor for "attribute was last reported" at least `minimumDurationMillis` ago.
132+
*
133+
* @param minimumDurationMillis - number of millis needed since last marked as dirty before we mark dirty again.
134+
* @return a functor usable for the `changedPredicate` arg of `SetValue()`
135+
*/
136+
static SufficientChangePredicate
137+
GetPredicateForSufficientTimeSinceLastDirty(System::Clock::Milliseconds64 minimumDurationMillis)
138+
{
139+
return [minimumDurationMillis](const SufficientChangePredicateCandidate & candidate) -> bool {
140+
return (candidate.lastDirtyValue != candidate.newValue) &&
141+
((candidate.nowTimestamp - candidate.lastDirtyTimestamp) >= minimumDurationMillis);
142+
};
143+
}
144+
145+
/**
146+
* @brief Factory to generate a functor that forces reportable now.
147+
* @return a functor usable for the `changedPredicate` arg of `SetValue()`
148+
*/
149+
static SufficientChangePredicate GetForceReportablePredicate()
150+
{
151+
return [](const SufficientChangePredicateCandidate & candidate) -> bool { return true; };
152+
}
153+
154+
Nullable<T> value() const { return mValue; }
155+
QuieterReportingPolicyFlags & policy() { return mPolicyFlags; }
156+
const QuieterReportingPolicyFlags & policy() const { return mPolicyFlags; }
157+
158+
/**
159+
* Set the updated value of the attribute, computing whether it needs to be reported according to `changedPredicate` and
160+
* policies.
161+
*
162+
* - Any change of nullability between `newValue` and the old value will be considered dirty.
163+
* - The policies from `QuieterReportingPolicyEnum` and set via `SetPolicy()` are self-explanatory by name.
164+
* - The changedPredicate will be called with last dirty <timestamp, value> and new <timestamp value> and may override
165+
* the dirty state altogether when it returns true. Use sparingly and default to a functor returning false.
166+
*
167+
* Internal recording will be done about last dirty value and last dirty timestamp based on the policies having applied.
168+
*
169+
* @param newValue - new value to set for the attribute
170+
* @param now - system monotonic timestamp at the time of the call
171+
* @param changedPredicate - functor to possibly override dirty state
172+
* @return AttributeDirtyState::kMustReport if attribute must be marked dirty right away, or
173+
* AttributeDirtyState::kNoReportNeeded otherwise.
174+
*/
175+
AttributeDirtyState SetValue(const chip::app::DataModel::Nullable<T> & newValue, Timestamp now,
176+
SufficientChangePredicate changedPredicate)
177+
{
178+
bool isChangeOfNull = newValue.IsNull() ^ mValue.IsNull();
179+
bool areBothValuesNonNull = !newValue.IsNull() && !mValue.IsNull();
180+
181+
bool changeToFromZero = areBothValuesNonNull && (*newValue == 0 || *mValue == 0);
182+
bool isIncrement = areBothValuesNonNull && (*newValue > *mValue);
183+
bool isDecrement = areBothValuesNonNull && (*newValue < *mValue);
184+
185+
bool isNewlyDirty = isChangeOfNull;
186+
isNewlyDirty =
187+
isNewlyDirty || (mPolicyFlags.Has(QuieterReportingPolicyEnum::kMarkDirtyOnChangeToFromZero) && changeToFromZero);
188+
isNewlyDirty = isNewlyDirty || (mPolicyFlags.Has(QuieterReportingPolicyEnum::kMarkDirtyOnDecrement) && isDecrement);
189+
isNewlyDirty = isNewlyDirty || (mPolicyFlags.Has(QuieterReportingPolicyEnum::kMarkDirtyOnIncrement) && isIncrement);
190+
191+
SufficientChangePredicateCandidate candidate{
192+
mLastDirtyTimestampMillis, // lastDirtyTimestamp
193+
now, // nowTimestamp
194+
mLastDirtyValue, // lastDirtyValue
195+
newValue // newValue
196+
};
197+
isNewlyDirty = isNewlyDirty || changedPredicate(candidate);
198+
199+
mValue = newValue;
200+
201+
if (isNewlyDirty)
202+
{
203+
mLastDirtyValue = newValue;
204+
mLastDirtyTimestampMillis = now;
205+
}
206+
207+
return isNewlyDirty ? AttributeDirtyState::kMustReport : AttributeDirtyState::kNoReportNeeded;
208+
}
209+
210+
/**
211+
* Same as the other `SetValue`, but assumes a changedPredicate that never overrides to dirty.
212+
*
213+
* This is the easy/common case.
214+
*
215+
* @param newValue - new value to set for the attribute
216+
* @param now - system monotonic timestamp at the time of the call
217+
* @return AttributeDirtyState::kMustReport if attribute must be marked dirty right away, or
218+
* AttributeDirtyState::kNoReportNeeded otherwise.
219+
*/
220+
AttributeDirtyState SetValue(const chip::app::DataModel::Nullable<T> & newValue, Timestamp now)
221+
{
222+
return SetValue(newValue, now, [](const SufficientChangePredicateCandidate &) -> bool { return false; });
223+
}
224+
225+
protected:
226+
// Current value of the attribute.
227+
chip::app::DataModel::Nullable<T> mValue;
228+
// Last value that was marked as dirty (to use in comparisons for change, e.g. by SufficientChangePredicate).
229+
chip::app::DataModel::Nullable<T> mLastDirtyValue;
230+
// Enabled internal change detection policies.
231+
QuieterReportingPolicyFlags mPolicyFlags{ 0 };
232+
// Timestamp associated with the last time the attribute was marked dirty (to use in comparisons for change).
233+
chip::System::Clock::Milliseconds64 mLastDirtyTimestampMillis{};
234+
};
235+
236+
} // namespace detail
237+
238+
using detail::QuieterReportingAttribute;
239+
240+
} // namespace app
241+
} // namespace chip
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Copyright (c) 2024 Project CHIP Authors
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
import("//build_overrides/chip.gni")
16+
import("${chip_root}/build/chip/chip_test_suite.gni")
17+
18+
chip_test_suite("tests") {
19+
output_name = "libAppClusterBuildingBlockTests"
20+
21+
test_sources = [ "TestQuieterReporting.cpp" ]
22+
23+
public_deps = [
24+
"${chip_root}/src/app/cluster-building-blocks",
25+
"${chip_root}/src/app/data-model:nullable",
26+
"${chip_root}/src/lib/core:error",
27+
"${chip_root}/src/lib/support/tests:pw-test-macros",
28+
"${chip_root}/src/system",
29+
]
30+
}

0 commit comments

Comments
 (0)