Group-Handling with blockly

Another thing that could make it more clear would perhaps to rename the “get state of item” block to “get state from item named” or something even more clear than color. Color alone often is not sufficient for many users including those with color blindness. But don’t get me wrong, I like color too, just not only color.

That change in text alone makes the blocks a little more clear what is going wrong and why it failed when trying to implement it in the obvious way by linking the Item block to “get state of item”. Color would provide more help and some docs to wrap it all up would be nice to.

On-the-other-hand, it seems pretty clear and intuitive from the end user perspective to make at least that one “get state of item” block be able to handle both names and Item Objects.

NOTE: There was a brief discussion on adding some of this sort of type conversion stuff to the JS Scripting library so, when the time comes to migrate Blockly to JS Scripting perhaps some of what currently looks ugly will just become a function call.

As I said, type issues are the number one issue with rules development so I personally want to push as much of that as is feasible away from the end users.

For one example, I’ve a “todatetime” function in my libraries which will convert just about anything you pass to it into a ZonedDateTime. For example:

  • “3h2m” → now.plusHours(3).plusMinutes(2)
  • DateTimeType → getZonedDateTime()
  • Number, DecimalType → now.plusNanos( number * 10000)
  • JavaScript Date → equivalent ZonedDateTime

and so on. So most of the time one can call todatetime and pass it almost anything that makes sense as a date time or a duration and you’ll get a ZonedDateTime suitable for use in a timer. The end user never needs to care about types here. I’d like to see something like that added to the library as well as something similar done for calculations and comparisons that deal with DecimalTypes, Numbers, primitives and quantity types.

My point is any ugliness added for this stuff now in Blockly might be temporary.

That’s exactly the kind of synergy I hope we can achieve in the end, with the openhab-js and Blockly parallel breakthroughs in JS scripting ultimately profiting from each other.
As another example, openhab-js now includes js-joda which has the same API as Java to handle ZonedDateTime and the like, so ultimately the Blockly blocks wouldn’t have to instantiate Java’s ZonedDateTime but work with the JS classes provided by js-joda.

I agree renaming labels to try to avoid confusion could do wonders too.

1 Like

Sorry for that dump question (perhaps it is the late hour,) but where I can find the “create text” block from your rule?

Thank you!

@johannesbonn Enjoy the awesome screenshot:

Ah ok, I saw this block but I think there is a problem configure it with my iPad and Safari. I will give it a try on my PC.

Thank you!

So, regarding my Bugfix-Pullrequest, which I think we should get out of the door soon, can you guys tell me what (or if) I should change because I am really not sure what is expected now from me. I would really like to have that fixed in a fail safe way so our users do not run into issues (by sacrificing a bit of a code generation elegance). I am not hesitating to change something for a better paradigm in the future but I think we shouldn’t do that for a bug fix.

The other topic I like to be decided in general if we favor blockly-user-fail-safeness over code-generation complexity or do we see blockly’s code generation as a “perfect example” for people how to write openHAB Javascript code? My opinion is that we should of course write very good blockly code but code that is being generated will always be worse than can code written by humans nor is it intended to be (and that IMHO is a general statement for code generation without becoming too philosophical about it :wink: ) .

The main reason I favour fail-safeness of usage to me is what Rich already said: it will reduce a lot of our time in guiding the people what and what not do to to avoid errors as 90% of at least my target user group are not developers (besides the effort I currently put into documentation).

The other reason why I like this to be merged soon is that I have a couple of other changes waiting and due to my git-incompetence I do not like to work at two things at the same time…

@Andrew_Rowe opened another thread for a generic discussion on this topic. [Discussion] Blockly implementation in OH

From my perspective, is less about making the code foolproof and more about not violating expectations. More details on that thread.

To be you’ve several options:

  1. Make the “get state of item” work with both item names and item Objects

  2. Make the block that iterates over members if a Group map the list so it’s just a list of item names instead of Item Objects

  3. Change the name of both blocks to make it intuitively clear that get item state only works on Item names and when iterating over a Groups members you are getting the whole Item Object.

I personally like 1 or 2 better as it gives a better end user experience. I’m OK with 3. I’m not OK with doing nothing (not that it matters much what I’m OK with in the first place).

No matter what approach is taken, when migrating off of Nashorn it should be revisited as a lot of code complexity might be hidden inside openhab-js which could give us the best of both worlds.

re 1) I vote for (1) as it is being implemented as it is the most intuitive from the user perspective
re 2) If I get you right that would make the behaviour incompatible with what is currently happening and would also require quite some rework.
re 3) I would maybe make the block clearer but makes our blocks less versatile like (1) would do.

