Implement startup behavior for EInit events #298

MandKastner · 2024-12-24T14:53:28Z

Unconnected event pins with the event type EInit of FBs will be triggered automatically during startup

mx990

I am somewhat troubled that we are adding a separate memory allocation to hold the input event connection counters for all FB instances. In case of the similar monitoring counters, users can at least disable monitoring on constrained devices or if it is simply not needed.

I would like to propose several alternative approaches for discussion:

we use a common counter for all EInit events ¹,
we use a mark-and-sweep search when the resource is started, marking all FBs with input connections on EInit events ¹ and then triggering all unmarked FBs (also resetting the mark),
we only allocate the counters when SFBInterfaceSpec::mEITypeNames is not nullptr, which would at least reduce the impact for most FBs ².

In any case, we may want to add a compile-time flag to disable automatic triggering of unconnected EInit events on startup, including any potential overhead incurred thereof.

This would slightly change the semantics if we had multiple EInit events, but maybe we should disallow that anyway? ↩ ↩²
In that case, we should probably also make sure that FB types in the source tree with only EEvent events have mEITypeNames set to nullptr. ↩

mx990 · 2024-12-24T18:32:35Z

src/core/funcbloc.cpp

@@ -50,6 +50,7 @@ bool CFunctionBlock::initialize() {
 #ifdef  FORTE_SUPPORT_MONITORING
  setupEventMonitoringData();
 #endif //FORTE_SUPPORT_MONITORING
+  mInputEventConnectionCount = std::make_unique<size_t[]>(getFBInterfaceSpec().mNumEIs);


I believe you need to re-create the array when there interface is reconfigured, see callers of setupEventMonitoringData().

I am not sure if i understand what you mean. The only caller of setupMonitoringData() is initialize(). In my understanding the existing unique_ptr will give up its ownership and take the new one with the second call of initialize() . With the release the managed memory should be freed and the new array created.

It is also called in

4diac-forte/src/core/genfb.tpp

Line 221 in 1511891

T::setupEventMonitoringData();

after setting up the interface of a generic FB.

But with @MandKastner current implementation using a std::unique_ptr this does not really matter. Or do I miss something?

Ah I missed the point. Yes we need to put a call there as well.

src/core/funcbloc.h

mx990 · 2024-12-24T18:40:57Z

src/core/funcbloc.cpp

@@ -432,6 +433,7 @@ EMGMResponse CFunctionBlock::changeExecutionState(EMGMCommandType paCommand){
      if((E_FBStates::Idle == mFBState) || (E_FBStates::Stopped == mFBState)){
        mFBState = E_FBStates::Running;
        nRetVal = EMGMResponse::Ready;
+        triggerEInitEvents();


It seems this will also trigger EInit events after a reconfiguration or warm restart? Is this the desired behavior?

MandKastner · 2025-01-09T11:40:07Z

@mx990 Thank you for your suggestions. I would prefer to keep the infrastructure to check if a event port is connected and go with your 3rd suggestion to reduce allocated memory. As you also suggested i moved the trigger mechanism to a SIFB and perform the recursive search there, this will allow users to decide about the triggering process.

franz-hoepfinger-4diac · 2025-01-17T11:02:07Z

I will test this now.
eagerly waiting for this.

franz-hoepfinger-4diac · 2025-01-17T13:56:10Z

so, First Test: Failed:
it seems none of the EInit Pins are triggered:

franz-hoepfinger-4diac · 2025-01-17T13:56:59Z

ahhhhhhhhhhh i see... there is a new FB: E_TRIG!

franz-hoepfinger-4diac · 2025-01-17T14:00:07Z

Working now ... Cool.

franz-hoepfinger-4diac · 2025-01-17T16:21:12Z

on bigger PRojects i get a lot of
ERROR: T#25000000000: External event queue is full, external event dropped!
Messages.

azoitl · 2025-01-17T16:27:18Z

src/core/funcbloc.cpp

@@ -671,6 +674,18 @@ size_t CFunctionBlock::getToStringBufferSize() const {

 }

+void CFunctionBlock::triggerEventsOfType(TEventTypeID paEventTypeId) {


please move to E_TRIG FB

azoitl · 2025-01-17T16:28:23Z

src/core/funcbloc.cpp

+  if (getFBInterfaceSpec().mEITypeNames != nullptr) {
+    for (TEventID eventId = 0; eventId < getFBInterfaceSpec().mNumEIs; eventId++) {
+      if (getEIType(eventId) == paEventTypeId && !isInputEventConnected(eventId)) {
+        getResource()->getResourceEventExecution()->startEventChain(CConnectionPoint(this, eventId));


use the eventchain execution thread which was used to triggere the R_TRIG fb and add the events to the internal event list.

azoitl · 2025-01-17T16:29:18Z

src/stdfblib/events/E_TRIG_fbt.cpp

+  return nullptr;
+}
+
+void FORTE_E_TRIG::triggerEvents(forte::core::CFBContainer* paContainer, TEventTypeID paEventType) {


paContainer is not really needed you could call the getResoruce here.

azoitl · 2025-01-17T16:30:37Z

src/stdfblib/events/E_TRIG_fbt.cpp

+    case scmEventREQID:
+      const TEventTypeID eventTypeId = CStringDictionary::getInstance().getId(var_EVENTTYPE.c_str());
+      if (eventTypeId != CStringDictionary::scmInvalidStringId) {
+        triggerEvents(getResource(), eventTypeId);


use the paECET for triggereEvents so that you do not need to use the external events everywhere.

azoitl · 2025-01-17T16:32:09Z

on bigger PRojects i get a lot of ERROR: T#25000000000: External event queue is full, external event dropped! Messages.

I would expect that, we need more memory to handle that. But with my comments from above you only need to make the internal event queue larger

franz-hoepfinger-4diac · 2025-01-17T19:18:01Z

on bigger PRojects i get a lot of ERROR: T#25000000000: External event queue is full, external event dropped! Messages.

I would expect that, we need more memory to handle that. But with my comments from above you only need to make the internal event queue larger

Understand.
yes, waiting for this Improvement.
but still a big step forward.

MandKastner added 2 commits December 18, 2024 13:48

fix typo

0f3f3e8

initial implementation of startup behavior for EInit events

4a2d9ea

mx990 self-requested a review December 24, 2024 18:31

mx990 suggested changes Dec 24, 2024

View reviewed changes

MandKastner added 3 commits January 8, 2025 11:06

only allocate memory if FB contains EInit event

888134e

add event type trigger FB

d057a4a

remove unused typedef

001d007

azoitl requested changes Jan 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement startup behavior for EInit events #298

Implement startup behavior for EInit events #298

MandKastner commented Dec 24, 2024

mx990 left a comment

mx990 Dec 24, 2024

MandKastner Jan 9, 2025

mx990 Jan 9, 2025

azoitl Jan 9, 2025

azoitl Jan 9, 2025

mx990 Dec 24, 2024

MandKastner commented Jan 9, 2025

franz-hoepfinger-4diac commented Jan 17, 2025

franz-hoepfinger-4diac commented Jan 17, 2025

franz-hoepfinger-4diac commented Jan 17, 2025

franz-hoepfinger-4diac commented Jan 17, 2025 •

edited

Loading

franz-hoepfinger-4diac commented Jan 17, 2025

azoitl Jan 17, 2025

azoitl Jan 17, 2025

azoitl Jan 17, 2025

azoitl Jan 17, 2025

azoitl commented Jan 17, 2025

franz-hoepfinger-4diac commented Jan 17, 2025

		@@ -671,6 +674,18 @@ size_t CFunctionBlock::getToStringBufferSize() const {

		}

		void CFunctionBlock::triggerEventsOfType(TEventTypeID paEventTypeId) {

Implement startup behavior for EInit events #298

Are you sure you want to change the base?

Implement startup behavior for EInit events #298

Conversation

MandKastner commented Dec 24, 2024

mx990 left a comment

Choose a reason for hiding this comment

Footnotes

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

MandKastner commented Jan 9, 2025

franz-hoepfinger-4diac commented Jan 17, 2025

franz-hoepfinger-4diac commented Jan 17, 2025

franz-hoepfinger-4diac commented Jan 17, 2025

franz-hoepfinger-4diac commented Jan 17, 2025 • edited Loading

franz-hoepfinger-4diac commented Jan 17, 2025

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

azoitl commented Jan 17, 2025

franz-hoepfinger-4diac commented Jan 17, 2025

franz-hoepfinger-4diac commented Jan 17, 2025 •

edited

Loading