Extending Blockly with new openHAB commands

ysc · November 21, 2021, 6:12pm

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.

ysc · November 21, 2021, 6:33pm

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.

stefan.hoehn · November 21, 2021, 6:49pm

I have a pretty simple approach here: I am using a variable name which is unlikely to be used and then I clear the map always before adding new value pairs. Sure, there could be unlikely situations where this could collide but I think this is very unlikely.

maniac103 · November 21, 2021, 7:52pm

With a lot of users using your code, even unlikely conditions are likely to happen in practice

stefan.hoehn · November 21, 2021, 8:33pm

Here’s the PR: [blockly] fix audio block code generation by stefan-hoehn · Pull Request #1211 · openhab/openhab-webui · GitHub

rlkoshak · November 22, 2021, 2:59pm

Well, all rules can bypass the whitelist using executeCommandLine. If an attacker has enough knowledge to know this, they have enough knowledge to choose Rules DSL or JavaScript and use a Script Action instead of using Blockly. So I don’t think leaving it out of Blockly adds much security while removing an important capability. Also, making the Blockly implementation more secure won’t do much for security either unless all the scripts also implement something similar.

The discussion occurred back when the white list was first implemented for the Exec binding. I submitted a security issue and I don’t remember who all was on that thread. Since it was a security issue it want’ a public issue unfortunately so I can’t provide a link (can’t even find it right now)

rlkoshak · November 22, 2021, 4:08pm

Something just came up in another thread.

Is it true that we don’t yet have access to event so that we can access the implicit variables like event.itemName, event.itemState and such?

If not that would be pretty high up on my list of priorities. I tried to get at the event variable somehow through Blockly without editing the code manually but couldn’t figure out a way to do it.

stefan.hoehn · November 22, 2021, 7:27pm

That is actually pretty easy to implement. How about calling it “currentItem” and adding it next to the item in the item section? I am not sure if event-item is an intuitive name but you may convince me

rlkoshak · November 22, 2021, 7:36pm

OK, but there are a lot of variables that could be carried by event. There in lies some complexity. See Rules | openHAB for all the variables and when they are available based on how the rule was triggered.

Variable	Rules DSL Equivalent	Contents
`event.itemName`	`triggeringItemName`	name of the Item that triggered the rule
`event.itemState`	`newState`	the state of the Item that triggered the rule
`evenyt.oldState`	`previousState`	the state the Item had before the event that triggered the rule
`events.itemCommand`	`receivedCommand`	the command that triggered the rule
`event.event`	`receivedEvent`	information about the Channel Event that triggered the rule

There is more stuff in there but those are the major ones.

To be intuitive it might be worth while sticking with the Rules DSL names for these.

Since they are variables related to the triggering of the rule, I would expect them to be in the variables section or in a separate “Implicit Variables” section under openHAB. Maybe there could be a single “event” variable and a selector to choose which of the above you want to use. That would reduce the proliferation of blocks down to just the one.

stefan.hoehn · November 22, 2021, 7:45pm

Ok, this actually more work. I thought it would have been just the currentItem that triggered the rule. This needs have more thought how to design it but still not a big deal. I will put it onto the list for the next topic to be done.

rlkoshak · November 22, 2021, 7:58pm

Oh, two more things that cropped up today in a couple of different threads.

