Extending Blockly with new openHAB commands

Where do you see “average state”? I only see values and the last one in the the drop-down has state.

Commenting based on the screenshots on the first post

  • send command "value" to item MyItem: replace "value" with "command" (directing reader to understand the meaning of command)


    Similarly for “post update”

  • get thing state: probably should be get thing status
    image

Comments on

  • item XX has changed since: would XX state has changed|updated since YY be better? Also, have the same past tense for changed and updated. I think this would emphasise the difference that we are talking about the state. That’s even more important now with these changes, as blockly commands deal with both item objects and the state values.

Comments on Extending Blockly with new openHAB commands - #176 by stefan.hoehn

  • How about: get state of item XX

@maniac103 broughts good points about the exact spelling of the persistence functions. I will add one more proposal for your consideration

  • get <average/delta/deviation> of item XX state since YY

Best,
Sami

First, Sami, I do appreciate that you take care, though keep in mind everybody like me is doing this whole work in our freetime, so I try to avoid double work (no offense, I just like to make that transparent), which is why I hesitate to do many changes afterwards - therefore I am posting everything I intend to implement here during the implementation before (I am aware you only jumped in now and have probably not seen that) Also every change needs to be reviewed later by someone else which is mostly Yannick who already has a big load of work to do (thx Yannick)

To make my life easier: Can you please edit your post above and add images of the blocks you want to have changed and even better scribble the changes what you want to have changed next to the block or on top of it. This avoids misunderstandings on my side.

Thank you
Stefan

PS:
“get <average/delta/deviation> of item XX state since YY” has already been amended by me, see above

1 Like

Hi Stefan, no offence taken, I do appreciate that.

Let me have a look if I can make it easier to pick up the proposed changes – makes it easier for others to comment as well .

I have updated the above post with pictures.

These are suggestions naturally and my feedback, and mostly bikeshedding. I leave it up to you if the changes make sense considering the effort involved etc.

Great, that definitely makes it much clearer and honestly this is almost no effort at all. I will post an update soon.

Here you go

and

Happy now? :wink:

Looks great! That was fast

One comment on the second one, we post state updates, not commands. Are you able to have different placeholders depending on the action, meaning

<send command> "command" to item <item MyItem> <post update> "state" to item <item MyItem>

It was my understanding that you wanted to replace ‘value’ by ‘state’, in which case ‘average value’ would become ‘average state’.
Nevermind though, this has been cleared up by @ssalonen’s comments and your changes :slight_smile:

Probably would be non-trivial, that’s why the original block had “value” as the placeholder, which I thought was a suitable generic term for both what you send as a command or a state update.
i.e. update the state using this string value & send a command with this string value (commands and states are actually typed in OH core).

1 Like

I agree Yannick, I just tried to find a way to check and change the shadow block dynamically but that turns out to be either not straight forward or not doable at all.

By the way, do you by chance have some time eventually to look at the merge request as I do not want to make that even bigger with new changes? Just asking… :relaxed:

1 Like

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.

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.
https://javascript.info/comparison#comparison-of-different-types

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)