Make blockly Else if to an actual else if

Use this forum to discuss possible implementation of a new feature before opening a ticket.
A developer shall edit the topic title with "[xxx]" where xxx is the id of the accompanying tracker id.
Duplicate posts about the same id. +1 posts are not allowed.
Xztraz
Posts: 126
Joined: Tuesday 31 January 2017 22:54
Target OS: Raspberry Pi
Domoticz version:
Contact:

Make blockly Else if to an actual else if

Post by Xztraz » Thursday 22 February 2018 11:22

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.
Last edited by Xztraz on Friday 23 February 2018 10:47, edited 1 time in total.

User avatar
heggink
Posts: 459
Joined: Tuesday 08 September 2015 21:44
Target OS: Raspberry Pi
Domoticz version: V3.9530
Location: NL
Contact:

Re: Make blockly Else if to an actual else if

Post by heggink » Thursday 22 February 2018 11:32

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
Pi3, latest beta
RFXCOM, z-wave (AEOTEC, switches, temhum, pir, contacts),
Plugwise2py, P1 'smart'meter & solar panel
Alexa, Wifi Cams motion detection
ESP: relays, PIR & Temp/TempHum
Geofence iCloud, Bluetooth & Wifi ping
Harmony hub, Nest

Xztraz
Posts: 126
Joined: Tuesday 31 January 2017 22:54
Target OS: Raspberry Pi
Domoticz version:
Contact:

Re: Make blockly Else if to an actual else if

Post by Xztraz » Thursday 22 February 2018 11:49

What i see from testing the logic blocks on blockly demo site they produce real else if and such.
blockly_real.PNG
blockly_real.PNG (20.2 KiB) Viewed 954 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.

Xztraz
Posts: 126
Joined: Tuesday 31 January 2017 22:54
Target OS: Raspberry Pi
Domoticz version:
Contact:

Re: Make blockly Else if to an actual else if

Post by Xztraz » Thursday 22 February 2018 12:38

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 :D

leonmoonen
Posts: 15
Joined: Thursday 25 May 2017 23:07
Target OS: Raspberry Pi
Domoticz version: current
Contact:

Make blockly Else if to an actual else if

Post by leonmoonen » Thursday 22 February 2018 18:31

I’m a bit lost in your argument... what do you think Domoticz is making of the blockly example you posted?

Xztraz
Posts: 126
Joined: Tuesday 31 January 2017 22:54
Target OS: Raspberry Pi
Domoticz version:
Contact:

Re: Make blockly Else if to an actual else if

Post by Xztraz » Thursday 22 February 2018 19:10

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.

blauwebuis
Posts: 315
Joined: Wednesday 21 December 2016 10:11
Target OS: Raspberry Pi
Domoticz version: current
Contact:

Re: Make blockly Else if to an actual else if

Post by blauwebuis » Thursday 22 February 2018 19:23

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.

leonmoonen
Posts: 15
Joined: Thursday 25 May 2017 23:07
Target OS: Raspberry Pi
Domoticz version: current
Contact:

Make blockly Else if to an actual else if

Post by leonmoonen » Friday 23 February 2018 6:30

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 Image

ash77
Posts: 11
Joined: Wednesday 10 June 2015 22:10
Target OS: Windows
Domoticz version:
Location: United States
Contact:

Re: Make blockly Else if to an actual else if

Post by ash77 » Friday 23 February 2018 6:43

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;

Xztraz
Posts: 126
Joined: Tuesday 31 January 2017 22:54
Target OS: Raspberry Pi
Domoticz version:
Contact:

Re: Make blockly Else if to an actual else if

Post by Xztraz » Friday 23 February 2018 9: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 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
elseifbug.PNG (12.41 KiB) Viewed 868 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. :mrgreen:
Last edited by Xztraz on Friday 23 February 2018 10:30, edited 1 time in total.

CLEMENT99
Posts: 27
Joined: Friday 26 January 2018 10:18
Target OS: Windows
Domoticz version: BETA
Location: Brussels
Contact:

Re: Make blockly Else if to an actual else if

Post by CLEMENT99 » Friday 23 February 2018 10:17

Xztraz wrote:
Friday 23 February 2018 9: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. :evil:

Xztraz
Posts: 126
Joined: Tuesday 31 January 2017 22:54
Target OS: Raspberry Pi
Domoticz version:
Contact:

Re: Make blockly Else if to an actual else if

Post by Xztraz » Friday 23 February 2018 10:32

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
elsifif.PNG (18.42 KiB) Viewed 856 times
-Second If- never runs

Xztraz
Posts: 126
Joined: Tuesday 31 January 2017 22:54
Target OS: Raspberry Pi
Domoticz version:
Contact:

Re: Make blockly Else if to an actual else if

Post by Xztraz » Friday 23 February 2018 10:55

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.

blauwebuis
Posts: 315
Joined: Wednesday 21 December 2016 10:11
Target OS: Raspberry Pi
Domoticz version: current
Contact:

Re: Make blockly Else if to an actual else if

Post by blauwebuis » Friday 23 February 2018 11:24

@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

User avatar
gizmocuz
Posts: 8608
Joined: Thursday 11 July 2013 18:59
Target OS: Raspberry Pi
Domoticz version: beta
Location: Top of the world
Contact:

Re: Make blockly Else if to an actual else if

Post by gizmocuz » Friday 23 February 2018 16:04

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
Quality outlives Quantity!

leonmoonen
Posts: 15
Joined: Thursday 25 May 2017 23:07
Target OS: Raspberry Pi
Domoticz version: current
Contact:

Re: Make blockly Else if to an actual else if

Post by leonmoonen » Friday 23 February 2018 16:40

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...

User avatar
gizmocuz
Posts: 8608
Joined: Thursday 11 July 2013 18:59
Target OS: Raspberry Pi
Domoticz version: beta
Location: Top of the world
Contact:

Re: Make blockly Else if to an actual else if

Post by gizmocuz » Friday 23 February 2018 16:43

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)
Quality outlives Quantity!

leonmoonen
Posts: 15
Joined: Thursday 25 May 2017 23:07
Target OS: Raspberry Pi
Domoticz version: current
Contact:

Re: Make blockly Else if to an actual else if

Post by leonmoonen » Friday 23 February 2018 18:16

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,
Image
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 ;)

User avatar
gizmocuz
Posts: 8608
Joined: Thursday 11 July 2013 18:59
Target OS: Raspberry Pi
Domoticz version: beta
Location: Top of the world
Contact:

Re: Make blockly Else if to an actual else if

Post by gizmocuz » Friday 23 February 2018 18:43

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
Quality outlives Quantity!

leonmoonen
Posts: 15
Joined: Thursday 25 May 2017 23:07
Target OS: Raspberry Pi
Domoticz version: current
Contact:

Re: Make blockly Else if to an actual else if

Post by leonmoonen » Friday 23 February 2018 21:07

cool, sounds like I’ve just got a project to work on :)

and yes, that multi-condition situation should of course be supported too

Post Reply

Who is online

Users browsing this forum: No registered users and 2 guests