Skip to content

Commit

Permalink
Add tests for pending notification map & fix review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
gjc13 committed Feb 17, 2022
1 parent 0996dc8 commit 5a8377f
Show file tree
Hide file tree
Showing 25 changed files with 1,774 additions and 41,737 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -911,7 +911,7 @@
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "RAM",
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "",
Expand Down Expand Up @@ -8738,7 +8738,7 @@
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "RAM",
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -954,7 +954,7 @@
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "RAM",
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "",
Expand Down Expand Up @@ -5986,7 +5986,7 @@
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "RAM",
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "",
Expand Down
4 changes: 2 additions & 2 deletions examples/thermostat/thermostat-common/thermostat.zap
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "RAM",
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "",
Expand Down Expand Up @@ -8190,7 +8190,7 @@
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "RAM",
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "",
Expand Down
2 changes: 1 addition & 1 deletion examples/tv-app/tv-common/tv-app.zap
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "RAM",
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "",
Expand Down
4 changes: 2 additions & 2 deletions examples/tv-casting-app/tv-casting-common/tv-casting-app.zap
Original file line number Diff line number Diff line change
Expand Up @@ -871,7 +871,7 @@
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "RAM",
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "",
Expand Down Expand Up @@ -8005,7 +8005,7 @@
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "RAM",
"storageOption": "External",
"singleton": 0,
"bounded": 0,
"defaultValue": "",
Expand Down
15 changes: 11 additions & 4 deletions src/app/app-platform/ContentAppPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -418,10 +418,17 @@ CHIP_ERROR ContentAppPlatform::CreateBindingWithCallback(OperationalDeviceProxy
.cluster = MakeOptional(bindingClusterId),
};
}
Binding::Attributes::Binding::TypeInfo::Type bindingList(entries);
CHIP_ERROR error = cluster.WriteAttribute(bindingList, nullptr, Binding::Id, Binding::Attributes::Binding::Id, successCb,
failureCb, NullOptional);
ChipLogDetail(Controller, "CreateBindingWithCallback: Sent write request, waiting for response");
using BindingListTypeInfo = Binding::Attributes::Binding::TypeInfo;
BindingListTypeInfo::Type bindingList(entries);
CHIP_ERROR error = cluster.WriteAttribute<BindingListTypeInfo>(bindingList, nullptr, successCb, failureCb);
if (error == CHIP_NO_ERROR)
{
ChipLogDetail(Controller, "CreateBindingWithCallback: Sent write request, waiting for response");
}
else
{
ChipLogError(Controller, "CreateBindingWithCallback: Failed to send write request: %" CHIP_ERROR_FORMAT, error.Format());
}

return error;
}
Expand Down
1 change: 1 addition & 0 deletions src/app/chip_data_model.gni
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ template("chip_data_model") {
"${_app_root}/util/attribute-storage.cpp",
"${_app_root}/util/attribute-table.cpp",
"${_app_root}/util/binding-table.cpp",
"${_app_root}/util/binding-table.h",
"${_app_root}/util/client-api.cpp",
"${_app_root}/util/ember-compatibility-functions.cpp",
"${_app_root}/util/ember-print.cpp",
Expand Down
12 changes: 3 additions & 9 deletions src/app/clusters/bindings/PendingNotificationMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,15 @@ CHIP_ERROR PendingNotificationMap::FindLRUConnectPeer(FabricIndex * fabric, Node
{
continue;
}
bool foundSamePeer = false;
for (auto checkIter = BindingTable::GetInstance().begin(); iter != BindingTable::GetInstance().end(); ++iter)
for (auto checkIter = BindingTable::GetInstance().begin(); checkIter != BindingTable::GetInstance().end(); ++checkIter)
{
if (checkIter->type == EMBER_UNICAST_BINDING && checkIter->fabricIndex == checkIter->fabricIndex &&
checkIter->nodeId == checkIter->nodeId)
if (checkIter->type == EMBER_UNICAST_BINDING && checkIter->fabricIndex == iter->fabricIndex &&
checkIter->nodeId == iter->nodeId)
{
foundSamePeer = true;
bindingWithSamePeer[iter.GetIndex()] = checkIter.GetIndex();
break;
}
}
if (!foundSamePeer)
{
bindingWithSamePeer[iter.GetIndex()] = iter.GetIndex();
}
}

uint16_t lastAppear[EMBER_BINDING_TABLE_SIZE];
Expand Down
2 changes: 2 additions & 0 deletions src/app/clusters/bindings/PendingNotificationMap.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ class PendingNotificationMap

bool operator!=(const Iterator & rhs) { return mIndex != rhs.mIndex; }

bool operator==(const Iterator & rhs) { return mIndex == rhs.mIndex; }

private:
PendingNotificationMap * mMap;
int16_t mIndex;
Expand Down
8 changes: 3 additions & 5 deletions src/app/clusters/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,7 @@ CHIP_ERROR CheckValidBindingList(const DecodableBindingListType & bindingList, F
auto iter = bindingList.begin();
while (iter.Next())
{
VerifyOrReturnError(iter.GetValue().fabricIndex == accessingFabricIndex && IsValidBinding(iter.GetValue()),
CHIP_IM_GLOBAL_STATUS(ConstraintError));
VerifyOrReturnError(IsValidBinding(iter.GetValue()), CHIP_IM_GLOBAL_STATUS(ConstraintError));
listSize++;
}
ReturnErrorOnFailure(iter.GetStatus());
Expand All @@ -83,7 +82,7 @@ CHIP_ERROR CheckValidBindingList(const DecodableBindingListType & bindingList, F
}
}
ReturnErrorCodeIf(BindingTable::GetInstance().Size() - oldListSize + listSize > EMBER_BINDING_TABLE_SIZE,
CHIP_IM_GLOBAL_STATUS(ConstraintError));
CHIP_IM_GLOBAL_STATUS(ResourceExhausted));
return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -204,10 +203,9 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath
}
else if (path.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem)
{
DecodableBindingListType newBindingList;
TargetStructType target;
ReturnErrorOnFailure(decoder.Decode(target));
if (target.fabricIndex != accessingFabricIndex || !IsValidBinding(target))
if (!IsValidBinding(target))
{
return CHIP_IM_GLOBAL_STATUS(ConstraintError);
}
Expand Down
12 changes: 9 additions & 3 deletions src/app/tests/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,13 @@ static_library("helpers") {
]
}

source_set("binding-table") {
sources = [ "${chip_root}/src/app/util/binding-table.cpp" ]
source_set("binding-test-srcs") {
sources = [
"${chip_root}/src/app/clusters/bindings/PendingNotificationMap.cpp",
"${chip_root}/src/app/clusters/bindings/PendingNotificationMap.h",
"${chip_root}/src/app/util/binding-table.cpp",
"${chip_root}/src/app/util/binding-table.h",
]

public_deps = [
"${chip_root}/src/app/util/mock:mock_ember",
Expand All @@ -64,6 +69,7 @@ chip_test_suite("tests") {
"TestInteractionModelEngine.cpp",
"TestMessageDef.cpp",
"TestNumericAttributeTraits.cpp",
"TestPendingNotificationMap.cpp",
"TestReadInteraction.cpp",
"TestReportingEngine.cpp",
"TestStatusIB.cpp",
Expand All @@ -84,7 +90,7 @@ chip_test_suite("tests") {
cflags = [ "-Wconversion" ]

public_deps = [
":binding-table",
":binding-test-srcs",
"${chip_root}/src/app",
"${chip_root}/src/app/common:cluster-objects",
"${chip_root}/src/app/tests:helpers",
Expand Down
147 changes: 147 additions & 0 deletions src/app/tests/TestPendingNotificationMap.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
/*
*
* Copyright (c) 2022 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 <app/clusters/bindings/PendingNotificationMap.h>
#include <app/util/binding-table.h>
#include <lib/support/UnitTestRegistration.h>
#include <nlunit-test.h>

using chip::BindingTable;
using chip::ClusterId;
using chip::FabricIndex;
using chip::MakeOptional;
using chip::NodeId;
using chip::NullOptional;
using chip::PendingNotificationEntry;
using chip::PendingNotificationMap;

namespace {

void ClearBindingTable(BindingTable & table)
{
auto iter = table.begin();
while (iter != table.end())
{
iter = table.RemoveAt(iter);
}
}

void CreateDefaultFullBindingTable(BindingTable & table)
{
for (uint8_t i = 0; i < EMBER_BINDING_TABLE_SIZE; i++)
{
table.Add(EmberBindingTableEntry::ForNode(i / 10, i % 5, 0, 0, MakeOptional<ClusterId>(i)));
}
}

void TestEmptyMap(nlTestSuite * aSuite, void * aContext)
{
PendingNotificationMap pendingMap;
NL_TEST_ASSERT(aSuite, pendingMap.begin() == pendingMap.end());
FabricIndex fabricIndex;
NodeId node;
NL_TEST_ASSERT(aSuite, pendingMap.FindLRUConnectPeer(&fabricIndex, &node) == CHIP_ERROR_NOT_FOUND);
}

void TestAddRemove(nlTestSuite * aSuite, void * aContext)
{
PendingNotificationMap pendingMap;
ClearBindingTable(BindingTable::GetInstance());
CreateDefaultFullBindingTable(BindingTable::GetInstance());
for (uint8_t i = 0; i < EMBER_BINDING_TABLE_SIZE; i++)
{
pendingMap.AddPendingNotification(i, nullptr);
}
auto iter = pendingMap.begin();
for (uint8_t i = 0; i < EMBER_BINDING_TABLE_SIZE; i++)
{
PendingNotificationEntry entry = *iter;
NL_TEST_ASSERT(aSuite, entry.mBindingEntryId == i);
++iter;
}
NL_TEST_ASSERT(aSuite, iter == pendingMap.end());
pendingMap.RemoveAllEntriesForNode(0, 0);
uint8_t expectedEntryIndecies[] = { 1, 2, 3, 4, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19 };
iter = pendingMap.begin();
for (uint8_t i = 0; i < sizeof(expectedEntryIndecies); i++)
{
PendingNotificationEntry entry = *iter;
NL_TEST_ASSERT(aSuite, entry.mBindingEntryId == expectedEntryIndecies[i]);
++iter;
}
NL_TEST_ASSERT(aSuite, iter == pendingMap.end());
pendingMap.RemoveAllEntriesForFabric(0);
iter = pendingMap.begin();
for (uint8_t i = 0; i < 10; i++)
{
PendingNotificationEntry entry = *iter;
NL_TEST_ASSERT(aSuite, entry.mBindingEntryId == 10 + i);
++iter;
}
NL_TEST_ASSERT(aSuite, iter == pendingMap.end());
pendingMap.RemoveAllEntriesForFabric(1);
NL_TEST_ASSERT(aSuite, pendingMap.begin() == pendingMap.end());
}

void TestLRUEntry(nlTestSuite * aSuite, void * aContext)
{
PendingNotificationMap pendingMap;
ClearBindingTable(BindingTable::GetInstance());
CreateDefaultFullBindingTable(BindingTable::GetInstance());
pendingMap.AddPendingNotification(0, nullptr);
pendingMap.AddPendingNotification(1, nullptr);
pendingMap.AddPendingNotification(5, nullptr);
pendingMap.AddPendingNotification(7, nullptr);
pendingMap.AddPendingNotification(11, nullptr);

FabricIndex fabricIndex;
NodeId node;

NL_TEST_ASSERT(aSuite, pendingMap.FindLRUConnectPeer(&fabricIndex, &node) == CHIP_NO_ERROR);
NL_TEST_ASSERT(aSuite, fabricIndex == 0 && node == 1);

pendingMap.RemoveEntry(1);
NL_TEST_ASSERT(aSuite, pendingMap.FindLRUConnectPeer(&fabricIndex, &node) == CHIP_NO_ERROR);
NL_TEST_ASSERT(aSuite, fabricIndex == 0 && node == 0);

pendingMap.RemoveAllEntriesForFabric(0);
NL_TEST_ASSERT(aSuite, pendingMap.FindLRUConnectPeer(&fabricIndex, &node) == CHIP_NO_ERROR);
NL_TEST_ASSERT(aSuite, fabricIndex == 1 && node == 1);
}

} // namespace

int TestPeindingNotificationMap()
{
static nlTest sTests[] = {
NL_TEST_DEF("TestEmptyMap", TestEmptyMap),
NL_TEST_DEF("TestAddRemove", TestAddRemove),
NL_TEST_DEF("TestLRUEntry", TestLRUEntry),
NL_TEST_SENTINEL(),
};

nlTestSuite theSuite = {
"PendingNotificationMap",
&sTests[0],
nullptr,
nullptr,
};
nlTestRunner(&theSuite, nullptr);
return (nlTestRunnerStats(&theSuite));
}

CHIP_REGISTER_TEST_SUITE(TestPeindingNotificationMap)
10 changes: 5 additions & 5 deletions src/app/tests/suites/TestBinding.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ tests:
arguments:
value:
[
{ FabricIndex: 1 },
{ FabricIndex: 0 },
{
FabricIndex: 1,
FabricIndex: 0,
Node: 1,
Group: 1,
Endpoint: 1,
Expand All @@ -64,9 +64,9 @@ tests:
arguments:
value:
[
{ FabricIndex: 1, Group: 1 },
{ FabricIndex: 1, Node: 1, Endpoint: 1, Cluster: 6 },
{ FabricIndex: 1, Node: 2, Endpoint: 1 },
{ FabricIndex: 0, Group: 1 },
{ FabricIndex: 0, Node: 1, Endpoint: 1, Cluster: 6 },
{ FabricIndex: 0, Node: 2, Endpoint: 1 },
]

- label: "Read binding table"
Expand Down
4 changes: 2 additions & 2 deletions src/app/util/binding-table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ CHIP_ERROR BindingTable::Add(const EmberBindingTableEntry & entry)
return CHIP_NO_ERROR;
}

EmberBindingTableEntry & BindingTable::GetAt(uint8_t index)
const EmberBindingTableEntry & BindingTable::GetAt(uint8_t index)
{
return mBindingTable[index];
}
Expand Down Expand Up @@ -116,7 +116,7 @@ uint8_t BindingTable::GetNextAvaiableIndex()
return EMBER_BINDING_TABLE_SIZE;
}

BindingTable::Iterator BindingTable::Iterator::operator++() const
BindingTable::Iterator BindingTable::Iterator::operator++()
{
if (mIndex != EMBER_BINDING_TABLE_SIZE)
{
Expand Down
Loading

0 comments on commit 5a8377f

Please sign in to comment.