Skip to content

Commit

Permalink
Start moving matter pre/post callbacks from __weak__ handling to a …
Browse files Browse the repository at this point in the history
…injectable class instead. (#32648)

* Add MatterCallbacks.cpp to include callbacks and redirection

* Fix compile dependencies

* Replace a few more callbacks

* Fix lint file - mattercallbacks is now tracked

* Restyle

* Update ApplicationCallbacks to DataModelCallbacks

* Update src/app/util/MatterCallbacks.cpp

Co-authored-by: Arkadiusz Bokowy <[email protected]>

* Update src/app/util/MatterCallbacks.h

Co-authored-by: Damian Królik <[email protected]>

* Updated comments

---------

Co-authored-by: Arkadiusz Bokowy <[email protected]>
Co-authored-by: Damian Królik <[email protected]>
  • Loading branch information
3 people authored Mar 21, 2024
1 parent 6b82061 commit 1f2aff4
Show file tree
Hide file tree
Showing 8 changed files with 185 additions and 48 deletions.
1 change: 0 additions & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ jobs:
--known-failure app/util/generic-callbacks.h \
--known-failure app/util/generic-callback-stubs.cpp \
--known-failure app/util/im-client-callbacks.h \
--known-failure app/util/MatterCallbacks.h \
--known-failure app/util/util.cpp \
--known-failure app/util/util.h \
--known-failure app/WriteHandler.h \
Expand Down
2 changes: 2 additions & 0 deletions src/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ static_library("interaction-model") {
"${chip_root}/src/app/icd/server:icd-server-config",
"${chip_root}/src/app/icd/server:observer",
"${chip_root}/src/app/util:af-types",
"${chip_root}/src/app/util:callbacks",
"${chip_root}/src/lib/address_resolve",
"${chip_root}/src/lib/support",
"${chip_root}/src/lib/support:static-support",
Expand Down Expand Up @@ -318,6 +319,7 @@ static_library("app") {
":interaction-model",
"${chip_root}/src/app/data-model",
"${chip_root}/src/app/icd/server:icd-server-config",
"${chip_root}/src/app/util:callbacks",
"${chip_root}/src/lib/address_resolve",
"${chip_root}/src/lib/support",
"${chip_root}/src/messaging",
Expand Down
17 changes: 4 additions & 13 deletions src/app/CommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -449,9 +449,9 @@ Status CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommandElem
{
ChipLogDetail(DataManagement, "Received command for Endpoint=%u Cluster=" ChipLogFormatMEI " Command=" ChipLogFormatMEI,
concretePath.mEndpointId, ChipLogValueMEI(concretePath.mClusterId), ChipLogValueMEI(concretePath.mCommandId));
SuccessOrExit(err = MatterPreCommandReceivedCallback(concretePath, GetSubjectDescriptor()));
SuccessOrExit(err = DataModelCallbacks::GetInstance()->PreCommandReceived(concretePath, GetSubjectDescriptor()));
mpCallback->DispatchCommand(*this, concretePath, commandDataReader);
MatterPostCommandReceivedCallback(concretePath, GetSubjectDescriptor());
DataModelCallbacks::GetInstance()->PostCommandReceived(concretePath, GetSubjectDescriptor());
}

exit:
Expand Down Expand Up @@ -555,11 +555,11 @@ Status CommandHandler::ProcessGroupCommandDataIB(CommandDataIB::Parser & aComman
continue;
}
}
if ((err = MatterPreCommandReceivedCallback(concretePath, GetSubjectDescriptor())) == CHIP_NO_ERROR)
if ((err = DataModelCallbacks::GetInstance()->PreCommandReceived(concretePath, GetSubjectDescriptor())) == CHIP_NO_ERROR)
{
TLV::TLVReader dataReader(commandDataReader);
mpCallback->DispatchCommand(*this, concretePath, dataReader);
MatterPostCommandReceivedCallback(concretePath, GetSubjectDescriptor());
DataModelCallbacks::GetInstance()->PostCommandReceived(concretePath, GetSubjectDescriptor());
}
else
{
Expand Down Expand Up @@ -1053,12 +1053,3 @@ void CommandHandler::TestOnlyInvokeCommandRequestWithFaultsInjected(Messaging::E

} // namespace app
} // namespace chip

CHIP_ERROR __attribute__((weak)) MatterPreCommandReceivedCallback(const chip::app::ConcreteCommandPath & commandPath,
const chip::Access::SubjectDescriptor & subjectDescriptor)
{
return CHIP_NO_ERROR;
}
void __attribute__((weak)) MatterPostCommandReceivedCallback(const chip::app::ConcreteCommandPath & commandPath,
const chip::Access::SubjectDescriptor & subjectDescriptor)
{}
17 changes: 10 additions & 7 deletions src/app/WriteHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,9 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeData
mProcessingAttributeIsList = dataAttributePath.IsListOperation();
mProcessingAttributePath.SetValue(dataAttributePath);

MatterPreAttributeWriteCallback(dataAttributePath);
DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Write,
DataModelCallbacks::OperationOrder::Pre, dataAttributePath);

TLV::TLVWriter backup;
DataVersion version = 0;
mWriteResponseBuilder.GetWriteResponses().Checkpoint(backup);
Expand All @@ -348,7 +350,9 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeData
mWriteResponseBuilder.GetWriteResponses().Rollback(backup);
err = AddStatus(dataAttributePath, StatusIB(err));
}
MatterPostAttributeWriteCallback(dataAttributePath);

DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Write,
DataModelCallbacks::OperationOrder::Post, dataAttributePath);
SuccessOrExit(err);
}

Expand Down Expand Up @@ -482,7 +486,8 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut

chip::TLV::TLVReader tmpDataReader(dataReader);

MatterPreAttributeWriteCallback(dataAttributePath);
DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Write,
DataModelCallbacks::OperationOrder::Pre, dataAttributePath);
err = WriteSingleClusterData(subjectDescriptor, dataAttributePath, tmpDataReader, this);

if (err != CHIP_NO_ERROR)
Expand All @@ -493,7 +498,8 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut
mapping.endpoint_id, ChipLogValueMEI(dataAttributePath.mClusterId),
ChipLogValueMEI(dataAttributePath.mAttributeId), err.Format());
}
MatterPostAttributeWriteCallback(dataAttributePath);
DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Write,
DataModelCallbacks::OperationOrder::Post, dataAttributePath);
}

dataAttributePath.mEndpointId = kInvalidEndpointId;
Expand Down Expand Up @@ -677,6 +683,3 @@ void WriteHandler::MoveToState(const State aTargetState)

} // namespace app
} // namespace chip

void __attribute__((weak)) MatterPreAttributeWriteCallback(const chip::app::ConcreteAttributePath & attributePath) {}
void __attribute__((weak)) MatterPostAttributeWriteCallback(const chip::app::ConcreteAttributePath & attributePath) {}
13 changes: 8 additions & 5 deletions src/app/reporting/Engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,15 @@ Engine::RetrieveClusterData(const SubjectDescriptor & aSubjectDescriptor, bool a
{
ChipLogDetail(DataManagement, "<RE:Run> Cluster %" PRIx32 ", Attribute %" PRIx32 " is dirty", aPath.mClusterId,
aPath.mAttributeId);
MatterPreAttributeReadCallback(aPath);

DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Read,
DataModelCallbacks::OperationOrder::Pre, aPath);

ReturnErrorOnFailure(ReadSingleClusterData(aSubjectDescriptor, aIsFabricFiltered, aPath, aAttributeReportIBs, aEncoderState));
MatterPostAttributeReadCallback(aPath);

DataModelCallbacks::GetInstance()->AttributeOperation(DataModelCallbacks::OperationType::Read,
DataModelCallbacks::OperationOrder::Post, aPath);

return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -996,9 +1002,6 @@ void Engine::ScheduleUrgentEventDeliverySync(Optional<FabricIndex> fabricIndex)
} // namespace app
} // namespace chip

void __attribute__((weak)) MatterPreAttributeReadCallback(const chip::app::ConcreteAttributePath & attributePath) {}
void __attribute__((weak)) MatterPostAttributeReadCallback(const chip::app::ConcreteAttributePath & attributePath) {}

// TODO: MatterReportingAttributeChangeCallback should just live in libCHIP,
// instead of being in ember-compatibility-functions. It does not depend on any
// app-specific generated bits.
Expand Down
12 changes: 12 additions & 0 deletions src/app/util/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,15 @@ source_set("af-types") {
"${chip_root}/src/protocols/interaction_model",
]
}

source_set("callbacks") {
sources = [
"MatterCallbacks.cpp",
"MatterCallbacks.h",
]

deps = [
"${chip_root}/src/access",
"${chip_root}/src/app:paths",
]
}
99 changes: 99 additions & 0 deletions src/app/util/MatterCallbacks.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
/*
* 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.
*/
#include "MatterCallbacks.h"

// The defines below are using link-time callback and should be removed
//
// TODO: applications should be converted to use DataModelCallbacks instead
// of relying on weak linkage
void __attribute__((weak)) MatterPreAttributeReadCallback(const chip::app::ConcreteAttributePath & attributePath) {}
void __attribute__((weak)) MatterPostAttributeReadCallback(const chip::app::ConcreteAttributePath & attributePath) {}
void __attribute__((weak)) MatterPreAttributeWriteCallback(const chip::app::ConcreteAttributePath & attributePath) {}
void __attribute__((weak)) MatterPostAttributeWriteCallback(const chip::app::ConcreteAttributePath & attributePath) {}
CHIP_ERROR __attribute__((weak)) MatterPreCommandReceivedCallback(const chip::app::ConcreteCommandPath & commandPath,
const chip::Access::SubjectDescriptor & subjectDescriptor)
{
return CHIP_NO_ERROR;
}
void __attribute__((weak)) MatterPostCommandReceivedCallback(const chip::app::ConcreteCommandPath & commandPath,
const chip::Access::SubjectDescriptor & subjectDescriptor)
{}

