openhab / openhab-core

Core framework of openHAB
https://www.openhab.org/
Eclipse Public License 2.0
930 stars 428 forks source link

Minus operator (-) does not work in rules (openHAB 4.2.0) #4304

Open quensen opened 4 months ago

quensen commented 4 months ago

The minus operator (-) for subtraction does not work in rules in some cases since I updated from 4.1.3 to 4.2.0 Release Build.

Expected Behavior

The following code was working in 4.1.3:

    val sollflur = Temp_Soll_EG_Flur.state as Number

    val DateTime vor1Y = now.minusYears(1)

    val istdiele = Temp_Ist_EG_Diele.averageSince (vor1Y)
    val istflur = Temp_Ist_EG_Flur.averageSince (vor1Y)

    val solldiele = sollflur - (istflur - istdiele) // this is line 1543

Current Behavior

After I updated to 4.2.0, I am getting this error: 2024-07-08 14:13:57.877 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID 'main-40' failed: Unknown variable or command '-'; line 1543, column 30, length 18 in main

Possible Solution

My quick-fix was to add type Number to the variables:

    val Number istdiele = Temp_Ist_EG_Diele.averageSince    (vor1Y)
    val Number istflur  = Temp_Ist_EG_Flur.averageSince     (vor1Y)

But that does not solve the original issue.

Steps to Reproduce (for Bugs)

Put the lines above in a text file rule and save it as test.rules in the rules folder in your config folder. Watch the log file.

Context

I was trying to perfom the subtraction to calculate a derived value.

Your Environment

runtimeInfo:
  version: 4.2.0
  buildString: Release Build
locale: de-DE
systemInfo:
  configFolder: /etc/openhab
  userdataFolder: /var/lib/openhab
  logFolder: /var/log/openhab
  javaVersion: 17.0.11
  javaVendor: Debian
  osName: Linux
  osVersion: 6.6.31+rpt-rpi-2712
  osArchitecture: aarch64
  availableProcessors: 4
  freeMemory: 224180480
  totalMemory: 801112064
  uptime: 2694
  startLevel: 100
addons:
  - automation-jrubyscripting
  - automation-jsscripting
  - binding-astro
  - binding-exec
  - binding-fsinternetradio
  - binding-http
  - binding-knx
  - binding-lgwebos
  - binding-miio
  - binding-mqtt
  - binding-network
  - binding-tr064
  - misc-openhabcloud
  - persistence-rrd4j
  - transformation-jsonpath
  - transformation-map
  - transformation-regex
  - transformation-xslt
  - ui-basic
clientInfo:
  device:
    ios: false
    android: false
    androidChrome: false
    desktop: true
    iphone: false
    ipod: false
    ipad: false
    edge: false
    ie: false
    firefox: false
    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: false
  locationbarVisible: true
  menubarVisible: true
  navigator:
    cookieEnabled: true
    deviceMemory: N/A
    hardwareConcurrency: 20
    language: de
    languages:
      - de
      - de-DE
      - en
      - en-GB
      - en-US
    onLine: true
    platform: Win32
  screen:
    width: 3440
    height: 1440
    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
    blocklyRenderer: null
  userAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML,
    like Gecko) Chrome/126.0.0.0 Safari/537.36 Edg/126.0.0.0
timestamp: 2024-07-08T13:05:25.900Z
mherwege commented 4 months ago

averageSince now returns a QuantityType for Items that have a dimension. This is listed in the breaking changes. In your rule it causes a subtraction of a QuantityType (result of the subtraction in brackets) from a DecimalType (you defined sollflur as a Number). You should not convert sollflur or indeed, define the two others as a Number to stay consistent in your rule. So I don’t think this is a bug.

Intenos commented 1 month ago

I have also a problem with some math operations since 4.2.0. I experienced the issue on different openHAB installations, with DSL rules as well as with blockly and with 4.2.0 as well as 4.2.2

On my side it is more about mulitplications and divisions. From there I´m not sure whether it is the same problem or should be reported separatly.

Anyhow, before 4.2.0 the calculations worked in the following way:

var Number kWh_verfuegbar_heute = (PV_Vorhersage_Solcast_Sued_Energie_HeuteVerbleibend.state as QuantityType<Number>).toUnit("kWh").toBigDecimal - 6 / 100 * (100 - PV_Batterie_Ladezustand.state as Number) 

kWh_verfuegbar_heute = kWh_verfuegbar_heute - (9 / 18 * (18 - now.getHour))

Since 4.2.0, it always subtracts 0. So the values remain the same. The really interesting part is, when I change around the multiplication and division, it works again!

var Number kWh_verfuegbar_heute = (PV_Vorhersage_Solcast_Sued_Energie_HeuteVerbleibend.state as QuantityType<Number>).toUnit("kWh").toBigDecimal - 6 * (100 - PV_Batterie_Ladezustand.state as Number) / 100

kWh_verfuegbar_heute = kWh_verfuegbar_heute - (9 * (18 - now.getHour) / 18)

The same with blockly:

This is not working: image

This is working: image

Intenos commented 1 month ago

I can confirm that adding "as Number" resolves the issue with "-". I also found one calculation today not working anymore and creating the same error message as written in the first post.

After adding "as Number" it works: if ((LWZ_Status_Verdichter.sumSince(ZonedDateTime.now().with(LocalTime.MIDNIGHT)) as Number - LWZ_Status_WW.sumSince(ZonedDateTime.now().with(LocalTime.MIDNIGHT)) as Number) > 0)