Don’t miss what I said last though. No matter what we do now, when we switch to JSScripting we can have 1 without the extra complicated code.

So wait and see might be better and go with clearer names for the blocks.

If we wait and see and do nothing for now, then we have the problem why the whole thread was started, haven’t we? Honestly, I was trying to help and now the discussion has cost me much more time than everything else which frustrates me. Maybe I should rollback the PR and someone else takes over. :cry:

I didn’t say do nothing. That’s not really acceptable in my mind because it doesn’t solve this problem.

I’m proposing we solve it but changing the names of the blocks to be more clear what’s going on.

When we move to JSScripting the whole problem goes away I think because if how the helper library already implements items. So even if we implement 1 or 2 more, it will have to change in the not to distant future anyway. Since it’s controversial, maybe it’s better to wait a little bit and revisit when we do move to JSScripting.

To state again, I’m not suggesting doing nothing. But I am suggesting we not spend too much time and stress over something we know will change soon anyway.

1 Like

We are actually discussing one line of code which looks a bit more complicated that allows the blocks to be much more resilient. Instead of doing that we would rename the block? I am against that but if the community thinks this is the better way if doing it, then please @ysc or anyone else take over and finalize the fix because it probably doesn’t make sense to explain me what should be done and then I do it but rather the ones who have the intended idea in mind finalize it. Thanks.

I would actually propose to take out the group handling fix discussed here from the mentioned PR because it blocks the other 2 fixes and if we ever come to a conclusion how to handle groups we can simply do one separate PR for that.

1 Like

I’ve not had time to reply fully here but I want to explain my position a bit further.

I completely agree that it’s just one line of code and it really does make a much better experience for the end user. Were I in charge of but hesitate to accept it as is.

However, we know that sometime soon we will be moving Blockly to JSScripting with openhab-js and when that happens then all of these blocks will need to be revisited and perhaps reimplemented.

Based on my own experience with JSScripting I’ve recently learned, I think this whole problem will simply go away on its own when that happens.

Given that, I just don’t think it’s worth fighting over this one line of code.

I do think it’s worth implementing the block to support but the item name and Item Object. I did think that the benefits to the end users fat out weights the cost of the slightly more complicated code.

I just don’t think it’s worth fighting for right now. Not when I’m a year or so from now (or how ever long it takes for JSScripting to become a default) the whole issue will need to be readdressed.

I’ve fought for so much in OH and lost so many battles I’ve learned to be patient and to pick my battles. This isn’t the ditch I want to die in. One the end I think it will all come out right now matter what we decide here.

So I’m ok with doing the bare minimum too address the op issue until a more permanent solution comes in JSScripting.

Ok, then let me ask maybe one closing thing: I have not heard a concrete alternative solution for the following problem from above:

Would you like me to document that as a solution in the blockly reference on how to officially use “get members of group”?

PS: Just for the record: I have asked the change I did to be taken out of the pull request. So nothing will happen regarding that topic and it will stay as is for the time being.

For me it’s ok. But I don’t know if that’s a good solution.

When will the switch to JSScripting take place?

@ysc Yannick I realize your stance on making the generated code clean so as to provide good example code for people trying to learn but…
I would like to add my (meaningless) vote that we consider making this small exception to that rational

I just bumped into this issue with a similar solution-in-mind as Christian, I wanted to check the battery status of my blinds. Thanks for the workarround!

I’m not sure if it’s clear from the discussion above. In the loop, this works:
image

But this doesn’t work:
image

On another note, I’m quite an experienced programmer but I’m currently converting my rules as much as possible to blockly. I don’t really care how the generated code looks like and actually expect to find it ‘sterile’, ‘heartless’, ‘more complicated then necessary’, in a way I would not write it. It might turn out to be useful later for some snippets. But then more in the general sense ‘how do I need to get from the output of this to the input of that’.
The biggest reason I’m converting my rules to blockly is to get my wife and kids onboard as well. A big thanks to you all for making this possible!

3 Likes

Thanks for providing this solution. I actually found this thread as a “Your topic is similar to …” suggestion as I was creating my own post about it.

I’m using openHAB3.3M2 on this installation, so whatever solutions are discussed above, they haven’t been implemented as of that release.

I have really enjoyed Blockly. I’m in the slow process of converting Rules I wrote in files using Rules DSL. Many of them were altered copy and pastes from the community from my early days when I had no idea what I was doing. I realize “if it ain’t broke, don’t fix it”, but Blockly makes things more intuitive and easier to improve or modfity.

