Skip to content

Commit

Permalink
Merge branch 'master' into feature/app-install-flow-public
Browse files Browse the repository at this point in the history
  • Loading branch information
lazarkov authored Jul 3, 2024
2 parents 36eaa5f + e5acdc9 commit 1a8f583
Show file tree
Hide file tree
Showing 14 changed files with 145 additions and 33 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/examples-linux-standalone.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -202,21 +202,21 @@ jobs:
run: |
./scripts/run_in_build_env.sh \
"./scripts/build/build_examples.py \
--target linux-x64-fabric-admin \
--target linux-x64-fabric-admin-rpc \
build"
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
linux debug fabric-admin \
out/linux-x64-fabric-admin/fabric-admin \
out/linux-x64-fabric-admin-rpc/fabric-admin \
/tmp/bloat_reports/
- name: Build example Fabric Bridge App
run: |
./scripts/run_in_build_env.sh \
"./scripts/build/build_examples.py \
--target linux-x64-fabric-bridge \
--target linux-x64-fabric-bridge-no-ble-rpc \
build"
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
linux debug fabric-bridge-app \
out/linux-x64-fabric-bridge/fabric-bridge-app \
out/linux-x64-fabric-bridge-no-ble-rpc/fabric-bridge-app \
/tmp/bloat_reports/
- name: Uploading Size Reports
uses: ./.github/actions/upload-size-reports
Expand Down
1 change: 1 addition & 0 deletions examples/common/pigweed/protos/fabric_bridge_service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,6 @@ message SynchronizedDevice {

service FabricBridge {
rpc AddSynchronizedDevice(SynchronizedDevice) returns (pw.protobuf.Empty){}
rpc RemoveSynchronizedDevice(SynchronizedDevice) returns (pw.protobuf.Empty){}
}

5 changes: 5 additions & 0 deletions examples/common/pigweed/rpc_services/FabricBridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ class FabricBridge : public pw_rpc::nanopb::FabricBridge::Service<FabricBridge>
{
return pw::Status::Unimplemented();
}

virtual pw::Status RemoveSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response)
{
return pw::Status::Unimplemented();
}
};

} // namespace rpc
Expand Down
6 changes: 4 additions & 2 deletions examples/fabric-admin/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ config("config") {
defines += [ "CONFIG_USE_LOCAL_STORAGE" ]
}

cflags = [ "-Wconversion" ]

if (chip_enable_pw_rpc) {
defines += [ "PW_RPC_ENABLED" ]
}
Expand Down Expand Up @@ -144,6 +142,10 @@ static_library("fabric-admin-utils") {
]

deps += pw_build_LINK_DEPS
} else {
# The system_rpc_server.cc file is in pigweed and doesn't compile with
# -Wconversion, remove check for RPC build only.
cflags = [ "-Wconversion" ]
}

if (chip_enable_transport_trace) {
Expand Down
1 change: 1 addition & 0 deletions examples/fabric-admin/args.gni
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,4 @@ matter_log_json_payload_decode_full = true
# make fabric-admin very strict by default
chip_tlv_validate_char_string_on_read = true
chip_tlv_validate_char_string_on_write = true
chip_enable_ble = true
46 changes: 45 additions & 1 deletion examples/fabric-admin/rpc/RpcClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ constexpr uint32_t kDefaultChannelId = 1;
// Fabric Bridge Client
rpc::pw_rpc::nanopb::FabricBridge::Client fabricBridgeClient(rpc::client::GetDefaultRpcClient(), kDefaultChannelId);
pw::rpc::NanopbUnaryReceiver<::pw_protobuf_Empty> addSynchronizedDeviceCall;
pw::rpc::NanopbUnaryReceiver<::pw_protobuf_Empty> removeSynchronizedDeviceCall;

// Callback function to be called when the RPC response is received
void OnAddDeviceResponseCompleted(const pw_protobuf_Empty & response, pw::Status status)
Expand All @@ -55,6 +56,19 @@ void OnAddDeviceResponseCompleted(const pw_protobuf_Empty & response, pw::Status
}
}

// Callback function to be called when the RPC response is received
void OnRemoveDeviceResponseCompleted(const pw_protobuf_Empty & response, pw::Status status)
{
if (status.ok())
{
ChipLogProgress(NotSpecified, "RemoveSynchronizedDevice RPC call succeeded!");
}
else
{
ChipLogProgress(NotSpecified, "RemoveSynchronizedDevice RPC call failed with status: %d", status.code());
}
}

} // namespace

CHIP_ERROR InitRpcClient(uint16_t rpcServerPort)
Expand All @@ -76,11 +90,41 @@ CHIP_ERROR AddSynchronizedDevice(chip::NodeId nodeId)
chip_rpc_SynchronizedDevice device;
device.node_id = nodeId;

// The RPC will remain active as long as `addSynchronizedDeviceCall` is alive.
// By assigning the returned call to the global 'addSynchronizedDeviceCall', the RPC
// call is kept alive until it completes. When a response is received, it
// will be logged by the handler function and the call will complete.
addSynchronizedDeviceCall = fabricBridgeClient.AddSynchronizedDevice(device, OnAddDeviceResponseCompleted);

