Migrate to new NetworkChannel interface #94

LasseRosenow · 2024-10-24T14:16:50Z

Closes #83
Closes #88

github-actions · 2024-10-24T14:17:57Z

Memory usage after merging this PR will be:

Memory Report

action_microstep_test_c

	from	to
text	39461	39461
data	744	744
bss	480	480
total	40685	40685

action_test_c

	from	to
text	39250	39250
data	744	744
bss	480	480
total	40474	40474

delayed_conn_test_c

	from	to
text	42745	42745
data	736	736
bss	480	480
total	43961	43961

event_payload_pool_test_c

	from	to
text	18297	18297
data	624	624
bss	320	320
total	19241	19241

event_queue_test_c

	from	to
text	26242	26242
data	720	720
bss	480	480
total	27442	27442

physical_action_test_c

	from	to
text	40463	40463
data	761	761
bss	2016	2016
total	43240	43240

port_test_c

	from	to
text	42620	42620
data	736	736
bss	480	480
total	43836	43836

reaction_queue_test_c

	from	to
text	25906	25906
data	720	720
bss	480	480
total	27106	27106

shutdown_test_c

	from	to
text	36731	36731
data	744	744
bss	2688	2688
total	40163	40163

startup_test_c

	from	to
text	36062	36062
data	744	744
bss	2464	2464
total	39270	39270

timer_test_c

	from	to
text	36248	36248
data	736	736
bss	480	480
total	37464	37464

github-actions · 2024-10-24T14:21:10Z

Memory usage after merging this PR will be:

Memory Report

action_microstep_test_c

	from	to
text	39461	39461
data	744	744
bss	480	480
total	40685	40685

action_test_c

	from	to
text	39250	39250
data	744	744
bss	480	480
total	40474	40474

delayed_conn_test_c

	from	to
text	42745	42745
data	736	736
bss	480	480
total	43961	43961

event_payload_pool_test_c

	from	to
text	18297	18297
data	624	624
bss	320	320
total	19241	19241

event_queue_test_c

	from	to
text	26242	26242
data	720	720
bss	480	480
total	27442	27442

physical_action_test_c

	from	to
text	40463	40463
data	761	761
bss	2016	2016
total	43240	43240

port_test_c

	from	to
text	42620	42620
data	736	736
bss	480	480
total	43836	43836

reaction_queue_test_c

	from	to
text	25906	25906
data	720	720
bss	480	480
total	27106	27106

shutdown_test_c

	from	to
text	36731	36731
data	744	744
bss	2688	2688
total	40163	40163

startup_test_c

	from	to
text	36062	36062
data	744	744
bss	2464	2464
total	39270	39270

timer_test_c

	from	to
text	36248	36248
data	736	736
bss	480	480
total	37464	37464

github-actions · 2024-10-24T14:34:57Z

Memory usage after merging this PR will be:

Memory Report

action_microstep_test_c

	from	to
text	39461	39461
data	744	744
bss	480	480
total	40685	40685

action_test_c

	from	to
text	39250	39250
data	744	744
bss	480	480
total	40474	40474

delayed_conn_test_c

	from	to
text	42745	42745
data	736	736
bss	480	480
total	43961	43961

event_payload_pool_test_c

	from	to
text	18297	18297
data	624	624
bss	320	320
total	19241	19241

event_queue_test_c

	from	to
text	26242	26242
data	720	720
bss	480	480
total	27442	27442

physical_action_test_c

	from	to
text	40463	40463
data	761	761
bss	2016	2016
total	43240	43240

port_test_c

	from	to
text	42620	42620
data	736	736
bss	480	480
total	43836	43836

reaction_queue_test_c

	from	to
text	25906	25906
data	720	720
bss	480	480
total	27106	27106

shutdown_test_c

	from	to
text	36731	36731
data	744	744
bss	2688	2688
total	40163	40163

startup_test_c

	from	to
text	36062	36062
data	744	744
bss	2464	2464
total	39270	39270

timer_test_c

	from	to