This was my first experience where Blocky was not intuitive - where something that looked like it should work flat out didn’t. I’m glad I found this work around, which solved my problem.

(The only other frustration I have had with Blockly has been a few times where a discarded fragment ended up hidden behind the block, and I got some strange error messages. Sometimes the fragments are really well hidden.)

Thanks to all of you who have done so much to provide this new, powerful tool.

1 Like

I can explain the difference between the blocks to why this might be neither straight forward nor intuitive to understand (I should mention that possibly in the blockly docs)

  1. image

This block already existed for a very long time. It has always a block output of type String

  1. image

When I implemented this block it is kind of smart. It detects depending of what attribute you select the different output type - though state is String equal to the block above.

So it should behave differently nor generate a different code when choosing state.

What I don’t understand is what you mean with

In the loop, this works… but this doesn’t work:

can you give an example where the two blocks behave differently along with the code it creates, so I can investigate it?

If you use both blocks wit Items they behave the same, but if you use them with a variable they are different.

Blockly8

itemRegistry.getItem('MyItem').getState();

itemRegistry.getItem('MyItem').getState();

var i_list = Java.from(itemRegistry.getItem('MyItem').members);
for (var i_index in i_list) {
  i = i_list[i_index];
  logger.info(itemRegistry.getItem(i).getState());
  logger.info(i.getState());
}

The first block is checking the itemRegistry for “i” but the variable is not stored in the itemRegistry
The second block is, as you said “smart”, and is just checking “i” for it’s State.

This is the log

2022-03-13 17:24:55.084 [WARN ] [e.automation.internal.RuleEngineImpl] - Fail to execute action: 2

