[DRAFT] Bugs Found while building FSW25 Demo #236

tanneberger · 2025-03-04T04:08:00Z

No description provided.

github-actions · 2025-03-04T04:08:59Z

Coverage after merging uart-cmake into main will be

65.13%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
src
action.c	76.42%	65.63%	100%	79.31%	136–137, 24, 42–45, 48, 50–51, 54–57, 63–64, 71–73, 73, 73–76, 82–84, 95–96
builtin_triggers.c	90.91%	70%	100%	96.77%	14, 18, 40, 43
connection.c	77.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.c	77.17%	53.13%	92.31%	84.15%	111–112, 124–126, 14–15, 20, 24–26, 26, 26–28, 28, 28, 34–35, 44, 48–49, 55–56, 72–73, 77–78
event.c	95.35%	92.86%	100%	96.15%	14–15
federated.c	6.83%	3.33%	10%	8.43%	10, 101, 101, 101–102, 104, 104, 104, 104, 104–105, 105, 105–107, 11, 110, 110, 110–112, 114–115, 118–120, 120, 120–121, 121, 121–123, 125, 128–129, 13, 131–135, 139, 14, 140–141, 143, 143, 143, 145–149, 15, 152, 152, 152–155, 158–159, 159, 159, 16, 160, 162–163, 166–167, 172–173, 173, 173–174, 176, 178, 178, 178–179, 18, 18, 18, 180–181, 181, 181, 181, 181, 181–189, 19, 19, 19, 190, 190, 190–191, 193, 195–204, 208, 21, 21, 21, 211, 211, 211–213, 217, 22, 220–221, 221, 221, 221–226, 228–229, 23, 23, 23, 230, 232, 238–239, 24, 240, 25, 253–254, 254, 254–255, 255, 255–256, 256, 256–257, 257, 257–258, 258, 258–259, 259, 259–260, 260, 260, 262, 262, 262–263, 263, 263–264, 264, 264–265, 265, 265–266, 266, 266, 268, 29–30, 34–35, 37–40, 42, 44, 44, 44–45, 45, 45–46, 46, 46–47, 47, 47, 49, 51–54, 56, 56, 56–59, 59, 59–60, 62, 64–65, 65, 65–66, 70, 73–74, 74, 74–75, 77–78, 80, 84–85, 87–89, 92, 94–99
logging.c	88.52%	83.33%	100%	89.36%	25, 38–40, 47, 60–61
network_channel.c	69.23%	62.50%	100%	70.59%	45, 45, 45, 50–53, 62
port.c	74.68%	42.86%	100%	91.49%	10, 10, 10, 15, 15, 15–16, 22, 26, 31, 31–33, 33, 33–34, 45, 45, 45–46
queues.c	86.21%	79.03%	100%	88.37%	114–118, 121–122, 139, 144, 150, 29, 29, 33–38, 61–62, 86, 86, 90–95
reaction.c	69.92%	52.17%	100%	79.17%	15, 17, 21, 28–30, 30, 30–32, 32, 32–33, 44, 47, 49, 54–55, 55, 55–57, 57, 57–58, 75, 91–93, 93, 93–96, 96, 96–97
reactor.c	69.39%	51.56%	100%	82.05%	10, 100–101, 14–19, 22, 28, 30, 32–36, 36, 36–37, 37, 37, 42, 54, 57–58, 58, 58–59, 59, 59–60, 62, 76–77, 80–81, 81, 81–82, 82, 82–83, 85, 90
serialization.c	37.50%	25%	50%	40%	16–17, 26–27, 33–34, 34, 34–35, 37–38, 41–42, 42, 42–43, 45–46
startup_coordinator.c	0%	0%	0%	0%	100–101, 101, 101, 101, 101–109, 111, 120–123, 123, 123–125, 127, 127, 127–131, 134, 136–139, 147–148, 161–162, 162, 162–164, 169–170, 170, 170–171, 174, 174, 174–176, 179–180, 180, 180–186, 188, 190–192, 195–198, 21, 210, 212, 214, 214, 214, 214, 214, 214–217, 217, 217, 217, 217, 217, 217–219, 221–223, 223, 223–228, 228, 228–229, 229, 229–230, 230, 230, 230, 230, 230, 230, 232, 232, 232, 234, 234, 234–236, 236, 236–238, 24, 240–241, 244, 247–249, 25, 25, 25, 251, 253, 253, 253, 253, 253, 253, 253–262, 262, 262–266, 269, 27, 270–273, 28–29, 29, 29–30, 32, 32, 32, 34, 34, 34–35, 37, 39, 39, 39–40, 45, 45, 45, 47, 50, 54–55, 61–63, 63, 63–65, 69, 71, 71, 71–75, 75, 75, 77, 80–81, 83, 85, 92–93, 93, 93, 93, 93–94, 94, 94
tag.c	40.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

github-actions · 2025-03-04T04:09:35Z

Memory usage after merging this PR will be:

Memory Report

action_empty_test_c

	from	to	increase (%)
text	62727	63001	0.44
data	752	752	0.00
bss	12224	12224	0.00
total	75703	75977	0.36