text	36248	36248
data	736	736
bss	480	480
total	37464	37464

github-actions · 2024-10-24T15:30:28Z

Memory usage after merging this PR will be:

Memory Report

action_microstep_test_c

	from	to
text	39461	39461
data	744	744
bss	480	480
total	40685	40685

action_test_c

	from	to
text	39250	39250
data	744	744
bss	480	480
total	40474	40474

delayed_conn_test_c

	from	to
text	42745	42745
data	736	736
bss	480	480
total	43961	43961

event_payload_pool_test_c

	from	to
text	18297	18297
data	624	624
bss	320	320
total	19241	19241

event_queue_test_c

	from	to
text	26242	26242
data	720	720
bss	480	480
total	27442	27442

physical_action_test_c

	from	to
text	40463	40463
data	761	761
bss	2016	2016
total	43240	43240

port_test_c

	from	to
text	42620	42620
data	736	736
bss	480	480
total	43836	43836

reaction_queue_test_c

	from	to
text	25906	25906
data	720	720
bss	480	480
total	27106	27106

shutdown_test_c

	from	to
text	36731	36731
data	744	744
bss	2688	2688
total	40163	40163

startup_test_c

	from	to
text	36062	36062
data	744	744
bss	2464	2464
total	39270	39270

timer_test_c

	from	to
text	36248	36248
data	736	736
bss	480	480
total	37464	37464

github-actions · 2024-10-24T15:31:26Z

Memory usage after merging this PR will be:

Memory Report

action_microstep_test_c

	from	to
text	39461	39461
data	744	744
bss	480	480
total	40685	40685

action_test_c

	from	to
text	39250	39250
data	744	744
bss	480	480
total	40474	40474

delayed_conn_test_c

	from	to
text	42745	42745
data	736	736
bss	480	480
total	43961	43961

event_payload_pool_test_c

	from	to
text	18297	18297
data	624	624
bss	320	320
total	19241	19241

event_queue_test_c

	from	to
text	26242	26242
data	720	720
bss	480	480
total	27442	27442

physical_action_test_c

	from	to
text	40463	40463
data	761	761
bss	2016	2016
total	43240	43240

port_test_c

	from	to
text	42620	42620
data	736	736
bss	480	480
total	43836	43836

reaction_queue_test_c

	from	to
text	25906	25906
data	720	720
bss	480	480
total	27106	27106

shutdown_test_c

	from	to
text	36731	36731
data	744	744
bss	2688	2688
total	40163	40163

startup_test_c

	from	to
text	36062	36062
data	744	744
bss	2464	2464
total	39270	39270

timer_test_c

	from	to
text	36248	36248
data	736	736
bss	480	480
total	37464	37464

github-actions · 2024-10-25T15:07:08Z

Memory usage after merging this PR will be:

Memory Report

action_microstep_test_c

	from	to
text	43239	43239
data	752	752
bss	480	480
total	44471	44471

action_test_c

	from	to
text	43028	43028
data	752	752
bss	480	480
total	44260	44260

delayed_conn_test_c

	from	to
text	47434	47434
data	744	744
bss	480	480
total	48658	48658

event_payload_pool_test_c

	from	to
text	18297	18297
data	624	624
bss	320	320
total	19241	19241

event_queue_test_c

	from	to
text	27185	27185
data	728	728
bss	480	480
total	28393	28393

nanopb_test_c

	from	to
text	42362	42362
data	744	744
bss	320	320
total	43426	43426

physical_action_test_c

	from	to
text	44240	44240
data	769	769
bss	2048	2048
total	47057	47057

port_test_c

	from	to
text	47309	47309
data	744	744
bss	480	480
total	48533	48533

reaction_queue_test_c

	from	to
text	26849	26849
data	728	728
bss	480	480
total	28057	28057

request_shutdown_test_c

	from	to
text	43819	43819
data	744	744
bss	480	480
total	45043	45043

shutdown_test_c

	from	to
text	40411	40411
data	752	752
bss	2720	2720
total	43883	43883

startup_test_c

	from	to
