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.

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

Processing order of rule actions combined with 'and' is not guaranteed #531

Open
Heizelmann opened this issue Feb 10, 2015 · 11 comments
Open

Comments

@Heizelmann
Copy link
Contributor

Sometimes the first action is first and sometimes the second:
Example:
... then switch Timer off and switch Timer on
To avoid this random behaviour I use a explicit delay:
... then switch Timer off and switch Timer on after 1 second

What I expect is a left to right processing.

@incmve
Copy link
Contributor

incmve commented Feb 10, 2015

Nope its 2 commands running at the same time.
your telling it to do A and B at the same time, after 1 seconds is the correct way.

@Heizelmann
Copy link
Contributor Author

Asynchrounus processing is fine but not for the rule engine. If this is realy the case this is a serious issue for me. Same with processing order of rules in the rule system. See also #529 and #530

@sweetpi
Copy link
Contributor

sweetpi commented Feb 12, 2015

What I expect is a left to right processing.

We've 3 options here:

  1. Process action left to right by default
  2. Add a checkbox for each rule that let you decide if the actions should run in parallel
  3. Introduce some keyword for sequential processing like and then

I've to think about what would be the best. Comments?

@cklam2
Copy link

cklam2 commented Feb 12, 2015

I'd prefer "and then", this looks more natural and in the overview of rules you can see whether an And will run in parallel or sequential.

It's also more flexible, for example

turn lamp1 on and then turn lamp2 on and turn lamp3 off

Based on this example i think that "and then" should have higher precedence than "and"

@Heizelmann
Copy link
Contributor Author

I would prefer a strict left to right processing and I think parallel processing is not necessary.

@sweebee
Copy link
Contributor

sweebee commented Feb 12, 2015

I have 4 lights in my living room and they are all turning on from left to right.

trigger: if woonkamer is turned on then turn light1 on and turn light2 on and turn light3 on and turn light4 on

@Heizelmann
Copy link
Contributor Author

Look at this experiment. The rule is

If $new increased then $delta = $new - $old and $old = $new

Now change the variable $new from lets say 10 to 15. $old might be 10. The result is
first set $old to 15 and then set $delta to 0

What a novice user (like me) not knows is the concurrent processing model.
A current workaround is to use an explicit delay

If $new increased and $delta = $new - $old and $old = $new after 0.1 seconds

which gives the right result:
first set $delta to 5 and then set $old to 15.

The result is fine . But the naturalness of the rule syntax becomes worse.

@thexperiments
Copy link
Contributor

I ran into the same problem. I also like "and then".

@mwittig
Copy link
Collaborator

mwittig commented Jan 6, 2017

I am also in favor of this, however, option "1" may be a lot easier to implement.

I think it can be done simply by replacing the line https://github.com/pimatic/pimatic/blob/master/lib/rules.coffee#L871 in _executeRuleActions with the following:

Promise.mapSeries rule.actions, (action) =>

At the end of the for loop the return statement in line https://github.com/pimatic/pimatic/blob/master/lib/rules.coffee#L894 needs to be replaced with the following:

.finally =>
  return actionResults

I'll give it a try later and report back. I'll also make an imapct review.

If the suggested change works as expected it could be ellaborated to provide for option "2".

My question is when and if "parallel" execution is really required. It should also be said that execution is not really done in "parallel", but it more of an interwoven execution of asynchronous functions which belong to different execution contexts of the list of actions. To me this makes not much sense and I don't see any benefits in doing so. As @sweebee pointed out a sequence of switch actions won't result in switching on all the lights at the same time. Any views on this? @sweetpi What's your view on this?

@sweetpi
Copy link
Contributor

sweetpi commented Jan 13, 2017

@mwittig I think we should implement option "1" as you suggested, so that left to right is the default. We could then add an checkbox in the rule or a keyword for parallel execution later (if necessary) Would you implement the suggested change?

@mwittig
Copy link
Collaborator

mwittig commented Jan 13, 2017

@sweetpi yes, will do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants