Actions without Types (#107)

* making action variadic

* fix action

* fixing up all the remaining tests

* fix test action microstep

* clean up

* more generic bounds checks for actions

* fixing overwrite test

Signed-off-by: tanneberger <github@tanneberger.me>

* formatting

* change name

* fix physical actions

* fix up tests

* fix

* add comments

* fix zephyre example

* Zephyr fixes + cannot return from a do-while

* More descriptive exeception

* integrating feedback from erling

* fix macro formatting

---------

Signed-off-by: tanneberger <github@tanneberger.me>
Co-authored-by: erlingrj <erling.jellum@gmail.com>

examples/zephyr/basic_federated/federated_sender/src/sender.c

@@ -17,16 +17,16 @@ static const struct gpio_dt_spec button = GPIO_DT_SPEC_GET_OR(SW0_NODE, gpios, {
 static struct gpio_callback button_cb_data;
 static const struct gpio_dt_spec led = GPIO_DT_SPEC_GET(LED0_NODE, gpios);
 
-DEFINE_ACTION_STRUCT(Action1, PHYSICAL_ACTION, 1, 0, bool, 2)
-DEFINE_ACTION_CTOR_FIXED(Action1, PHYSICAL_ACTION, 1, 0, bool, 2, MSEC(0))
+DEFINE_ACTION_STRUCT(Action1, PHYSICAL_ACTION, 1, 0, 2, bool)
+DEFINE_ACTION_CTOR(Action1, PHYSICAL_ACTION, MSEC(0), 1, 0, 2, bool)
 DEFINE_REACTION_STRUCT(Sender, 0, 1)
 DEFINE_OUTPUT_PORT_STRUCT(Out, 1, 2)
 DEFINE_OUTPUT_PORT_CTOR(Out, 1)
 Action1 *action_ptr = NULL;
 
 void button_pressed(const struct device *dev, struct gpio_callback *cb, uint32_t pins) {
   printk("Button pressed!\n");
-  lf_schedule(action_ptr, true, 0);
+  lf_schedule(action_ptr, 0, true);
 }
 
 void setup_button() {

include/reactor-uc/action.h

@@ -19,6 +19,8 @@ struct Action {
   TriggerEffects effects;        // The reactions triggered by this Action.
   TriggerSources sources;        // The reactions that can write to this Action.
   EventPayloadPool payload_pool; // Pool of memory for the data associated with the events scheduled on this action.
+  size_t max_pending_events;
+  size_t events_scheduled;
   /**
    * @brief  Schedule an event on this action.
    */
@@ -27,6 +29,6 @@ struct Action {
 
 void Action_ctor(Action *self, ActionType type, interval_t min_offset, Reactor *parent, Reaction **sources,
                  size_t sources_size, Reaction **effects, size_t effects_size, void *value_ptr, size_t value_size,
-                 void *payload_buf, bool *payload_used_buf, size_t payload_buf_capacity);
+                 void *payload_buf, bool *payload_used_buf, size_t event_bound);
 
 #endif

include/reactor-uc/error.h

@@ -12,6 +12,7 @@
 typedef enum {
   LF_OK = 0,
   LF_ERR,
+  LF_FATAL,
   LF_SLEEP_INTERRUPTED,
   LF_INVALID_TAG,
   LF_AFTER_STOP_TAG,

include/reactor-uc/macros.h

@@ -28,12 +28,29 @@
  * @brief Schedule an event on an action
  *
  */
-#define lf_schedule(action, val, offset)                                                                               \
+
+#define lf_schedule_with_val(action, offset, val)                                                                      \
   do {                                                                                                                 \
     __typeof__(val) __val = (val);                                                                                     \
-    (action)->super.schedule(&(action)->super, (offset), (const void *)&__val);                                        \
+    lf_ret_t ret = (action)->super.schedule(&(action)->super, (offset), (const void *)&__val);                         \
+    if (ret == LF_FATAL) {                                                                                             \
+      LF_ERR(TRIG, "Scheduling an value, that doesn't have value!");                                                   \
+      Scheduler *sched = &(action)->super.super.parent->env->scheduler;                                                \
+      sched->do_shutdown(sched, sched->current_tag);                                                                   \
+      throw("Tried to schedule a value onto an action without a type!");                                               \
+    }                                                                                                                  \
+  } while (0)
+
+#define lf_schedule_without_val(action, offset)                                                                        \
+  do {                                                                                                                 \
+    (action)->super.schedule(&(action)->super, (offset), NULL);                                                        \
   } while (0)
 
+#define GET_ARG4(arg1, arg2, arg3, arg4, ...) arg4
+#define LF_SCHEDULE_CHOOSER(...) GET_ARG4(__VA_ARGS__, lf_schedule_with_val, lf_schedule_without_val)
+
+#define lf_schedule(...) LF_SCHEDULE_CHOOSER(__VA_ARGS__)(__VA_ARGS__)
+
 /**
  * @brief Convenience macro for registering a reaction as an effect of a trigger.
  * The input must be a pointer to a derived Trigger type with an effects field.
@@ -178,28 +195,54 @@
                         sizeof(self->effects) / sizeof(self->effects[0]));                                             \
   }
 
-#define DEFINE_ACTION_STRUCT(ActionName, ActionType, EffectSize, SourceSize, BufferType, BufferSize)                   \
+#define DEFINE_ACTION_STRUCT_WITHOUT_VALUE(ActionName, ActionType, EffectSize, SourceSize, MaxPendingEvents)           \
+  typedef struct {                                                                                                     \
+    Action super;                                                                                                      \
+    Reaction *sources[(SourceSize)];                                                                                   \
+    Reaction *effects[(EffectSize)];                                                                                   \
+  } ActionName;
+
+#define DEFINE_ACTION_STRUCT_WITH_VALUE(ActionName, ActionType, EffectSize, SourceSize, MaxPendingEvents, BufferType)  \
   typedef struct {                                                                                                     \
     Action super;                                                                                                      \
     BufferType value;                                                                                                  \
-    BufferType payload_buf[(BufferSize)];                                                                              \
-    bool payload_used_buf[(BufferSize)];                                                                               \
+    BufferType payload_buf[(MaxPendingEvents)];                                                                        \
+    bool payload_used_buf[(MaxPendingEvents)];                                                                         \
     Reaction *sources[(SourceSize)];                                                                                   \
     Reaction *effects[(EffectSize)];                                                                                   \
   } ActionName;
 
-#define DEFINE_ACTION_CTOR_FIXED(ActionName, ActionType, EffectSize, SourceSize, BufferType, BufferSize, MinDelay)     \
+#define GET_ARG8(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, ...) arg8
+#define GET_ARG7(arg1, arg2, arg3, arg4, arg5, arg6, arg7, ...) arg7
+
+// based on if the user gives 6 or 7 parameters either DEFINE_ACTION_STRUCT_WITH_VALUE or
+// DEFINE_ACTION_STRUCT_WITHOUT_VALUE is returned.
+#define DEFINE_ACTION_STRUCT_CHOOSER(...)                                                                              \
+  GET_ARG7(__VA_ARGS__, DEFINE_ACTION_STRUCT_WITH_VALUE, DEFINE_ACTION_STRUCT_WITHOUT_VALUE)
+
+// this enum first figures out if we need to use the constructor for an action with or
+// without an value an then invokes the corresponding macro
+#define DEFINE_ACTION_STRUCT(...) DEFINE_ACTION_STRUCT_CHOOSER(__VA_ARGS__)(__VA_ARGS__)
+
+#define DEFINE_ACTION_CTOR_WITH_VALUE(ActionName, ActionType, MinDelay, EffectSize, SourceSize, MaxPendingEvents,      \
+                                      BufferType)                                                                      \
   void ActionName##_ctor(ActionName *self, Reactor *parent) {                                                          \
-    Action_ctor(&self->super, ActionType, MinDelay, parent, self->sources, SourceSize, self->effects, EffectSize,      \
-                &self->value, sizeof(BufferType), (void *)&self->payload_buf, self->payload_used_buf, BufferSize);     \
+    Action_ctor(&self->super, ActionType, MinDelay, parent, self->sources, (SourceSize), self->effects, (EffectSize),  \
+                &self->value, sizeof(BufferType), (void *)&self->payload_buf, self->payload_used_buf,                  \
+                (MaxPendingEvents));                                                                                   \
   }
 
-#define DEFINE_ACTION_CTOR(ActionName, ActionType, EffectSize, SourceSize, BufferType, BufferSize)                     \
-  void ActionName##_ctor(ActionName *self, Reactor *parent, interval_t min_delay) {                                    \
-    Action_ctor(&self->super, ActionType, min_delay, parent, self->sources, SourceSize, self->effects, EffectSize,     \
-                &self->value, sizeof(BufferType), (void *)&self->payload_buf, self->payload_used_buf, BufferSize);     \
+#define DEFINE_ACTION_CTOR_WITHOUT_VALUE(ActionName, ActionType, MinDelay, EffectSize, SourceSize, MaxPendingEvents)   \
+  void ActionName##_ctor(ActionName *self, Reactor *parent) {                                                          \
+    Action_ctor(&self->super, ActionType, (MinDelay), parent, self->sources, (SourceSize), self->effects,              \
+                (EffectSize), NULL, 0, NULL, NULL, (MaxPendingEvents));                                                \
   }
 
+#define DEFINE_ACTION_CTOR_CHOOSER(...)                                                                                \
+  GET_ARG8(__VA_ARGS__, DEFINE_ACTION_CTOR_WITH_VALUE, DEFINE_ACTION_CTOR_WITHOUT_VALUE)
+
+#define DEFINE_ACTION_CTOR(...) DEFINE_ACTION_CTOR_CHOOSER(__VA_ARGS__)(__VA_ARGS__)
+
 #define DEFINE_LOGICAL_CONNECTION_STRUCT(ConnectionName, DownstreamSize)                                               \
   typedef struct {                                                                                                     \
     LogicalConnection super;                                                                                           \

src/action.c

@@ -25,6 +25,7 @@ void Action_prepare(Trigger *self, Event *event) {
     }
   }
 
+  act->events_scheduled--;
   self->is_present = true;
   self->payload_pool->free(self->payload_pool, event->payload);
 }
@@ -34,24 +35,37 @@ lf_ret_t Action_schedule(Action *self, interval_t offset, const void *value) {
   Environment *env = self->super.parent->env;
   Scheduler *sched = &env->scheduler;
   void *payload = NULL;
-  validate(value);
 
   env->enter_critical_section(env);
 
   // Dont accept events before we have started
   // TODO: Do we instead need some flag to signal that we have started?
   if (sched->start_time == NEVER) {
-    LF_ERR(TRIG, "Action %p cannot schedule events before start tag", self);
     env->leave_critical_section(env);
+    LF_ERR(TRIG, "Action %p cannot schedule events before start tag", self);
     return LF_ERR;
   }
 
-  ret = self->super.payload_pool->allocate(self->super.payload_pool, &payload);
-  if (ret != LF_OK) {
-    return ret;
+  if (self->super.payload_pool->capacity == 0 && value != NULL) {
+    // user tried to schedule a action that does not have any value
+    env->leave_critical_section(env);
+
+    return LF_FATAL;
   }
 
-  memcpy(payload, value, self->payload_pool.size);
+  if (self->events_scheduled >= self->max_pending_events) {
+    LF_ERR(TRIG, "Scheduled action to often bound: %i", self->max_pending_events);
+    return LF_ERR;
+  }
+
+  if (value != NULL) {
+    ret = self->super.payload_pool->allocate(self->super.payload_pool, &payload);
+    if (ret != LF_OK) {
+      return ret;
+    }
+
+    memcpy(payload, value, self->payload_pool.size);
+  }
 
   tag_t base_tag = ZERO_TAG;
   interval_t total_offset = lf_time_add(self->min_offset, offset);
@@ -66,6 +80,7 @@ lf_ret_t Action_schedule(Action *self, interval_t offset, const void *value) {
   Event event = EVENT_INIT(tag, (Trigger *)self, payload);
 
   ret = sched->schedule_at_locked(sched, &event);
+  self->events_scheduled++;
 
   if (self->type == PHYSICAL_ACTION) {
     env->platform->new_async_event(env->platform);
@@ -78,12 +93,18 @@ lf_ret_t Action_schedule(Action *self, interval_t offset, const void *value) {
 
 void Action_ctor(Action *self, ActionType type, interval_t min_offset, Reactor *parent, Reaction **sources,
                  size_t sources_size, Reaction **effects, size_t effects_size, void *value_ptr, size_t value_size,
-                 void *payload_buf, bool *payload_used_buf, size_t payload_buf_capacity) {
-  EventPayloadPool_ctor(&self->payload_pool, payload_buf, payload_used_buf, value_size, payload_buf_capacity);
+                 void *payload_buf, bool *payload_used_buf, size_t event_bound) {
+  int capacity = 0;
+  if (payload_buf != NULL) {
+    capacity = event_bound;
+  }
+  EventPayloadPool_ctor(&self->payload_pool, payload_buf, payload_used_buf, value_size, capacity);
   Trigger_ctor(&self->super, TRIG_ACTION, parent, &self->payload_pool, Action_prepare, Action_cleanup);
   self->type = type;
   self->value_ptr = value_ptr;
   self->min_offset = min_offset;
+  self->max_pending_events = event_bound;
+  self->events_scheduled = 0;
   self->schedule = Action_schedule;
   self->sources.reactions = sources;
   self->sources.num_registered = 0;

src/event.c

@@ -11,6 +11,10 @@ lf_ret_t EventPayloadPool_free(EventPayloadPool *self, void *payload) {
 }
 
 lf_ret_t EventPayloadPool_allocate(EventPayloadPool *self, void **payload) {
+  if (self->capacity == 0) {
+    return LF_NO_MEM;
+  }
+
   for (size_t i = 0; i < self->capacity; i++) {
     if (!self->used[i]) {
       self->used[i] = true;
@@ -26,9 +30,13 @@ void EventPayloadPool_ctor(EventPayloadPool *self, char *buffer, bool *used, siz
   self->used = used;
   self->capacity = capacity;
   self->size = size;
-  for (size_t i = 0; i < capacity; i++) {
-    self->used[i] = false;
+
+  if (self->used != NULL) {
+    for (size_t i = 0; i < capacity; i++) {
+      self->used[i] = false;
+    }
   }
+
   self->allocate = EventPayloadPool_allocate;
   self->free = EventPayloadPool_free;
 }

src/scheduler.c

@@ -132,7 +132,6 @@ void Scheduler_do_shutdown(Scheduler *self, tag_t shutdown_tag) {
   LF_INFO(SCHED, "Scheduler terminating at tag %" PRId64 ":%" PRIu32, shutdown_tag.time, shutdown_tag.microstep);
   Environment *env = self->env;
   self->prepare_timestep(self, shutdown_tag);
-
   env->leave_critical_section(env);
 
   Trigger *shutdown = &self->env->shutdown->super;

test/lf/src/Action.lf

@@ -7,7 +7,7 @@ main reactor {
 
   reaction(startup) -> a {=
     for (int i = 0; i<10; i++) {
-      lf_schedule(a, self->cnt, MSEC(self->cnt++));
+      lf_schedule(a, MSEC(self->cnt++), self->cnt);
     }
   =}
 

test/unit/action_microstep_test.c

@@ -1,8 +1,8 @@
 #include "reactor-uc/reactor-uc.h"
 #include "unity.h"
 
-DEFINE_ACTION_STRUCT(MyAction, LOGICAL_ACTION, 1, 1, int, 1);
-DEFINE_ACTION_CTOR_FIXED(MyAction, LOGICAL_ACTION, 1, 1, int, 1, MSEC(0));
+DEFINE_ACTION_STRUCT(MyAction, LOGICAL_ACTION, 1, 1, 1, int);
+DEFINE_ACTION_CTOR(MyAction, LOGICAL_ACTION, MSEC(0), 1, 1, 1, int);
 DEFINE_STARTUP_STRUCT(MyStartup, 1);
 DEFINE_STARTUP_CTOR(MyStartup, 1);
 DEFINE_REACTION_STRUCT(MyReactor, 0, 1);
@@ -41,7 +41,7 @@ DEFINE_REACTION_BODY(MyReactor, 0) {
   TEST_ASSERT_EQUAL(0, env->get_elapsed_logical_time(env));
 
   if (self->cnt < 100) {
-    lf_schedule(my_action, ++self->cnt, 0);
+    lf_schedule(my_action, 0, ++self->cnt);
   }
 }
 

test/unit/action_overwrite_test.c

@@ -1,8 +1,8 @@
 #include "reactor-uc/reactor-uc.h"
 #include "unity.h"
 
-DEFINE_ACTION_STRUCT(MyAction, LOGICAL_ACTION, 1, 1, int, 2);
-DEFINE_ACTION_CTOR_FIXED(MyAction, LOGICAL_ACTION, 1, 1, int, 2, MSEC(0));
+DEFINE_ACTION_STRUCT(MyAction, LOGICAL_ACTION, 1, 1, 2, int);
+DEFINE_ACTION_CTOR(MyAction, LOGICAL_ACTION, MSEC(0), 1, 1, 2, int);
 DEFINE_STARTUP_STRUCT(MyStartup, 1);
 DEFINE_STARTUP_CTOR(MyStartup, 1)
 DEFINE_REACTION_STRUCT(MyReactor, 0, 1);
@@ -22,8 +22,8 @@ DEFINE_REACTION_BODY(MyReactor, 0) {
   MyAction *my_action = &self->my_action;
   if (self->cnt == 0) {
     TEST_ASSERT_EQUAL(lf_is_present(my_action), false);
-    lf_schedule(my_action, 41, MSEC(1));
-    lf_schedule(my_action, 42, MSEC(1));
+    lf_schedule(my_action, MSEC(1), 41);
+    lf_schedule(my_action, MSEC(1), 42);
   } else {
     TEST_ASSERT_EQUAL(1, self->cnt);
     TEST_ASSERT_EQUAL(lf_is_present(my_action), true);

test/unit/action_test.c

@@ -1,8 +1,8 @@
 #include "reactor-uc/reactor-uc.h"
 #include "unity.h"
 
-DEFINE_ACTION_STRUCT(MyAction, LOGICAL_ACTION, 1, 1, int, 2);
-DEFINE_ACTION_CTOR_FIXED(MyAction, LOGICAL_ACTION, 1, 1, int, 2, MSEC(0));
+DEFINE_ACTION_STRUCT(MyAction, LOGICAL_ACTION, 1, 1, 2, int);
+DEFINE_ACTION_CTOR(MyAction, LOGICAL_ACTION, MSEC(0), 1, 1, 2, int);
 DEFINE_STARTUP_STRUCT(MyStartup, 1);
 DEFINE_STARTUP_CTOR(MyStartup, 1)
 DEFINE_REACTION_STRUCT(MyReactor, 0, 1);
@@ -32,7 +32,7 @@ DEFINE_REACTION_BODY(MyReactor, 0) {
     TEST_ASSERT_EQUAL(self->cnt, my_action->value);
   }
 
-  lf_schedule(my_action, ++self->cnt, MSEC(1));
+  lf_schedule(my_action, MSEC(1), ++self->cnt);
 }
 
 DEFINE_REACTION_CTOR(MyReactor, 0);

test/unit/empty_action_test.c

@@ -0,0 +1,63 @@
+#include "reactor-uc/reactor-uc.h"
+#include "unity.h"
+
+DEFINE_ACTION_STRUCT(MyAction, LOGICAL_ACTION, 1, 1, 6);
+DEFINE_ACTION_CTOR(MyAction, PHYSICAL_ACTION, MSEC(0), 1, 1, 6);
+DEFINE_STARTUP_STRUCT(MyStartup, 1);
+DEFINE_STARTUP_CTOR(MyStartup, 1)
+DEFINE_REACTION_STRUCT(MyReactor, 0, 1);
+
+typedef struct {
+  Reactor super;
+  MyReactor_Reaction0 my_reaction;
+  MyAction my_action;
+  MyStartup startup;
+  Reaction *_reactions[1];
+  Trigger *_triggers[2];
+  int cnt;
+} MyReactor;
+
+DEFINE_REACTION_BODY(MyReactor, 0) {
+  MyReactor *self = (MyReactor *)_self->parent;
+  MyAction *my_action = &self->my_action;
+
+  printf("Hello World\n");
+  printf("Action = %d\n");
+
+  lf_schedule(my_action, MSEC(1));
+}
+
+DEFINE_REACTION_CTOR(MyReactor, 0);
+
+void MyReactor_ctor(MyReactor *self, Environment *env) {
+  self->_reactions[0] = (Reaction *)&self->my_reaction;
+  self->_triggers[0] = (Trigger *)&self->startup;
+  self->_triggers[1] = (Trigger *)&self->my_action;
+  Reactor_ctor(&self->super, "MyReactor", env, NULL, NULL, 0, self->_reactions, 1, self->_triggers, 2);
+  MyAction_ctor(&self->my_action, &self->super);
+  MyReactor_Reaction0_ctor(&self->my_reaction, &self->super);
+  MyStartup_ctor(&self->startup, &self->super);
+  ACTION_REGISTER_EFFECT(self->my_action, self->my_reaction);
+  REACTION_REGISTER_EFFECT(self->my_reaction, self->my_action);
+  ACTION_REGISTER_SOURCE(self->my_action, self->my_reaction);
+  BUILTIN_REGISTER_EFFECT(self->startup, self->my_reaction);
+
+  self->cnt = 0;
+}
+
+void test_simple() {
+  MyReactor my_reactor;
+  Environment env;
+  Environment_ctor(&env, (Reactor *)&my_reactor);
+  MyReactor_ctor(&my_reactor, &env);
+  env.scheduler.duration = MSEC(600);
+  env.assemble(&env);
+  env.start(&env);
+  Environment_free(&env);
+}
+
+int main() {
+  UNITY_BEGIN();
+  RUN_TEST(test_simple);
+  return UNITY_END();
+}