Skip to content

Commit

Permalink
One more decouple from ember from src/app and ensure we can compile w…
Browse files Browse the repository at this point in the history
…ithout `ember-compatibility-functions.cpp` (#35919)

* Add ability to skip compiling of ember-compatibility-functions

* Fix build and make sure that CI will validate a data-model-enabled build

* Restyle

* Fix compilation

* Restyled by clang-format

* Typo fix

* Fix includes

* Rename method to something that seems a bit clearer

* Fix pwrpc usage to use datamodel interface

* Restyle

* Remove todo

* Update for attribute is list to support not know value

* Update src/app/WriteHandler.h

Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>

---------

Co-authored-by: Restyled.io <commits@restyled.io>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
  • Loading branch information
3 people authored Oct 8, 2024
1 parent 52a5589 commit 07a28ac
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 15 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/examples-linux-arm.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ jobs:
--target linux-arm64-chip-tool-nodeps-ipv6only \
--target linux-arm64-lock-clang \
--target linux-arm64-minmdns-clang \
--target linux-arm64-light-rpc-ipv6only-clang \
--target linux-arm64-light-data-model-enabled-rpc-ipv6only-clang \
--target linux-arm64-thermostat-no-ble-clang \
--target linux-arm64-lit-icd-no-ble-clang \
--target linux-arm64-fabric-admin-clang-rpc \
Expand Down
38 changes: 38 additions & 0 deletions examples/common/pigweed/rpc_services/Attributes.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "pigweed/rpc_services/internal/StatusUtils.h"

#include <app-common/zap-generated/attribute-type.h>
#include <app/AppConfig.h>
#include <app/InteractionModelEngine.h>
#include <app/MessageDef/AttributeReportIBs.h>
#include <app/util/attribute-storage.h>
Expand All @@ -32,6 +33,13 @@
#include <lib/core/TLVTypes.h>
#include <platform/PlatformManager.h>

#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
#include <app/AttributeValueEncoder.h>
#include <app/data-model-provider/ActionReturnStatus.h>
#include <app/data-model-provider/OperationTypes.h>
#include <app/data-model-provider/Provider.h>
#endif

namespace chip {
namespace rpc {

Expand Down Expand Up @@ -202,7 +210,37 @@ class Attributes : public pw_rpc::nanopb::Attributes::Service<Attributes>
writer.Init(tlvBuffer);
PW_TRY(ChipErrorToPwStatus(writer.StartContainer(TLV::AnonymousTag(), TLV::kTLVType_Structure, outer)));
PW_TRY(ChipErrorToPwStatus(attributeReports.Init(&writer, kReportContextTag)));

#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
// TODO: this assumes a singleton data model provider
app::DataModel::Provider * provider = app::InteractionModelEngine::GetInstance()->GetDataModelProvider();

app::DataModel::ReadAttributeRequest request;
request.path = path;
request.operationFlags.Set(app::DataModel::OperationFlags::kInternal);
request.subjectDescriptor = subjectDescriptor;

std::optional<app::DataModel::ClusterInfo> info = provider->GetClusterInfo(path);
if (!info.has_value())
{
return ::pw::Status::NotFound();
}

app::AttributeValueEncoder encoder(attributeReports, subjectDescriptor, path, info->dataVersion,
false /* isFabricFiltered */, nullptr /* attributeEncodingState */);
app::DataModel::ActionReturnStatus result = provider->ReadAttribute(request, encoder);

if (!result.IsSuccess())
{
app::DataModel::ActionReturnStatus::StringStorage storage;
ChipLogError(Support, "Failed to read data: %s", result.c_str(storage));
return ::pw::Status::Internal();
}

#else
PW_TRY(ChipErrorToPwStatus(app::ReadSingleClusterData(subjectDescriptor, false, path, attributeReports, nullptr)));
#endif

attributeReports.EndOfContainer();
PW_TRY(ChipErrorToPwStatus(writer.EndContainer(outer)));
PW_TRY(ChipErrorToPwStatus(writer.Finalize()));
Expand Down
44 changes: 33 additions & 11 deletions src/app/WriteHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include <app/MessageDef/StatusIB.h>
#include <app/StatusResponse.h>
#include <app/WriteHandler.h>
#include <app/data-model-provider/MetadataTypes.h>
#include <app/data-model-provider/OperationTypes.h>
#include <app/reporting/Engine.h>
#include <app/util/MatterCallbacks.h>
Expand All @@ -32,6 +33,7 @@
#include <lib/core/CHIPError.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/TypeTraits.h>
#include <lib/support/logging/TextOnlyLogging.h>
#include <messaging/ExchangeContext.h>
#include <protocols/interaction_model/StatusCode.h>

Expand All @@ -41,8 +43,7 @@ namespace chip {
namespace app {

using namespace Protocols::InteractionModel;
using Status = Protocols::InteractionModel::Status;
constexpr uint8_t kListAttributeType = 0x48;
using Status = Protocols::InteractionModel::Status;

CHIP_ERROR WriteHandler::Init(DataModel::Provider * apProvider, WriteHandlerDelegate * apWriteHandlerDelegate)
{
Expand Down Expand Up @@ -79,6 +80,30 @@ void WriteHandler::Close()
MoveToState(State::Uninitialized);
}

std::optional<bool> WriteHandler::IsListAttributePath(const ConcreteAttributePath & path)
{
#if CHIP_CONFIG_USE_DATA_MODEL_INTERFACE
VerifyOrReturnValue(mDataModelProvider != nullptr, std::nullopt,
ChipLogError(DataManagement, "Null data model while checking attribute properties."));

auto info = mDataModelProvider->GetAttributeInfo(path);
if (!info.has_value())
{
return std::nullopt;
}

return info->flags.Has(DataModel::AttributeQualityFlags::kListAttribute);
#else
constexpr uint8_t kListAttributeType = 0x48;
const auto attributeMetadata = GetAttributeMetadata(path);
if (attributeMetadata == nullptr)
{
return std::nullopt;
}
return (attributeMetadata->attributeType == kListAttributeType);
#endif
}

Status WriteHandler::HandleWriteRequestMessage(Messaging::ExchangeContext * apExchangeContext,
System::PacketBufferHandle && aPayload, bool aIsTimedWrite)
{
Expand Down Expand Up @@ -317,10 +342,7 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeData
err = element.GetData(&dataReader);
SuccessOrExit(err);

const auto attributeMetadata = GetAttributeMetadata(dataAttributePath);
bool currentAttributeIsList = (attributeMetadata != nullptr && attributeMetadata->attributeType == kListAttributeType);

if (!dataAttributePath.IsListOperation() && currentAttributeIsList)
if (!dataAttributePath.IsListOperation() && IsListAttributePath(dataAttributePath).value_or(false))
{
dataAttributePath.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll;
}
Expand Down Expand Up @@ -446,7 +468,7 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut
mProcessingAttributePath, mStateFlags.Has(StateBits::kProcessingAttributeIsList), dataAttributePath);
bool shouldReportListWriteBegin = false; // This will be set below.

const EmberAfAttributeMetadata * attributeMetadata = nullptr;
std::optional<bool> isListAttribute = std::nullopt;

while (iterator->Next(mapping))
{
Expand All @@ -460,11 +482,11 @@ CHIP_ERROR WriteHandler::ProcessGroupAttributeDataIBs(TLV::TLVReader & aAttribut
// Try to get the metadata from for the attribute from one of the expanded endpoints (it doesn't really matter which
// endpoint we pick, as long as it's valid) and update the path info according to it and recheck if we need to report
// list write begin.
if (attributeMetadata == nullptr)
if (!isListAttribute.has_value())
{
attributeMetadata = GetAttributeMetadata(dataAttributePath);
bool currentAttributeIsList =
(attributeMetadata != nullptr && attributeMetadata->attributeType == kListAttributeType);
isListAttribute = IsListAttributePath(dataAttributePath);
bool currentAttributeIsList = isListAttribute.value_or(false);

if (!dataAttributePath.IsListOperation() && currentAttributeIsList)
{
dataAttributePath.mListOp = ConcreteDataAttributePath::ListOperation::ReplaceAll;
Expand Down
6 changes: 6 additions & 0 deletions src/app/WriteHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,12 @@ class WriteHandler : public Messaging::ExchangeDelegate
CHIP_ERROR WriteClusterData(const Access::SubjectDescriptor & aSubject, const ConcreteDataAttributePath & aPath,
TLV::TLVReader & aData);

/// Checks whether the given path corresponds to a list attribute
/// Return values:
/// true/false: valid attribute path, known if list or not
/// std::nulloptr - path not available/valid, unknown if attribute is a list or not
std::optional<bool> IsListAttributePath(const ConcreteAttributePath & path);

Messaging::ExchangeHolder mExchangeCtx;
WriteResponseMessage::Builder mWriteResponseBuilder;
Optional<ConcreteAttributePath> mProcessingAttributePath;
Expand Down
14 changes: 12 additions & 2 deletions src/app/chip_data_model.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,17 @@ endfunction()
#
function(chip_configure_data_model APP_TARGET)
set(SCOPE PRIVATE)
cmake_parse_arguments(ARG "" "SCOPE;ZAP_FILE;IDL" "EXTERNAL_CLUSTERS" ${ARGN})
set(ADD_EMBER_INTERFACE_FILES TRUE)
cmake_parse_arguments(ARG "SKIP_EMBER_INTERFACE" "SCOPE;ZAP_FILE;IDL" "EXTERNAL_CLUSTERS" ${ARGN})

if(ARG_SCOPE)
set(SCOPE ${ARG_SCOPE})
endif()

if(ARG_SKIP_EMBER_INTERFACE)
set(ADD_EMBER_INTERFACE_FILES FALSE)
endif()

# CMAKE data model auto-includes the server side implementation
target_sources(${APP_TARGET} ${SCOPE}
${CHIP_APP_BASE_DIR}/server/AclStorage.cpp
Expand Down Expand Up @@ -159,7 +164,6 @@ function(chip_configure_data_model APP_TARGET)
${CHIP_APP_BASE_DIR}/util/attribute-table.cpp
${CHIP_APP_BASE_DIR}/util/binding-table.cpp
${CHIP_APP_BASE_DIR}/util/DataModelHandler.cpp
${CHIP_APP_BASE_DIR}/util/ember-compatibility-functions.cpp
${CHIP_APP_BASE_DIR}/util/ember-global-attribute-access-interface.cpp
${CHIP_APP_BASE_DIR}/util/ember-io-storage.cpp
${CHIP_APP_BASE_DIR}/util/generic-callback-stubs.cpp
Expand All @@ -169,4 +173,10 @@ function(chip_configure_data_model APP_TARGET)
${APP_GEN_FILES}
${APP_TEMPLATES_GEN_FILES}
)

if(ADD_EMBER_INTERFACE_FILES)
target_sources(${APP_TARGET} ${SCOPE}
${CHIP_APP_BASE_DIR}/util/ember-compatibility-functions.cpp
)
endif()
endfunction()
6 changes: 5 additions & 1 deletion src/app/chip_data_model.gni
Original file line number Diff line number Diff line change
Expand Up @@ -209,11 +209,15 @@ template("chip_data_model") {
"${_app_root}/util/DataModelHandler.cpp",
"${_app_root}/util/attribute-storage.cpp",
"${_app_root}/util/attribute-table.cpp",
"${_app_root}/util/ember-compatibility-functions.cpp",
"${_app_root}/util/ember-global-attribute-access-interface.cpp",
"${_app_root}/util/ember-io-storage.cpp",
"${_app_root}/util/util.cpp",
]

if (chip_use_data_model_interface != "enabled") {
# This applies to "check" or "disabled" (i.e. use ember-only or use ember for double-checking)
sources += [ "${_app_root}/util/ember-compatibility-functions.cpp" ]
}
}

if (defined(invoker.zap_file)) {
Expand Down

0 comments on commit 07a28ac

Please sign in to comment.