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

LFC federated RIOT fixes #192

Merged
merged 70 commits into from
Jan 22, 2025
Merged

LFC federated RIOT fixes #192

merged 70 commits into from
Jan 22, 2025

Conversation

LasseRosenow
Copy link
Collaborator

@LasseRosenow LasseRosenow commented Jan 20, 2025

Build:

FEDERATION=r1 make all

or

FEDERATION=r2 make all

Or directly Build and Run:

FEDERATION=r1 make all term

or

FEDERATION=r2 make all term

Base automatically changed from lfc-federated to main January 21, 2025 15:50
@LasseRosenow LasseRosenow marked this pull request as ready for review January 21, 2025 15:53
@@ -0,0 +1,3 @@
#!/bin/bash
REMOTE_ADDRESS=fe80::8cc3:33ff:febb:1b3 make all -C ./sender
REMOTE_ADDRESS=fe80::8cc3:33ff:febb:1b3 make all -C ./receiver
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I decided to add build.sh files to all riot examples, because a simple make all does not always work. Especially if you also want to build both federations.

Instead of making the runAll.sh file complicated with different commands per project I let it run build.sh in each folder it can find. This will also prevent the bug that the coap_federated example wasn't build in the CI because I forgot to add it to buildAll.sh

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds reasonable. We must find a good way to build federated programs. Your current proposal, with a single Makefile, works now. But I think in the future we want independent projects for each federate because there might several difference between the boards, and their configuration.

Two questions:

  1. Are you now able to pick a IP6 address from the tap interface?
  2. Why do they both specify the same remote?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah sorry github showed me this comment at the bottom and I replied in the main thread and not directly to this comment

@LasseRosenow LasseRosenow requested a review from erlingrj January 21, 2025 16:52
@@ -242,17 +242,24 @@ class UcTcpIpChannel(
class UcCoapUdpIpChannel(
Copy link
Member

Choose a reason for hiding this comment

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

Is this homogeneous enough that one code-generator class works potentially for all platforms?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We will find out when we add support for native and Zephyr

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.

Looks good, but I wasnt able to run the coap_federated_lf on my end

@@ -0,0 +1,3 @@
#!/bin/bash
REMOTE_ADDRESS=fe80::8cc3:33ff:febb:1b3 make all -C ./sender
REMOTE_ADDRESS=fe80::8cc3:33ff:febb:1b3 make all -C ./receiver
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds reasonable. We must find a good way to build federated programs. Your current proposal, with a single Makefile, works now. But I think in the future we want independent projects for each federate because there might several difference between the boards, and their configuration.

Two questions:

  1. Are you now able to pick a IP6 address from the tap interface?
  2. Why do they both specify the same remote?

@LasseRosenow
Copy link
Collaborator Author

Are you now able to pick a IP6 address from the tap interface?

Not yet. That will come later the build.sh is only to let the CI build the example

@LasseRosenow
Copy link
Collaborator Author

Why do they both specify the same remote

Because the remote doesn't matter just for building the project, but I have updated the build.sh files to make them more realistic to how a real scenario would look like :)

This all will be better when I have a reliable way to extract the IP addresses, but my focus was on getting the tests run reliably right now, so I couldn't look into it yet

Copy link
Contributor

Coverage after merging lfc-federated-riot-fixes into main will be

71.16%

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.c78.52%51.16%100%88.66%10, 104, 11, 110, 123–124, 136–137, 14, 14, 143, 145, 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.44%2.91%7.69%6.51%10, 100, 100, 100–101, 104, 107–108, 108, 108–109, 111–112, 114, 118–119, 12, 121–123, 126, 128–133, 135–137, 14, 14, 14, 140–142, 142, 142–143, 143, 143–145, 147, 15, 150–151, 153–157, 159, 16, 160–164, 166, 166, 166–169, 17, 171, 171, 171–173, 173, 173–174, 178–179, 179, 179, 18, 18, 18, 182–183, 187–189, 191, 191, 191, 193–197, 200, 200, 200–203, 206–207, 207, 207–208, 21, 210–211, 214–215, 22, 220–221, 221, 221–222, 224, 226, 226, 226–229, 229, 229, 229, 229, 23, 23, 23, 230–238, 238, 238–239, 24, 241, 243–249, 25, 25, 25, 253, 256, 256, 256–258, 26, 262, 265–266, 266, 266, 266–269, 27, 270–274, 276, 28, 28, 28, 282–284, 29, 29, 29, 29, 29, 296–297, 30, 300–303, 305, 305, 305–306, 310–311, 311, 311, 313, 315–316, 316, 316–317, 317, 317–318, 318, 318–319, 319, 319, 32, 320, 320, 320–321, 321, 321, 323, 323, 323–324, 324, 324–325, 325, 325–326, 326, 326, 328, 35, 35, 35, 35, 35–36, 40–41, 45–46, 48–51, 53, 53, 53–54, 54, 54, 56, 56, 56–58, 58, 58–60, 64–65, 69–70, 72–75, 77, 79, 79, 79–80, 80, 80–81, 81, 81–82, 82, 82, 85–86, 88–89, 9, 90–91, 93, 93, 93–96, 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.c50%50%50%50%16–17, 26–27, 33–35, 38–40
   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, 276–277, 277, 277–278, 280–281, 281, 281–282, 282, 282, 284–287, 297, 297–298, 298, 298–299, 323–324, 324, 324–326,

@LasseRosenow
Copy link
Collaborator Author

Your current proposal, with a single Makefile, works now. But I think in the future we want independent projects for each federate because there might several difference between the boards, and their configuration.

This PR is only about fixing the riot makefile integration to be able to build. The structure of the example is without any indication or proposal in that regard :) This is just the bare minimal that is needed to let it compile and find the makefiles in the correct federate directories in src-gen.

Once we have agreed on a solution how to structure it, I can align it :)

@LasseRosenow LasseRosenow requested a review from erlingrj January 22, 2025 12:57
@erlingrj
Copy link
Collaborator

OK, thanks for the explanations! Lets go!

@erlingrj erlingrj merged commit 5144e03 into main Jan 22, 2025
8 checks passed
@erlingrj erlingrj deleted the lfc-federated-riot-fixes branch January 22, 2025 13:48
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.

3 participants