text	39742	39742
data	752	752
bss	2496	2496
total	42990	42990

timer_test_c

	from	to
text	39815	39815
data	744	744
bss	2528	2528
total	43087	43087

github-actions · 2024-10-25T15:09:39Z

Memory usage after merging this PR will be:

Memory Report

action_microstep_test_c

	from	to
text	43239	43239
data	752	752
bss	480	480
total	44471	44471

action_test_c

	from	to
text	43028	43028
data	752	752
bss	480	480
total	44260	44260

delayed_conn_test_c

	from	to
text	47434	47434
data	744	744
bss	480	480
total	48658	48658

event_payload_pool_test_c

	from	to
text	18297	18297
data	624	624
bss	320	320
total	19241	19241

event_queue_test_c

	from	to
text	27185	27185
data	728	728
bss	480	480
total	28393	28393

nanopb_test_c

	from	to
text	42362	42362
data	744	744
bss	320	320
total	43426	43426

physical_action_test_c

	from	to
text	44240	44240
data	769	769
bss	2048	2048
total	47057	47057

port_test_c

	from	to
text	47309	47309
data	744	744
bss	480	480
total	48533	48533

reaction_queue_test_c

	from	to
text	26849	26849
data	728	728
bss	480	480
total	28057	28057

request_shutdown_test_c

	from	to
text	43819	43819
data	744	744
bss	480	480
total	45043	45043

shutdown_test_c

	from	to
text	40411	40411
data	752	752
bss	2720	2720
total	43883	43883

startup_test_c

	from	to
text	39742	39742
data	752	752
bss	2496	2496
total	42990	42990

timer_test_c

	from	to
text	39815	39815
data	744	744
bss	2528	2528
total	43087	43087

github-actions · 2024-10-25T16:14:18Z

Memory usage after merging this PR will be:

Memory Report

action_microstep_test_c

	from	to
text	43239	43239
data	752	752
bss	480	480
total	44471	44471

action_test_c

	from	to
text	43028	43028
data	752	752
bss	480	480
total	44260	44260

delayed_conn_test_c

	from	to
text	47434	47434
data	744	744
bss	480	480
total	48658	48658

event_payload_pool_test_c

	from	to
text	18297	18297
data	624	624
bss	320	320
total	19241	19241

event_queue_test_c

	from	to
text	27185	27185
data	728	728
bss	480	480
total	28393	28393

nanopb_test_c

	from	to
text	42362	42362
data	744	744
bss	320	320
total	43426	43426

physical_action_test_c

	from	to
text	44240	44240
data	769	769
bss	2048	2048
total	47057	47057

port_test_c

	from	to
text	47309	47309
data	744	744
bss	480	480
total	48533	48533

reaction_queue_test_c

	from	to
text	26849	26849
data	728	728
bss	480	480
total	28057	28057

request_shutdown_test_c

	from	to
text	43819	43819
data	744	744
bss	480	480
total	45043	45043

shutdown_test_c

	from	to
text	40411	40411
data	752	752
bss	2720	2720
total	43883	43883

startup_test_c

	from	to
text	39742	39742
data	752	752
bss	2496	2496
total	42990	42990

timer_test_c

	from	to
text	39815	39815
data	744	744
bss	2528	2528
total	43087	43087

…hannel test inside the unit tests folder

LasseRosenow · 2024-10-25T19:19:41Z

Okay this should be finished now.

I am not sure why these tests fail. It seems my unit test cannot find the tcp_ip_channel.c file, but locally it compiles and runs on my computer.

The client and server seem to be non blocking now, so the configuration in tcp_ip_channel.c was successful from what I can see. I added tests for this also.
The way the test works is a bit rough for now: If the tcp_ip_channel blocks even though not allowed, the test will probably never finish. I am not sure if the CI has timeouts for that.

But as long as everything finishes running at least we know it is non-blocking. :) 🤓

LasseRosenow · 2024-10-25T19:22:13Z

test/unit/mock.c

