Extending Blockly with new openHAB commands

Hi Stefan,
very cool work, thank you for your effort and all the work. Are there any plans for a map transformation block to work with map files? A second need could be a block to detect the item of a group which triggered the rule. Is there a chance for such blocks?

BR

Johannes

Thanks Johannes, appreciated. Map transform is already there - see above somewhere in the thread and part of the next code drop. And the other are event ones that are mentioned as well above. Check it out above and see if that is what you are missing.

I made significant changes to Stefan’s PR and merged so we can test them in the upcoming milestones and release candidates.

I’m very pleased with Blockly’s new capabilities!

image

generates:

function addFrameworkService (serviceClass) {
  var bundleContext = Java.type('org.osgi.framework.FrameworkUtil').getBundle(scriptExtension.class).getBundleContext();
  var serviceReference = bundleContext.getServiceReference(serviceClass);
  return bundleContext.getService(serviceReference);
}

var ruleManager = addFrameworkService('org.openhab.core.automation.RuleManager');

function convertDictionaryToHashMap (dict) {
  if (!dict || dict.length === 0) return null;
  var map = new java.util.HashMap();
  Object.keys(dict).forEach(function (key) {
    map.put(key, dict[key]);
  });
  return map;
}

var dtf = Java.type("java.time.format.DateTimeFormatter");

var zdt = Java.type("java.time.ZonedDateTime");

function getZonedDateTime (datetime) {
  return zdt.parse(datetime + ' 00:00:00 +00:00', dtf.ofPattern('yyyy-MM-dd HH:mm:ss z'))
}

var persistence = Java.type('org.openhab.core.persistence.extensions.PersistenceExtensions');


ruleManager.runNow('ruleUID', true, convertDictionaryToHashMap({'key0': 'abc', 'key1': 'hey', 'key2': 'there'}));
if (persistence.updatedSince(itemRegistry.getItem('MyItem'), getZonedDateTime('2021-04-06'))) {
  print(zdt.now().minusMonths(4));
  print((persistence.averageSince(itemRegistry.getItem('MyItem'), zdt.now().minusHours(1))));
}
1 Like

Hi Yannick,

I currently comparing your changes to mine. Actually you were too fast with merging :wink: because I had some stuff not pushed because I was waiting for your review first to avoid to many commits. My fault that I did not mention that.

All in all, Yannick, can you do me a favor :hugs: : I try to be very transparent during development what I do and what I am about to implement and I put a lot of thoughts into it and almost all the time I take your comments into account and I also prioritise my work based on your comments.

I am happy to learn but when you just change some blocks without discussing it here before (which I am aware is the most time efficient way of doing so for you and you are the only one I think who are doing webui reviews which is an awful lot of work) it makes my life much harder afterwards as I have to find out later like I did today what was changed.

Almost anything you do makes really good sense and there is no doubt about that but it would be really great if you either propose block changes first (not code of course) or at least quickly list what you have changed so I don’t have to do the research, ok? :hugs:

Things missing due to not having been pushed by me:

  • I had added (and not pushed) the “previous state” in persistence which is now missing like mentioned here

    • “‘previousState’: ‘Gets the previous value of the State of a persisted Item’”
    • the mutation of the block in “oh_get_persistvalue” for the previous value therefore is missing as well
  • ‘oh_sleep’ is still saying “thread sleep (ms)” instead “Wait for…”

More like a summary from my side that I hopefully found the most important changes you did:

  • you made sure that the names created by provideFunction_ are then used in the code to avoid variable conflicts (understood)
  • you use const instead of let consistently (understood)
  • I wasn’t aware that I could return and use something like that in JS. (learned :slight_smile: )

const { dtf, zdt, getZonedDatetime } = addDateSupport()

Questions

Ephemeris blocks

  • you moved all ephemeris input blocks into “blocks-dateoffsets”. Did you only move it there but also changed something important (except renaming the blocks)?
    • at least one thing I discovered is the following

image

Anything else?

Transformation