@mherwege May I ask you what you think about the multiplication and dividison issue I wrote above? Shall I create a separate bug report for it? Or is there also an explanation for it?

mherwege commented 1 month ago

May I ask you what you think about the multiplication and dividison issue I wrote above? Shall I create a separate bug report for it? Or is there also an explanation for it?

Sorry for the late reply. It is a bit hectic at work.

What are the types of the items in your Blockly rules? If these are QuantityTypes, you should not use the straigth mutiplication and division operators, but the methods provided with QuantityTypes in Blockly.

For DSL, I often struggle myself with the logic. But behind it is Java, and if you give Java 2 integers, it will divide into an integer result. That's why first multiplying and then dividing will probably work. You can also try first multiplying with 1.0 to make sure it is treated as a float. DSL does all kind of type magic as it tries to infer types, and that's where it often goes wrong.

Intenos commented 1 month ago

Thank you very much for your reply.

How can I find out if they are QuantityTypes and what do I have then to use for other blocks for the calculation?

The relevant code lines look as follows. From my understanding they are declared as normal variables and I would expect it to work (like it did before 4.2.0).

var Laufzeit, Sollwert, dStellwert_25, dZeit;
Laufzeit = 50;
dZeit = (Laufzeit * dStellwert_25) / 100;
mherwege commented 1 month ago

The relevant code lines look as follows. From my understanding they are declared as normal variables and I would expect it to work (like it did before 4.2.0).

var Laufzeit, Sollwert, dStellwert_25, dZeit;
Laufzeit = 50;
dZeit = (Laufzeit * dStellwert_25) / 100;

And where do you set dStellwert_25?

Intenos commented 1 month ago

Oh, good point:

dStellwert_25 = Sollwert - parseFloat(items.getItem('Heizung_DG_Mischer_Ist').state);

mherwege commented 1 month ago

Please provide the full information:

How is Sollwert set? What are the items types of all items involved?

I notice in this specific line you are probably using the parseNumber block. Why do you use that? This is meant to parse a string, not the item state, which is an object. You should be careful, follow the tip from here. But this conversion to String is not needed if you do the arithmetic directly with QuantityType: see https://www.openhab.org/docs/configuration/blockly/rules-blockly-uom.html#quantity-computation.

Intenos commented 1 month ago

That´s the total Blockly code:

var Laufzeit, Sollwert, dStellwert_25, dZeit;

if (items.getItem('Heizung_DG_Mischer_Auf').state == 'OFF' && items.getItem('Heizung_DG_Mischer_Zu').state == 'OFF' && (cache.private.exists('MyTimer') && cache.private.get('MyTimer').isRunning()) == false && items.getItem('Heizung_DG_Mischer_Kalibrieren').state == 'OFF') {
  // Sekunden
  Laufzeit = 50;
  Sollwert = parseFloat(items.getItem('Heizung_DG_Mischer_Soll').state);
  if (Sollwert < 0) {
    Sollwert = 0;
  } else if (Sollwert > 100) {
    Sollwert = 100;
  }
  if (Sollwert < parseFloat(items.getItem('Heizung_DG_Mischer_Zwangsoeffnung').state)) {
    Sollwert = parseFloat(items.getItem('Heizung_DG_Mischer_Zwangsoeffnung').state);
  }
  dStellwert_25 = Sollwert - parseFloat(items.getItem('Heizung_DG_Mischer_Ist').state);
  // Vermeidung zu kurzer Verfahrzeiten.
  // Um vollends auf 0 oder 100 % zu fahren.
  if (dStellwert_25 > 3 || dStellwert_25 < -3 || (Sollwert == 0 || Sollwert == 100) && dStellwert_25 != 0) {
    dZeit = (Laufzeit * dStellwert_25) / 100;
    if (dZeit > 0) {
      items.getItem('Heizung_DG_Mischer_Auf').sendCommand('ON');
    } else if (dZeit < 0) {
      items.getItem('Heizung_DG_Mischer_Zu').sendCommand('ON');
      dZeit = dZeit * -1;
      // Regelmäßige Autokalibrierung beim Anfahren von 0 %.
      if (Sollwert == 0) {
        dZeit = dZeit + 10;
      }
    }
    if (cache.private.exists('MyTimer') === false || cache.private.get('MyTimer').hasTerminated()) {
      cache.private.put('MyTimer', actions.ScriptExecution.createTimer('MyTimer', time.ZonedDateTime.now().plusSeconds(dZeit), function (timer_context) {
        items.getItem('Heizung_DG_Mischer_Auf').sendCommand('OFF');
        items.getItem('Heizung_DG_Mischer_Zu').sendCommand('OFF');
        items.getItem('Heizung_DG_Mischer_Ist').sendCommand(Sollwert);
        }, ));
    };
  }
}

The items "Heizung_DG_Mischer_Ist", "Heizung_DG_Mischer_Zwangsoeffnung" and "Heizung_DG_Mischer_Soll" are all Number items without defined Dimension.

The parsing to Numbers I was used to from the DSL ruls where it is (was?) required to get comparisons and calculations working.

mherwege commented 1 month ago

The parsing to Numbers I was used to from the DSL ruls where it is (was?) required to get comparisons and calculations working.

And yet Blockly (and javascript) is not DSL. The parseNumber block expects a String as an input. And what you get back as an item state may not be a String. In most cases you should not do that. Just put the result of the get state of item block straight into the comparison. Note there is a block, get ... of item that solves this typing problem in a better way and should be used instead: https://www.openhab.org/docs/configuration/blockly/rules-blockly-items-things.html#get-particular-attributes-of-an-item.