action_microstep_test_c

	from	to	increase (%)
text	63598	63872	0.43
data	760	760	0.00
bss	12288	12288	0.00
total	76646	76920	0.36

action_overwrite_test_c

	from	to	increase (%)
text	63435	63709	0.43
data	752	752	0.00
bss	12288	12288	0.00
total	76475	76749	0.36

action_test_c

	from	to	increase (%)
text	63339	63613	0.43
data	760	760	0.00
bss	12288	12288	0.00
total	76387	76661	0.36

deadline_test_c

	from	to	increase (%)
text	58855	58900	0.08
data	768	768	0.00
bss	11648	11648	0.00
total	71271	71316	0.06

delayed_conn_test_c

	from	to	increase (%)
text	64113	64364	0.39
data	752	752	0.00
bss	13056	13056	0.00
total	77921	78172	0.32

event_payload_pool_test_c

	from	to
text	18331	18331
data	624	624
bss	320	320
total	19275	19275

event_queue_test_c

	from	to
text	28667	28667
data	728	728
bss	480	480
total	29875	29875

nanopb_test_c

	from	to
text	43346	43346
data	1288	1288
bss	320	320
total	44954	44954

port_test_c

	from	to	increase (%)
text	64029	64280	0.39
data	752	752	0.00
bss	12928	12928	0.00
total	77709	77960	0.32

reaction_queue_test_c

	from	to
text	27897	27897
data	728	728
bss	480	480
total	29105	29105

request_shutdown_test_c

	from	to	increase (%)
text	63570	63844	0.43
data	752	752	0.00
bss	12288	12288	0.00
total	76610	76884	0.36

startup_test_c

	from	to	increase (%)
text	58453	58498	0.08
data	760	760	0.00
bss	11648	11648	0.00
total	70861	70906	0.06

tcp_channel_test_c

	from	to	increase (%)
text	95024	94952	-0.08
data	1640	1640	0.00
bss	14656	14656	0.00
total	111320	111248	-0.06

timer_test_c

	from	to	increase (%)
text	58453	58498	0.08
data	752	752	0.00
bss	11648	11648	0.00
total	70853	70898	0.06

github-actions · 2025-03-04T04:10:05Z

Benchmark results after merging this PR:

Benchmark results

Performance:

PingPongUc:
Best Time: 164.439 msec
Worst Time: 167.521 msec
Median Time: 164.995 msec

PingPongC:
Best Time: 168.193 msec
Worst Time: 177.522 msec
Median Time: 170.290 msec

ReactionLatencyUc:
Best latency: 26315 nsec
Median latency: 60046 nsec
Worst latency: 352342 nsec

ReactionLatencyC:
Best latency: 49962 nsec
Median latency: 59874 nsec
Worst latency: 115285 nsec

Memory usage:

PingPongUc:
text data bss dec hex filename
40154 760 8072 48986 bf5a bin/PingPongUc

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

ReactionLatencyUc:
text data bss dec hex filename
30154 744 2312 33210 81ba bin/ReactionLatencyUc

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

erlingrj

This looks mostly good. I think your solution to the handshake works.

erlingrj · 2025-03-04T09:26:37Z

cmake/lfc.cmake

@@ -61,15 +61,16 @@ function(lf_build_generated_code MAIN_TARGET SOURCE_GEN_DIR)
  endif()

  # Check if the Include.cmake file exists in SOURCE_GEN_DIR
-  if (NOT EXISTS ${SOURCE_GEN_DIR}/Include.cmake)
+  if (NOT EXISTS ${SOURCE_GEN_DIR}/${FEDERATE}/Include.cmake)


Instead of introducing a global variable, the caller of lf_build_generated_code should pass the correct SOURCE_GEN_DIR to this function. If you code-generate a federated program, source-gen dir must include the federate name

erlingrj · 2025-03-04T09:27:23Z

cmake/lfc.cmake

    message(FATAL_ERROR "Include.cmake does not exist in src-gen directory: ${SOURCE_GEN_DIR}/Include.cmake")
  endif()

-  include(${SOURCE_GEN_DIR}/Include.cmake)
+  include(${SOURCE_GEN_DIR}/${FEDERATE}/Include.cmake)
+  message(${REACTOR_UC_PATH})


Again, dont think we should introduce the dependency on this global variable. ALso no need to print the REACTOR_UC_PATH here

erlingrj · 2025-03-04T09:28:18Z

include/reactor-uc/network_channel.h