Why did you change the dropdown of the transformation to become a textfield? It is clear to me that a text field is more generic but I don’t like the idea here.

I think a dropdown is more safe to be used instead of a text field because people could do nonsense here

I see you have kept the dropdown in the code. I would prefer and like putting that back in instead of the text input:

like so

Cheers
Stefan

Rich, can you have a look here? There is a reschedule-timer-bug that I don’t really understand but I am sure you do and know what the fix has to be as you had been involved with providing guidance to me.

cc @enrico.mcc

@rlkoshak @ysc

I need your advice regarding the bug I once created here

Basically the reason is because it is converted to Javascript and comparison works well if

  • the state does not contain a metric (and therefore only contains a number)
    *and one of both has the type of Number

In the case a String (containing a number) is compared with a Number then Javascript converts the String into a Number and the comparison works.

So the reason why it works above in the first case is actually not the reason that is assigned to a variable but because the comparison is done agains a number.

So this all goes together with also the metrics comparison containing units. We would actually need some mechanism where we detect both operands and react accordingly.

@ysc:
I checked the blockly API and tried out several things but have found no way yet to intercept the code generation of the if-block.
What I could imagine though that we implement our own comparison block for OH that could handle that. Do have a better idea?

I started working on these

which results into

logger.info((‘item name=’ + String(event.itemName)));
logger.info((‘item new state=’ + String(event.itemState)));
logger.info(('previous state = ’ + String(event.oldItemState)));
logger.info(('received command = ’ + String(event.itemCommand)));
logger.info((‘received event =’ + String(event.event)));

note that I was not able to add it to the vars-section but added it to the “items & things” section.

  1. With respect to Event Object Attributes — openHAB Helper Libraries documentation there are a few missing - should we add more?

  2. Is it correct that all return a String?

Any other ideas?

Sorry - I ended up working on your PR for longer than I thought (most of the afternoon) and then I had to run away so I didn’t have time to elaborate on what I did.

Actually I had this idea that this one could warrant its own block, because it is quite different than the others (sum, average, min/max etc.) as it doesn’t necessarily return a decimal number, rather a state corresponding to the type of the item. Also there is the quite useful skipEqual option that could be added as a checkbox on the block. So I removed it with the intent of readding it this week as a separate block.

I saw that you reused the EphemerisDate & EphemerisDay types & blocks in the persistence category to specify the dates, which wasn’t quite right. So I renamed them to ZonedDateTime and DayOffset. Note that the PersistenceExtensions methods don’t accept a day offset, only a ZonedDateTime.
https://next.openhab.org/javadoc/latest/org/openhab/core/persistence/extensions/persistenceextensions

But by having a dropdown with hardcoded values, you arbitrarily exclude other transformations (JS, Jinja, Scale, XPath etc.) from being able to be used, and also the user still has to figure out that the corresponding add-ons have to be installed. So I would prefer keeping it open and reinstating the dropdown when we figure out a way to list only those transformations that can be used. Or figure out an UI that has the suggestions, like with the dropdown, but doesn’t limit the available choices to them (I haven’t got a solution for that).

I would prefer the original comparison block to work, but I don’t know if it’ll be possible. The problem is that indeed the oh_getitem_state block returns a string because it is, in fact, a string, as the API always returns states as strings. But behind it could be any type of state. So at first it could make sense to not set an output type so it becomes compatible with any block. That’s basically what happens when you put the state in a variable - you lose the type information.
However that’s not the best either because while in many cases it will work as expected, due to the way JavaScript handles comparisons, it is not at all guaranteed.

image

The 2 last results in the example above are unfortunate.

The other issue is that standard JS comparisons don’t work with QuantityTypes i.e.:

var QuantityType = Java.type("org.openhab.core.library.types.QuantityType");
var x = new QuantityType("100 °F");
var y = new QuantityType("35 °C");
var z = new QuantityType("40 °C");
print(x);
print(y);
print(x < y);
print(x < z);
print(x.compareTo(y));
print(x.compareTo(z));

returns:

100 °F
35 °C
false // correct
false // incorrect!
1 // correct
-1 // correct

So in the end we might indeed have to provide a comparison block explicitely for QuantityTypes which would use “compareTo” under the hood - even though I’m not very happy about it as it adds to the confusion.

Thanks for your comprehensive answer.

“So I removed it with the intent of readding it this week as a separate block.”

Let me do it as I already have started with new stuff and me being continuously fighting with git when working on forks don’t want to get into git hell again :wink: It’s a pity though because I loved the fact I finally found out how to make the mutation happen :wink:

As far as the condition thing is concerned - I am pretty sure there is no way to use the original condition block. I have seen threads in that regard to actually fork the original one which clearly points into that direction.

I will now focus on the events block to get it out before christmas on M5 (your merge is already in there) and maybe I will then tackle the first part MVP on the condition thing (see post above)

This points back to the units of measurement/QuantityType discussion above.

There is a baby step that could be used though. If we add a new block that lets the user assign the units to the constant value in the comparison, e.g. 5 w instead of just the 5 that’s in place now then at least the user has the option to handle this case instead of it just blowing up in their face.

A next step would be to detect whether the state is a DecimalType or a QuantityType. If it’s a DecimalType then you can only compare it to a plain old number. If it’s a QuantityType there are some alternatives:

  • QuantityType to QuantityType
  • QuantityType.floatValue to Number
  • QuantityType to new QuantityType(Number, Units from first operand)

The logic becomes a bit complicated as you can see.

I’m hoping to have most of this encapsulated into the JSScripting library but for now we have to do it ourselves.

The JavaScript would look something like this.

var lessThan = function(op1, op2) {
  if(op1.class == QuantityType.class || op2.class == QuantityType) {
    if(op1.class == QuantityType.class && op2.class == QuantityType.class) {
        return op1.compareTo(op2) < 0;
    }
    else if(op1.class == QuantityType.class) { // If op2 isn't a QuantityType convert it to one using the units from op1
        return op1.compareTo(new QuantityType(op2, op1.getUnits()) < 0;
    }
  }
  else {
    // do the comparison normally
  }
}

The above isn’t complete but it kind of shows the logic we’d need.

The “5 w” type block would turn into

new QuantityType(5, "w")

So in the above, if the 5 were replaced with 5 “w” then the first if in the function above would run and the comparison would be done with units. In all other cases where one or the other operand has units we need to convert the one that isn’t units to have units, or strip the units off of the one with units.

That’s the next best place to put it I think. It will only ever have content that is related to Items or Things.

We need to include event.channel I think. The rest can be left out. Many of them are not supported anyway.

No, newState, and previousState return a State and receivedCommand returns a Command. For example, if a Number Item changed, newState and oldState would be DecimalTypes.

1 Like

Unfortunately I need to come back to the event-Block because if want to return anything back different from String then we need to think about it more in depth and we should not mix that up with the comparison thing above because that would it make even harder.

        ['newState', 'itemState'],            // Change and Updated Event -> State => String or State-Type?
        ['previousState', 'oldItemState'],    // Change and Updated Event -> State =>  String or State-Type?
        ['triggering item name', 'itemName'], // all -> String
        ['type', 'type']]),                   // all -> String
        ['receivedCommand', 'itemCommand'],   // Command -> has attributes but hard to handle the different attributes per type => String
        ['channel', 'channel'],               // ChannelTriggeredEvent  -> has attributes but hard to handle the different attributes per type => String

Look at the comments above

State
The first two return a State-Object which in OH is basically a type. A type has several subclasses all of which have different methods. Most methods of those types should not be of any interest (like OnOff, Decimal…) as the only thing you would probably want is the value anyway. HSBType does have getters for RGB but that is pretty specific. A State-Block would need to have a “free-attribute” name or you would have to provide the State-Type and from there it could “intelligently” tell which attribute would be available as an attribute.

Note that these can be undefined in case it is CommandEvent.

So in summary at least for the time being I would not implement a Blockly-State-Block which would provide a state back but just return String and name that field “state value”. Later we could provide the state as well that would feed into a block that takes that state and with choosing the expected type it would allow you to retrieve specific attributes for that block (we could also make that an individual block as it returns a different type State then)

Agreed?

Type and Name
Are just Strings. Type actually provides the “short name” of the types class

Channel
is only available when the channel was triggered
This is actually a Subtype of Event which has the name attribute and a type which we already have. As the only interesting information is the channel name, I would not give back the channel object but again a String that contains the channel name only.

Agreed?

Event
event.event doesn’t exist actually. We could return “event.” in the code and something (any attribute or method) could be added as a free string but if there was anything we would need that is not covered above in any of the fields for one of the event’s subtypes we should rather explicitly provide them as an attribute in this block here like the channel.

Agreed?

Hopefully you say “Yes” to all because then I would be able finish that topic tonight and can provide a PR by tomorrow which should not be too big for Yannick to review and merge soon (it contains some very minor fixes and additions).

PS: The comparison topic will be only tackled next year (or maybe over the winter holidays). I really want to put some major thoughts into that before doing it.

I’d call it “state string” to be clear that we a reworking with Strings.

My only concern is you can’t do number comparisons and math with a String and that’s a pretty big need.

If I can’t do something like event.itemState > 20 or event.oldItemState > event.itemState that’s a pretty big limitation.

Agreed.

event.event exists sometimes when a rule is triggered by an event Channel and it contains information about the event as a String. We can’t skip this but you don’t need to do anything special about it. Just return it as it is and specify it as a String.

Oh sure, I think that should work because as mentioned further above Javascript is rather resilient to this. If you compare a string with a number (like I described in my ‘own bug’ that I files about that) that comparing is working as JS coverts the string into a number… but to be very sure in this case I do a test case after the implementation.

One thing to be careful of though is that probably won’t work in JSScripting or when using strict.

I’ve been spending a lot of time working with JSScripting and that’s one place where I ran into trouble. We will have to get Blockly to support JSScripting at some point so if we can avoid extra pain later it’s worth at least consideration.

For the short term though if it works in Nashron as Blockly is generating it then that’s fine by me.

Yep, then this is a general program not only at this place. We will have a look as soon as we work on GraalVM. I am sure that Yannick has that somewhat on the roadmap.

Back to work - the PR will come soon :wave: @ysc

I think he already has a PR started and I posted a couple tutorials. It’s not too bad really making something work and I learned something here that I didn’t know about which might make it even better (namely event.type) in some cases.

I started a tutorial over at Migrate from Nashorn to JSScripting for UI Rules. To start I would think Blocky would use the Object.require approach so the differences between Nashorn and JSScripting are minimal. Long therm though I think a number of things that will exist in the Helper Library could make the Blocky implementation easier. I think the biggest gotcha is Thread.sleep breaks and there is as of yet no alternative. That and the String issue are the two big gotchas.

For example, what if the library provided a method to compare two States or compare a State to anything and as long as they are compatible it will give the right answer? That should make your job with the comparison stuff easier I would think.

Will the helper library be part of openhab by default?

Not having sleep is really a big deal breaker. I am sure a lot of people are using it. The problem is that I just quickly researched that topic that this was actually a design session not to have multi-Threading.

Yep. It’s embedded in the add-on now. :slight_smile:

Yes but the usual work around is to do a Promise and await but that only works when you have a loop which normal JS in the browser and Node have but seems to be lacking in GraalVM JavaScript. So you make the promise and try to await and the rule just exits.

We might have to do something ugly in the library like a busy while loop. It’ll be super ugly though because we can’t even yield or nice the loop so it will spike the CPU while “sleeping”.

Here are the final blocks and I can confirm that the comparison works (I used a number = Decimal Type state)

The blocks also have tooltips that tell the user with which events the blocks to use, e.g.

@ysc here is the PR to be reviewed and merged.

TIA
Stefan