Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Blockly: Upgrade to v9, add JSScripting (GraalVM) implementations, UoM block types #1617

Merged
merged 40 commits into from
Jan 22, 2023

Conversation

stefan-hoehn
Copy link
Contributor

@stefan-hoehn stefan-hoehn commented Dec 30, 2022

Preface

As this goes mainly to you, Yannick (@ghys) , Florian (@florian-h05) and I need to apologise that this PR has become huge which is of course a challenge to review and definitely needs time to review and test. For us it was the only way to really approach it in a quality driven and effective way. We both joint forces and intensively worked together on that topic for the last two weeks and didn't even know where it would drive is. Basically, we then pushed it forward as much as possible as we also had to learn what would work and what not which in the end has led to that PR, though it already has been tested thoroughly both for Nashorn and JSScripting (I even added a lot of "tests" to the repository whiche I had never done before).

Description

The intent of this PR is to provide a new implementation that creates Blockly code based on the latest ECMAScript and the latest JSScripting library that is provided alongside with the GraalVM which is introduced for openHAB 4.0. Our most important goal was not to break anything for the current Nashorn-based Blockly rules nor when moving on to the JSScripting library and that goal was achieved. We were able to provide a Blockly code generation mechanism that is able to detect whether it generates code for Nashorn or JSScripting, which even allows users to gradually migrate one rule after the other let both rule-"scripttypes" run alongside until finally everything is migrated.

The way to migrate a rule is to just chose a new script type (see #1601) and then once save the rule and then the rule is automatically migrated to JSScripting (it is also possible to just change the type in the code tab of the rule and re-save).

The PR is based on Blockly 9.2. We decided to do that jump as part of that PR because I had to test ALL blocks for Nashorn and GraalVM/JSScripting intensively, so it made sense to test this in one step (which went pretty smoothly).

The new implementation also allows us to provide new blocks based on JSScripting that are not able with Nashorn or even with an additional features for these blocks which are not available for Nashorn (e.g. the store value block exists for both but with JSScripting it offers the choice of a private or public cache) which may incentify our users to migrate more quickly to openHAB to 4.0 and the new blocks.

New Features

  • full block support for Nashorn and GraalVM/JSScripting at the same time
  • Blockly 9.2 Upgrade
  • Quantity / Unit of Measurement support for all OpenHAB units (GraalVM/JSScripting only)

image

(this goes along with several already merged PRs of @florian-h05 who implemented the UoM support in JSScripting)
  • Support for Private & Shared Cache storage of Values (GraalVM/JSScripting only)

image

Dependencies
Depends on #1613.
Depends on #1601
Fixes #1309

@stefan-hoehn stefan-hoehn requested a review from a team as a code owner December 30, 2022 12:13
@relativeci
Copy link

relativeci bot commented Dec 30, 2022

Job #730: Bundle Size — 15.97MiB (-0.03%).

043fc2e(current) vs 0c193b9 main#729(baseline)

Metrics (3 changes)
                 Current
Job #730
     Baseline
Job #729
Initial JS 1.73MiB 1.73MiB
Initial CSS 608.52KiB 608.52KiB
Cache Invalidation 90.48% 90.48%
Chunks 218 218
Assets 688 688
Modules 2009(+0.1%) 2007
Duplicate Modules 108 108
Duplicate Code 1.77%(-1.67%) 1.8%
Packages 135(+1.5%) 133
Duplicate Packages 15 15
Total size by type (2 changes)
                 Current
Job #730
     Baseline
Job #729
CSS 856.56KiB 856.56KiB
Fonts 1.08MiB 1.08MiB
HTML 1.23KiB 1.23KiB
IMG 140.74KiB 140.74KiB
JS 9.01MiB (-0.35%) 9.04MiB
Media 295.6KiB 295.6KiB
Other 4.61MiB (+0.59%) 4.59MiB

View job #730 reportView main branch activity

@florian-h05
Copy link
Contributor

florian-h05 commented Dec 30, 2022

References #1597.

Thanks for the great summary 👍
This depends as well on a new openHAB-js version being released and included in the addon, I’ll take care of this.

I have to add that working on this PR also pushed the openHAB JavaScript library much forward as we (I) added support for UoM handling, ISO Date and Time string parsing and some other improvements.

And finally, I want to ping @rkoshak that his Christmas present arrived (although a bit too late, it is still this year).

@ghys
Copy link
Member

ghys commented Jan 3, 2023

Thanks, let me know when this is ready and I'll review it ASAP, so there's plenty of time to adjust.

Kudos for including some form of tests though I'm not completely convinced there's no way to build some automated ones with Jest - simply initializing the blockSource, calling the Blockly function that converts to code and check the result.
I'll try to write one eventually, if I succeed there's potential for many others.

@stefan-hoehn
Copy link
Contributor Author

Thanks, let me know when this is ready

It is actually ready to be reviewed. Thanks, Yannick.

I'm not completely convinced there's no way to build some automated ones

I actually agree on and eventually that would be nice that but I honestly I didn't feel like open up that topic as well 😄

florian-h05 and others added 20 commits January 22, 2023 08:47
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
…available

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Based on openhab#1170.

Also-by: Yannick Schaus <github@schaus.net>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Stefan Höhn <stefan@andreaundstefanhoehn.de>
… notification

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Rebasing onto upstream/main brought a new blockly update with additional blocks. This commit covers these additions if their categories were already covered.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Stefan Höhn <stefan.hoehn@nfon.com>
Signed-off-by: Stefan Höhn <stefan.hoehn@nfon.com>
Signed-off-by: Stefan Höhn <stefan.hoehn@nfon.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
…storage

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Stefan Höhn <stefan.hoehn@nfon.com>
Signed-off-by: Stefan Höhn <stefan.hoehn@nfon.com>
* Prepare for MIME type swap of GraalJS and NashornJS.
* Do NOT change the MIME type of existing Blockly scripts
* Default the MIME type to GraalJS on Blockly creation

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Stefan Höhn <stefan.hoehn@nfon.com>
Signed-off-by: Stefan Höhn <stefan.hoehn@nfon.com>
…ace obj

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
stefan-hoehn-nfon and others added 14 commits January 22, 2023 08:47
Signed-off-by: Stefan Höhn <stefan.hoehn@nfon.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Stefan Höhn <stefan.hoehn@nfon.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
…openhab-js

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Stefan Höhn <stefan.hoehn@nfon.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Stefan Höhn <stefan.hoehn@nfon.com>
Signed-off-by: Stefan Höhn <mail@stefanhoehn.com>
When changing the MIME type of Blockly, the editor did not notice that change and therefore code was generated for the old MIME type.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Stefan Höhn <mail@stefanhoehn.com>
Signed-off-by: Stefan Höhn <mail@stefanhoehn.com>
@ghys ghys force-pushed the blockly_jsscripting branch from 94977f0 to 1f06efe Compare January 22, 2023 07:47
@ghys
Copy link
Member

ghys commented Jan 22, 2023

It's great to be able to switch script engines with the list in the drawer. Maybe there could be a way to fire an event to regenerate the code on update, instead of having to hit ctrl-B twice to switch to the blockly view and back in the future.

Ok so I've tested, not exhaustively, with the following simple scripts:

image

(The "from/to cache" part disappears when the script type is changed to ECMAScript-5.1, nice)

JSScripting

var runtime = require('@runtime');
var itemRegistry = runtime.itemRegistry;
var events = runtime.events;

console.info(items.getItem('NewItem'));
cache.private.put('key', 'value');
console.info((cache.private.get('key')));

Nashorn

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

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


logger.info(itemRegistry.getItem('NewItem'));
this.storedValues['key'] = 'value';
logger.info((this.storedValues['key']));

image

JSScripting

var runtime = require('@runtime');
var itemRegistry = runtime.itemRegistry;
var events = runtime.events;

rules.runRule('ruleUID', {});

Nashorn

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;
}


