openhab / openhab-webui

Web UIs of openHAB
Eclipse Public License 2.0
213 stars 234 forks source link

[Blockly] Variables do not work in Quantity block #2057

Closed Dash closed 4 weeks ago

Dash commented 9 months ago

Discussion on this issue where I was asked to raise this: https://community.openhab.org/t/uom-blockly-quantity-not-working-with-variable/149372

The problem

When assigning using a variable in conjunction with the Quantity block: c73cc75e9d8478c9e1d81d19eb7fc7fc9d7928e0_2_690x181 The following code is generated: b10d225f95eab7b9a6c1e03d39ed2225de4ac482

Expected behavior

Primarily I would expect this to work as adding a literal value to the Quantity function works just fine. At the very least, if this isn't supported functionality then the Quantity function should disallow variables in the UI.

Steps to reproduce

As above

  1. Create a Blockly script
  2. Create a variable
  3. Assign a value to the variable (string is the most honest)
  4. Use the variable as the first parameter for the Quantity block

Your environment

runtimeInfo:
  version: 4.0.2
  buildString: Release Build
locale: en-GB
systemInfo:
  configFolder: /etc/openhab
  userdataFolder: /var/lib/openhab
  logFolder: /var/log/openhab
  javaVersion: 17.0.8
  javaVendor: N/A
  osName: Linux
  osVersion: 5.14.21-150400.24.81-default
  osArchitecture: amd64
  availableProcessors: 2
  freeMemory: 335846760
  totalMemory: 778043392
  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: true
    cordova: false
    phonegap: false
    electron: false
    nwjs: false
    webView: false
    webview: false
    standalone: false
    os: windows
    pixelRatio: 1
    prefersColorScheme: dark
  isSecureContext: true
  locationbarVisible: true
  menubarVisible: true
  navigator:
    cookieEnabled: true
    deviceMemory: N/A
    hardwareConcurrency: 12
    language: en-GB
    languages:
      - en-GB
      - en
    onLine: true
    platform: Win32
  screen:
    width: 1920
    height: 1080
    colorDepth: 24
  support:
    touch: false
    pointerEvents: true
    observer: true
    passiveListener: true
    gestures: false
    intersectionObserver: true
  themeOptions:
    dark: dark
    filled: true
    pageTransitionAnimation: default
    bars: light
    homeNavbar: default
    homeBackground: default
    expandableCardAnimation: default
  userAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101
    Firefox/116.0
timestamp: 2023-09-06T18:24:49.125Z

Browser console

Browser network traffic

Additional information

florian-h05 commented 8 months ago

I cannot reproduce this on 4.1.0 SNAPSHOT 3676, and we had a major PR on Quantity blocks, so this seems to be solved.

stefan-hoehn commented 1 month ago

@florian-h05 I cannot confirm this on openHAB 4.2.0.M2

I have the same problem:

image

which produces

var ItemPower;

ItemPower = 123;
console.info(undefined);
ItemPower = '123';
console.info(undefined);

Can you please reopen this one?

stefan-hoehn commented 1 month ago

The problem as always is the interpretation of the variable type which can only be done during runtime. These are the options that the original block can take:

image

So it knows how to handle a string, an item name and an item object and will the extract the number from it.

However, when a variable is given it doesn't know what is the type of the number, so like in all blocks it needs to expect a certain type. In almost all other blocks this is treated as item object with the rationale behind that often this is used when you loop/iterate over items (where the loop uses a list of item objects).

I could easily allow expecting the variable of type number or a string but that behavior wouldn't be consistent with other places (where we expect the item object):

image

This would generate a simple code like

code = Quantity(${value} + ${unit})

@florian-h05

I could support this as well:

image

but that would lead to a runtime detection code (that you once proposed in a different thread) which would become like


console.info(
(typeof ItemPower === 'object') ? ((ItemPower.quantityState !== null) ? ItemPower.quantityState.toUnit('W') : Quantity(ItemPower.numericState + 'W') ): Quantity(ItemPower + 'W')
);

WDYT?

florian-h05 commented 1 month ago

I totally agree that missing variable support is a bug in the code generation, seems like the default case is missing:

https://github.com/openhab/openhab-webui/blob/d8016b6ddf154aeb01c77708b98986f472d41bfc/bundles/org.openhab.ui/web/src/assets/definitions/blockly/blocks-uom.js#L35-L46

With regards on how to fix that, I am not sure how to do that. On one hand, I like the flexibility of the runtime type detection, but on the other hand it makes the code „ugly“ and more difficult to learn from as a beginner.

In any case, handling string or number as variable type should be made possible for Quantity, this would mean we have to add a default case to the switch statement above with Stefan‘s proposed code Quantity(${value} + ${unit}).

As the normal Quantity block, which only takes one argument, is able to support both strings and Item objects as variable types (handled inside openhab-js), I would tend to recommend users to use a combination of this block together with the „Qty to unit“ block to achieve the same behaviour as it would be possible with the proposed runtime type detection code on the extended block.

stefan-hoehn commented 4 weeks ago

@Dash Finally I found the time to actually change the root cause of the problem which is the fact that Blockly doesn't know the type of the variable. Fortunately there is a solution which I only found out yesterday. That solution also solves your issue (as you can see from the samples I provided there).

So, thanks for bringing this up as it finally pushed me again to search for the generic solution I was looking for so long.