Extending Blockly with new openHAB commands

Not after a reschedule. Only after a cancel and as the last thing that the timer’s function does when the timer goes off.

It doesn’t make sense to set it to null after a reschedule.

OK, just a though for what might be the cause of the problem. If that’s how Blockly does it it’s probably right.

HI Rich,

I finally found out why I didn’t use the variable’s content to produce the right code, so now I can do but unfortunately this introduces a new problem in code generation.

Let’s take example 1 where just use a String like before:

and it correctly produces what we expect - please take a particular note on the initialization part line13 to 15 in the beginning of the code as we will compare that in a moment.

well done … now let’s do the variable version:

which generates fine code - see lines 18 onwards: it assigns the string to the variable and then uses the variable as an indirection for accessing the timers. Very well …

but wait! Have a lock on the initialization part line13 to 15

It also uses the variable indirection for rightfully initializing the timer to null if undefined but this is always done in the beginning of the source by a blockly technique that makes sure that the same code that is required by many blocks is produced only once and then put to the beginning.

Of course, this must fail, because the variable neither exists at that point nor is it set correctly.

So why do we need this code in the beginning? It makes the whole code later on much easier. Instead we could put all the check into the code everywhere we we access the timer-variable.

So the only way I could think of is that we remove this part but then we have to make sure that everywhere where we use the timer[…]-field we first have to check if is not only not undefined or nor null.

So for the following that could fail if timerName wasn’t existing