ruleManager.runNow('ruleUID', true, convertDictionaryToHashMap({}));

image

JSScripting

var runtime = require('@runtime');
var itemRegistry = runtime.itemRegistry;
var events = runtime.events;

console.info((actions.Ephemeris.isWeekend(0)));
console.info((Quantity('100 W').equal(Quantity('0.1 kW'))));
console.info((Quantity('21 °C').add(Quantity('79 °F'))));
actions.NotificationAction.sendLogNotification('message', 'temperature_hot', 'info');

Nashorn

image

(could probably use a nicer message, like the one appearing in the console:)

image

Without the UoM blocks:

var ephemeris = Java.type("org.openhab.core.model.script.actions.Ephemeris");

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

var notifications = Java.type('org.openhab.io.openhabcloud.NotificationAction');


logger.info((ephemeris.isWeekend(0)));
notifications.sendLogNotification('message', 'temperature_hot', 'info');

image

JSScripting

var runtime = require('@runtime');
var itemRegistry = runtime.itemRegistry;
var events = runtime.events;

if (cache.private.exists('MyTimer') === false || cache.private.get('MyTimer').hasTerminated()) {
  cache.private.put('MyTimer', actions.ScriptExecution.createTimer('MyTimer', time.ZonedDateTime.now().plusSeconds(10), function () {
    console.info(time.ZonedDateTime.now().plusSeconds(10));
    cache.private.remove('MyTimer');
  }));
} else {
  cache.private.get('MyTimer').reschedule(time.ZonedDateTime.now().plusSeconds(10));
};

Nashorn

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

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

function getZonedDateTime(datetime) {
  datetime = String(datetime).replace('T', ' ')
  var regex_time_min =         /^\d{2}:\d{2}$/;
  var regex_time_sec =         /^\d{2}:\d{2}:\d{2}$/;
  var regex_date =             /^\d{4}-\d{2}-\d{2}$/;
  var regex_date_time_min =    /^\d{4}-\d{2}-\d{2} \d{2}:\d{2}$/;
  var regex_date_time_sec =    /^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}$/;
  var regex_date_time_sec_tz = /^\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2}[+-]\d{2}:\d{2}$/;
  var regex_date_time_ms =     /^\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2}\.\d{3}$/;
  var regex_date_time_us =     /^\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2}\.\d{6}$/;
  var regex_date_time_ms_tz =  /^\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2}\.\d{3}[+-]\d{2}:\d{2}$/;
  var regex_date_time_us_tz =  /^\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2}\.\d{6}[+-]\d{2}:\d{2}$/;
  var regex_oh =               /^\d{4}-\d{2}-\d{2}[T ]\d{2}:\d{2}:\d{2}\.\d{3}[+-]\d{4}$/;
  var now = zdt.now();
  var now_year = now.getYear();
  var now_month = now.getMonthValue();
  var now_day = now.getDayOfMonth();
  var today = '' + now_year;
  today += '-' + ('0' + now_month).slice(-2);
  today += '-' + ('0' + now_day).slice(-2)+' ';
  switch (true) {
    case regex_time_min.test(datetime): return zdt.parse(today + datetime + ':00+00:00', dtf.ofPattern('yyyy-MM-dd HH:mm:ssz'));
    case regex_time_sec.test(datetime): return zdt.parse(today + datetime + '+00:00', dtf.ofPattern('yyyy-MM-dd HH:mm:ssz'));
    case regex_date.test(datetime): return zdt.parse(datetime + ' 00:00:00+00:00', dtf.ofPattern('yyyy-MM-dd HH:mm:ssz'));
    case regex_date_time_min.test(datetime): return zdt.parse(datetime + ':00+00:00', dtf.ofPattern('yyyy-MM-dd HH:mm:ssz'));
    case regex_date_time_sec.test(datetime): return zdt.parse(datetime + '+00:00', dtf.ofPattern('yyyy-MM-dd HH:mm:ssz'));
    case regex_date_time_sec_tz.test(datetime): return zdt.parse(datetime, dtf.ofPattern('yyyy-MM-dd HH:mm:ssz'));
    case regex_date_time_ms.test(datetime): return zdt.parse(datetime + ' +00:00', dtf.ofPattern('yyyy-MM-dd HH:mm:ss.SSS z'));
    case regex_date_time_us.test(datetime): return zdt.parse(datetime + ' +00:00', dtf.ofPattern('yyyy-MM-dd HH:mm:ss.SSSSSS z'));
    case regex_date_time_ms_tz.test(datetime): return zdt.parse(datetime, dtf.ofPattern('yyyy-MM-dd HH:mm:ss.SSSSz'));
    case regex_date_time_us_tz.test(datetime): return zdt.parse(datetime, dtf.ofPattern('yyyy-MM-dd HH:mm:ss.SSSSSSSz'));
    case regex_oh.test(datetime): return zdt.parse(datetime.slice(0,26) + ':' + datetime.slice(26,28), dtf.ofPattern('yyyy-MM-dd HH:mm:ss.SSSSz'));
    default: return zdt.parse(datetime);
  }
}

