openhab-webui icon indicating copy to clipboard operation
openhab-webui copied to clipboard

Allow editing transformation configurations in UI

Open J-N-K opened this issue 2 years ago • 16 comments

Depends on https://github.com/openhab/openhab-core/pull/3036

This should be merged for the next major version of openHAB.

Bildschirmfoto 2022-07-10 um 14 21 34 Bildschirmfoto 2022-07-10 um 18 18 08 Bildschirmfoto 2022-07-10 um 14 23 13 Bildschirmfoto 2022-07-10 um 14 22 33

Signed-off-by: Jan N. Klug [email protected]

J-N-K avatar Jul 10 '22 12:07 J-N-K

Job #892: Bundle Size — 15.69MiB (+0.18%).

cea09d2(current) vs 6492179 main#891(baseline)

:warning: Bundle contains 16 duplicate packages

Metrics (2 changes)
                 Current
Job #892
     Baseline
Job #891
Initial JS 1.72MiB(+0.02%) 1.72MiB
Initial CSS 608.59KiB 608.59KiB
Cache Invalidation 93.91% 93.91%
Chunks 219 219
Assets 689 689
Modules 1710(+0.23%) 1706
Duplicate Modules 82 82
Duplicate Code 1.72% 1.72%
Packages 137 137
Duplicate Packages 15 15
Total size by type (3 changes)
                 Current
Job #892
     Baseline
Job #891
CSS 858.03KiB (+0.02%) 857.87KiB
Fonts 526.1KiB 526.1KiB
HTML 1.23KiB 1.23KiB
IMG 140.74KiB 140.74KiB
JS 9.2MiB (+0.23%) 9.18MiB
Media 295.6KiB 295.6KiB
Other 4.71MiB (+0.16%) 4.7MiB

View job #892 reportView main branch activity

relativeci[bot] avatar Jul 10 '22 12:07 relativeci[bot]

I wonder, if we need to get the vocabulary straight:

Screenshot 2022-07-10 at 18 34 06

Are those now "Transformations" or "Transformation Configurations"? I'd actually say the header is correct - these are instances of "Transformations" that are created, managed and configured, wdyt?

Is the "editor mode", what you call "context" in https://github.com/openhab/openhab-core/pull/3036 or what is that about?

kaikreuzer avatar Jul 10 '22 16:07 kaikreuzer

Exactly, the "editor mode" and in the case of blockly the "non-scripted block information" is what the context is. Maybe the wording "context" is not good, but we need additional information. What is not shown in the pictures: for MAP the editor shows a hint that you have to enter a value behind the "=".

Yes, in fact they are configurations for transformations (file-based are shown, but not editable, and managed can be created/edited/deleted, similar to things or items). "Transformation configuration" very long term which does not properly fit in the UI. I'm very happy if you have a good proposal what we should name it.

J-N-K avatar Jul 10 '22 16:07 J-N-K

I'm very happy if you have a good proposal what we should name it.

Well, as stated above, I think what we deal with here are "Transformations".

To be honest, I still haven't understood what TransformationConfigurations are supposed to be.

A Transformation itself is defined by a type, and a filename or function (depending on the type) - let's call it parameter. Its String representation is so far simply <Type>(<Parameter>) and with this, the Transformation should be fully described.

If certain Transformation types now require specific information, this should imho simply be part of the <Parameter> of it.

The Parameter can imho be seen as the configuration of the Transformation. In order to make Transformations nicely manageable through the UI, I could imagine that we can add a unique id and a label to it and make the Parameter a map, so that it can be treated as the configuration and we could provide according ConfigDescriptions through the Transformation services. "Classic" transformation types that rely on files could e.g. always use "file" as a key and the filename as a value for the configuration. Script transformations could have a "script language" key, and refer either to a "file" or have an in-line script parameter.

As a result, we should have a TransformationRegistry, just like we have a ThingRegistry and an ItemRegistry. The entities managed within those registries then all have configurations. Wouldn't this be the most logical solution?

kaikreuzer avatar Jul 10 '22 18:07 kaikreuzer

Sounds reasonable. I'll prepare that.

J-N-K avatar Jul 10 '22 20:07 J-N-K

Screenshot 2022-07-10 at 18 34 06

I suppose the header "Transformations" is fine even if it's a transformation parameter/configuration that's actually being edited, as it still has to look good, so a short one-word header is better.

Regarding its position in the menu, I'm wondering if those "deserve" that top-level spot along with the "big four" (things/inbox, items/model, pages, rules/scripts), to me they're far less important and merely a helper, like Profiles, or Rule Templates, or UI widgets: they have no purpose of their own but are meant to be used by top-level objects.