@@ -150,8 +150,11 @@ struct AsyncNetworkChannel {
 #endif

 #elif defined(PLATFORM_PICO)
-#ifdef NETWORK_CHANNEL_TCP_POSIX
-#error "NETWORK_POSIX_TCP not supported on PICO"
+//#ifdef NETWORK_CHANNEL_TCP_POSIX


Why is this commented out?

erlingrj · 2025-03-04T09:31:37Z

lfc/core/src/main/kotlin/org/lflang/generator/uc/UcCmakeGenerator.kt

@@ -52,7 +52,8 @@ abstract class UcCmakeGenerator(
            |set(LF_MAIN_TARGET ${mainTarget})
            |set(CMAKE_BUILD_TYPE ${targetConfig.getOrDefault(BuildTypeProperty.INSTANCE)})
            |set(PLATFORM POSIX CACHE STRING "Target platform")
-            |include($S{CMAKE_CURRENT_SOURCE_DIR}/Include.cmake)
+            |#include($S{CMAKE_CURRENT_SOURCE_DIR}/src-gen/$S{LF_MAIN}/$S{FEDERATE}/Include.cmake)


We shouldnt code-generated commented out code. And this code change includes the same file but makes it more brittle by depending on a relative path

erlingrj · 2025-03-04T09:32:15Z

src/network_channel.c

-#error "NETWORK_POSIC_TCP not supported on PICO"
-#endif
+//#ifdef NETWORK_CHANNEL_TCP_POSIX
+//#error "NETWORK_POSIC_TCP not supported on PICO"


Why is this removed?

erlingrj · 2025-03-04T09:33:59Z

src/platform/pico/uart_channel.c

+  UartPolledChannel *self = (UartPolledChannel *)untyped_self;
+  UART_CHANNEL_DEBUG("Open connection - Sending Ping");
+  char connect_message[] = UART_OPEN_MESSAGE;
+  uart_write_blocking(self->uart_device, (const uint8_t *)connect_message, sizeof(connect_message));


We shouldnt send the UART_OPEN_MESSAGE if we already are connected?

erlingrj · 2025-03-04T09:35:03Z

src/reaction.c

@@ -34,6 +34,7 @@ int calculate_port_level(Port *port) {
    }
  }

+  printf("Input port %p has level %d", port, current);


Should probably be removed

erlingrj · 2025-03-04T09:35:13Z

src/reaction.c

@@ -25,7 +25,7 @@ int calculate_port_level(Port *port) {
    }
  }

-  for (size_t i = 0; i < port->sources.size; i++) {
+  for (size_t i = 0; i < port->sources.num_registered; i++) {


num_registered and size should be the same

erlingrj · 2025-03-04T09:36:08Z

src/reactor.c

@@ -30,7 +30,6 @@ void Reactor_validate(Reactor *self) {
    if (trigger->type == TRIG_INPUT || trigger->type == TRIG_OUTPUT) {
      Port *port = (Port *)trigger;
      validate(port->effects.num_registered == port->effects.size);
-      validate(port->sources.num_registered == port->sources.size);


Why remove this check? It is a code-gen bug if we say that a port has N sources but then we do not register N

erlingrj · 2025-03-04T09:46:30Z

src/platform/pico/uart_channel.c

+      if (memcmp(connect_message, &self->receive_buffer[self->receive_buffer_index - sizeof(connect_message)],
+                 sizeof(connect_message)) == 0) {
+        self->receive_buffer_index -= sizeof(connect_message);
+        printf("Found Byte Signature of Open Message\n");


Should probably not do any printf here since we are in an ISR?

tanneberger added 3 commits March 3, 2025 19:14

pico uart without encryption_layer

bb74c3a

uart-cmake

322d8e3

fsw25 prep

38f274c

erlingrj requested changes Mar 4, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DRAFT] Bugs Found while building FSW25 Demo #236

[DRAFT] Bugs Found while building FSW25 Demo #236

tanneberger commented Mar 4, 2025

github-actions bot commented Mar 4, 2025

github-actions bot commented Mar 4, 2025

action_empty_test_c

action_microstep_test_c

action_overwrite_test_c

action_test_c

deadline_test_c

delayed_conn_test_c

event_payload_pool_test_c

event_queue_test_c

nanopb_test_c

port_test_c

reaction_queue_test_c

request_shutdown_test_c

startup_test_c

tcp_channel_test_c

timer_test_c

github-actions bot commented Mar 4, 2025

Performance:

Memory usage:

erlingrj left a comment

erlingrj Mar 4, 2025

erlingrj Mar 4, 2025

erlingrj Mar 4, 2025

erlingrj Mar 4, 2025

erlingrj Mar 4, 2025

erlingrj Mar 4, 2025

erlingrj Mar 4, 2025

erlingrj Mar 4, 2025

erlingrj Mar 4, 2025

erlingrj Mar 4, 2025

[DRAFT] Bugs Found while building FSW25 Demo #236

Are you sure you want to change the base?

[DRAFT] Bugs Found while building FSW25 Demo #236

Conversation

tanneberger commented Mar 4, 2025

github-actions bot commented Mar 4, 2025

github-actions bot commented Mar 4, 2025

action_empty_test_c

action_microstep_test_c

action_overwrite_test_c

action_test_c

deadline_test_c

delayed_conn_test_c

event_payload_pool_test_c

event_queue_test_c

nanopb_test_c

port_test_c

reaction_queue_test_c

request_shutdown_test_c

startup_test_c

tcp_channel_test_c

timer_test_c

github-actions bot commented Mar 4, 2025

Performance:

Memory usage:

erlingrj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment