Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce a building block usable for all Q attributes #34266

Merged
merged 12 commits into from
Jul 10, 2024
12 changes: 6 additions & 6 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 @@ -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.
tcarmelveilleux marked this conversation as resolved.
Show resolved Hide resolved
*/
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)
tcarmelveilleux marked this conversation as resolved.
Show resolved Hide resolved
{
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; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want policy to be randomly mutable? I would expect it to be a constructor argument and immutable...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer being able to update the policy, and I don't think it's a big deal. Will leave like this.

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,
tcarmelveilleux marked this conversation as resolved.
Show resolved Hide resolved
SufficientChangePredicate changedPredicate)
{
bool isChangeOfNull = newValue.IsNull() ^ mValue.IsNull();
tcarmelveilleux marked this conversation as resolved.
Show resolved Hide resolved
tcarmelveilleux marked this conversation as resolved.
Show resolved Hide resolved
bool areBothValuesNonNull = !newValue.IsNull() && !mValue.IsNull();

bool changeToFromZero = areBothValuesNonNull && (*newValue == 0 || *mValue == 0);
tcarmelveilleux marked this conversation as resolved.
Show resolved Hide resolved
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;
tcarmelveilleux marked this conversation as resolved.
Show resolved Hide resolved
// 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{};
tcarmelveilleux marked this conversation as resolved.
Show resolved Hide resolved
};

} // 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
Loading