namespace chip {
namespace {

class WeakRedirectCallbacks : public DataModelCallbacks
{
public:
void AttributeOperation(OperationType operation, OperationOrder order, const chip::app::ConcreteAttributePath & path) override
{
switch (operation)
{
case OperationType::Read:
switch (order)
{
case OperationOrder::Pre:
MatterPreAttributeReadCallback(path);
break;
case OperationOrder::Post:
MatterPostAttributeReadCallback(path);
break;
}
break;
case OperationType::Write:
switch (order)
{
case OperationOrder::Pre:
MatterPreAttributeWriteCallback(path);
break;
case OperationOrder::Post:
MatterPostAttributeWriteCallback(path);
break;
}
break;
}
}

CHIP_ERROR PreCommandReceived(const chip::app::ConcreteCommandPath & commandPath,
const chip::Access::SubjectDescriptor & subjectDescriptor) override
{
return MatterPreCommandReceivedCallback(commandPath, subjectDescriptor);
}

void PostCommandReceived(const chip::app::ConcreteCommandPath & commandPath,
const chip::Access::SubjectDescriptor & subjectDescriptor) override
{

MatterPostCommandReceivedCallback(commandPath, subjectDescriptor);
}
};

WeakRedirectCallbacks gWeakCallbacks;
DataModelCallbacks * gInstance = &gWeakCallbacks;

} // namespace

DataModelCallbacks * DataModelCallbacks::GetInstance()
{
return gInstance;
}

DataModelCallbacks * DataModelCallbacks::SetInstance(DataModelCallbacks * newInstance)
{
return std::exchange(gInstance, newInstance);
}

} // namespace chip
72 changes: 50 additions & 22 deletions src/app/util/MatterCallbacks.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/*
*
* Copyright (c) 2021 Project CHIP Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
Expand All @@ -14,32 +13,61 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/

// THIS FILE IS GENERATED BY ZAP

#pragma once

#include <access/SubjectDescriptor.h>
#include <app/ConcreteAttributePath.h>
#include <app/ConcreteCommandPath.h>

void MatterPreAttributeReadCallback(const chip::app::ConcreteAttributePath & attributePath);
void MatterPostAttributeReadCallback(const chip::app::ConcreteAttributePath & attributePath);
void MatterPreAttributeWriteCallback(const chip::app::ConcreteAttributePath & attributePath);
void MatterPostAttributeWriteCallback(const chip::app::ConcreteAttributePath & attributePath);
namespace chip {

/** @brief Matter Pre Command Received
*
* This callback is called once the message has been determined to be a command, and
* before the command is dispatched to the receiver.
*/
CHIP_ERROR MatterPreCommandReceivedCallback(const chip::app::ConcreteCommandPath & commandPath,
const chip::Access::SubjectDescriptor & subjectDescriptor);
/// Allows for application hooks for processing attribute and command operations
class DataModelCallbacks
{
public:
enum class OperationType
{
Read,
Write
};

/** @brief Matter Post Command Received
*
* This callback is called once the message has been determined to be a command, but
* after it being dispatched to the receiver.
*/
void MatterPostCommandReceivedCallback(const chip::app::ConcreteCommandPath & commandPath,
const chip::Access::SubjectDescriptor & subjectDescriptor);
enum class OperationOrder
{
Pre,
Post
};

virtual ~DataModelCallbacks() = default;

/// This callback is called on attribute operations:
/// - for reads and writes
/// - both before and after attribute read/writes
///
/// NOTE: PostRead is only called on read success.
virtual void AttributeOperation(OperationType operation, OperationOrder order, const chip::app::ConcreteAttributePath & path) {}

/// This callback is called once for every command dispatch, before the dispatch is actually
/// done towards the receiver.
///
/// This method is called once for every CommandDataIB (i.e. it may be called several times
/// in the case of batch invoke, where a single `InvokeRequestMessage` may contain several
/// CommandDataIB entries).
///
/// Returning an error here will prevent the command to be dispatched further
virtual CHIP_ERROR PreCommandReceived(const chip::app::ConcreteCommandPath & commandPath,
const chip::Access::SubjectDescriptor & subjectDescriptor)
{
return CHIP_NO_ERROR;
}

/// This callback is called once the message has been determined to be a command, but
/// after it being dispatched to the receiver.
virtual void PostCommandReceived(const chip::app::ConcreteCommandPath & commandPath,
const chip::Access::SubjectDescriptor & subjectDescriptor)
{}

static DataModelCallbacks * GetInstance();
static DataModelCallbacks * SetInstance(DataModelCallbacks * newInstance);
};

} // namespace chip

0 comments on commit 1f2aff4

Please sign in to comment.