I could imagine them in Developer Tools under "Advanced Object Management" instead, along with those future managed "secondary" objects - templates, managed profile configurations, managed metadata namespace descriptions, etc.

I know they are actually configuration so it also makes sense to put them under Configuration in the main Settings, but just wanted to throw out the idea.

Regarding Blockly in transformations, how would that work exactly? Does the script engine have the same scope as the script rule modules? Because then a lot of blocks in the Blockly toolbox would make no sense and more so, critical blocks to build a transformation script like value or the the eventual parameters are not available. And in case of JS, it's recommended to code transformation scripts as self-invoking functions (https://www.openhab.org/addons/transformations/javascript/#examples); so I don't say it can't be done, I'm sure it's just a general idea and not a working implementation yet.

ghys avatar Jul 11 '22 15:07 ghys

From an end user perspective I don't like the idea of hiding them under Developer Tools. I wouldn't intuitively think to look for them there. While they may only be a helper, in the docs they are treated as their own thing on the same level as persistence, rules, items, etc. The other secondary objects you mention don't have their own top level page in the docs (or any page in the docs in some cases) so those make more sense to put into Developer Tools but I'm not so sure about transformations.

Of course, its prominence in the docs is also an accident of the history of OH itself. But given that transformations can be used in a lot of places (Things/Channels, Profiles, Items, Rules) I'm not sure it makes sense to bury that page in one of the others either.

Anyway, what's not clear is if one would be able to somehow launch the creation of a new transformation from, for example, the MQTT Channel configuration page. It would be pretty cool if I could launch the creation of it from there (and from other bindings that use transformations, the State Description metadata config form, and the transform Profile config form). Then it may not matter as much where they are placed.

If not, the user would have to

  1. open a new tab
  2. click to open Developer Tools
  3. click to open transformations
  4. click to add a new transformation config
  5. config it
  6. return to the previous page
  7. probably reload that page to pick up the new transformation.

rkoshak avatar Jul 11 '22 16:07 rkoshak

Fair enough, as I've said it just me throwing out an idea to better surface essential concepts and distinguish them from others that are more technical. Maybe some reorganizing at some point would be in order.

Anyway, what's not clear is if one would be able to somehow launch the creation of a new transformation from, for example, the MQTT Channel configuration page. It would be pretty cool if I could launch the creation of it from there (and from other bindings that use transformations, the State Description metadata config form, and the transform Profile config form). Then it may not matter as much where they are placed.

Agreed that could also make sense. But if we're giving transformations the top-level treatment like Things or Items, then they would also have to appear on the left sidebar under Settings and have support in the developer toolbar (so you can 1. search for them with the omni searchbar there, 2. pin them while you're working on them and 3. have a create shortcut in the 4th tab).

ghys avatar Jul 11 '22 16:07 ghys

Maybe some reorganizing at some point would be in order.

I'm already working, slowly, on a complete reorganization of the rules docs. They are in the worst shape of all the sections right now. But I agree. Ultimately I think a complete rethink of the docs structure needs to occur. We've been mostly coasting almost since OH 2.0 and I'm not sure the current structure works the best in a 3.x world.

But if we're giving transformations the top-level treatment like Things or Items, then they would also have to appear on the left sidebar under Settings and have support in the developer toolbar

Could they be on the right hand column under System Services or Other Services? Note we'll have a similar problem when Persistence configs come to the UI so we should keep that in mind. What ever is decided here will probably apply to Persistence so we are talking about adding two new entries in the end, not just this one.

Just because it's in the "Configuration" section does it have to be in the sidebar?

I too am just throwing out ideas. I do agree they transformations are not as "big" as rules, items and things. But they are also more than item metadata and templates and such for the average user.

rkoshak avatar Jul 11 '22 18:07 rkoshak

@rkoshak: IMO MQTT and HTTP state/commandTransformation parameters should be removed and replaced by a chaining transformation which can be applied as a profile. Another question is if chaining multiple transformations is really needed when we can use all scripting languages for transformations (in fact, this is already possible, the profile for applying the transformation on a channel/item link is missing).

I have no preference where on the settings page it is placed, but to me the transformations are too important to be hidden in the developer section.

J-N-K avatar Jul 11 '22 18:07 J-N-K

Maybe some reorganizing at some point would be in order.

I'm already working, slowly, on a complete reorganization of the rules docs.

Just so we're clear, my remark was about the settings menu, not the docs (though we could agree it could apply to both).

Could they be on the right hand column under System Services or Other Services?

Depends, these sections are automatically generated (System Services are the configuration services whose id begins with system:, Other Services are the rest of them), so there's a direct backend equivalent to those. If you're talking about making another section under these sections then again yes but the transformations would be kind of lonely there.