function createZonedDateTime(year, month, day, hour, minute, second, nano, offsetString, timezoneString) {
  stringToParse = '' + year;
  stringToParse += '-' + ('0' + month).slice(-2);
  stringToParse += '-' + ('0' + day).slice(-2);
  stringToParse += 'T' + ('0' + hour).slice(-2);
  stringToParse += ':' + ('0' + minute).slice(-2);
  stringToParse += ':' + ('0' + second).slice(-2);
  stringToParse += '.' + nano + offsetString + '[' + timezoneString + ']';
  return stringToParse;
}

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

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

var scriptExecution = Java.type('org.openhab.core.model.script.actions.ScriptExecution');


if (typeof this.timers['MyTimer'] === 'undefined' || this.timers['MyTimer'].hasTerminated()) {
  this.timers['MyTimer'] = scriptExecution.createTimer(zdt.now().plusSeconds(10), function () {
    logger.info(zdt.now().plusSeconds(10));
    })
} else {
  this.timers['MyTimer'].reschedule(zdt.now().plusSeconds(10));
}

Note that it gives slightly different results, the precision/timezone is different, shows the differences between the vanilla ZonedDateTime and the JS implementation (no region/timezone configured):

08:53:24.146 [INFO ] [org.openhab.rule.b4                  ] - 2023-01-22T08:53:34.134283224Z[Europe/London]
08:53:55.309 [INFO ] [org.openhab.automation.script.ui.b4  ] - 2023-01-22T08:54:05.280Z[SYSTEM]

One thing I have to say: I think the benefits of having the openhab-js library available for use are pretty clear! 🙂 The code is really lighter & nicer.

Copy link
Member

@ghys ghys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @stefan-hoehn and @florian-h05! It's pretty clear there was a lot of effort involved.

I still think some unit tests would be valuable but this can be postponed to another PR.
I'll admit I would have implemented this slightly differently, separating the 2 implementations completely (I don't really mind the duplicated code) in different folders, instead of carrying a isGraalJs flag over - but this implementation is perfectly fine too.

The things that probably remains are the block libraries, they have facilities to inject framework services or Java types etc., not sure how they work (if at all) in JSScripting - a lot of block libraries are for calling thing actions, which might not even require that.
So there would be a need to alter/augment the YAML specification for these libraries, to eventually allow the author to provide code templates for both implementations (JSScripting & Nashorn).
This can be taken care of later, but relatively soon to have the opportunity to change the libraries currently in the marketplace.

For now, I think we can safely merge this PR as I've not been able to fault it, and fix what needs to be fixed as we go.

