Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RIOT federated CoAP TEST #189

Merged
merged 27 commits into from
Jan 29, 2025
Merged

Add RIOT federated CoAP TEST #189

merged 27 commits into from
Jan 29, 2025

Conversation

LasseRosenow
Copy link
Collaborator

No real test yet, but at least I finally managed to run the sender and the receiver in parallel from one script file.
I had a strange race condition in RIOT perhaps? The sender and receiver were unable to talk over the tap interface, if they started to fast in parallel.

The solution for now is: sleep 3 .... But it wörks :)

image

@LasseRosenow
Copy link
Collaborator Author

The next step is to let the CI execute this and add a small test case that fails if the receiver does not receive the Hello message within a reasonable margin of time.

Copy link
Contributor

github-actions bot commented Jan 15, 2025

Benchmark results after merging this PR:

Benchmark results

Performance:

PingPongUc:
Best Time: 204.908 msec
Worst Time: 206.210 msec
Median Time: 206.342 msec

PingPongC:
Best Time: 169.787 msec
Worst Time: 177.327 msec
Median Time: 170.210 msec

ReactionLatencyUc:
Best latency: 31702 nsec
Median latency: 60490 nsec
Worst latency: 779339 nsec

ReactionLatencyC:
Best latency: 29635 nsec
Median latency: 60491 nsec
Worst latency: 97100 nsec

Memory usage:

PingPongUc:
text data bss dec hex filename
40932 760 7440 49132 bfec bin/PingPongUc

PingPongC:
text data bss dec hex filename
45826 880 360 47066 b7da bin/PingPongC

ReactionLatencyUc:
text data bss dec hex filename
30737 744 2080 33561 8319 bin/ReactionLatencyUc

ReactionLatencyC:
text data bss dec hex filename
41536 848 360 42744 a6f8 bin/ReactionLatencyC

Copy link
Contributor

github-actions bot commented Jan 15, 2025

Memory usage after merging this PR will be:

Memory Report

action_empty_test_c

from to increase (%)
text 60860 60821 -0.06
data 752 752 0.00
bss 11360 11360 0.00
total 72972 72933 -0.05

action_microstep_test_c

from to increase (%)
text 61731 61692 -0.06
data 760 760 0.00
bss 11424 11424 0.00
total 73915 73876 -0.05

action_overwrite_test_c

from to increase (%)
text 61568 61529 -0.06
data 752 752 0.00
bss 11424 11424 0.00
total 73744 73705 -0.05

action_test_c

from to increase (%)
text 61472 61433 -0.06
data 760 760 0.00
bss 11424 11424 0.00
total 73656 73617 -0.05

deadline_test_c

from to increase (%)
text 57102 57063 -0.07
data 768 768 0.00
bss 10784 10784 0.00
total 68654 68615 -0.06

delayed_conn_test_c

from to increase (%)
text 62464 62425 -0.06
data 752 752 0.00
bss 10272 10272 0.00
total 73488 73449 -0.05

event_payload_pool_test_c

from to increase (%)
text 18331 18331 0.00
data 624 624 0.00
bss 320 320 0.00
total 19275 19275 0.00

event_queue_test_c

from to increase (%)
text 27616 27616 0.00
data 736 736 0.00
bss 480 480 0.00
total 28832 28832 0.00

nanopb_test_c

from to increase (%)
text 42886 42886 0.00
data 904 904 0.00
bss 320 320 0.00
total 44110 44110 0.00

port_test_c

from to increase (%)
text 62412 62373 -0.06
data 752 752 0.00
bss 10272 10272 0.00
total 73436 73397 -0.05

reaction_queue_test_c

from to increase (%)
text 27448 27448 0.00
data 736 736 0.00
bss 480 480 0.00
total 28664 28664 0.00

request_shutdown_test_c

from to increase (%)
text 61703 61664 -0.06
data 752 752 0.00
bss 11424 11424 0.00
total 73879 73840 -0.05

