Improved Trigger API #35
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 10 commits
September 18, 2016 15:07
This should clear some terminology confusion, as event are not required to use the engine (e.g. when using constant time triggers).
Using the 'event' terminology for both triggers and events was very confusing, as events are not required for triggers (i.e. constant time triggers). This will make it a lot easier to mentally differentiate the two concepts.
These three triggers have exactly the same functionality.
The 'TIME OF an_event' (the primary time) is the clinically relevant time, e.g. the time when a sample was taken. The 'EVENTTIME' is the time when the relevant data, e.g. the samples measured values, are stored in the database. This allows using the 'EVENTTIME', to get the matching database entry, while 'TIME OF an_event' can be used for calculation. Therefore the primary time and eventtime are stored separately.
… callEvent() This seperates the triggers (expressions in the evoke slot) and the events (variable). As a result the the ArdenValue superclass is no longer required for Triggers, which removes unused primary time handling for triggers. Previously it was possible to return FixedDateTriggers or more complex triggers. Pros: - It is possible to change evoke conditions for MLMs at runtime, without changing the code of the MLM itself. Cons: - It mixes the concept of triggers and events. - It allows building invalid statements. E.g. for the statement '5 SECONDS AFTER TIME OF an_event', if an_event is a CyclicTrigger, then the behavior is not defined. - This approach invites to not use evoke statements, which were designed specifically for this task. - Evoke logic is knowledge and should therefore be part of an MLM. - Variable events mean that the evoke engine must regularly check (poll) every MLM, if its events have changed, instead of waiting for external events (push).
runOnEvent() can then be used to find all MLMs that should run for the event, but not trigger cyclic or delayed triggers. This will be used for event calls in the logic slot, that should not start triggers.
This should make it possible to calculate EVENTTIME and TRIGGERTIME
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This includes the following changes:
Rename EvokeEvents to Triggers. Using the "event" terminology for triggers (expressions in the evoke slot), engine and events was very confusing, as events are not required for the evoke slot (i.e. constant time triggers). This will make it a lot easier to mentally differentiate the concepts. For more thoughts on this, see the message of commit a41c445.
There is now an
ArdenEventclass for events, that stores itsEVENTTIMEas well as its primary time (see #8).Improvements to the Trigger API. There are now methods to:
This will be useful to calculate
EVENTTIMEandTRIGGERTIME, and to support the event call statement in the logic slot, which only checks whether an MLM should run for an event and does not trigger it.TODO: