Page 1 of 2
Make blockly Else if to an actual else if
Posted: Thursday 22 February 2018 10:22
by Xztraz
As people have figured out domoticz implementation of blocklys Else-If just works as a bunch of stacked if's. Not very logical.
This should be rewritten to work as a real else-if and ordinary if-blocks should be stackable.
Not so easy but blockly is one of the strongest part of domoticzs. it should be given higher priority.
Re: Make blockly Else if to an actual else if
Posted: Thursday 22 February 2018 10:32
by heggink
Blockly being the strongest part is an opinion IMHO. I personally think dzVents is the strongest part but that's just me
.
Back to Blockly, I may be wrong here but my understanding is that this is a blockly issue, not a domoticz issue. I think you would have to change the blockly behaviour. Not sure how many people in the domoticz community know how to do that to be honest.
Again, I may be completely mistaken.
H
Re: Make blockly Else if to an actual else if
Posted: Thursday 22 February 2018 10:49
by Xztraz
What i see from testing the logic blocks on blockly demo site they produce real else if and such.
- blockly_real.PNG (20.2 KiB) Viewed 8735 times
Produces this lua code:
Code: Select all
if not true then
for count = 1, 10 do
print('abc')
end
elseif blah == 0 then
print('abc')
else
blah = 1 + blah
end
if true and not false then
blah = blah + 1
end
so it's the implementation of blockly in domoticz that's weird.
Re: Make blockly Else if to an actual else if
Posted: Thursday 22 February 2018 11:38
by Xztraz
And yes it's an opinion. But i thought blockly was something unique in domoticz. but after some googling other projects are implementing this too.
It should still follow programming logic
Make blockly Else if to an actual else if
Posted: Thursday 22 February 2018 17:31
by leonmoonen
I’m a bit lost in your argument... what do you think Domoticz is making of the blockly example you posted?
Re: Make blockly Else if to an actual else if
Posted: Thursday 22 February 2018 18:10
by Xztraz
The example was just to show that blocky outputs real ifs and elses and such. the blockly code is just jibberish but shows it's translated to real code.
Re: Make blockly Else if to an actual else if
Posted: Thursday 22 February 2018 18:23
by blauwebuis
Fixing this would rock.
I agree that Blockly is a valuable feature that most other open source home automation didn't/don't have. For me it was also the main reason to stick with Domoticz: it has potential to make home automation accessible to a less tech-savvy audience.
Make blockly Else if to an actual else if
Posted: Friday 23 February 2018 5:30
by leonmoonen
Xztraz wrote:The example was just to show that blocky outputs real ifs and elses and such. the blockly code is just jibberish but shows it's translated to real code.
I understood that part, but you seem to say that Domoticz either doesn’t allow you to create that control flow, or that it interprets it wrongly. Can you give the translation to code that you think Domoticz is using for that same piece of blockly?
nb: I have very similar else-if blockly logic in Domoticz and haven’t seen an issue , which may explain my confusion - I’m not saying there’s nothing wrong, just trying to pinpoint what exactly needs to be fixed
Re: Make blockly Else if to an actual else if
Posted: Friday 23 February 2018 5:43
by ash77
It's a pretty simple change. There are four calls to parseBlocklyActions() in CEventSystem::EvaluateBlockly(). Change all four the same way:
Change:
Code: Select all
parseBlocklyActions(it->Actions, it->Name, it->ID);
to:
Code: Select all
parseBlocklyActions(it->Actions, it->Name, it->ID);
break;
Re: Make blockly Else if to an actual else if
Posted: Friday 23 February 2018 8:55
by Xztraz
Ah! maybe my initial explanation was a bit short
If setting up an -if-else- block. its intepreted as 2 -if- blocks after eachother. its not an else condition.
both -if- and - if-else- are ran independent ignoring if it matches the if condition.
This example should never reach the else if. if the first is tru the second should not run
- elseifbug.PNG (12.41 KiB) Viewed 8651 times
and when pushing the button. both rows are executed as expected with 2 separate ifs
Code: Select all
2018-02-23 08:53:10.091 EventSystem: Event triggered: elseiftest_1
2018-02-23 08:53:10.091 -IF-
2018-02-23 08:53:10.091 EventSystem: Event triggered: elseiftest_2
2018-02-23 08:53:10.091 -ELSE IF-
so its converted to something like:
Code: Select all
if nexach3 = on
print('-IF-')
end
if nexach3 = on
print('-ELSE IF-')
end
enabling -ELSE- (only else) in the if-else block would also be freaking super.
Re: Make blockly Else if to an actual else if
Posted: Friday 23 February 2018 9:17
by CLEMENT99
Xztraz wrote: ↑Friday 23 February 2018 8:55
Ah! maybe my initial explanation was a bit short
If setting up an -if-else- block. its intepreted as 2 -if- blocks after eachother. its not an else condition.
both -if-
and - if-else- are ran independent if there is something true in the first row.
OK, thanks for this explanation, I understand now my problems with blocky.
Re: Make blockly Else if to an actual else if
Posted: Friday 23 February 2018 9:32
by Xztraz
people end up with super huge and comparator blocks instead of neat nestling.
This needs to be fixed too in the same time if you now would need multiple if rows in a blockly script
- elsifif.PNG (18.42 KiB) Viewed 8639 times
-Second If- never runs
Re: Make blockly Else if to an actual else if
Posted: Friday 23 February 2018 9:55
by Xztraz
The Blockly code generator should just make code in lua and run under drventz or something.
Then the fullish blockly set would be possible to get working. i'm not sure how its interpreted now but its quite limited.
This should be something to look at in the long run. Then the step to lua for even more advanced stuff is easy. just klick edit raw lua and continue from there..
i know lots of work.. but would be a big leap forward.
Re: Make blockly Else if to an actual else if
Posted: Friday 23 February 2018 10:24
by blauwebuis
@ash77 - wow, is that really the solution? If so, let's create a pull request!
I've created a pull request here:
https://github.com/domoticz/domoticz/pull/2164
Re: Make blockly Else if to an actual else if
Posted: Friday 23 February 2018 15:04
by gizmocuz
Best to test this solution first locally...
But i don't think this is the fix unfortunately, and it's the way it was designed/implemented years ago.
The OP from this topic also created a github issue where i try to explain how to overcome (his/some) this issue:
https://github.com/domoticz/domoticz/issues/2154
you will see what i mean if you create both blockly's and have a look in the database what is created in the EventRules table
if this is correct, then all will work, but if an advanced system dzVents/Lua/Python is not desired, indeed someone have spent some time on this
Re: Make blockly Else if to an actual else if
Posted: Friday 23 February 2018 15:40
by leonmoonen
Ok, it’s also clear now why I don’t see the issue, as all my conditions turn out to be mutually exclusive.
One more question for @gizmocuz: Although technically ‘more correct’, fixing this behavior would break the logic of existing scripts. Would that be a concern for possibly ever accepting such a patch? If not, I’d be happy to look into this and try to give something back to my favorite home automation system...
Re: Make blockly Else if to an actual else if
Posted: Friday 23 February 2018 15:43
by gizmocuz
You mean the patch from 'ash77' ? i dont think that this is 'the' patch and causes only the first 'then' to be executed and then it stops
If the 'EventRules' is correctly populated by a patch, this will not cause previous scripts to break....
(in this case there should be more rules created)
Re: Make blockly Else if to an actual else if
Posted: Friday 23 February 2018 17:16
by leonmoonen
I wasn't specifically referring to asf77's patch but let's say we have somehow come up with a magical patch that addresses the issue. I think that this
does mean that the behavior would change and my question was aimed at finding out if that would be a reason to never accept the patch (there can be good reasons not to want to change the current behavior, but IMO it also would mean it's not worth exploring a potential patch, since it inherently changes behavior).
Let me try to explain why I think it changes behavior / may break existing scripts: after applying the patch, the blockly else-if branch will be interpreted as a real else-if branch, that means its condition will only be evaluated when the condition of the corresponding if branch is false. Then existing scripts that rely on the current interpretation of the blockly else-if logic would
"break" to the extend that their else-if branch is no longer evaluated when the if condition is true.
In terms of Xztraz' example,
before the patch EventRules would be populated in such a way that the log would show
Code: Select all
2018-02-23 08:53:10.091 EventSystem: Event triggered: elseiftest_1
2018-02-23 08:53:10.091 -IF-
2018-02-23 08:53:10.091 EventSystem: Event triggered: elseiftest_2
2018-02-23 08:53:10.091 -ELSE IF-
and after the patch EventRules would be populated in a way the log would show
Code: Select all
2018-02-23 08:53:10.091 EventSystem: Event triggered: elseiftest_1
2018-02-23 08:53:10.091 -IF-
If a user expected their script to behave as before the patch, that script is now "broken".
Hope this makes sense, it is surprisingly hard to write down unambiguously
Re: Make blockly Else if to an actual else if
Posted: Friday 23 February 2018 17:43
by gizmocuz
Thanks, this makes sense!
This patch could be accepted, as all will happen in the first IF, and it should not even go to the ELSE because the above is true.
But, there can be more conditions then one
if (A=1 and B=2) then
...
else if (A=2 and B=1) then
...
else if (A=2 and B=2) then
...
else
..
will this also work? i hope it does, and then of course the patch is more then welcome!
Edit: made a small type above, and corrected it
Re: Make blockly Else if to an actual else if
Posted: Friday 23 February 2018 20:07
by leonmoonen
cool, sounds like I’ve just got a project to work on
and yes, that multi-condition situation should of course be supported too