Thanks a lot!

@ghys ghys merged commit 4dfb912 into openhab:main Jan 22, 2023
@ghys ghys changed the title Blockly goes JSScripting and GraalVM Blockly: Upgrade to v9, add JSScripting (GraalVM) implementations, UoM block types Jan 22, 2023
@ghys ghys added enhancement New feature or request main ui Main UI labels Jan 22, 2023
@ghys ghys added this to the 4.0 milestone Jan 22, 2023
@florian-h05 florian-h05 deleted the blockly_jsscripting branch January 22, 2023 10:37
@florian-h05
Copy link
Contributor

florian-h05 commented Jan 22, 2023

Maybe there could be a way to fire an event to regenerate the code on update, instead of having to hit ctrl-B twice to switch to the blockly view and back in the future.

This could be implemented for sure 👍

Units of Measurements Blocks

[Nashorn] could probably use a nicer message, like the one appearing in the console:)

I‘m wondering what‘s the underlying issue of this message, will have to take a look.
I‘m not sure how/if we can change the message, I have two other solutions in mind:

  • Hide UoM blocks on Nashorn
  • Generate comments that say „UoM blocks are not supported on Nashorn“ instead of having such an issue

Time based stuff

Note that it gives slightly different results, the precision/timezone is different, shows the differences between the vanilla ZonedDateTime and the JS implementation (no region/timezone configured)

We‘ve discussed the thing with the time zone and came to conclusion, that it‘s no problem.
Regarding the precision: Timers in JS Scripting (GraalJS) give no guarantee to run on the exact time, since we have to synchronise their execution onto a Lock, this behaviour is comparable to Node‘s event loop. But I guess those millis (or nanos?) don’t matter for our use cases :-)

One thing I have to say: I think the benefits of having the openhab-js library available for use are pretty clear! 🙂 The code is really lighter & nicer.

Yes 😊
It will become even cleaner once I wrap some of the actions that require PercentType, we can then even remove the prepend code that is added to every GraalJS Blockly script. See openhab/openhab-js#98 (comment).

The things that probably remains are the block libraries, they have facilities to inject framework services or Java types etc., not sure how they work (if at all) in JSScripting - a lot of block libraries are for calling thing actions, which might not even require that.

Injecting Java types using Java.type works in GraalJS as well, but the framework services aren’t available by default. I can tell what works with Graal, but I lack the knowledge of how the Blockly libraries work to help fixing them.

Thanks for reviewing and merging, Yannick! This was a big PR.

I have to add one remaining to-do:

  • Update the UoM comparison Blocks for breaking changes in the library (not released yet).

@ghys
Copy link
Member

ghys commented Jan 22, 2023

I‘m wondering what‘s the underlying issue of this message, will have to take a look.
I‘m not sure how/if we can change the message

Could be as simple as throwing errors instead of logging warnings here:

console.warn('arithmetic function unit of measurement only supported with jsscripting')

they will normally be caught when saving here:

if (this.isBlockly) {
try {
this.currentModule.configuration.blockSource = this.$refs.blocklyEditor.getBlocks()
this.script = this.$refs.blocklyEditor.getCode()
} catch (e) {
this.$f7.dialog.alert(e)
return Promise.reject(e)
}
}

and the message will be displayed instead of "undefined".

@florian-h05
Copy link
Contributor

Thanks for the tip.

Maybe there could be a way to fire an event to regenerate the code on update, instead of having to hit ctrl-B twice to switch to the blockly view and back in the future.

I am wondering what‘s the benefit from this. If you run a Blockly script from the editor, it‘s code is regenerated and saved before it is run. And if you switch to the source code tab, the code is regenerated as well. Not sure if you was aware of this.

@florian-h05
Copy link
Contributor

@ghys See PR #1657 for the UoM message and some library breaking change I have to adjust to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request main ui Main UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[blockly] Add historicState option to Persistence blocks
4 participants