Skip to content

Commit

Permalink
Enable the src/controller tests on all platforms except nrfconnect. (#…
Browse files Browse the repository at this point in the history
…9638)

1) Modify our CHIPClientCallbacks template to suppress the "possibly
   unbounded stack" warning from a VLA we know we want to remove.
2) Don't try to link in BLELayer in the TestDevice test if BLE
   networking is disabled (so that BLELayer symbols don't actually
   exist).
3) Heap-allocate the huge FabricTable so we don't run into compile
   errors due to a stack frame being too big in TestDevice.
4) Fix the fact that SystemLayerImplSelect did not default-initialize
   mDispatchQueue.  This matters because TestDevice does weird things
   with a separate SystemLayer that the PlatformManager does not know
   about, nor does it call InitChipStack in any case.

The tests are still disabled on nrfconnect because I could not get
them to link there so far.

Fixes #9173
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Oct 1, 2021
1 parent 5df0b2a commit 1023098
Show file tree
Hide file tree
Showing 7 changed files with 221 additions and 13 deletions.
6 changes: 3 additions & 3 deletions src/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ if (chip_build_tests) {
deps += [ "${chip_root}/src/ble/tests" ]
}

if (!is_clang && chip_device_platform != "darwin" &&
chip_device_platform != "nrfconnect" &&
chip_device_platform != "efr32") {
# On nrfconnect, the controller tests run into
# https://github.com/project-chip/connectedhomeip/issues/9630
if (chip_device_platform != "nrfconnect") {
deps += [ "${chip_root}/src/controller/tests" ]
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,11 @@ bool emberAfDiscoverCommandsReceivedResponseCallback(ClusterId clusterId, uint16
{{#if (chip_server_has_list_attributes)}}
{{#chip_server_cluster_attributes}}
{{#if isList}}
{{! TODO: This stack usage bit is not OK. See https://github.com/project-chip/connectedhomeip/issues/8558 }}
#if !defined(__clang__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wstack-usage="
#endif // __clang__
void {{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}ListAttributeFilter(TLV::TLVReader * tlvData, Callback::Cancelable * onSuccessCallback, Callback::Cancelable * onFailureCallback)
{
// TODO: Add actual support for array and lists.
Expand Down Expand Up @@ -387,6 +392,10 @@ void {{asUpperCamelCase parent.name}}Cluster{{asUpperCamelCase name}}ListAttribu
Callback::Callback<{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}ListAttributeCallback> * cb = Callback::Callback<{{asUpperCamelCase parent.name}}{{asUpperCamelCase name}}ListAttributeCallback>::FromCancelable(onSuccessCallback);
cb->mCall(cb->mContext, count, data);
}
{{! TODO: This stack usage bit is not OK. See https://github.com/project-chip/connectedhomeip/issues/8558 }}
#if !defined(__clang__)
#pragma GCC diagnostic pop
#endif // __clang__

{{/if}}
{{/chip_server_cluster_attributes}}
Expand Down
8 changes: 2 additions & 6 deletions src/controller/tests/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ chip_test_suite("tests") {

test_sources = [ "TestCommissionableNodeController.cpp" ]

if (chip_device_platform == "linux" || chip_device_platform == "Darwin") {
test_sources += [ "TestDevice.cpp" ]
}
test_sources += [ "TestDevice.cpp" ]

cflags = [ "-Wconversion" ]

Expand All @@ -34,7 +32,5 @@ chip_test_suite("tests") {
"${nlunit_test_root}:nlunit-test",
]

if (chip_device_platform == "linux" || chip_device_platform == "Darwin") {
public_deps += [ "${chip_root}/src/controller/data_model" ]
}
public_deps += [ "${chip_root}/src/controller/data_model" ]
}
13 changes: 10 additions & 3 deletions src/controller/tests/TestDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
* limitations under the License.
*/

#if CONFIG_NETWORK_LAYER_BLE
#include <ble/BleLayer.h>
#endif // CONFIG_NETWORK_LAYER_BLE
#include <controller/CHIPDevice.h>
#include <inet/IPAddress.h>
#include <inet/InetLayer.h>
Expand Down Expand Up @@ -47,8 +49,12 @@ void TestDevice_EstablishSessionDirectly(nlTestSuite * inSuite, void * inContext
ExchangeManager exchangeMgr;
Inet::InetLayer inetLayer;
System::LayerImpl systemLayer;
#if CONFIG_NETWORK_LAYER_BLE
Ble::BleLayer blelayer;
FabricTable fabrics;
#endif // CONFIG_NETWORK_LAYER_BLE
// Heap-allocate the fairly large FabricTable so we don't end up with a huge
// stack.
FabricTable * fabrics = Platform::New<FabricTable>();
secure_channel::MessageCounterManager messageCounterManager;
SessionIDAllocator idAllocator;

Expand All @@ -65,7 +71,7 @@ void TestDevice_EstablishSessionDirectly(nlTestSuite * inSuite, void * inContext
BleListenParameters(&blelayer)
#endif
);
sessionMgr.Init(&systemLayer, &transportMgr, &fabrics, &messageCounterManager);
sessionMgr.Init(&systemLayer, &transportMgr, fabrics, &messageCounterManager);
exchangeMgr.Init(&sessionMgr);
messageCounterManager.Init(&exchangeMgr);

Expand All @@ -76,7 +82,7 @@ void TestDevice_EstablishSessionDirectly(nlTestSuite * inSuite, void * inContext
.inetLayer = &inetLayer,
.storageDelegate = nullptr,
.idAllocator = &idAllocator,
.fabricsTable = &fabrics,
.fabricsTable = fabrics,
};
Device device;
NodeId mockNodeId = 1;
Expand All @@ -93,6 +99,7 @@ void TestDevice_EstablishSessionDirectly(nlTestSuite * inSuite, void * inContext
messageCounterManager.Shutdown();
exchangeMgr.Shutdown();
sessionMgr.Shutdown();
Platform::Delete(fabrics);
transportMgr.Close();
inetLayer.Shutdown();
systemLayer.Shutdown();
Expand Down
2 changes: 1 addition & 1 deletion src/system/SystemLayerImplSelect.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ class LayerImplSelect : public LayerSocketsLoop
#endif // CHIP_SYSTEM_CONFIG_POSIX_LOCKING

#if CHIP_SYSTEM_CONFIG_USE_DISPATCH
dispatch_queue_t mDispatchQueue;
dispatch_queue_t mDispatchQueue = nullptr;
#endif
};

Expand Down
Loading

0 comments on commit 1023098

Please sign in to comment.