java.lang.RuntimeException: org.openhab.core.items.ItemNotFoundException: Item 'Z9F_State (Type=ContactItem, State=CLOSED, Label=Z09, Category=Contact, Tags=[OpenState, Z-OG-F], Groups=[Z9F, Windows])' could not be found in the item registry

	at jdk.nashorn.internal.runtime.ScriptRuntime.apply(ScriptRuntime.java:531) ~[jdk.scripting.nashorn:?]

	at jdk.nashorn.api.scripting.NashornScriptEngine.evalImpl(NashornScriptEngine.java:456) ~[jdk.scripting.nashorn:?]

	at jdk.nashorn.api.scripting.NashornScriptEngine.evalImpl(NashornScriptEngine.java:413) ~[jdk.scripting.nashorn:?]

	at jdk.nashorn.api.scripting.NashornScriptEngine.evalImpl(NashornScriptEngine.java:409) ~[jdk.scripting.nashorn:?]

	at jdk.nashorn.api.scripting.NashornScriptEngine.eval(NashornScriptEngine.java:162) ~[jdk.scripting.nashorn:?]

	at javax.script.AbstractScriptEngine.eval(AbstractScriptEngine.java:264) ~[java.scripting:?]

	at org.openhab.core.automation.module.script.internal.handler.ScriptActionHandler.lambda$0(ScriptActionHandler.java:62) ~[?:?]

	at java.util.Optional.ifPresent(Optional.java:183) ~[?:?]

	at org.openhab.core.automation.module.script.internal.handler.ScriptActionHandler.execute(ScriptActionHandler.java:59) ~[?:?]

	at org.openhab.core.automation.internal.RuleEngineImpl.executeActions(RuleEngineImpl.java:1181) ~[?:?]

	at org.openhab.core.automation.internal.RuleEngineImpl.runNow(RuleEngineImpl.java:1033) ~[?:?]

	at org.openhab.core.automation.internal.RuleEngineImpl.runNow(RuleEngineImpl.java:1049) ~[?:?]

	at org.openhab.core.automation.rest.internal.RuleResource.runNow(RuleResource.java:327) ~[?:?]

	at jdk.internal.reflect.GeneratedMethodAccessor337.invoke(Unknown Source) ~[?:?]

	at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:?]

	at java.lang.reflect.Method.invoke(Method.java:566) ~[?:?]

	at org.apache.cxf.service.invoker.AbstractInvoker.performInvocation(AbstractInvoker.java:179) ~[bundleFile:3.4.5]

	at org.apache.cxf.service.invoker.AbstractInvoker.invoke(AbstractInvoker.java:96) ~[bundleFile:3.4.5]

	at org.apache.cxf.jaxrs.JAXRSInvoker.invoke(JAXRSInvoker.java:201) ~[bundleFile:3.4.5]

	at org.apache.cxf.jaxrs.JAXRSInvoker.invoke(JAXRSInvoker.java:104) ~[bundleFile:3.4.5]

	at org.apache.cxf.interceptor.ServiceInvokerInterceptor$1.run(ServiceInvokerInterceptor.java:59) ~[bundleFile:3.4.5]

	at org.apache.cxf.interceptor.ServiceInvokerInterceptor.handleMessage(ServiceInvokerInterceptor.java:96) ~[bundleFile:3.4.5]

	at org.apache.cxf.phase.PhaseInterceptorChain.doIntercept(PhaseInterceptorChain.java:308) ~[bundleFile:3.4.5]

	at org.apache.cxf.transport.ChainInitiationObserver.onMessage(ChainInitiationObserver.java:121) ~[bundleFile:3.4.5]

	at org.apache.cxf.transport.http.AbstractHTTPDestination.invoke(AbstractHTTPDestination.java:265) ~[bundleFile:3.4.5]

	at org.apache.cxf.transport.servlet.ServletController.invokeDestination(ServletController.java:234) ~[bundleFile:3.4.5]

	at org.apache.cxf.transport.servlet.ServletController.invoke(ServletController.java:208) ~[bundleFile:3.4.5]

	at org.apache.cxf.transport.servlet.ServletController.invoke(ServletController.java:160) ~[bundleFile:3.4.5]

	at org.apache.cxf.transport.servlet.CXFNonSpringServlet.invoke(CXFNonSpringServlet.java:225) ~[bundleFile:3.4.5]

	at org.apache.cxf.transport.servlet.AbstractHTTPServlet.handleRequest(AbstractHTTPServlet.java:298) ~[bundleFile:3.4.5]

	at org.apache.cxf.transport.servlet.AbstractHTTPServlet.doPost(AbstractHTTPServlet.java:217) ~[bundleFile:3.4.5]

	at javax.servlet.http.HttpServlet.service(HttpServlet.java:707) ~[bundleFile:3.1.0]

	at org.apache.cxf.transport.servlet.AbstractHTTPServlet.service(AbstractHTTPServlet.java:273) ~[bundleFile:3.4.5]

	at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:799) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:550) ~[bundleFile:9.4.43.v20210629]

	at org.ops4j.pax.web.service.jetty.internal.HttpServiceServletHandler.doHandle(HttpServiceServletHandler.java:71) ~[bundleFile:?]

	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:602) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:235) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1624) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1434) ~[bundleFile:9.4.43.v20210629]

	at org.ops4j.pax.web.service.jetty.internal.HttpServiceContext.doHandle(HttpServiceContext.java:294) ~[bundleFile:?]

	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:501) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1594) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1349) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141) ~[bundleFile:9.4.43.v20210629]

	at org.ops4j.pax.web.service.jetty.internal.JettyServerHandlerCollection.handle(JettyServerHandlerCollection.java:82) ~[bundleFile:?]

	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.server.Server.handle(Server.java:516) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:388) ~[bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:633) [bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:380) [bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:277) [bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311) [bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105) [bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.io.ChannelEndPoint$1.run(ChannelEndPoint.java:104) [bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:338) [bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:315) [bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:173) [bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:131) [bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:386) [bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:883) [bundleFile:9.4.43.v20210629]

	at org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1034) [bundleFile:9.4.43.v20210629]

	at java.lang.Thread.run(Thread.java:829) [?:?]
Caused by: org.openhab.core.items.ItemNotFoundException: Item 'Z9F_State (Type=ContactItem, State=CLOSED, Label=Z09, Category=Contact, Tags=[OpenState, Z-OG-F], Groups=[Z9F, Windows])' could not be found in the item registry

	at org.openhab.core.internal.items.ItemRegistryImpl.getItem(ItemRegistryImpl.java:86) ~[?:?]

	at jdk.nashorn.internal.scripts.Script$Recompilation$7831$\^eval\_$cu1$restOf/0x85686ab8.:program(<eval>:9) ~[?:?]

	at jdk.nashorn.internal.runtime.ScriptFunctionData.invoke(ScriptFunctionData.java:655) ~[jdk.scripting.nashorn:?]

	at jdk.nashorn.internal.runtime.ScriptFunction.invoke(ScriptFunction.java:513) ~[jdk.scripting.nashorn:?]

	at jdk.nashorn.internal.runtime.ScriptRuntime.apply(ScriptRuntime.java:527) ~[jdk.scripting.nashorn:?]

	... 67 more

That’s what i figured out.

1 Like