String.splt doesn’t seem to be implemented as a block under Text. That would return a value that could be saved to a variable that could be processed by all the List operators. That’s a pretty useful way to write more generic rules and doing some very simple text parsing and processing.
String.contains is another very useful test to perform on a String. For example: if(mySring.contains("Foo")){. So the myString.contains would be a block that returns a boolean which can be plugged into a condition under Logic.

These are pretty low level one liners which shouldn’t be too hard to implement I would hope. But the forum is littered with code that uses them so having them would help users write Blockly code that does similar things more easily.

stefan.hoehn · November 22, 2021, 8:37pm

regarding those requiremnts I would have been surprised if it hadn’t been implemented. It is kind of not too obvious though

What about this here?

and this?

Does that suffice?

rlkoshak · November 22, 2021, 8:52pm

Doh! That’s the one. That definitely implements split. I just didn’t recognize it when I looked through all the blocks.

Again, I completely missed that what that block did.

It looks like both String operations are indeed already implemented. That’s good news! I just need to learn how to read the blocks more carefully I suppose. And to think about where the right place to look is.

stefan.hoehn · November 22, 2021, 8:54pm

Finally something >I< can show you

stefan.hoehn · November 23, 2021, 6:44am

This is the block I am creating

This is how I do it and it returns a string

let scriptParameters = Blockly.JavaScript.valueToCode(block, 'parameters', Blockly.JavaScript.ORDER_ATOMIC)
console.log(typeof(scriptParameters))

it reveals “string”

And therefore it out each character into the map, which is not what we want.

 let scriptParameters = Blockly.JavaScript.valueToCode(block, 'parameters', Blockly.JavaScript.ORDER_ATOMIC)
    const myMap = scriptParameters
    Object.keys(myMap).forEach(function (key) {
      code += `  callscript_map.put('${key}', ${myMap[key]})\n`
    })

If we look into the string it turns out to be

{‘jonas’: ‘16’, ‘stefan’: ‘55’}

If I try to convert that string into a JSON Object with

const myMap = JSON.parse(scriptParameters)
or

const myMap = JSON.parse("{\'jonas\': \'16\', \'stefan\': \'55\'}")

I get

SyntaxError: Unexpected token ’ in JSON at position 1

However, if I converted the string by replacing the ’ with double quotes " like so

const workingMap = JSON.parse(‘{“jonas”: “16”, “stefan”: “55”}’)

then it works and I do not get this error. Like so

    let scriptParameters = Blockly.JavaScript.valueToCode(block, 'parameters', Blockly.JavaScript.ORDER_ATOMIC)
    const stringParametersMap = JSON.parse(scriptParameters.replace(/\'/g, '"'))

    Object.keys(stringParametersMap).forEach(function (key) {
      code += `  callscript_map.put('${key}', ${stringParametersMap[key]})\n`
    })

Do you have an idea why I get a string back and not an object as you had expected and secondly why I have to replace the single quotes by double quotes to make the JSON.parse work?

rlkoshak · November 23, 2021, 4:03pm

Do you need the escapes on the single quotes at all? Maybe that’s the problem as you don’t need them in that particular context and in fact it might be telling JSON.parse to not interpret them as ’ which means your key name isn’t in quotes at all which means it’s not valid JSON.

Your working example isn’t using escapes at all so I bet "{'jonas': '16', 'stefan': '55'}" would work too.

stefan.hoehn · November 23, 2021, 4:49pm

The escapes are only there because I added this in the source code as a string. They are not in the response of th component.

stefan.hoehn · November 23, 2021, 6:01pm

I tried using the caching like you see in the “var paramMapName = …”

let scriptname = Blockly.JavaScript.valueToCode(block, 'scriptname', Blockly.JavaScript.ORDER_ATOMIC)
    const stringParametersMap = JSON.parse(scriptParameters.replace(/\'/g, '"'))

    var paramMapName = Blockly.JavaScript.provideFunction_(
      'scriptParamsMap',
      ['var ' + Blockly.JavaScript.FUNCTION_NAME_PLACEHOLDER_ + ' = new java.util.HashMap();'])

    let code = `  ${paramMapName}.clear();\n`
    Object.keys(stringParametersMap).forEach(function (key) {
      code += `  ${paramMapName}.put('${key}', ${stringParametersMap[key]})\n`
    })

    code += `ruleManager.runNow(${scriptname}, true, ${paramMapName});\n`

I would have hoped that in case I use the block twice that the return value from the second block code generation would return a new string but it is always the same and of course it makes sense because in other areas I exactly use it for the purpose it only insert that code ONCE intentionally. I read the linked several times but I just don’t understand how I can derive different variable names in case whenever I use a new block.

btw, thanks for merging the bugfix so quickly!

ysc · November 24, 2021, 2:37pm

Here’s a correct (I hope, not tested at all) implementation of the oh_callscript block, hope it answers your questions:

  Blockly.JavaScript['oh_callscript'] = function (block) {
    const ruleManager = addOSGiService('ruleManager', 'org.openhab.core.automation.RuleManager')
    const parameterDict = Blockly.JavaScript.valueToCode(block, 'parameters', Blockly.JavaScript.ORDER_ATOMIC)
    const scriptName = Blockly.JavaScript.valueToCode(block, 'scriptname', Blockly.JavaScript.ORDER_ATOMIC)

    const convertDictionaryToHashMap = Blockly.JavaScript.provideFunction_(
      'convertDictionaryToHashMap',
      [
        'function ' + Blockly.JavaScript.FUNCTION_NAME_PLACEHOLDER_ + ' (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;',
        '}'
      ])

    return `${ruleManager}.runNow(${scriptName}, true, ${convertDictionaryToHashMap}(${parameterDict}));\n`
  }

...

  function addOSGiService (serviceName, serviceClass) {
    const addServiceName = Blockly.JavaScript.provideFunction_(
      'addFrameworkService', [
        'function ' + Blockly.JavaScript.FUNCTION_NAME_PLACEHOLDER_ + ' (serviceClass) {',
        '  var bundleContext = Java.type(\'org.osgi.framework.FrameworkUtil\').getBundle(scriptExtension.class).getBundleContext();',
        '  var serviceReference = bundleContext.getServiceReference(serviceClass);',
        '  return bundleContext.getService(serviceReference);',
        '}'
      ])

    return Blockly.JavaScript.provideFunction_(
      serviceName,
      [`var ${Blockly.JavaScript.FUNCTION_NAME_PLACEHOLDER_} = ${addServiceName}('${serviceClass}');`])
  }

in particular:

if you ever have to define a temporary global variable for the block (not to be reused), then you must get an unique name with Blockly.JavaScript.variableDB_.getDistinctName:

const listVar = Blockly.JavaScript.variableDB_.getDistinctName(
    'temp_list', Blockly.Variables.NAME_TYPE);
let code = 'var ' + listVar + ' = ' + arg0 + ';\n';

if you use Blockly.JavaScript.provideFunction_ to provide a reusable function (or global variable) then you must use the return value to get the actual name, which may change if the user has defined a variable with the same name (note that it’s not done at all in the current code and that would have to be fixed):

const functionName = Blockly.JavaScript.provideFunction_(
    'list_lastElement',
    [ 'function ' + Blockly.JavaScript.FUNCTION_NAME_PLACEHOLDER_ + '(aList) {',
      '  // Return the last element of a list.',
      '  return aList[aList.length - 1];',
      '}']);
// Generate the function call for this block.
var code = functionName + '(' + arg0 + ')';

you misunderstood the example I gave you to convert to dictionary to a HashMap, it was not meant to put in the code generation routine, but part of the actual code to be generated. You rarely have to analyze the inputs themselves, valueToCode returns a string because that’s code that you have to insert in the final code somewhere.

NB, that addOSGiService function above could be exported from another file and imported because it’s likely that it will be used in other blocks.

stefan.hoehn · November 24, 2021, 2:53pm

Thanks, appreciate it. I will follow up soon. Did you see already my PR as I have mostly implemented everything except the caching I struggled with. The code generation is done.