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.

Sign up for GitHub

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

[DRAFT] Bugs Found while building FSW25 Demo #236

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[DRAFT] Bugs Found while building FSW25 Demo #236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bb74c3a
pico uart without encryption_layer
tanneberger Feb 21, 2025
322d8e3
uart-cmake
tanneberger Feb 22, 2025
38f274c
fsw25 prep
tanneberger Mar 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion benchmarks/src/PingPongUc.lf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,4 @@ main reactor
ping.finished -> pong.finish;
ping.send -> pong.receive;
pong.send -> ping.receive;
}
}
7 changes: 4 additions & 3 deletions cmake/lfc.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

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})
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

add_subdirectory(${REACTOR_UC_PATH})
target_sources(${MAIN_TARGET} PRIVATE ${LFC_GEN_MAIN} ${LFC_GEN_SOURCES})
target_include_directories(${MAIN_TARGET} PRIVATE ${LFC_GEN_INCLUDE_DIRS})
target_link_libraries(${MAIN_TARGET} PUBLIC reactor-uc)
target_compile_definitions(reactor-uc PUBLIC LF_LOG_LEVEL_ALL=${LOG_LEVEL})
target_compile_definitions(reactor-uc PUBLIC ${LFC_GEN_COMPILE_DEFS})
endfunction()
endfunction()
7 changes: 5 additions & 2 deletions include/reactor-uc/macros_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,17 @@
*
*/
#define LF_TRIGGER_REGISTER_EFFECT(trigger, effect) \
do { \
do { \
assert((effect) != NULL); \
assert((trigger)->effects.num_registered < (trigger)->effects.size); \
(trigger)->effects.reactions[(trigger)->effects.num_registered++] = (effect); \
} while (0)

// Register a reaction as a source of a trigger. `trigger` must be a pointer to
// a derived Trigger type.
#define LF_TRIGGER_REGISTER_SOURCE(trigger, source) \
do { \
do { \
assert((source) != NULL); \
assert((trigger)->sources.num_registered < (trigger)->sources.size); \
(trigger)->sources.reactions[(trigger)->sources.num_registered++] = (source); \
} while (0)
Expand All @@ -27,6 +29,7 @@
// a derived Trigger type.
#define LF_TRIGGER_REGISTER_OBSERVER(trigger, observer) \
do { \
assert((observer) != NULL); \
assert((trigger)->observers.num_registered < (trigger)->observers.size); \
(trigger)->observers.reactions[(trigger)->observers.num_registered++] = (observer); \
} while (0)
Expand Down
9 changes: 6 additions & 3 deletions include/reactor-uc/network_channel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ struct PolledNetworkChannel {
/**
* @brief Polls for new data and calls the callback handler if a message is successfully decoded
*/
void (*poll)(NetworkChannel *self);
void (*poll)(PolledNetworkChannel *self);
};

struct AsyncNetworkChannel {
Expand Down Expand Up @@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this commented out?

//#error "NETWORK_POSIX_TCP not supported on PICO"
//#endif
#ifdef NETWORK_CHANNEL_UART
#include "platform/pico/uart_channel.h"
#endif

#elif defined(PLATFORM_FLEXPRET)
Expand Down
34 changes: 34 additions & 0 deletions include/reactor-uc/platform/pico/uart_channel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#ifndef REACTOR_UC_PICO_UART_CHANNEL_H
#define REACTOR_UC_PICO_UART_CHANNEL_H

#include "reactor-uc/network_channel/uart_channel.h"
#include "reactor-uc/network_channel.h"
#include "reactor-uc/environment.h"

#include "pico/stdlib.h"
#include "hardware/uart.h"
#include "hardware/irq.h"

typedef struct FederatedConnectionBundle FederatedConnectionBundle;
typedef struct UartPolledChannel UartPolledChannel;
typedef struct UartAsyncChannel UartAsyncChannel;

#define UART_CHANNEL_BUFFERSIZE 1024

struct UartPolledChannel {
PolledNetworkChannel super;
NetworkChannelState state;
FederateMessage output;
unsigned char receive_buffer[UART_CHANNEL_BUFFERSIZE];
unsigned char send_buffer[UART_CHANNEL_BUFFERSIZE];
unsigned int receive_buffer_index;
uart_inst_t *uart_device;

FederatedConnectionBundle *bundle;
void (*receive_callback)(FederatedConnectionBundle *bundle, const FederateMessage *message);
};

void UartPolledChannel_ctor(UartPolledChannel *self, uint32_t uart_device, uint32_t baud, UartDataBits data_bits,
UartParityBits parity, UartStopBits stop_bits);

#endif
3 changes: 2 additions & 1 deletion lfc/core/src/main/kotlin/org/lflang/generator/uc/UcCmakeGenerator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

|include(./Include.cmake)
|add_executable($S{LF_MAIN_TARGET} $S{LFC_GEN_SOURCES} $S{LFC_GEN_MAIN})
|install(TARGETS $S{LF_MAIN_TARGET}
| RUNTIME DESTINATION $S{CMAKE_INSTALL_BINDIR}
Expand Down
4 changes: 2 additions & 2 deletions lfc/core/src/main/kotlin/org/lflang/generator/uc/UcNetworkChannel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ abstract class UcNetworkChannel(

UART -> {
val srcEp = (srcIf as UcUARTInterface).createEndpoint()
val destEp = (srcIf as UcUARTInterface).createEndpoint()
val destEp = (destIf as UcUARTInterface).createEndpoint()
channel = UcUARTChannel(srcEp, destEp)
}

Expand Down Expand Up @@ -327,7 +327,7 @@ class UcUARTChannel(private val uart_src: UcUARTEndpoint, private val uart_dest:
"Uart${if (uart_src.async) "Async" else "Polled"}Channel_ctor(&self->channel, ${uart_src.uart_device}, ${uart_src.baud_rate}, UC_${uart_src.data_bits}, UC_${uart_src.parity}, UC_${uart_src.stop_bits});"

override fun generateChannelCtorDest() =
"Uart${if (uart_src.async) "Async" else "Polled"}Channel_ctor(&self->channel, ${uart_dest.uart_device}, ${uart_dest.baud_rate}, UC_${uart_dest.data_bits}, UC_${uart_dest.parity}, UC_${uart_dest.stop_bits});"
"Uart${if (uart_dest.async) "Async" else "Polled"}Channel_ctor(&self->channel, ${uart_dest.uart_device}, ${uart_dest.baud_rate}, UC_${uart_dest.data_bits}, UC_${uart_dest.parity}, UC_${uart_dest.stop_bits});"

override val codeType: String
get() =
Expand Down
Empty file added runAll.sh
View file
Open in desktop
Empty file.
9 changes: 6 additions & 3 deletions src/network_channel.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@
#endif

#elif defined(PLATFORM_PICO)
#ifdef NETWORK_CHANNEL_TCP_POSIX
#error "NETWORK_POSIC_TCP not supported on PICO"
#endif
//#ifdef NETWORK_CHANNEL_TCP_POSIX
//#error "NETWORK_POSIC_TCP not supported on PICO"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this removed?

//#endif
// #ifdef NETWORK_CHANNEL_UART
#include "platform/pico/uart_channel.c"
// #endif

#elif defined(PLATFORM_FLEXPRET)
#ifdef NETWORK_CHANNEL_TCP_POSIX
Expand Down
2 changes: 1 addition & 1 deletion src/platform/pico/pico.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ void Platform_vprintf(const char *fmt, va_list args) { vprintf(fmt, args); }

lf_ret_t PlatformPico_initialize(Platform *self) {
PlatformPico *p = (PlatformPico *)self;
stdio_init_all();
//stdio_init_all();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why no stdio?

// init sync structs
critical_section_init(&p->crit_sec);
sem_init(&p->sem, 0, 1);
Expand Down
Loading
Loading