What ever is decided here will probably apply to Persistence so we are talking about adding two new entries in the end, not just this one.

Right with you, that's why I'm conflicted.

Just because it's in the "Configuration" section does it have to be in the sidebar?

It's kind of funny because I asked myself this very question along with a lot of others some time ago when building the UI and I finally decided that it does, as a principle. So either we break from that principle or we stick to it, up for debate - and it influenced my assessment on whether to feature it in those sections, and whether it could be under Developer Tools, where it would have its sidebar entry there for sure. It's easier to debate with yourself ;) but now that the question is asked we can have a discussion.

ghys avatar Jul 11 '22 19:07 ghys

but now that the question is asked we can have a discussion.

I personally never draw any association between the sidebar elements and the configuration (+automation!) section of the settings page - i.e. I never noticed and as such I'd say that there is absolutely no need to have exactly the same entries. So having transformations only in the section, but not in the sidebar would imho be just fine.

kaikreuzer avatar Jul 11 '22 19:07 kaikreuzer

Another thing I've considered, for the record, is that the menu acts as a "wizard" of sorts. As in you can follow the menu entries, have your Things, then define the Model, act on Items if needed and then design some Pages. Then you can go on to automation. Somehow the transformations don't fit in that story.

But whatever, let's keep the transformations there for lack of a better place!

ghys avatar Jul 11 '22 19:07 ghys

IMO MQTT and HTTP state/commandTransformation parameters should be removed and replaced by a chaining transformation which can be applied as a profile

I could not agree more. I've said for years that binding authors shouldn't be responsible for implementing transformations. It's less work for the developers and easier for the end users to have just one way to apply them instead of needing to learn the various minor differences between add-ons.

Another question is if chaining multiple transformations is really needed when we can use all scripting languages for transformations (in fact, this is already possible, the profile for applying the transformation on a channel/item link is missing).

I haven't thought about that too much, but I can see a number of cases where it feels more straight forward to chain them than to force users into a manually written script. For example extracting a value using JSONPATH or XPATH and using a MAP to convert that to ON/OFF is a pretty straight forward transformation config but less so as a script I think.

There is also the REGEX filter on the MQTT binding which, while I'm really glad it's implemented I've always been a little uncomfortable with how it's implemented. It feels a little like a misuse of transformations. But the need is still there because sometimes you have multiple devices publishing to the same topic (looking at you Shelly 😠 ). And to make this work my understanding is that REGEX was modified to behave differently from all the rest. If there's no match to a JSONPATH, you get the whole JSON but if there's no match on the REGEX you get nothing back.

I wonder if chaining profiles makes sense. Consider the case where an MQTT channel receives a JSON that we need to extract a value from but then want to apply the hysteresis profile or scale profile or the like. With transformations removed from the binding and without the ability to chain profiles that'd have to be implemented by hand which some users would consider a step backwards in usability, even if it's a step forward in flexibility. If you do that then chaining transformations happens as a matter of course plus the flexibility of profiles in general improves.

rkoshak avatar Jul 11 '22 19:07 rkoshak

This goes a little bit off-topic here, but chaining profiles is difficult to implement and also difficult to model in the UI. A single profile that allows chaining transformations is by far easier and behaves like the state/commandTransformation parameters of the MQTT/HTTP binding. This is less than 200 loc.

J-N-K avatar Jul 11 '22 20:07 J-N-K

Chaining of profiles is possible and in some ways, a lot easier than for transformations which rely on passing around a string value. Profiles can abstain from firing a callback which is ideal for building up logical flows. I think some of arguments you have are close to what was discussed in opensmarthouse/opensmarthouse-core#35. I made such functionality working with custom configuration mechanism I made for connectorio-addons. It is operational for me since almost a year: https://github.com/ConnectorIO/connectorio-addons/commit/5ee23bb8a5a2b3edfc8d18519be320e52f724ecf#diff-ca023f108105de2d6ad5d690fd92b61c6775d88ebce3375c0ddf2f98b4efa633.

I had small adjustments in there which were related to configuration handling. Main problem comes from fact that openHAB does not support a dynamic configuration with list of complex types (ie. profile configurations). For my own case I made it working through proper structuring of single profile configuration ie. profile1=urn:profile profile1_option1=val profile1_option2=val2 profile2=urn:xyz profile3=urn:foo.

splatch avatar Jul 29 '22 00:07 splatch

What is the state of this PR? I could help with reviewing it, sounds like a very nice addition.

florian-h05 avatar Feb 27 '23 22:02 florian-h05

I planned to review it but you can have a look.

These items remain IMO:

  • decide whether transformations would be on the top level similarly to Things, Items, Rules (and derivatives) etc. or something more indirect like Profiles or Inbox.
  • re: Blockly, don't offer the block types meant for rules that aren't applicable to transformation scripts (or remove Blockly altogether for now)
  • I found the initial creation a little bit unwelcoming (manual selection of a editor mode, perhaps it could be somehow derived from the transformation type/language)

ghys avatar Mar 01 '23 17:03 ghys

The problem with selecting the correct editor mode automatically is that especially for SCRIPT nearly all of them apply, depending on your preferred scripting language. Maybe the selection can be improved so that e.g. for MAP no selection is necessary.

J-N-K avatar Mar 01 '23 18:03 J-N-K

Yes I was thinking something along those lines, first of all it would be nice to pick the transformation type instead of having to type it (that may require an API endpoint). Then I'm not opposed to some type/editor mode hardcoded defaults in the UI code, and for the SCRIPT transformations, you would select the desired language for your transformation which would set everything up - language & editor mode.

ghys avatar Mar 01 '23 19:03 ghys

What would be a good endpoint for that? We could probably handle it under /transformations/services because services is not a valid UID, so we would not have collisions.

Would

[
  "MAP",
  "REGEX",
  "SCALE",
  "SCRIPT"
]

be sufficient? That can be easily extracted. It's more difficult if the also need a description, because that is currently not exposed by the service component.

J-N-K avatar Mar 01 '23 19:03 J-N-K

It sure works! Having no description is not a problem. Maybe there would be a way to link the transformation to the add-on somehow so that when creating a JSONPATH transformation we would have a handy link to https://www.openhab.org/addons/transformations/jsonpath/ but that's just wishful thinking for now!

ghys avatar Mar 01 '23 21:03 ghys

That‘s the difficult part. Currently we only add a property for the transformation „id“, but that is not linked in any way to the add-on. We can probably add more properties, but that requires some more work. One add-on could expose different services.

J-N-K avatar Mar 01 '23 21:03 J-N-K

@ghys Given that you already had a look at this PR and you follow the discussion and development here longer, I will leave the review to you.

florian-h05 avatar Mar 01 '23 22:03 florian-h05

I planned to review it but you can have a look.

I will try to review it in the next time, so we can get this merged for the next milestone.

These items remain IMO:

  • decide whether transformations would be on the top level similarly to Things, Items, Rules (and derivatives) etc. or something more indirect like Profiles or Inbox.

IMO we can keep Transformations under the configuration section with the „big four“ and place it after the pages. Transformations are just to heavily used (at least on my system) to hide them in the dev tools. And TBH I don‘t have a better proposal where to put them.

  • re: Blockly, don't offer the block types meant for rules that aren't applicable to transformation scripts (or remove Blockly altogether for now)

Without trying this out yet, I‘d vote to remove Blockly for now and focus on finishing that. The Blockly stuff can be done later.

  • I found the initial creation a little bit unwelcoming (manual selection of a editor mode, perhaps it could be somehow derived from the transformation type/language)

first of all it would be nice to pick the transformation type instead of having to type it (that may require an API endpoint).

Yes, there is room for improvement after reading the posts, with the new endpoint we could provide a menu to select the transformation type and if SCRIPT is chosen, we can get the available script engines and display a picker similar to the script editor/creation wizard.

@J-N-K Do you want to implement that here or should I review and merge the existing functionality and we or I add these enhancements in another PR?

florian-h05 avatar Mar 19 '23 14:03 florian-h05

@florian-h05 I'm also fine with you taking over here. I'm not very good when it comes to frontend, it always takes me hours to figure out how to do something in vue.

J-N-K avatar Mar 25 '23 15:03 J-N-K

@J-N-K Can you please explain what JSON data core expects if I want to add SCRIPT transformation for application/javascript?

florian-h05 avatar Mar 26 '23 00:03 florian-h05

{
  "uid": "config:script:3adf5789e",
  "label": "My First Script Transformation",
  "type": "script",
  "configuration": {
      "function": "(function(data) {\n var returnValue = \"String has \" + data.length + \" characters\"\n return returnValue\n})(input)\n"
  }
}

This is the "needed" part to make it work. You can add additional configuration as you like, it's just a Map, I added configuration.mode to store the editor mode.

J-N-K avatar Mar 26 '23 06:03 J-N-K

For what is the language input box meant?

florian-h05 avatar Mar 26 '23 07:03 florian-h05

Here? Just to select which editor is used, so syntax-highlighting works. This works for all transformations, not just scripts (e.g. for value-pairs like MAP and SCALE).

Edit: My fault. language is used to select transformations based on locales. You could have a transformation "switch" which depending on the locale translated ON to different values. This is also available in file-based transformations by adding a _de suffix to the filename (before the extension).

J-N-K avatar Mar 26 '23 07:03 J-N-K