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

RIOT: auto execute LFC on build #166

Merged
merged 5 commits into from
Dec 13, 2024
Merged

Conversation

LasseRosenow
Copy link
Collaborator

@LasseRosenow LasseRosenow commented Dec 12, 2024

This adds the lfc command to riot.mk and also moves more configuration over from the lingo template.

For more info see: lf-lang/lf-riot-uc-template#1

Copy link
Contributor

github-actions bot commented Dec 12, 2024

Benchmark results after merging this PR:

Benchmark results

Performance:

PingPongUc:
Best Time: 166.182 msec
Worst Time: 177.775 msec
Median Time: 172.270 msec

PingPongC:
Best Time: 168.904 msec
Worst Time: 169.313 msec
Median Time: 168.586 msec

ReactionLatencyUc:
Best latency: 41147 nsec
Median latency: 59760 nsec
Worst latency: 93663 nsec

ReactionLatencyC:
Best latency: 21085 nsec
Median latency: 59730 nsec
Worst latency: 96243 nsec

Memory usage:

PingPongUc:
text data bss dec hex filename
39564 752 8496 48812 beac bin/PingPongUc

PingPongC:
text data bss dec hex filename
46044 872 360 47276 b8ac bin/PingPongC

ReactionLatencyUc:
text data bss dec hex filename
29689 736 2112 32537 7f19 bin/ReactionLatencyUc

ReactionLatencyC:
text data bss dec hex filename
41666 840 360 42866 a772 bin/ReactionLatencyC

make/riot/riot.mk Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Dec 13, 2024

Memory usage after merging this PR will be:

Memory Report

action_empty_test_c

from to increase (%)
text 59715 59715 0.00
data 744 744 0.00
bss 10112 10112 0.00
total 70571 70571 0.00

action_microstep_test_c

from to increase (%)
text 60586 60586 0.00
data 752 752 0.00
bss 10112 10112 0.00
total 71450 71450 0.00

action_overwrite_test_c

from to increase (%)
text 60423 60423 0.00
data 744 744 0.00
bss 10112 10112 0.00
total 71279 71279 0.00

action_test_c

from to increase (%)
text 60327 60327 0.00
data 752 752 0.00
bss 10112 10112 0.00
total 71191 71191 0.00

deadline_test_c

from to increase (%)
text 55913 55913 0.00
data 760 760 0.00
bss 10784 10784 0.00
total 67457 67457 0.00

delayed_conn_test_c

from to increase (%)
text 61342 61342 0.00
data 744 744 0.00
bss 10112 10112 0.00
total 72198 72198 0.00

event_payload_pool_test_c

from to increase (%)
text 18330 18330 0.00
data 624 624 0.00
bss 320 320 0.00
total 19274 19274 0.00

event_queue_test_c

from to increase (%)
text 27597 27597 0.00
data 736 736 0.00
bss 480 480 0.00
total 28813 28813 0.00

nanopb_test_c

from to increase (%)
text 42888 42888 0.00
data 904 904 0.00
bss 320 320 0.00
total 44112 44112 0.00

port_test_c

from to increase (%)
text 61290 61290 0.00
data 744 744 0.00
bss 10112 10112 0.00
total 72146 72146 0.00

reaction_queue_test_c

from to increase (%)
text 27319 27319 0.00
data 736 736 0.00
bss 480 480 0.00
total 28535 28535 0.00

request_shutdown_test_c

from to increase (%)
text 60558 60558 0.00
data 744 744 0.00
bss 10112 10112 0.00
total 71414 71414 0.00

startup_test_c

from to increase (%)
text 55612 55612 0.00
data 752 752 0.00
bss 10784 10784 0.00
total 67148 67148 0.00

tcp_channel_test_c

from to increase (%)
text 93837 93837 0.00
data 1224 1224 0.00
bss 21280 21280 0.00
total 116341 116341 0.00

timer_test_c

from to increase (%)
text 55503 55503 0.00
data 744 744 0.00
bss 10784 10784 0.00
total 67031 67031 0.00

@LasseRosenow LasseRosenow marked this pull request as ready for review December 13, 2024 12:41
@LasseRosenow
Copy link
Collaborator Author

@erlingrj Ready for review again :)

# Build lf example
pushd hello_lf
run/build.sh
popd
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Integrating the LFC into the make file is really nice! Now we don't need to handle _LF examples different from c-code based examples anymore!

@LasseRosenow LasseRosenow force-pushed the riot-auto-execute-lfc-on-build branch from e12ba6a to a9bfafd Compare December 13, 2024 14:06
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 good. Just a few minor comments. I will let you merge it when you've seen them

