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

Reintroduce inline script support for ECMAScript #1981

Closed
ulfwin opened this issue Jul 22, 2023 · 11 comments · Fixed by openhab/openhab-core#3730
Closed

Reintroduce inline script support for ECMAScript #1981

ulfwin opened this issue Jul 22, 2023 · 11 comments · Fixed by openhab/openhab-core#3730
Labels
enhancement New feature or request main ui Main UI

Comments

@ulfwin
Copy link

ulfwin commented Jul 22, 2023

The problem

Inline script support for channel transformation profiles seems to exist for python and DSL, but is not there for ECMAScript since it presents a list of existing scripts rather than a text field (see below screenshot)

I believe earlier milestones had the custom text field also for ECMAScript.

image

Your suggestion

The list of scripts is also nice, so perhaps the list and an additional text field as well?

Your environment

runtimeInfo:
  version: 4.0.0.M4
  buildString: Milestone Build
locale: en-US
systemInfo:
  configFolder: /etc/openhab
  userdataFolder: /var/lib/openhab
  logFolder: /var/log/openhab
  javaVersion: 17.0.7
  javaVendor: Raspbian
  osName: Linux
  osVersion: 6.1.21-v8+
  osArchitecture: arm
  availableProcessors: 4
  freeMemory: 46981712
  totalMemory: 479059968
  startLevel: 100
bindings: null
clientInfo:
  device:
    ios: false
    android: false
    androidChrome: false
    desktop: true
    iphone: false
    ipod: false
    ipad: false
    edge: false
    ie: false
    firefox: true
    macos: false
    windows: false
    cordova: false
    phonegap: false
    electron: false
    nwjs: false
    webView: false
    webview: false
    standalone: false
    pixelRatio: 1
    prefersColorScheme: light
  isSecureContext: false
  locationbarVisible: true
  menubarVisible: true
  navigator:
    cookieEnabled: true
    deviceMemory: N/A
    hardwareConcurrency: 4
    language: en-US
    languages:
      - en-US
      - en
    onLine: true
    platform: Linux x86_64
  screen:
    width: 1920
    height: 1080
    colorDepth: 24
  support:
    touch: false
    pointerEvents: true
    observer: true
    passiveListener: true
    gestures: false
    intersectionObserver: true
  themeOptions:
    dark: light
    filled: true
    pageTransitionAnimation: default
    bars: filled
    homeNavbar: default
    homeBackground: default
    expandableCardAnimation: default
  userAgent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:109.0) Gecko/20100101
    Firefox/115.0
timestamp: 2023-07-22T17:51:17.492Z
@ulfwin ulfwin added enhancement New feature or request main ui Main UI labels Jul 22, 2023
@ulfwin
Copy link
Author

ulfwin commented Jul 22, 2023

@florian-h05
Copy link
Contributor

@openhab/core-maintainers I believe this is an core issu because UI gets its options from the REST API.

@J-N-K
Copy link
Member

J-N-K commented Jul 24, 2023

The script transformation regards an entry starting with | as inline script. The REST API provides only the available transformations from the registry (which of course do not contain the inline script).

UI should present a "free text" option at this point, together with the available transformations.

@rkoshak
Copy link

rkoshak commented Jul 24, 2023

That's what I was thinking too. The UI should provide the option for a text field where the user can enter an arbitrary inline script.

@florian-h05
Copy link
Contributor

Would be one option, but e.g. for DSL, the UI's input field is only there because core provides a config description for it. I would prefer to have it the same for all profiles, otherwise we need to hard-code it for some and for others not.

For example for SCRIPT DSL:

http://localhost:8080/rest/config-descriptions/profile:transform:DSL

{
    "uri": "profile:transform:DSL",
    "parameters": [
        {
            "description": "The Script for transforming state updates and commands from the Thing handler to the item. The script may return null to discard the updates/commands and not pass them through.",
            "label": "Thing To Item Transformation",
            "name": "toItemScript",
            "required": false,
            "type": "TEXT",
            "readOnly": false,
            "multiple": false,
            "advanced": false,
            "verify": false,
            "limitToOptions": true,
            "options": [],
            "filterCriteria": []
        },
        {
            "description": "The Script for transforming commands from the item to the Thing handler. The script may return null to discard the commands and not pass them through.",
            "label": "Item To Thing Transformation",
            "name": "toHandlerScript",
            "required": false,
            "type": "TEXT",
            "readOnly": false,
            "multiple": false,
            "advanced": false,
            "verify": false,
            "limitToOptions": true,
            "options": [],
            "filterCriteria": []
        }
    ],
    "parameterGroups": []
}