// timerName = 'Stefan';
if (this.timers[timerName] && this.timers['timerName].isActive()) {
}

we had to generate this boilerplate code even for the simplest part of the code

if (timerName!== undefined && !timerName && this.timers[timerName] && this.timers[timerName].isActive()) {
}

Now imagine you had this code

if (this.timers[timerName] === null) {
	this.timers[timerName] = scriptExecution.createTimer(zonedDateTime.now().plusSeconds(10), function(){
		  this.timers[timerName] = null
	})

}else {
	this.timers[timerName].reschedule(zonedDateTime.now().plusSeconds(10))
}

which would result into

if (timerName!==undefined && !timerName && this.timers[timerName] === null) {
	this.timers[timerName] = scriptExecution.createTimer(zonedDateTime.now().plusSeconds(10), function(){
		  this.timers[timerName] = null
	})

}else if (timerName!== undefined && !timerName)  {
	  this.timers[timerName].reschedule(zonedDateTime.now().plusSeconds(10))
}

… and even after thinking quite some time I am not sure if this is completely correct … and now if we had this?

You could for sure argue that “who cares about the generated code” as it is in the background where noone sees it. Well, at least I do because have to be sure that it works and that one could at least comprehend what is going on… :slight_smile:

If even tried to encapsulate the check into a function but couldn’t quite find a good way of doing it.

What I am trying to say is that this doable but I have a bad feeling delivering the generated code due to the many permutations we could run into that would not work as expected. I am honestly torn back and forth about doing it out not. I am kind of stuck here… :cry:

Is there no way to make the definition of the variable always occur above the if that checks to see if the timer exists?

Maybe we can/should step back and use undefined instead of null. In that case we would elimnate the initialization to null at the top entirely.

Instead of

if(this.timers[timerName] === null)

we would have

if(this.timers[timerName] === undefined)

Instead of

this.timers[timerName] = null

we would have

this.timers[timerName] = undefined

Looking at this code I think that just might work. We’ve replaced the use of null to determine whether the timer exists or not with undefined which is actually perfectly reasonable. However, I’m not 100% positive that we can set a variable to undefined like that or not.

It’s worth a try. This could not only address this problem but make the code over all simpler.

We still absolutely need the initialization of timers[] though. But I’m thinking we don’t for the individual timers themselves. We won’t get undefined errors like we would if the timers were just variables instead of being managed in a dict.

Is there no way to make the definition of the variable always occur above the if that checks to see if the timer exists?

Unfortunately not because it is a standard blockly block which is out of my control.

I can try the idea with undefined - setting a var to undefined is technically possible.

Hi Rich,

I have good news for you that hopefully makes you happy: the idea with “undefined” did work out fine which also fulfills your wish to use blockly variables throughout the new openhab timer blocks. Here are some working and tested examples together with the code

"Simple Timer"

new:

  • Simple timers provide reschedule, cancel and none-functionality on rule retriggering
  • As requested by Yannick even simple timers are named timers
  • Timer name can be provided via blockly variable

var logger = Java.type('org.slf4j.LoggerFactory').getLogger('org.openhab.rule.' + ctx.ruleUID)
var scriptExecution = Java.type("org.openhab.core.model.script.actions.ScriptExecution")
var zonedDateTime = Java.type("java.time.ZonedDateTime")

if (typeof this.timers === 'undefined') {
	 this.timers =[]
}

function runme() {
  logger.info('runme')
}


logger.info('Starting')
if (typeof this.timers['stefansTimer'] === 'undefined') {
	this.timers['stefansTimer'] = scriptExecution.createTimer(zonedDateTime.now().plusSeconds(10), function(){
	  runme();
	  this.timers['stefansTimer'] = undefined
	})

}else {
	this.timers['stefansTimer'].reschedule(zonedDateTime.now().plusSeconds(10))
}

Simple timer with variable

var timerVar;
var scriptExecution = Java.type("org.openhab.core.model.script.actions.ScriptExecution")
var zonedDateTime = Java.type("java.time.ZonedDateTime")
if (typeof this.timers === 'undefined') {
	 this.timers =[]
}

var logger = Java.type('org.slf4j.LoggerFactory').getLogger('org.openhab.rule.' + ctx.ruleUID)

function runme() {
  logger.info('runme')
}

timerVar = 'stefansTimer';
if (typeof this.timers[timerVar] === 'undefined') {
	this.timers[timerVar] = scriptExecution.createTimer(zonedDateTime.now().plusSeconds(10), function(){
	  runme();
	  this.timers[timerVar] = undefined
	})

}else {
	this.timers[timerVar].reschedule(zonedDateTime.now().plusSeconds(10))
}

Individual timer functions

  • can be used to program specific complex logic with the individual methods
  • Uses named timers to related the functions
  • Timer name can also be provided via blockly variable

The following shows the example with a blockly variable - instead strings can be used as before

(note that the following example doesn’t really make sense as the cancel timer prevents the code from doing anything which is intended as I wanted to use as many blocks as possible in one go)

var timer1;
var scriptExecution = Java.type("org.openhab.core.model.script.actions.ScriptExecution")
var zonedDateTime = Java.type("java.time.ZonedDateTime")
var logger = Java.type('org.slf4j.LoggerFactory').getLogger('org.openhab.rule.' + ctx.ruleUID)
if (typeof this.timers === 'undefined') {
	 this.timers =[]
}

timer1 = 'stefansTimer';
if (typeof this.timers[timer1] === 'undefined') {
	this.timers[timer1] = scriptExecution.createTimer(zonedDateTime.now().plusSeconds(3), function() {
	  logger.error('abc')
	  this.timers[timer1] = undefined
	})

}
if (typeof this.timers[timer1] !== 'undefined') {
	this.timers[timer1].cancel()
	this.timers[timer1] = undefined
}
if (typeof this.timers[timer1] !== 'undefined') { this.timers[timer1].reschedule(zonedDateTime.now().plusSeconds(5)) }

of course of checks work as well and produce the following code (which again makes no sense in the way it is used here - it is only here to show the produced code)

var timer1;
if (typeof this.timers === 'undefined') {
	 this.timers =[]
}
var logger = Java.type('org.slf4j.LoggerFactory').getLogger('org.openhab.rule.' + ctx.ruleUID)

if ((typeof this.timers[timer1] !== 'undefined' && this.timers[timer1].isActive()) 
&& (typeof this.timers[timer1] !== 'undefined' && this.timers[timer1].isRunning()) 
&& (typeof this.timers[timer1] !== 'undefined' && this.timers[timer1].hasTerminated())) {
  logger.error('abc')
}

I hope that this finally closes that chapter and it can be released.

Save values for retriggered rules.

As I was working so comprehensively on the named timers I finally thought it would be nice to have the functionality to be able to save own variables in a way that they survive a rule retrigger which is something I use a lot when I am developing DSL rules. So I added that as well which is now provided as follows.

Note that it does not persist the values in openhab but just remembers the value in a this-variable-Collection (see code) similar to the timers. I might eventually use the API “PersistenceExtensions” to provide that functionality if this is asked by the community.

@ysc As I know you like looking at how the blocks present themselves and what the naming is like -please respond if you like it to be changed (e.g. we could use store instead of save). These are the last things I will do before I do the finally push to the git repo for your final review.

Here is the blockly representation

and this is the code that is generated (it uses its own Set named globalvars to keep track of the variables)

The first time the rule is triggered it says “Stefan”, the next time it says “Rich” which proves the rule remembers the content of that variable

if (typeof this.globalvars === 'undefined') {
	 this.globalvars =[]
}
var logger = Java.type('org.slf4j.LoggerFactory').getLogger('org.openhab.rule.' + ctx.ruleUID)
if ((this.globalvars['myName']) == 'Stefan') {
  this.globalvars['myName'] = 'Rich'
} else {
  this.globalvars['myName'] = 'Stefan'
}
logger.info((this.globalvars['myName']))

This following one is a pretty confusing one: It actually uses a Blockly Variable as an indirection that contains the variable name (hence the named slot) into which the value should be stored. I would rather say that this a seldom situation where you would use something like that and usually I wouldn’t recommend doing that but it shows that even this is working and I am sure it will trigger your creativity to do something weird with it :wink:

var theVariableName;
if (typeof this.globalvars === 'undefined') {
	 this.globalvars =[]
}
var logger = Java.type('org.slf4j.LoggerFactory').getLogger('org.openhab.rule.' + ctx.ruleUID)


theVariableName = 'varname';
if ((this.globalvars[theVariableName]) == 'Stefan') {
  this.globalvars[theVariableName] = 'Rich'
} else {
  this.globalvars[theVariableName] = 'Stefan'
}
logger.info((this.globalvars[theVariableName]))

I hope that this all concludes the first drop for the new blockly enhancements.

@ysc let me know if you are fine with (or not), so I can push it. Note that I tried to squash the multiple commits but it tells that some of the commits already have multiple parents so I failed and gave up doing it. Hopefully you can do it when merging the pull request. Thanks for the support.

cheers
Stefan

I indeed think “store value {string} into {string}” and “value stored in {string}” would be a little clearer.

Hopefully this can be clarified in the tooltips and the eventual docs - that these blocks define variables that persist across rule runs (and maybe across different rules too?) but are lost if e.g. openHAB is restarted is perhaps a little hard to grasp at first.

You seem to have missed a space before []. Also In your code generators make sure there is a semi-colon at the end (it seems to be the convention in other built-in blocks).

  • Sure, I will rename them.
  • I added it already to the tooltips :+1:
  • I will add the Space
  • I actually omitted the semicolons everywhere because I thought that in ECMAscript it is the convention not to use them anymore?!

It’s a matter of choice - yours and/or the eventual coding style framework that you adopt - and it’s the “vim vs emacs” of JavaScript really. For Blockly’s standard blocks generated JS code, I assume the Google team used the same conventions as Blockly’s code itself: Web Blockly Style Guide  |  Google Developers - so 2-space indents and semicolons.

So it’s best to stick to it so the final generated code remains consistent when blocks from different sources are assembled.

So it is ok from your perspective l if I use no semicolons and tabs consistently?

No, like I said you have to stick to the convention of the blocks that are already there in the non-openHAB categories.

For example
image

generates this:

var i1;

function mathRandomInt(a, b) {
  if (a > b) {
    // Swap a and b to ensure a is smaller.
    var c = a;
    a = b;
    b = c;
  }
  return Math.floor(Math.random() * (b - a + 1) + a);
}

function colourRandom() {
  var num = Math.floor(Math.random() * Math.pow(2, 24));
  return '#' + ('00000' + num.toString(16)).substr(-6);
}

var repeat_end2 = mathRandomInt(1, 100);
for (var count2 = 0; count2 < repeat_end2; count2++) {
  i1 = colourRandom();
}

so that’s what your code should look like too.

:sob: … That will throw me back again in time

Great news! I’m glad that worked.

The blocks and the code look good to me. I’m glad we can get fully functional timer support in Blockly. It will be really important to making Blockly viable for more users.

I mostly use this for Timers but I think it too is important for other variable types so I’m glad you implemented it too.

That might be a bit of a pain though because You’ll have to create an Item(s) to store the values. I think this is sufficient for most use cases. And for the more advanced use cases where the data needs to survive a restart, the users should have access to Persistence to do the saving themselves, or Item metadata might be a viable alternative approach.

Calling this globalvars is probably a misnomer. These are not global in any way, really. Maybe “savedvars” would be better. It more accurately depicts what is really happening and it isn’t confused with Persistence in any way.

In Rules DSL we called them global because the only way to save a variable from one run to the next in a rule was to put the variable outside of the rule. That’s not an option here. There is no outside the rule.

Nope. There is an issue opened to address how that might work and there was even a poll and an approach decided upon. But since then there has been no activity. The approach is to use the ScripExtensions as a central repository for shared variables. But there really isn’t a good way to achieve that from rules. There is one example of doing it in GraalVM in @jpg0’s helper library but I’ve not been able to figure out how to use it in practice.

Glad that it works out for you so far.
I renamed the variable to “storedvars” as I also changed the block into “store … into” as proposed by Yannick.

I also added the semicolons.

@ysc I am going to push this soon. (Please do me a favour and squash the commits as I wasn’t able to do it myself … it says “multiple parents…” and take care of the missing sign-off, thanks)

Sending the notification was finally very easy. I only had to load the right java class which works perfectly well (provided of course that the OH cloud has been configured). So the code is then


var NotificationAction = Java.type("org.openhab.io.openhabcloud.NotificationAction");
NotificationAction.sendNotification('me@you.de','Hello');
NotificationAction.sendBroadcastNotification('Hi all','battery','high');
NotificationAction.sendLogNotification('Hi Log','attic','warn');

1 Like

Well, congratulations Stefan, bigbasec and Yannik as the first enhancement to the Blockly interface not only got merged but it seems to be included in the new Milestone 3.2.0.M4 as well
:+1:

5 Likes

Thank you very much for notifying, Andrew and Yannick for finalizing it. Thanks also to Rich for the many ideas and proposals.

I have edited the very first post to show what the current outcome is.

@rlkoshak Now on to the next adventures. Based on your proposals here, what would you think would be the next priority to be implemented?

2 Likes

in random order (some ideas I had while reviewed but didn’t have time to fix, or after):

  • blocks to retrieve metadata from items

    • “get {text:namespace} metadata value of item {text:MyItem}”
    • “get {text:namespace} metadata parameter {text:parameter} of item {text:MyItem}”

    (note: I don’t think item/things/metadata update blocks are needed in Blockly at this stage, the scripts should be primarily for acting of things & items, not managing them. Also they would clutter the toolbox and ultimately maybe interested users could install an “Admin” library of blocks from the marketplace.)

  • the “get thing state” block is missing an example “thing MyThing” shadow block

  • thread sleep block: it should probably accept a integer block so the delay can be changed by a calculation instead of a field where it’s hardcoded

then (basically what’s been put aside)

  • Ephemeris

  • Network category with ping & HTTP requests (not too complex if possible)

  • Calling other rules

  • the Colours category has a nice picker but it outputs RGB, it would be nice to have a RGB-to-HSB block (and probably vice-versa) to be able to use them as commands.

For the rest (publish MQTT message, Mail, Telegram…), I’ll try to finish the user-defined library thing so they could be made available with those.

1 Like

As I might think we want design a bit around the ephemeris I have copied your ideas from the github discussion to here:

From Yannick,

"I still don’t think it’s right to mix several functions that do unrelated things together. Having a block for each operation seems the right way to go.

One suggestion to have the ability to do offsets or not is to introduce new types that are only compatible with inputs in the Ephemeris blocks (and fill the input with a shadow “today” block by default), like this:

image

Then there must be a way to figure out the day/day-with-offset input in the code generation of the actual blocks, I haven’t checked yet."

I like the design as above, only that I think that the comparison shouldn’t be static but be rather coming from a drop down like so (Otherwise we would get more permutations of blocks).

image
(of course the “today” is a puzzle piece like you did above - I wasn’t clickly able to prototype that with the blockly demo tool)

Also the question would be if want “is a weekday” which basically is the same like NOT → "today is a weekend) but doesn’t sound as nice. Instead I could invert it internally and provide the weekday function for the block.

I agree with @ysc’s list of priorities.

My biggest priority though would be to implement those things that would bring Blockly at least up to Rules DSL in terms of what it can do. So the core actions (Actions | openHAB) would be at the top of my list, perhaps even above Item metadata.

I’ve found the ability to call other rules to also be super powerful so I might place that even above Item metadata.

I like what I see there. It seems simple enough and supports the core Ephemeris capabilities. I’m not sure Blockly needs to ever support custom day types or custom holiday files.