if (!addSynchronizedDeviceCall.active())
{
// The RPC call was not sent. This could occur due to, for example, an invalid channel ID. Handle if necessary.
return CHIP_ERROR_INTERNAL;
}

return CHIP_NO_ERROR;
}

CHIP_ERROR RemoveSynchronizedDevice(chip::NodeId nodeId)
{
ChipLogProgress(NotSpecified, "RemoveSynchronizedDevice");

if (removeSynchronizedDeviceCall.active())
{
ChipLogError(NotSpecified, "Remove Synchronized Device operation is in progress\n");
return CHIP_ERROR_BUSY;
}

chip_rpc_SynchronizedDevice device;
device.node_id = nodeId;

// By assigning the returned call to the global 'removeSynchronizedDeviceCall', the RPC
// call is kept alive until it completes. When a response is received, it
// will be logged by the handler function and the call will complete.
removeSynchronizedDeviceCall = fabricBridgeClient.RemoveSynchronizedDevice(device, OnRemoveDeviceResponseCompleted);

if (!removeSynchronizedDeviceCall.active())
{
// The RPC call was not sent. This could occur due to, for example, an invalid channel ID. Handle if necessary.
return CHIP_ERROR_INTERNAL;
}

Expand Down
15 changes: 15 additions & 0 deletions examples/fabric-admin/rpc/RpcClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,18 @@ CHIP_ERROR InitRpcClient(uint16_t rpcServerPort);
* - CHIP_ERROR_INTERNAL: An internal error occurred while activating the RPC call.
*/
CHIP_ERROR AddSynchronizedDevice(chip::NodeId nodeId);

/**
* @brief Removes a synchronized device from the RPC client.
*
* This function attempts to remove a device identified by its `nodeId` from the synchronized device list.
* It logs the progress and checks if a `RemoveSynchronizedDevice` operation is already in progress.
* If an operation is in progress, it returns `CHIP_ERROR_BUSY`.
*
* @param nodeId The Node ID of the device to be removed.
* @return CHIP_ERROR An error code indicating the success or failure of the operation.
* - CHIP_NO_ERROR: The RPC command was successfully sent.
* - CHIP_ERROR_BUSY: Another operation is currently in progress.
* - CHIP_ERROR_INTERNAL: An internal error occurred while activating the RPC call.
*/
CHIP_ERROR RemoveSynchronizedDevice(chip::NodeId nodeId);
6 changes: 4 additions & 2 deletions examples/fabric-bridge-app/linux/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ executable("fabric-bridge-app") {
"${chip_root}/src/lib",
]

cflags = [ "-Wconversion" ]

include_dirs = [ "include" ]

if (bridge_enable_pw_rpc) {
Expand Down Expand Up @@ -87,6 +85,10 @@ executable("fabric-bridge-app") {
"${chip_root}/examples/common",
"${chip_root}/examples/platform/linux",
]
} else {
# The system_rpc_server.cc file is in pigweed and doesn't compile with
# -Wconversion, remove check for RPC build only.
cflags = [ "-Wconversion" ]
}

output_dir = root_out_dir
Expand Down
16 changes: 16 additions & 0 deletions examples/fabric-bridge-app/linux/RpcServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class FabricBridge final : public chip::rpc::FabricBridge
{
public:
pw::Status AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) override;
pw::Status RemoveSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response) override;
};

pw::Status FabricBridge::AddSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response)
Expand All @@ -64,6 +65,21 @@ pw::Status FabricBridge::AddSynchronizedDevice(const chip_rpc_SynchronizedDevice
return pw::OkStatus();
}

pw::Status FabricBridge::RemoveSynchronizedDevice(const chip_rpc_SynchronizedDevice & request, pw_protobuf_Empty & response)
{
NodeId nodeId = request.node_id;
ChipLogProgress(NotSpecified, "Received RemoveSynchronizedDevice: " ChipLogFormatX64, ChipLogValueX64(nodeId));

int removed_idx = DeviceMgr().RemoveDeviceByNodeId(nodeId);
if (removed_idx < 0)
{
ChipLogError(NotSpecified, "Failed to remove device with nodeId=0x" ChipLogFormatX64, ChipLogValueX64(nodeId));
return pw::Status::NotFound();
}

return pw::OkStatus();
}

FabricBridge fabric_bridge_service;
#endif // defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE

Expand Down
3 changes: 3 additions & 0 deletions examples/fabric-bridge-app/linux/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ using namespace chip::app::Clusters::AdministratorCommissioning;
namespace {

constexpr uint16_t kPollIntervalMs = 100;

#if defined(PW_RPC_FABRIC_BRIDGE_SERVICE) && PW_RPC_FABRIC_BRIDGE_SERVICE
constexpr uint16_t kRetryIntervalS = 3;
#endif

bool KeyboardHit()
{
Expand Down
2 changes: 1 addition & 1 deletion src/app/codegen-data-model/CodegenDataModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ void LoadAttributeInfo(const ConcreteAttributePath & path, const EmberAfAttribut
InteractionModel::AttributeInfo * info)
{
info->readPrivilege = RequiredPrivilege::ForReadAttribute(path);
if (attribute.IsReadOnly())
if (!attribute.IsReadOnly())
{
info->writePrivilege = RequiredPrivilege::ForWriteAttribute(path);
}
Expand Down
22 changes: 21 additions & 1 deletion src/app/codegen-data-model/tests/TestCodegenModelViaMocks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ constexpr NodeId kTestNodeId = 0xFFFF'1234'ABCD'4321;

constexpr EndpointId kEndpointIdThatIsMissing = kMockEndpointMin - 1;

constexpr AttributeId kReadOnlyAttributeId = 0x5001;

static_assert(kEndpointIdThatIsMissing != kInvalidEndpointId);
static_assert(kEndpointIdThatIsMissing != kMockEndpoint1);
static_assert(kEndpointIdThatIsMissing != kMockEndpoint2);
Expand Down Expand Up @@ -190,6 +192,11 @@ const MockNodeConfig gTestNodeConfig({
}),
MockClusterConfig(MockClusterId(3), {
ClusterRevision::Id, FeatureMap::Id,
MockAttributeConfig(
kReadOnlyAttributeId,
ZCL_INT32U_ATTRIBUTE_TYPE,
ATTRIBUTE_MASK_NULLABLE // NOTE: explicltly NOT ATTRIBUTE_MASK_WRITABLE
)
}),
MockClusterConfig(MockClusterId(4), {
ClusterRevision::Id,
Expand Down Expand Up @@ -809,9 +816,22 @@ TEST(TestCodegenModelViaMocks, GetAttributeInfo)
ASSERT_TRUE(info.has_value());
EXPECT_FALSE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access)

// Mocks always set everything as R/W with administrative privileges
EXPECT_EQ(info->readPrivilege, chip::Access::Privilege::kAdminister); // NOLINT(bugprone-unchecked-optional-access)
EXPECT_EQ(info->writePrivilege, chip::Access::Privilege::kAdminister); // NOLINT(bugprone-unchecked-optional-access)

info = model.GetAttributeInfo(ConcreteAttributePath(kMockEndpoint2, MockClusterId(2), MockAttributeId(2)));
ASSERT_TRUE(info.has_value());
EXPECT_TRUE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access)
EXPECT_TRUE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access)
EXPECT_EQ(info->readPrivilege, chip::Access::Privilege::kAdminister); // NOLINT(bugprone-unchecked-optional-access)
EXPECT_EQ(info->writePrivilege, chip::Access::Privilege::kAdminister); // NOLINT(bugprone-unchecked-optional-access)

// test a read-only attribute, which will not have a write privilege
info = model.GetAttributeInfo(ConcreteAttributePath(kMockEndpoint3, MockClusterId(3), kReadOnlyAttributeId));
ASSERT_TRUE(info.has_value());
EXPECT_FALSE(info->flags.Has(AttributeQualityFlags::kListAttribute)); // NOLINT(bugprone-unchecked-optional-access)
EXPECT_EQ(info->readPrivilege, chip::Access::Privilege::kAdminister); // NOLINT(bugprone-unchecked-optional-access)
EXPECT_FALSE(info->writePrivilege.has_value()); // NOLINT(bugprone-unchecked-optional-access)
}

// global attributes are EXPLICITLY not supported
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,15 +252,16 @@ CHIP_ERROR P256Keypair::ECDH_derive_secret(const P256PublicKey & remote_public_k

return_status = trustm_ecdh_derive_secret(OPTIGA_KEY_ID_E100, (uint8_t *) remote_key, (uint16_t) rem_pubKeyLen + 3,
out_secret.Bytes(), (uint8_t) secret_length);

VerifyOrExit(return_status == OPTIGA_LIB_SUCCESS, error = CHIP_ERROR_INTERNAL);
out_secret.SetLength(secret_length);
error = CHIP_NO_ERROR;

exit:
if (error != CHIP_NO_ERROR)
{
trustm_close();
}
return out_secret.SetLength(secret_length);
return error;
#endif
}

Expand Down Expand Up @@ -295,7 +296,7 @@ CHIP_ERROR P256PublicKey::ECDSA_validate_hash_signature(const uint8_t * hash, si
(uint8_t *) bytes, (uint8_t) kP256_PublicKey_Length);

VerifyOrExit(return_status == OPTIGA_LIB_SUCCESS, error = CHIP_ERROR_INTERNAL);

error = CHIP_NO_ERROR;
exit:
if (error != CHIP_NO_ERROR)
{
Expand Down Expand Up @@ -407,7 +408,7 @@ CHIP_ERROR P256PublicKey::ECDSA_validate_msg_signature(const uint8_t * msg, size
(uint8_t *) bytes, (uint8_t) kP256_PublicKey_Length);

VerifyOrExit(return_status == OPTIGA_LIB_SUCCESS, error = CHIP_ERROR_INTERNAL);

error = CHIP_NO_ERROR;
exit:
if (error != CHIP_NO_ERROR)
{
Expand Down
Loading

0 comments on commit 1a8f583

Please sign in to comment.