I had this idea to combine “today” and “today +/- x day(s)” into one, and eventually get rid of the block (just implement the conditional visibility into the final blocks), but it’s not mandatory.
As for the color, no strong opinion, as long as it’s significantly different than the other colors currently in use, I think the black is fine.
Next feature is working 
as defined in Actions Subsystem
with the script in the script folder
being
logInfo(“helloScript”,“hello script”)
resulting into
2021-11-16 22:55:09.976 [INFO ] [penhab.core.model.script.helloScript] - hello script
Question: would there be any way to call a script as defined in the UI? Probably not
Calling “helloWorld” doesn’t find it as it really seems to look for a file.
Question: Does it make sense to support the transform function?
transform(String type, String function, String value)
If yes, can you provide an example that I could easily try out for testing
Question: This blockly feels a bit lonesome here 
Should we put it into some other category?
The other option we have is to add the
- transform
- and maybe the exec function ?
Regarding the exec function
- Does it require the exec binding?
- Do I have to allow the path for the exec binding to be allowed to accessed? (I think this was added for security reasons on M2 or so)
Yes but it’s just a regular rule so it would be like I posted up in post 103.
A “Script” in the UI is just a rule with a single script action tagged with “Script”. It’s actually a little confusing in that regard which, now that you’ve implemented it I wonder if this is going to cause some confusion.
Honestly the overloaded “Script” term is already causing confusion among users. Maybe if we change this action to “call script file” that might help. It makes it a bit more clear that we are calling a .script file, not a rule under the Script category in the UI.
As you found in your experiments, they are really two very different things.
Absolutely!
It’s going to be a little different based on which transformation you’re calling. For examples:
vat Transformation = Java.type("org.openhab.core.transform.actions.Transformation");
var map = Transformation.transform("MAP", "MyFile.map", "Some sting, maybe Item's state"); // Calls the map transformation
var regex = Transformation.transform("REGEX", ".*Begin(.*)End.*", "Some sting, maybe Item's state"); // Calls the REGEX transformation
var jsonpath = Transformation.transform("JSONPATH", "$.foo.bar", "Some JSON string with foo and bar elements, maybe Item's state");
It always takes three Strings, the first is the transformation to apply. The second is the argument to the transformation, such as the file to use or some other string telling the transformation what to look for. The last argument is the string that the transformation will transform.
It gets tricky because the first argument can only be one of the currently installed transformation add-ons. The second argument wholly depends on what the first argument is. So I’m not entirely certain if you can do much to help the user beyond just letting them entry arbitrary strings for the first two arguments. But if there were a way to provide just the list of installed transformation for the first argument and then a sort of builder for the second argument that would be fantastic, but I’m certain well beyond what’s possible.
I would expect “call rule” to go here as well. But in case I’d expect the category to become Scripts and Rules. I can also see executeCommandLine here too. I big gotcha with that one though is can you do a function with an arbitrary number of arguments?
Maybe to avoid confusion between executeCommandLine maybe we should name the block you’ve already built “call openhab script file” and then executeCommandLine can be something like “execute command line”.
An argument could be made that transform goes here too. In all these cases you are calling out to something outside of the current rule.
No, it’s a core Action available by default on all openHAB instances.
No, the whitelist doesn’t apply to executeCommandLine, for which I’m not super happy with as I think it too should be protected in the same way. Either the authorization built into OH 3 is sufficient for both the add-on and Action or it’s not. But I lost that argument.
So you are OK with calling executeCommandLine without any other external dependencies.
1 Like
I renamed the original block to “Call Script File”
Another good news. Even though I was worried about the complex code I got it working pretty fast (thanks to your perfect example) and I also support variables already.
This is how the blocks look like:
This is the code I create for Call-Script Block
var FrameworkUtil = Java.type('org.osgi.framework.FrameworkUtil');
var _bundle = FrameworkUtil.getBundle(scriptExtension.class);
var bundle_context = _bundle.getBundleContext()
var ruleManager_Ref = bundle_context.getServiceReference('org.openhab.core.automation.RuleManager');
var ruleManager = bundle_context.getService(ruleManager_Ref);
var map = new java.util.HashMap();
map.clear();
map.put('key','value');
map.put('key2','value2');
ruleManager.runNow('helloWorld', true, map);
and the script name “helloWorld” is actually called. The code of the script looks like
var logger = Java.type('org.slf4j.LoggerFactory').getLogger('org.openhab.rule.' + ctx.ruleUID);
logger.info('helloWorld was called')
logger.info('Param='+context.getAttribute('key'))
logger.info('Param='+context.getAttribute('key2'))
And it logs out
[INFO ] [org.openhab.rule.helloWorld ] - helloWorld was called
[INFO ] [org.openhab.rule.helloWorld ] - Param=value
[INFO ] [org.openhab.rule.helloWorld ] - Param=value2
So this is a really cool block now.
- Do you by chance now if this is synchronous call ? to me it appears it is
- The question is also if we should support one block with and one block without params. Otherwise it would look like this (doing something like the gear icon of the if-else block from blockly is probably pretty tough for me at the moment)
Now on to the transform function… (I have to check the rest api to see if I can retrieve the transformation from the backend)
I do not know the answer to that. I would assume it is but I never really looked into it.
That’s a stylistic question I don’t have a strong opinion about. We have to weight the cost of the block looking “weird” with the empty subelement against the cost of adding another block. Neither approach bothers me too much.
You GET on /addons to get a JSON with all the possible add-ons listed. Those that are installed have an “installed: true” property. The ID for all the transformation services is prepended with “transformation-”, for example “transformation-regex” for the REGEX Transformation add-on.
Here is the entry for the JavaScript transformation in my system.
{
"id": "transformation-javascript",
"label": "Javascript Transformation",
"version": "3.2.0.M4",
"contentType": "application/vnd.openhab.feature;type=karaf",
"link": "https://www.openhab.org/addons/transformations/javascript/",
"author": "openHAB",
"verifiedAuthor": true,
"installed": true,
"type": "transformation",
"description": "",
"configDescriptionURI": "",
"keywords": "",
"countries": "",
"connection": ""
},
Unfortunately though that doesn’t give you the information needed to actually use it in a rule with the tranform Action. 
For example, to call the the JavaScript transformation one needs to use “JS” for the first argument. That bit of information is not captured by the REST API call.
Ok, thanks, then wouldn’t probably put the effort into retrieving the transformation from the backend but just provide the three strings.
@ysc
Do you have an preference on the “empty” statement block? I would actually just provide one with and one without params or do you know an easy way? The only way I could think of would be a mutator but then we would have to provide a drop down that would select type like “with params / without params” which also isn’t so nice.
Type Checking on the inner block
I wanted to make sure that only my paramBlocks can be put into the “statementSection” (though this can be questioned actually - see below).
So I did
Blockly.Blocks['oh_callscript'] = {
init: function () {
this.appendValueInput('scriptname')
.setCheck('String')
.appendField('Call Script')
this.appendStatementInput("parameters")
.setCheck('ScriptParam');
this.setInputsInline(true)
}
}
and in the param-block I did
Blockly.Blocks['oh_scriptparam'] = {
init: function () {
this.appendValueInput('paramName')
.appendField('set param name')
this.appendValueInput('value')
.appendField('to')
.setAlign(Blockly.ALIGN_RIGHT)
.setCheck('String')
this.setInputsInline(true)
this.setOutput(true, 'ScriptParam')
}
However, it seems the
this.setOutput(true, ‘ScriptParam’)
isn’t allowed here because blockly throws an error say
Remove output connection prior to adding previous connection.:undefined
Here is blocklies code
Blockly.Block.prototype.setOutput = function(newBoolean, opt_check) {
if (newBoolean) {
if (opt_check === undefined) {
opt_check = null;
}
if (!this.outputConnection) {
if (this.previousConnection) {
throw Error('Remove previous connection prior to adding output ' +
'connection.');
}
this.outputConnection = this.makeConnection_(Blockly.OUTPUT_VALUE);
}
this.outputConnection.setCheck(opt_check);
} else {
if (this.outputConnection) {
if (this.outputConnection.isConnected()) {
throw Error('Must disconnect output value before removing connection.');
}
this.outputConnection.dispose();
this.outputConnection = null;
}
}
};
Does that tell you anything? I didn’t find anything nor does I understand the blockly code here.
btw, the
this.appendStatementInput(“parameters”)
.setCheck(‘ScriptParam’);
doesn’t prevent any other statement to be added which is weird.
However, I don’t think it is too bad. It would “even” allow to put any statement into the script call block before actually calling the block. It doesn’t therefore really hurt, I think:
results into
map.clear();
logger.error('abc');
map.put('key','value');
ruleManager.runNow('myscript', true, map);
So, probably we shouldn’t worry to much about restricting it to only the param block, should we?
Persistence
I will work on the persistence blocks after the transform. Should we use a dropdown for the different methods using ZonedTimeDate?
like so?
Note that the first two would result into a different return type (Boolean?) like the others (Value), so it might make sense to split these?
It actually becomes pretty interesting
| Attribute | Value
| ------------- | -----------
| changedSince | true/false
| updatedSince | true/false
| maximumSince | org.openhab.core.persistence.extensions.PersistenceExtensions$1@accc66
| minimumSince | 19/11/2021, 01:00: F2_Office_Main_Light -> OFF
| averageSince | 0.5107299806325430
| deltaSince | 1
| evolutionRate | null
| deviationSince| 0.4998657671823248
| varianceSince | 0.2498561628629385
Looking at the code it reveals the minimumSince and maximumSince actually should a HistoricItem which itself has a timestamp, the currentState and the item’s name. It is strange that the maximum returns a reference and the minimum a string representation even though it is of the same type.
Checking the class-name MinimumSince return an RRD4jItem and MaximumSince returns the inner class PersistenceExtensions$1. So it seems these two are not really useful. I wonder how you use them in the UI? What would we use these two for anyway? Can we leave them out or do you have an idea?
This works nicely already
generating
var Transformation = Java.type("org.openhab.core.transform.actions.Transformation");
var map = Transformation.transform("MAP", "nanoleaf.map", "hs");
and this
generating
var jsonpath2= Transformation.transform("JSONPATH", "$.device.status.temperature", '{ "device": { "location": "Outside", "status": { "temperature": 23.2 }}}');
and this
resulting into
logger.info(transformation.transform('REGEX', '.*(\\snot).*', 'My network does not work.'));
note that the \ are doubled to make sure the are escaped.
Are you fine with that block idea?
This concludes the script section:
I can’t think of any better way to do it. It looks really good. I’m looking forward to this more complete Blockly implementation to go live. I think a lot of users will be very happy.
3 Likes
I do
I was able to make the help url and the tooltip context sensitive to the choice of the dropdown:
so the tooltip tells you what to provide and context menu link jumps to the right help page of openHAB.
1 Like
Completely agree with you on this one - I don’t remember being involved in this discussion - and imho even touching files on the filesystem outside a sandbox should be restricted in scripts. After all if you end up being able to write to the whitelist file, what’s the point in having it in the first place…
I think executing commands is kind of touchy and if Blockly allows you to ease circumventing the whitelist of the Exec binding/transformation then we should leave it out, for now. Users can configure Exec things separately and use an item to launch the (shell) command with Blockly in a safer manner because of the whitelist.
Maybe there is something to be done still, similar to functions, where you have the mutator to define what the parameters are:
and then you can call the block and the parameters you defined can be assigned values:
In the Functions’ case, these are two separate blocks but in the “call script” all would be handled within the same block.
The implementation is here: https://github.com/google/blockly/blob/master/blocks/procedures.js and it sure looks pretty complex… maybe start with one without parameters and see how to evolve it later?
To be honest I’m not too sure about the statements inside the call script blocks to set the parameters, because 1) you can add arbitrary statements inside this surrounding structure which makes little sense (as you mentioned) and 2) you can use the “set param name” blocks outside the call script blocks which makes little sense as well.
Actually, a cleaner and probably easier solution would be to offer blocks to manipulate generic key/value maps - similar to those for lists:
(just imagine “create empty key/value map”, “create map with…”, “in map {mapVariable} get key k”, “in map {mapVariable} set key k as v”, “in map {mapVariable} remove key k” instead of those above).
The implementations for the lists are in:
- https://github.com/google/blockly/blob/master/blocks/lists.js
- https://github.com/google/blockly/blob/master/generators/javascript/lists.js
Then the user can build the map above the “call script” block and pass it as an (optional) argument. You could even initialize it on the fly: “[call script helloWorld with parameters [create map with x=1, y=2, …]]”. Of course you would have a check that the passed parameters is indeed a map.
IMHO it reflects better the flow of the actual generated code. Note that the Blockly maps could be simple JS objects and the call script block would be in charge of transforming it into a java.util.Hashmap on the fly (with for...in or Object.keys().forEach() or similar).
And it could make sense to manipulate key/value maps outside the call script scenario as well.
I know I’m asking a lot so that’s why I suggested to first have the call script block without params first (I didn’t even know you could pass params, which is admittedly a very cool and powerful feature with a lot of potential) then figure out a solution that works best for the params; it would be an addition so likely backward compatible.
Side note - it is also possible apparently to disallow blocks outside a certain context, see:
Yes that’s something that I also noticed with the “get {name/label/tags/groups} of item…” block. Depending on the choice made in the dropdown the output type is different! (array of strings in tags/groups cases, string otherwise).
The “easy” choice is to not define an output type at all, but it’s not ideal because then you expect the user to figure it out - which is unfortunate if we consider Blockly’s target audience to be novices or those less proficient in programming.
I wouldn’t mind having multiple similar blocks based on the output type: boolean, historical value, decimal number, date (of last update)… but it’s not ideal either as it wasn’t done in the Items case.
The ideal solution IMO would be to update the output type of the block based on the choice in the dropdown. I’m sure there are examples of blocks in the standard Blockly code that does that, though I can’t find any right now. If you can update tooltips, you can almost certainly update output types as well.
I think the transformation part looks good at first sight - I’d still make the block “vertical” by default like those in the Voice & Multimedia category because I suspect the horizontal version would quickly become quite long. Remember the user can switch between the two with the context menu (“External Inputs”), I’m just talking about the default.
Your work is really appreciated @stefan.hoehn - thank you a lot!
Could I ask you however to submit PRs to GitHub as soon as possible, even with non-working code, and ideally not submit a monster PR with several features - the larger the PRs, the longer it’ll take to review them.
Based on your “executeCommandLine” discussion with Rich I would refrain from providing it as a block because I feel it would be to easy to cause bad things with easy to use blocks.
Thanks, yes, that was actually one of the question I wanted to ask how we should proceed because I noticed that the comments in github became pretty comprehensive. So if I understand you right I would probably do one PR for each category? Though I would want to discuss these first here and then do the PR for that one category.
Thanks for all the input. I am kind of thinking about what is more important: Getting things done or dive even deeper into the complexity of blocks. Mutators are actually pretty complex to be implemented (at least yet I haven’t understood them) so I am not sure I wanted to head there. Changing the URL and Tooltips was “rather” easy because I was lucky to find out the setTooltip() actually takes a function that can dynamically return something (similar URL).
Great that you pointed out that functions can take parameters. I DID NOT know this 
@rlkoshak I was obviously wrong about that in my statement somewhere above.
Maybe I’ll check the function implementation (even though I am sure it uses mutators). It definitely looks like a nice solution.
And yes, I’ll have a look at the list idea as well.
All in all good ideas, let me see how much time I find because I like to have ephemeris, scripts and persistence done by end of this year 
It never hurts to see even WIP code as we discuss the feature. And yes I believe making PRs as small & focused as possible helps (when there are no dependencies between them), if you wait for feature A, B & C to be ready before submitting the PR, and features A & C are okay to merge but there are still concerns with B, then you hold off A & C from becoming available sooner.
The main issue I saw in the the were the many commits I wasn’t able to merge and rebase (did I already mention I hate git 
) and the many comments there and yes, therefore I tried to keep them smaller.
As I suspected there are already discussions and partial implementations of dictionaries (key/value maps) in Blockly that we could maybe reuse or take inspiration from:
https://groups.google.com/g/blockly/c/sOBRz5d4LPg
dict_get and dict_create_with blocks for blockpy · GitHub (Python only implementation + needs a good cleanup + no license).
1 Like
this defines a block to define a dictionary with an arbitrary number of keys.
2 Likes
Wow,.I am amazed, Yannick. That really caught your curiosity? 
Kudos.
How can we get this reviewed quickly, so I can merge and use that?
AND: Being the ultimate git nerd, how do I merge this into my fork?
Like so?
git checkout main
git pull upstream main
Would that retrieve it to my local repo?
git fetch upstream && git rebase upstream/main (while you’re in your work branch) should do the trick.
With this all you need is an input with a check set to “Dictionary” and add the code to populate the Java HashMap from the dictionary (if it’s not null or empty, otherwise you could even omit it), something like the following:
var scriptParameters = <dictionary>;
Object.keys(scriptParameters).forEach(function (key) {
map.put(key, scriptParameters[key]);
});
You could even pre-populate an empty dictionary as a shadow block.
Then you don’t need neither the statements input in the call script block, nor the “set param name” block.
Works nicely. almost done (didn’t have much time today). However, you need to concert scriptParameters which is a String into a Json-Object first.
One another topic: I just noticed a bug in the audio block which can be fixed by changing one line. Should I do a PR with that single line fix so it will go out quickly for those who use that block?
You have to replace <dictionary> above with something like Blockly.JavaScript.valueToCode("the name of your input") - it will be something like { 'key1': 0, 'key2': 'test', ...} which is already an object.
Also be aware when you generate code with common variable names, like var map = ... it could collide with user-defined variables easily.
Sure.
this is actually nicely explained in the Blockly docs:
- use Blockly.JavaScript.variableDB_.getDistinctName to generate a variable name that will not collide
- when providing utility functions you should use Blockly.JavaScript.FUNCTION_NAME_PLACEHOLDER and use the return value to get the actual function name to call the function. It could be different if Blockly detected a name collision.
1 Like