@J-N-K
Copy link
Member

J-N-K commented Jul 24, 2023

That's really strange, because DSL is in no way different from the other languages, it is handled by the exact same code. Maybe the difference is that you have defined transformations for other languages?

@rkoshak
Copy link

rkoshak commented Jul 24, 2023

Would be one option, but e.g. for DSL, the UI's input field is only there because core provides a config description for it.

If you are going to do an inline script, you'd still have to supply the whole thing, right? So there'd be no advantage or even a point in selecting the language first.

I was thinking there being an "other" option in the list which just consists of the text field.

I think @J-N-K is right, if you don't have a transform defined for a given language, you get a text field. However, if you have even one transform defined for a language, you only can choose from the the already existing transforms without the possibility to define an inline transform.

@J-N-K
Copy link
Member

J-N-K commented Jul 24, 2023

Without defined transformation:

{
    "uri": "profile:transform:GROOVY",
    "parameters": [
      {
        "description": "The Script for transforming state updates and commands from the Thing handler to the item. The script may return null to discard the updates/commands and not pass them through.",
        "label": "Thing To Item Transformation",
        "name": "toItemScript",
        "required": false,
        "type": "TEXT",
        "readOnly": false,
        "multiple": false,
        "advanced": false,
        "verify": false,
        "limitToOptions": true,
        "options": [],
        "filterCriteria": []
      },
      {
        "description": "The Script for transforming commands from the item to the Thing handler. The script may return null to discard the commands and not pass them through.",
        "label": "Item To Thing Transformation",
        "name": "toHandlerScript",
        "required": false,
        "type": "TEXT",
        "readOnly": false,
        "multiple": false,
        "advanced": false,
        "verify": false,
        "limitToOptions": true,
        "options": [],
        "filterCriteria": []
      }
    ],
    "parameterGroups": []
  },

With defined transformation:

{
  "uri": "profile:transform:GROOVY",
  "parameters": [
    {
      "description": "The Script for transforming state updates and commands from the Thing handler to the item. The script may return null to discard the updates/commands and not pass them through.",
      "label": "Thing To Item Transformation",
      "name": "toItemScript",
      "required": false,
      "type": "TEXT",
      "readOnly": false,
      "multiple": false,
      "advanced": false,
      "verify": false,
      "limitToOptions": true,
      "options": [
        {
          "label": "fooo",
          "value": "config:groovy:8c19ab652e"
        }
      ],
      "filterCriteria": []
    },
    {
      "description": "The Script for transforming commands from the item to the Thing handler. The script may return null to discard the commands and not pass them through.",
      "label": "Item To Thing Transformation",
      "name": "toHandlerScript",
      "required": false,
      "type": "TEXT",
      "readOnly": false,
      "multiple": false,
      "advanced": false,
      "verify": false,
      "limitToOptions": true,
      "options": [
        {
          "label": "fooo",
          "value": "config:groovy:8c19ab652e"
        }
      ],
      "filterCriteria": []
    }
  ],
  "parameterGroups": []
}

I think limitToOptions should probably be false, if other free text is allowed.

@florian-h05
Copy link
Contributor

I think limitToOptions should probably be false, if other free text is allowed.

I have just checked (manipulating the config sheet using the Vue dev tools), and yes, it should be:

image

@jimtng
Copy link
Contributor

jimtng commented Aug 20, 2023

After openhab/openhab-core#3730 (e.g. tested on openHAB 4.0.2), all I'm getting is the text input. It does not present me with a list of existing .rb transformations that exist in conf/transform directory or managed transformation). I'd imagine this applies to JS and any other script transformation profile types.

image

Can we have both text input and a list of existing compatible transformations?
Perhaps the text input can be the top / first option in the radio buttons so it's mutually exclusive, i.e. either text input, or one of the existing scripts.

@jimtng
Copy link
Contributor

jimtng commented Aug 21, 2023

Can we have both text input and a list of existing compatible transformations?
Perhaps the text input can be the top / first option in the radio buttons so it's mutually exclusive, i.e. either text input, or one of the existing scripts.

My apologies. It seems that there's a hidden drop down selection that would pop up when you click on the input field. However, there seems to be a bug but I'll open a separate issue about it.

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 a pull request may close this issue.

5 participants