startup_test_c

from to increase (%)
text 56801 56762 -0.07
data 760 760 0.00
bss 10784 10784 0.00
total 68345 68306 -0.06

tcp_channel_test_c

from to increase (%)
text 96867 96828 -0.04
data 1256 1256 0.00
bss 21376 21376 0.00
total 119499 119460 -0.03

timer_test_c

from to increase (%)
text 56692 56653 -0.07
data 752 752 0.00
bss 10784 10784 0.00
total 68228 68189 -0.06

@erlingrj
Copy link
Collaborator

Looks promising. It would be really great to understand what this race condition in the startup is about. Adding explicit sleeps to enforce some ordering often works great until we have some freak CI run where the timing is completely messed up because it is running in the cloud and maybe the CI task was suspended at the wrong point etc...

@LasseRosenow LasseRosenow force-pushed the add-riot-federated-coap-test branch 2 times, most recently from 36a6fac to 851f513 Compare January 23, 2025 15:55
@LasseRosenow LasseRosenow force-pushed the add-riot-federated-coap-test branch from 55d56c3 to c4360c5 Compare January 24, 2025 16:06
@LasseRosenow
Copy link
Collaborator Author

Okay this is pretty much done. The only problem that remains is that I am having issues setting up the tap interfaces in the CI. I will look more into it

@LasseRosenow
Copy link
Collaborator Author

Holy f** the CI has finally passed :D

@LasseRosenow
Copy link
Collaborator Author

Finally I got the tap interfaces working in the CI, this is ready for review!! 🥳

@LasseRosenow LasseRosenow marked this pull request as ready for review January 28, 2025 13:53
@LasseRosenow LasseRosenow requested a review from erlingrj January 28, 2025 13:53
Copy link
Collaborator

@erlingrj erlingrj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

THis looks great! Added a commit fixing the example program. And a few comments

examples/riot/coap_federated/receiver/main.c Outdated Show resolved Hide resolved
test/platform/riot/coap_channel_federated_test/run.sh Outdated Show resolved Hide resolved
Copy link
Contributor

Coverage after merging add-riot-federated-coap-test into main will be