LF_SRC_GEN_PATH = $(CURDIR)/src-gen/$(LF_MAIN)
# Execute the LF compiler if build target is "all"
ifeq ($(MAKECMDGOALS),all)
_ := $(shell $(REACTOR_UC_PATH)/lfc/bin/lfc-dev src/$(LF_MAIN).lf)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just for my own understanding, what does this syntax here achieve? Will the command be invoked before any target is run?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a small hack, it copies the output of the shell script over to the variable in this case underscore. This way the evaluation of the makefile is blocked until the script is finished ;)

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's very clever!

APPLICATION ?= $(LF_MAIN)

# Path of generated lf c-code
LF_SRC_GEN_PATH ?= $(CURDIR)/src-gen/$(LF_MAIN)
Copy link
Collaborator

Choose a reason for hiding this comment

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

An unfortunate detail about lfc is that where the generated-code is put is a little complicated:

  1. ~/src/Hello.lf -> ~/src-gen/Hello
  2. ~/Hello.lf -> ~/src-gen/Hello
  3. ~/src/lib/Hello.lf -> `~/src-gen/lib/Hello

I think we should ignore (2) where it is not in a src directory, for our template we say that the LF files must reside under a src directory. But for (3) to work, LF_MAIN must be set to lib/Hello, which might actually be reasonable

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Aaahh that is good to know, I will think about a good solution

make/riot/riot-lfc.mk Outdated Show resolved Hide resolved
@erlingrj erlingrj merged commit 1b80d88 into main Dec 13, 2024
7 of 8 checks passed
@erlingrj erlingrj deleted the riot-auto-execute-lfc-on-build branch December 13, 2024 17:46
Copy link
Contributor

Coverage after merging riot-auto-execute-lfc-on-build into main will be

71.23%

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.c78.35%55.56%84.62%83.33%12–13, 18, 20–21, 31, 35–36, 42–43, 51–52, 55–56, 60–61, 93–95
   event.c95.35%92.86%100%96.15%14–15
   federated.c5.61%2.97%7.69%6.76%101, 104–105, 105, 105–106, 108–109, 11, 111, 115–116, 118–120, 123, 125–129, 13, 13, 13, 130, 132–134, 137–139, 139, 139, 14, 140, 140, 140–142, 144, 147–148, 15, 150–154, 156–159, 16, 160–161, 163, 163, 163–166, 168, 168, 168–169, 17, 17, 17, 170, 170, 170–171, 175–176, 176, 176, 179–180, 184–186, 188, 188, 188, 190–194, 197, 197, 197–199, 20, 200, 203–204, 204, 204–205, 207–208, 21, 211–212, 217–218, 218, 218–219, 22, 22, 22, 221, 223, 223, 223–226, 226, 226, 226, 226–229, 23, 230–238, 24, 24, 24, 242, 245, 245, 245–247, 25, 251, 254–255, 255, 255, 255–259, 26, 260–263, 265, 27, 27, 27, 271–273, 28, 28, 28, 28, 28, 285–286, 289, 29, 290–292, 294, 294, 294–295, 299–300, 300, 300, 302, 304–305, 305, 305–306, 306, 306–307, 307, 307–308, 308, 308–309, 309, 309, 31, 310, 310, 310, 312, 312, 312–313, 313, 313–314, 314, 314–315, 315, 315, 317, 34, 34, 34, 34, 34–35, 38, 42–43, 45–48, 50, 50, 50–51, 51, 51, 53, 53, 53–55, 55, 55–57, 61–62, 66–67, 69–72, 74, 76, 76, 76–77, 77, 77–78, 78, 78–79, 79, 79, 82–83, 85–88, 9, 90, 90, 90–93, 95, 97, 97, 97–98
   logging.c87.50%80%100%88.64%24, 37–39, 46, 59–60
   network_channel.c57.69%50%100%58.82%36, 36, 36, 36, 41–44, 49–50, 53
   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.c70.34%54.55%100%78.26%15, 17, 21–22, 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.c0%0%0%0%10, 3–4, 4, 4–5, 5, 5–6, 8–9
src/platform/posix
   posix.c52.73%30%66.67%56%100, 100, 100–102, 106, 16, 18, 20–21, 34–36, 38–40, 48–49, 54–59, 59, 59–62, 62, 62–64, 67, 73–74, 78, 81, 92–94, 94, 94–96, 98–99
   tcp_ip_channel.c66.54%51.61%100%76.16%101, 103, 106–107, 117–118, 118, 118–119, 124–125, 125, 125–127, 131–132, 132, 132–134, 148, 154, 154, 154, 154, 154–155, 155, 155–156, 158, 158, 158–159, 168, 175–176, 180, 184, 184, 184–186, 186, 186–187, 189, 189, 189–191, 195, 195, 195–196, 216, 229–230, 230, 230–231, 237, 242–243, 243, 243–244, 244, 244–245, 247–248, 248, 248–249, 249, 249, 251–254, 264, 264–265, 265, 265–266, 290–291, 291, 291–293, 293, 293–294, 294, 294–295, 304, 304, 309, 311, 313, 315, 327–328,

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