I am not sure. It seems there is something wrong with the cmake configuration.
I am using these functions in the new network_channels_test.c and the compiler was complaining about multiple implementations for these functions.

By commenting out these implementations in mock.c it compiled.

Hm, on my end I get undefined reference errors when these functions are not there...

Ah, I just defined them that way because we are not using them for any other test

LasseRosenow · 2024-10-25T19:23:47Z

src/platform/posix/tcp_ip_channel.c

-    }
-  }
-  self->super.close((NetworkChannel *)self);
+  self->receive_thread = 0;


This is needed so that the free function does not cleanup threads that don't exist anymore if you clean and recreate the same tcpchannel. (I noticed this when running my new unit test)

Because sometimes we recreate the tcp_ip_channel but without a new thread so the thread is still closed in reality

LasseRosenow · 2024-10-25T19:25:41Z

src/platform/posix/tcp_ip_channel.c

@@ -220,7 +232,7 @@ void TcpIpChannel_register_callback(NetworkChannel *untyped_self,
    throw("pthread_attr_init failed");
  }
 /* TODO: RIOT posix-wrappers don't have pthread_attr_setstack yet */
-#ifdef PLATFORM_RIOT
+#if defined(PLATFORM_RIOT) && !defined(__USE_XOPEN2K)


When compiling for RIOT native BOARD we otherwise get a deprecated warning. Native does support the new setstack function

LasseRosenow · 2024-10-25T19:26:32Z

src/platform/posix/tcp_ip_channel.c

  LF_DEBUG(NET, "Closing TCP/IP Channel");
  TcpIpChannel *self = (TcpIpChannel *)untyped_self;