70.49%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   action.c77.69%65.63%100%81.18%134–135, 24, 42–45, 48, 50–51, 54–56, 62–63, 70–72, 72, 72–75, 81–82, 93–94
   builtin_triggers.c90.91%70%100%96.77%14, 18, 40, 43
   connection.c77.33%51.16%100%86.73%10, 104, 11, 110, 123–124, 136–137, 14, 14, 143, 145–146, 148, 16–17, 21–22, 22, 22–23, 25, 27–28, 33, 48, 48, 48–49, 55, 60–62, 97
   environment.c82.18%60%92.31%86.76%12–13, 18, 20–21, 31, 35–36, 42–43, 59–60, 64–65, 97–99
   event.c95.35%92.86%100%96.15%14–15
   federated.c5.23%2.73%7.69%6.33%10, 100, 102, 104, 104, 104–105, 109, 11, 112–113, 113, 113–114, 116–117, 119, 123–124, 126–128, 13, 131, 133–138, 140–142, 145–147, 147, 147–148, 148, 148–149, 15, 15, 15, 150, 152, 155–156, 158–159, 16, 160–162, 164–169, 17, 171, 171, 171–174, 176, 176, 176–178, 178, 178–179, 18, 183–184, 184, 184, 187–188, 19, 19, 19, 192–194, 196, 196, 196, 198–202, 205, 205, 205–208, 211–212, 212, 212–213, 215–216, 219, 22, 220, 225–226, 226, 226–227, 229, 23, 231, 231, 231–234, 234, 234, 234, 234, 234–239, 24, 24, 24, 240–243, 243, 243–244, 246, 248–249, 25, 250–256, 26, 26, 26, 260, 263, 263, 263–265, 269, 27, 272–273, 273, 273, 273–279, 28, 280–281, 283, 289, 29, 29, 29, 290–291, 30, 30, 30, 30, 30, 303–304, 307–309, 31, 310, 312, 312, 312–313, 317–318, 318, 318, 320, 322–323, 323, 323–324, 324, 324–325, 325, 325–326, 326, 326–327, 327, 327–328, 328, 328–329, 329, 329, 33, 331, 331, 331–332, 332, 332–333, 333, 333–334, 334, 334–335, 335, 335, 337, 36, 36, 36, 36, 36–37, 41–42, 46–47, 49–52, 54, 54, 54–55, 55, 55, 57, 57, 57–59, 59, 59–61, 65–66, 70–71, 73–76, 78, 80, 80, 80–81, 81, 81–82, 82, 82–83, 83, 83, 86–87, 89–92, 94, 94, 94–97, 97, 97–98
   logging.c88.52%83.33%100%89.36%25, 38–40, 47, 60–61
   network_channel.c69.23%62.50%100%70.59%40, 40, 40, 45–48, 57
   port.c78.08%45.83%100%93.33%10, 10, 10, 16, 20, 25, 25–27, 27, 27–28, 39, 39, 39–40
   queues.c89.94%80.36%100%94.06%108, 113, 119, 21–23, 47–48, 60–61, 84–88, 91–92
   reaction.c71.19%54.55%100%79.71%15, 17, 21, 28–31, 31, 31–32, 42, 45, 47, 52–53, 53, 53–55, 55, 55–56, 73, 89–91, 91, 91–94, 94, 94–95
   reactor.c69.33%51.52%100%82.28%10, 101–102, 14–19, 22, 28, 30, 32–37, 37, 37–38, 38, 38, 43, 55, 58–59, 59, 59–60, 60, 60–61, 63, 77–78, 81–82, 82, 82–83, 83, 83–84, 86, 91
   serialization.c37.50%25%50%40%16–17, 26–27, 33–34, 34, 34–35, 37–38, 41–42, 42, 42–43, 45–46
   tag.c40.19%31.48%60%47.92%14, 14–15, 17, 17–18, 23–24, 24, 24, 24, 24–25, 27, 27, 27, 27, 27–28, 30, 30, 30–31, 33–34, 34, 34–35, 37, 37, 37, 37, 37–38, 40, 40, 40, 40, 40–41, 43, 53–54, 63, 63–64, 83–85, 85, 85, 85, 85, 85, 85, 85, 85, 85, 85–87, 89
   timer.c95%66.67%100%100%14, 25
   trigger.c100%100%100%100%
   util.c41.67%33.33%33.33%46.67%12–13, 13, 13–16, 18–20, 4–5
src/platform/posix
   posix.c53.15%30%66.67%56.58%100–101, 101, 101–103, 107, 16, 18, 20–21, 35–37, 39–41, 49–50, 55–60, 60, 60–63, 63, 63–65, 68, 74–75, 79, 82, 93–95, 95, 95–97, 99
   tcp_ip_channel.c65.10%51.42%94.12%74.36%100, 103–104, 104, 104–105, 119–120, 122, 124, 128–129, 137, 140–141, 141, 141–142, 147–148, 148, 148–149, 155–156, 156, 156–158, 172, 175, 179, 179, 179, 179, 179–180, 180, 180–181, 181, 181–182, 184, 186, 186, 186–187, 196, 203–204, 208–209, 213, 213, 213–215, 215, 215–216, 218, 218, 218–219, 219, 219, 221, 223–224, 228, 228, 228–229, 249, 262–263, 263, 263–264, 270, 275–276, 276,

@LasseRosenow
Copy link
Collaborator Author

Okay I think all the feedback is addressed, thank you!

@LasseRosenow LasseRosenow requested a review from erlingrj January 29, 2025 11:12
@erlingrj erlingrj merged commit f5cd92b into main Jan 29, 2025
8 checks passed
@erlingrj erlingrj deleted the add-riot-federated-coap-test branch January 29, 2025 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants