Skip to content

Commit

Permalink
Introduce a building block usable for all Q attributes (#34266)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
tcarmelveilleux and restyled-commits authored Jul 10, 2024
1 parent 76a7b0b commit f83d67b
Show file tree
Hide file tree
Showing 6 changed files with 565 additions and 7 deletions.
14 changes: 7 additions & 7 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ on:
run-codeql:
required: false
type: boolean

concurrency:
group: ${{ github.ref }}-${{ github.workflow }}-${{ (github.event_name == 'pull_request' && github.event.number) || (github.event_name == 'workflow_dispatch' && github.run_number) || github.sha }}
cancel-in-progress: true

env:
CHIP_NO_LOG_TIMESTAMPS: true

jobs:
build_linux_gcc_debug:
name: Build on Linux (gcc_debug)
Expand Down Expand Up @@ -210,7 +210,7 @@ jobs:
./scripts/run_in_build_env.sh \
"./scripts/run-clang-tidy-on-compile-commands.py \
--compile-database out/sanitizers/compile_commands.json \
--file-exclude-regex '/(repo|zzz_generated|lwip/standalone)/|-ReadImpl|-InvokeSubscribeImpl|CodegenDataModel_Write' \
--file-exclude-regex '/(repo|zzz_generated|lwip/standalone)/|-ReadImpl|-InvokeSubscribeImpl|CodegenDataModel_Write|QuieterReporting' \
check \
"
- name: Clean output
Expand Down Expand Up @@ -243,7 +243,7 @@ jobs:
run: |
rm -rf ./zzz_pregenerated
mv scripts/codegen.py.renamed scripts/codegen.py
mv scripts/tools/zap/generate.py.renamed scripts/tools/zap/generate.py
mv scripts/tools/zap/generate.py.renamed scripts/tools/zap/generate.py
- name: Run fake linux tests with build_examples
run: |
./scripts/run_in_build_env.sh \
Expand All @@ -253,7 +253,7 @@ jobs:
uses: ./.github/actions/perform-codeql-analysis
with:
language: cpp

- name: Uploading core files
uses: actions/upload-artifact@v4
if: ${{ failure() && !env.ACT }}
Expand Down Expand Up @@ -430,7 +430,7 @@ jobs:
./scripts/run_in_build_env.sh \
"./scripts/run-clang-tidy-on-compile-commands.py \
--compile-database out/default/compile_commands.json \
--file-exclude-regex '/(repo|zzz_generated|lwip/standalone)/|CodegenDataModel_Write' \
--file-exclude-regex '/(repo|zzz_generated|lwip/standalone)/|CodegenDataModel_Write|QuieterReporting' \
check \
"
- name: Uploading diagnostic logs
Expand All @@ -445,7 +445,7 @@ jobs:
uses: ./.github/actions/perform-codeql-analysis
with:
language: cpp

# TODO Log Upload https://github.com/project-chip/connectedhomeip/issues/2227
# TODO https://github.com/project-chip/connectedhomeip/issues/1512

Expand Down
1 change: 1 addition & 0 deletions src/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ if (chip_build_tests) {
deps = []
tests = [
"${chip_root}/src/app/data-model/tests",
"${chip_root}/src/app/cluster-building-blocks/tests",
"${chip_root}/src/app/data-model-interface/tests",
"${chip_root}/src/access/tests",
"${chip_root}/src/crypto/tests",
Expand Down
24 changes: 24 additions & 0 deletions src/app/cluster-building-blocks/BUILD.gn
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Copyright (c) 2024 Project CHIP Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
import("//build_overrides/chip.gni")

source_set("cluster-building-blocks") {
sources = [ "QuieterReporting.h" ]

public_deps = [
"${chip_root}/src/app/data-model:nullable",
"${chip_root}/src/lib/support:support",
"${chip_root}/src/system",
]
}
241 changes: 241 additions & 0 deletions src/app/cluster-building-blocks/QuieterReporting.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,241 @@
/*
*
* Copyright (c) 2024 Project CHIP Authors
* All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#pragma once

#include <functional>
#include <stdbool.h>
#include <type_traits>

#include <app/data-model/Nullable.h>
#include <lib/support/BitFlags.h>
#include <system/SystemClock.h>

namespace chip {
namespace app {

enum class QuieterReportingPolicyEnum
{
kMarkDirtyOnChangeToFromZero = (1u << 0),
kMarkDirtyOnDecrement = (1u << 1),
kMarkDirtyOnIncrement = (1u << 2),
};

enum class AttributeDirtyState
{
kNoReportNeeded = 0,
kMustReport = 1,
};

using QuieterReportingPolicyFlags = BitFlags<QuieterReportingPolicyEnum>;

namespace detail {

using Timestamp = System::Clock::Milliseconds64;
template <typename T>
using Nullable = DataModel::Nullable<T>;

/**
* This class helps track reporting state of an attribute to properly keep track of whether
* it needs to be marked as dirty or not for purposes of reporting using
* "7.7.9 Quieter Reporting Quality" (Q quality)
*
* The class can be configured via `policy()` to have some/all of the common reasons
* for reporting (e.g. increment only, decrement only, change to/from zero).
*
* Changes of null to non-null or non-null to null are always considered dirty.
*
* It is possible to force mark the attribute as dirty (see `ForceDirty()`) such as
* for conditions like "When there is any increase or decrease in the estimated time
* remaining that was due to progressing insight of the server's control logic".
*
* Class maintains a `current value` and a timestamped `dirty` state. The `SetValue()`
* method must be used to update the `current value` and will return AttributeDirtyState::kMustReport
* if the attribute should be marked dirty/
*
* - `SetValue()` has internal rules for null/non-null changes and policy-based rules
* - `SetValue()` with a `SufficientChangePredicate` uses the internal rules in addition to
* the predicate to determine dirty state
*
* See [QuieterReportingPolicyEnum] for policy flags on when a value is considered dirty
* beyond non/non-null changes.
*
* Common quieter reporting usecases that can be supported by this class are:
* - If attribute has changed due to a change in the X or Y attributes
* - Use SufficientChangePredicate version
* - When it changes from 0 to any other value and vice versa
* - Use `kMarkDirtyOnChangeToFromZero` internal policy.
* - When it changes from null to any other value and vice versa
* - Built-in rule.
* - When it increases
* - Use `kMarkDirtyOnIncrement` internal policy.
* - When it decreases
* - Use `kMarkDirtyOnDecrement` internal policy.
* - When there is any increase or decrease in the estimated time remaining that was
* due to progressing insight of the server's control logic
* - Use SufficientChangePredicate version with an always-true predicate.
* - When it changes at a rate significantly different from one unit per second.
* - Use SufficientChangePredicate version.
* Example usage in-situ:
*
* Class has:
* QuieterReportingAttribute<uint8_t> mAttrib;
*
* Code at time of setting new value has:
*
* uint8_t newValue = driver.GetNewValue();
* auto now = SystemClock().GetMonotonicTimestamp();
* if (mAttrib.SetValue(newValue, now) == AttributeDirtyState::kMustReport)
* {
* MatterReportingAttributeChangeCallback(path_for_attribute);
* }
*
* @tparam T - the type of underlying numerical value that will be held by the class.
*/
template <typename T, std::enable_if_t<std::is_arithmetic<T>::value, bool> = true>
class QuieterReportingAttribute
{
public:
explicit QuieterReportingAttribute(const Nullable<T> & initialValue) : mValue(initialValue), mLastDirtyValue(initialValue) {}

struct SufficientChangePredicateCandidate
{
// Timestamp of last time attribute was marked dirty.
Timestamp lastDirtyTimestamp;
// New (`now`) timestamp passed in `SetValue()`.
Timestamp nowTimestamp;
// Value last marked as dirty.
const Nullable<T> & lastDirtyValue;
// New value passed in `SetValue()`, to compare against lastDirtyValue for sufficient change if needed.
const Nullable<T> & newValue;
};

using SufficientChangePredicate = std::function<bool(const SufficientChangePredicateCandidate &)>;

/**
* @brief Factory to generate a functor for "attribute was last reported" at least `minimumDurationMillis` ago.
*
* @param minimumDurationMillis - number of millis needed since last marked as dirty before we mark dirty again.
* @return a functor usable for the `changedPredicate` arg of `SetValue()`
*/
static SufficientChangePredicate
GetPredicateForSufficientTimeSinceLastDirty(System::Clock::Milliseconds64 minimumDurationMillis)
{
return [minimumDurationMillis](const SufficientChangePredicateCandidate & candidate) -> bool {
return (candidate.lastDirtyValue != candidate.newValue) &&
((candidate.nowTimestamp - candidate.lastDirtyTimestamp) >= minimumDurationMillis);
};
}

/**
* @brief Factory to generate a functor that forces reportable now.
* @return a functor usable for the `changedPredicate` arg of `SetValue()`
*/
static SufficientChangePredicate GetForceReportablePredicate()
{
return [](const SufficientChangePredicateCandidate & candidate) -> bool { return true; };
}

Nullable<T> value() const { return mValue; }
QuieterReportingPolicyFlags & policy() { return mPolicyFlags; }
const QuieterReportingPolicyFlags & policy() const { return mPolicyFlags; }

/**
* Set the updated value of the attribute, computing whether it needs to be reported according to `changedPredicate` and
* policies.
*
* - Any change of nullability between `newValue` and the old value will be considered dirty.
* - The policies from `QuieterReportingPolicyEnum` and set via `SetPolicy()` are self-explanatory by name.
* - The changedPredicate will be called with last dirty <timestamp, value> and new <timestamp value> and may override
* the dirty state altogether when it returns true. Use sparingly and default to a functor returning false.
*
* Internal recording will be done about last dirty value and last dirty timestamp based on the policies having applied.
*
* @param newValue - new value to set for the attribute
* @param now - system monotonic timestamp at the time of the call
* @param changedPredicate - functor to possibly override dirty state
* @return AttributeDirtyState::kMustReport if attribute must be marked dirty right away, or
* AttributeDirtyState::kNoReportNeeded otherwise.
*/
AttributeDirtyState SetValue(const chip::app::DataModel::Nullable<T> & newValue, Timestamp now,
SufficientChangePredicate changedPredicate)
{
bool isChangeOfNull = newValue.IsNull() ^ mValue.IsNull();
bool areBothValuesNonNull = !newValue.IsNull() && !mValue.IsNull();

bool changeToFromZero = areBothValuesNonNull && (*newValue == 0 || *mValue == 0);
bool isIncrement = areBothValuesNonNull && (*newValue > *mValue);
bool isDecrement = areBothValuesNonNull && (*newValue < *mValue);

bool isNewlyDirty = isChangeOfNull;
isNewlyDirty =
isNewlyDirty || (mPolicyFlags.Has(QuieterReportingPolicyEnum::kMarkDirtyOnChangeToFromZero) && changeToFromZero);
isNewlyDirty = isNewlyDirty || (mPolicyFlags.Has(QuieterReportingPolicyEnum::kMarkDirtyOnDecrement) && isDecrement);
isNewlyDirty = isNewlyDirty || (mPolicyFlags.Has(QuieterReportingPolicyEnum::kMarkDirtyOnIncrement) && isIncrement);

SufficientChangePredicateCandidate candidate{
mLastDirtyTimestampMillis, // lastDirtyTimestamp
now, // nowTimestamp
mLastDirtyValue, // lastDirtyValue
newValue // newValue
};
isNewlyDirty = isNewlyDirty || changedPredicate(candidate);

mValue = newValue;

if (isNewlyDirty)
{
mLastDirtyValue = newValue;
mLastDirtyTimestampMillis = now;
}

return isNewlyDirty ? AttributeDirtyState::kMustReport : AttributeDirtyState::kNoReportNeeded;
}

/**
* Same as the other `SetValue`, but assumes a changedPredicate that never overrides to dirty.
*
* This is the easy/common case.
*
* @param newValue - new value to set for the attribute
* @param now - system monotonic timestamp at the time of the call
* @return AttributeDirtyState::kMustReport if attribute must be marked dirty right away, or
* AttributeDirtyState::kNoReportNeeded otherwise.
*/
AttributeDirtyState SetValue(const chip::app::DataModel::Nullable<T> & newValue, Timestamp now)
{
return SetValue(newValue, now, [](const SufficientChangePredicateCandidate &) -> bool { return false; });
}

protected:
// Current value of the attribute.
chip::app::DataModel::Nullable<T> mValue;
// Last value that was marked as dirty (to use in comparisons for change, e.g. by SufficientChangePredicate).
chip::app::DataModel::Nullable<T> mLastDirtyValue;
// Enabled internal change detection policies.
QuieterReportingPolicyFlags mPolicyFlags{ 0 };
// Timestamp associated with the last time the attribute was marked dirty (to use in comparisons for change).
chip::System::Clock::Milliseconds64 mLastDirtyTimestampMillis{};
};

} // namespace detail

using detail::QuieterReportingAttribute;

} // namespace app
} // namespace chip
30 changes: 30 additions & 0 deletions src/app/cluster-building-blocks/tests/BUILD.gn
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Copyright (c) 2024 Project CHIP Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import("//build_overrides/chip.gni")
import("${chip_root}/build/chip/chip_test_suite.gni")

chip_test_suite("tests") {
output_name = "libAppClusterBuildingBlockTests"

test_sources = [ "TestQuieterReporting.cpp" ]

public_deps = [
"${chip_root}/src/app/cluster-building-blocks",
"${chip_root}/src/app/data-model:nullable",
"${chip_root}/src/lib/core:error",
"${chip_root}/src/lib/support/tests:pw-test-macros",
"${chip_root}/src/system",
]
}
Loading

0 comments on commit f83d67b

Please sign in to comment.