-  if (self->server) {
+  if (self->server && self->client != 0) {


This extra check is needed because in my test I sometimes just open a connection only and don't actually create a client socket. Then cleaning a client socket here creates an error.

LasseRosenow · 2024-10-25T19:27:21Z

include/reactor-uc/platform/posix/tcp_ip_channel.h

@@ -33,7 +33,6 @@ struct TcpIpChannel {

  fd_set set;
  bool server;
-  bool blocking;


This variable is unused

LasseRosenow · 2024-10-25T19:27:52Z

Okay I added some comments to explain some important code parts

…s a hack/workaround)

…en in each test file

github-actions · 2024-10-25T19:41:48Z

Coverage after merging new-network-channel-interface into main will be

58.96%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
external/nanopb
pb_common.c	47.87%	34.38%	63.64%	52.94%	100, 100, 100, 102, 110, 14–15, 152, 166, 168, 171–172, 172, 172, 177, 181, 184–185, 20, 20, 201, 203, 210, 215, 226, 231–232, 238, 238, 238, 241–242, 246, 248, 248, 248, 250, 254, 260, 263, 265, 265, 265, 267, 269, 269, 269, 272–273, 295, 297, 300, 302, 302, 302, 304, 306, 306, 306, 308, 308, 308, 308, 308, 310, 313, 313, 313, 313, 313, 315, 320, 44, 46–48, 50–55, 58, 60–63, 65–70, 74, 77–78, 84, 86, 88–89, 89, 89–90, 90, 90, 93
pb_decode.c	26.86%	17.14%	54.05%	34.59%	100, 1007, 1011–1012, 1012, 1012, 1022, 1024, 1024, 1024–1025, 1027, 1030, 1032, 1032, 1032, 1034, 1038, 1038, 1038, 104, 1042, 1042, 1045, 1045, 1045, 1047, 1047, 1047, 1049, 105, 105, 105, 1050, 1053, 1053, 1053, 1055, 1059, 1059, 1059, 1061, 1063, 1063, 1063–1064, 1066, 1066, 1066, 1069, 1074, 1074, 1074–1076, 108, 1082, 1082, 1082, 1084, 1084, 1084, 1089, 1089, 1089, 109, 109, 109, 1090, 1090, 1090, 1092, 1092, 1092, 1095–1097, 1100, 1103–1104, 1110–1111, 1115–1116, 1116, 1116, 1118, 1118, 1118, 1125, 1129–1130, 1133, 1135, 1135, 1135–1136, 1136, 1136, 1140, 1143, 1145, 1145, 1145, 115, 1154, 1158, 1158, 1158, 116, 1160, 1165, 1165, 1165–1166, 1168, 1170, 1170, 1170–1171, 1179, 127–128, 128, 128, 131–132, 132, 132, 1336, 1341, 1346, 1349, 1349, 1349–1350, 1352–1353, 1356, 1359, 1359, 1359–1360, 1362, 1362, 1362–1363, 1365, 1367, 1370, 1377, 1377, 1377–1378, 1382, 1389, 1393, 1400, 1400, 1400–1401, 1405, 1416, 1420, 1422, 1430–1431, 1434–1436, 1438, 1438, 1438–1440, 1440, 1440–1441, 1443, 1443, 1443, 1445–1446, 1446, 1446, 1456, 1458, 1458, 1458–1459, 1463–1464, 1481, 1483, 1483, 1483–1485, 1485, 1485–1486, 1488, 1488, 1488, 1490–1491, 1491, 1491, 1503–1504, 1506–1507, 1507, 1507, 1510–1511, 1511, 1511, 1513, 1516, 1516, 1516, 1528–1529, 1529, 1529, 1537, 1541, 1543, 1543, 1543–1544, 1546, 1546, 1546–1547, 1547, 1547, 1550, 1552, 1552, 1552–1553, 1553, 1553, 1555, 1555, 1555, 1558, 1558, 1558, 1570, 1570, 1570–1571, 1571, 1571, 1574, 1576, 1576, 1576–1577, 1584, 1593–1594, 1596–1597, 1597, 1597, 1602, 1602, 1602, 1605–1606, 1606, 1606, 1608, 1610, 1610, 1610, 1612, 1618, 1618, 1624–1625, 1633–1634, 1639, 1643, 1643, 1643–1644, 1646, 1646, 1646–1647, 1647, 1647, 1649, 1649, 1649, 1652–1653, 1656, 1656, 1656–1657, 1657, 1657, 1659, 176, 178, 178, 178, 180, 180, 180, 182, 186, 189, 197–198, 202, 202, 202–203, 205, 205, 205, 208, 208, 208–209, 209, 209–210, 210, 210, 210, 210, 212, 212, 212, 212, 212, 214, 214, 214, 217, 217, 217, 219, 219, 219, 219, 219, 221, 221, 221, 223, 227, 229–230, 230, 230, 251–252, 254, 254, 254–255, 255, 255, 266, 271, 271, 271–273, 273, 273–274, 277, 280, 280, 280–281, 288, 298, 300, 308, 310, 310, 310, 310, 310, 310, 312–316, 316, 316, 323, 325–326, 326, 326, 326, 326, 328–329, 332–333, 333, 333–334, 334, 334, 336, 336, 336–338, 338, 338–339, 341–343, 345–347, 349, 355, 355, 355, 365–366, 369–370, 370, 370, 379–380, 380, 380–381, 398, 398, 398, 398, 398, 398, 400–401, 401, 401, 401, 401–402, 402, 402, 404,

LasseRosenow · 2024-10-25T19:55:16Z

About the test name:

I named it network channel test because except the setup function the test is implementation independent.

In theory once we have more channels we could run these tests all in one place and share the code.

We only need to call the correct constructors accordingly in setup

But it is okay to call it tcp for now. Maybe we can revisit once we have more channels

erlingrj · 2024-10-25T21:06:02Z

src/platform/posix/tcp_ip_channel.c

@@ -132,7 +144,7 @@ lf_ret_t TcpIpChannel_send(NetworkChannel *untyped_self, TaggedMessage *message)
  return LF_OK;
 }

-TaggedMessage *TcpIpChannel_receive(NetworkChannel *untyped_self) {
+static TaggedMessage *TcpIpChannel_receive(NetworkChannel *untyped_self) {


I like static functions, but I like const pointers better, we should start using them also

erlingrj · 2024-10-25T21:13:18Z

This looks great to me. A good first step towards stabilizing the NetworkChannel!

LasseRosenow added 5 commits October 25, 2024 15:05

Migrate to new NetworkChannel interface

18e2c08

Fix close_connection interface

361a0e4

Fix network channel free

d9c1094

Fix tcp_channel_try_connect return types

c15b407

Add WIP test_tcp_channel inside riot examples

70e8764

LasseRosenow force-pushed the new-network-channel-interface branch from 8c68249 to 70e8764 Compare October 25, 2024 15:06

Fix tcp-ip-channel function prefix

3d359c0

Fix function name

dbb90ea

LasseRosenow added 8 commits October 25, 2024 17:11

Remove tcp ip channel tests and move them to a more generic network c…

ff1eb7c

…hannel test inside the unit tests folder

Fix zephyr and posix federated examples

0003576

Fix

9dde67a

Test

9f3d1d2

Revert test

434ca79

Make accept non blocking

9fb2861

Fix tcpchannel close

6574a65

Improve networkchannel unit test

638ff5e

LasseRosenow commented Oct 25, 2024

View reviewed changes

LasseRosenow requested review from erlingrj and tanneberger October 25, 2024 19:28

erlingrj added 3 commits October 25, 2024 12:39

Rename to tcp_channel_test as this is more accurate

663a07d

Include the TcpIpChannel in the build when we build the tests (this i…

1cd197c

…s a hack/workaround)

Use weak linkage on setUp and tearDown such that they can be overridd…

9c59e41

…en in each test file

erlingrj reviewed Oct 25, 2024

View reviewed changes

erlingrj merged commit 94366f8 into main Oct 25, 2024
1 of 2 checks passed

erlingrj mentioned this pull request Oct 25, 2024

Move some examples to unit-test #85

Closed

erlingrj deleted the new-network-channel-interface branch October 25, 2024 23:27

Migrate to new NetworkChannel interface #94

Migrate to new NetworkChannel interface #94

Conversation

LasseRosenow commented Oct 24, 2024 • edited Loading

github-actions bot commented Oct 24, 2024

action_microstep_test_c

action_test_c

delayed_conn_test_c

event_payload_pool_test_c

event_queue_test_c

physical_action_test_c

port_test_c

reaction_queue_test_c

shutdown_test_c

startup_test_c

timer_test_c

github-actions bot commented Oct 24, 2024

action_microstep_test_c

action_test_c

delayed_conn_test_c

event_payload_pool_test_c

event_queue_test_c

physical_action_test_c

port_test_c

reaction_queue_test_c

shutdown_test_c

startup_test_c

timer_test_c

github-actions bot commented Oct 24, 2024

action_microstep_test_c

action_test_c

delayed_conn_test_c

event_payload_pool_test_c

event_queue_test_c

physical_action_test_c

port_test_c

reaction_queue_test_c

shutdown_test_c

startup_test_c

timer_test_c

github-actions bot commented Oct 24, 2024

action_microstep_test_c

action_test_c

delayed_conn_test_c

event_payload_pool_test_c

event_queue_test_c

physical_action_test_c

port_test_c

reaction_queue_test_c

shutdown_test_c

startup_test_c

timer_test_c

github-actions bot commented Oct 24, 2024

action_microstep_test_c

action_test_c

delayed_conn_test_c

event_payload_pool_test_c

event_queue_test_c

physical_action_test_c

port_test_c

reaction_queue_test_c

shutdown_test_c

startup_test_c

timer_test_c

github-actions bot commented Oct 25, 2024

action_microstep_test_c

action_test_c

delayed_conn_test_c

event_payload_pool_test_c

event_queue_test_c

nanopb_test_c

physical_action_test_c

port_test_c

reaction_queue_test_c

request_shutdown_test_c

shutdown_test_c

startup_test_c

timer_test_c

github-actions bot commented Oct 25, 2024

action_microstep_test_c

LasseRosenow commented Oct 24, 2024 •

edited

Loading

LasseRosenow commented Oct 25, 2024 •

edited

Loading