Fix QuantityType arithmetic of mixed units (#147)

This PR addressed the following problems:
1. Operates under the rhs unit
```
a = QuantityType.new("20 °C")
b = QuantityType.new("9 °F")
logger.warn a + b # => 45 °F - this should be 25 °C 
logger.warn b + a # => 25 °C - this should be 45 °F
```
self gets converted to other's unit + other converted to self.unit then
added to self. This gave the wrong result in the wrong unit.

The lhs just needs to be converted to the unit defined by the block, and
rhs needs to be converted using `to_unit_relative` to the unit defined
by the block.

2. Inside a unit block
```
a = QuantityType.new("20 °C")
b = QuantityType.new("9 °F")
unit("°C") do
  logger.warn a + b # => 7.2222 °C => This should be 25 °C
  logger.warn b + a # => 7.2222 °C => correct. 9 °F + 20 °C -> -12.78 °C + 20 °C
  logger.warn 2 + b # -> - 20.999 °F => Should be 2°C + 5°C = 7 °C
  logger.warn 2 - b # => 24.999 °F => Should be 2 - 5 = -3 °C
end
```

3. Multiplications inside a unit block didn't take up the block's unit
```
kw = 5 | "kW"
unit("W") do
  logger.warn (kw * 2).format("%.0f %unit%") # => 10 kW => Should turn into 10000 W
  logger.warn (2 * kw).format("%.0f %unit%") # => 10 kW => Should turn into 10000 W
end
```

4. +/- against non-quantity type is still allowed. It takes up the other
operand's unit. This should raise an exception instead.
```
kw = 5 | "kW"
logger.warn kw + 5 # => 10kW
logger.warn 5 + kw # => java.lang.NullPointerException: Cannot read field "quantity" because "state" is null
```

5. Fix failing specs due to core changes in
openhab/openhab-core#3792

---------

Signed-off-by: Jimmy Tanagra <[email protected]>

lib/openhab/core/types/quantity_type.rb

@@ -174,44 +174,54 @@ def coerce(other)
           add: :+,
           subtract: :-
         }.each do |java_op, ruby_op|
-          convert = "self.class.new(other, DSL.unit(dimension) || unit)"
+          convert = "self.class.new(other, thread_unit)"
 
           class_eval( # rubocop:disable Style/DocumentDynamicEvalDefinition https://github.com/rubocop/rubocop/issues/10179
             # def +(other)
             #   logger.trace("#{self} + #{other} (#{other.class})")
             #   if other.is_a?(QuantityType)
             #     add_quantity(other)
-            #   elsif other.is_a?(DecimalType)
-            #     other = other.to_big_decimal
-            #     add_quantity(self.class.new(other, DSL.unit(dimension) || unit))
-            #   elsif other.is_a?(java.math.BigDecimal)
-            #     add_quantity(self.class.new(other, DSL.unit(dimension) || unit))
-            #   elsif other.respond_to?(:to_d)
-            #     other = other.to_d.to_java
-            #     add_quantity(self.class.new(other, DSL.unit(dimension) || unit))
-            #   elsif other.respond_to?(:coerce) && (lhs, rhs = other.coerce(to_d))
-            #     lhs + rhs
+            #   elsif (thread_unit = DSL.unit(dimension))
+            #     if other.is_a?(DecimalType)
+            #       other = other.to_big_decimal
+            #       add_quantity(self.class.new(other, thread_unit))
+            #     elsif other.is_a?(java.math.BigDecimal)
+            #       add_quantity(self.class.new(other, thread_unit))
+            #     elsif other.respond_to?(:to_d)
+            #       other = other.to_d.to_java
+            #       add_quantity(self.class.new(other, thread_unit))
+            #     elsif other.respond_to?(:coerce) && (lhs, rhs = other.coerce(to_d))
+            #       lhs + rhs
+            #     else
+            #       raise TypeError, "#{other.class} can't be coerced into #{self.class}"
+            #     end
             #   else
-            #     raise TypeError, "#{other.class} can't be coerced into #{self.class}"
+            #     raise TypeError,
+            #       "#{self.class} can only be added with another #{self.class} outside a unit block"
             #   end
             # end
             <<~RUBY, __FILE__, __LINE__ + 1
               def #{ruby_op}(other)
                 logger.trace("\#{self} #{ruby_op} \#{other} (\#{other.class})")
                 if other.is_a?(QuantityType)
                   #{java_op}_quantity(other)
-                elsif other.is_a?(DecimalType)
-                  other = other.to_big_decimal
-                  #{java_op}_quantity(#{convert})
-                elsif other.is_a?(java.math.BigDecimal)
-                  #{java_op}_quantity(#{convert})
-                elsif other.respond_to?(:to_d)
-                  other = other.to_d.to_java
-                  #{java_op}_quantity(#{convert})
-                elsif other.respond_to?(:coerce) && (lhs, rhs = other.coerce(to_d))
-                  lhs #{ruby_op} rhs
+                elsif (thread_unit = DSL.unit(dimension))
+                  if other.is_a?(DecimalType)
+                    other = other.to_big_decimal
+                    #{java_op}_quantity(#{convert})
+                  elsif other.is_a?(java.math.BigDecimal)
+                    #{java_op}_quantity(#{convert})
+                  elsif other.respond_to?(:to_d)
+                    other = other.to_d.to_java
+                    #{java_op}_quantity(#{convert})
+                  elsif other.respond_to?(:coerce) && (lhs, rhs = other.coerce(to_d))
+                    lhs #{ruby_op} rhs
+                  else
+                    raise TypeError, "\#{other.class} can't be coerced into \#{self.class}"
+                  end
                 else
-                  raise TypeError, "\#{other.class} can't be coerced into \#{self.class}"
+                  raise TypeError,
+                    "\#{self.class} can only be #{java_op}ed with another \#{self.class} outside a unit block"
                 end
               end
             RUBY
@@ -243,13 +253,13 @@ def #{ruby_op}(other)
               def #{ruby_op}(other)
                 logger.trace("\#{self} #{ruby_op} \#{other} (\#{other.class})")
                 if other.is_a?(QuantityType)
-                  #{java_op}_quantity(other)
+                  #{java_op}_quantity(other).unitize
                 elsif other.is_a?(DecimalType)
-                  #{java_op}(other.to_big_decimal)
+                  #{java_op}(other.to_big_decimal).unitize
                 elsif other.is_a?(java.math.BigDecimal)
-                  #{java_op}(other)
+                  #{java_op}(other).unitize
                 elsif other.respond_to?(:to_d)
-                  #{java_op}(other.to_d.to_java)
+                  #{java_op}(other.to_d.to_java).unitize
                 elsif other.respond_to?(:coerce) && (lhs, rhs = other.coerce(to_d))
                   lhs #{ruby_op} rhs
                 else
@@ -262,7 +272,7 @@ def #{ruby_op}(other)
 
         # if it's a dimensionless quantity, change the unit to match other_unit
         # @!visibility private
-        def unitize(other_unit = unit)
+        def unitize(other_unit = unit, relative: false)
           # prefer converting to the thread-specified unit if there is one
           other_unit = DSL.unit(dimension) || other_unit
           logger.trace("Converting #{self} to #{other_unit}")
@@ -273,7 +283,7 @@ def unitize(other_unit = unit)
           when other_unit
             self
           else
-            to_unit(other_unit)
+            relative ? to_unit_relative(other_unit) : to_unit(other_unit)
           end
         end
 
@@ -288,16 +298,16 @@ def deunitize
 
         private
 
-        # do addition directly against a QuantityType while ensuring we unitize
-        # both sides
+        # do addition directly against a QuantityType while ensuring we unitize both sides
         def add_quantity(other)
-          unitize(other.unit).add(other.unitize(unit))
+          self_unit = (unit == Units::ONE && DSL.unit(other.dimension)) || unit
+          unitize(self_unit).add(other.unitize(relative: true))
         end
 
-        # do subtraction directly against a QuantityType while ensuring we
-        # unitize both sides
+        # do subtraction directly against a QuantityType while ensuring we unitize both sides
         def subtract_quantity(other)
-          unitize(other.unit).subtract(other.unitize(unit))
+          self_unit = (unit == Units::ONE && DSL.unit(other.dimension)) || unit
+          unitize(self_unit).subtract(other.unitize(relative: true))
         end
 
         # do multiplication directly against a QuantityType while ensuring

spec/openhab/core/types/quantity_type_spec.rb

@@ -7,24 +7,65 @@
     expect(50.to_d | "°F").to eql QuantityType.new("50.0 °F") # rubocop:disable Performance/BigDecimalWithNumericArgument
   end
 
-  it "responds to math operations" do
-    # quantity type operand
-    expect(QuantityType.new("50 °F") + QuantityType.new("50 °F")).to eql QuantityType.new("100.0 °F")
-    expect(QuantityType.new("50 °F") - QuantityType.new("25 °F")).to eql QuantityType.new("25.0 °F")
-    expect((QuantityType.new("100 °F") / QuantityType.new("2 °F")).to_i).to be 50
-    expect(QuantityType.new("50 °F") + -QuantityType.new("25 °F")).to eql QuantityType.new("25.0 °F")
-
-    # numeric operand
-    expect(QuantityType.new("50 °F") * 2).to eql QuantityType.new("100.0 °F")
-    expect(QuantityType.new("100 °F") / 2).to eql QuantityType.new("50.0 °F")
-    expect(QuantityType.new("50 °F") * 2.0).to eql QuantityType.new("100.0 °F")
-    expect(QuantityType.new("100 °F") / 2.0).to eql QuantityType.new("50.0 °F")
-
-    # DecimalType operand
-    expect(QuantityType.new("50 °F") * DecimalType.new(2)).to eql QuantityType.new("100.0 °F")
-    expect(QuantityType.new("100 °F") / DecimalType.new(2)).to eql QuantityType.new("50.0 °F")
-    expect(QuantityType.new("50 °F") * DecimalType.new(2.0)).to eql QuantityType.new("100.0 °F")
-    expect(QuantityType.new("100 °F") / DecimalType.new(2.0)).to eql QuantityType.new("50.0 °F")
+  describe "math operations" do
+    describe "additions and subtractions" do
+      it "support quantity type operand" do
+        expect(QuantityType.new("50 °F") + QuantityType.new("50 °F")).to eql QuantityType.new("100.0 °F")
+        expect(QuantityType.new("50 °F") - QuantityType.new("25 °F")).to eql QuantityType.new("25.0 °F")
+        expect(QuantityType.new("50 °F") + -QuantityType.new("25 °F")).to eql QuantityType.new("25.0 °F")
+      end
+
+      it "raise exception with non QuantityType operand" do
+        expect { QuantityType.new("50 °F") + 50 }.to raise_exception(TypeError)
+        expect { QuantityType.new("50 °F") - 50 }.to raise_exception(TypeError)
+        expect { 50 + QuantityType.new("50 °F") }.to raise_exception(javax.measure.UnconvertibleException)
+        expect { 50 - QuantityType.new("50 °F") }.to raise_exception(javax.measure.UnconvertibleException)
+      end
+    end
+
+    describe "multiplications and divisions" do
+      it "support quantity type operand" do
+        expect(QuantityType.new("100 W") * QuantityType.new("2 W")).to eql QuantityType.new("200 W²")
+        expect(QuantityType.new("100 W") / QuantityType.new("2 W")).to eql QuantityType.new("50")
+      end
+
+      it "support numeric operand" do
+        expect(QuantityType.new("50 W") * 2).to eql QuantityType.new("100.0 W")
+        expect(QuantityType.new("50 kW") * 2).to eql QuantityType.new("100.0 kW")
+        expect(2 * QuantityType.new("50 W")).to eql QuantityType.new("100.0 W")
+        expect(2 * QuantityType.new("50 kW")).to eql QuantityType.new("100.0 kW")
+        expect(QuantityType.new("100 W") / 2).to eql QuantityType.new("50.0 W")
+        expect(QuantityType.new("50 W") * 2.0).to eql QuantityType.new("100.0 W")
+        expect(2.0 * QuantityType.new("50 W")).to eql QuantityType.new("100.0 W")
+        expect(2.0 * QuantityType.new("50 kW")).to eql QuantityType.new("100.0 kW")
+        expect(QuantityType.new("100 W") / 2.0).to eql QuantityType.new("50.0 W")
+      end
+
+      it "support DecimalType operand" do
+        expect(QuantityType.new("50 W") * DecimalType.new(2)).to eql QuantityType.new("100.0 W")
+        expect(QuantityType.new("100 W") / DecimalType.new(2)).to eql QuantityType.new("50.0 W")
+        expect(QuantityType.new("50 W") * DecimalType.new(2.0)).to eql QuantityType.new("100.0 W")
+        expect(QuantityType.new("100 W") / DecimalType.new(2.0)).to eql QuantityType.new("50.0 W")
+      end
+    end
+
+    describe "with mixed units" do
+      it "normalizes units in complex expression" do
+        expect(((23 | "°C") | "°F") - (70 | "°F")).to be < 4 | "°F"
+      end
+
+      it "supports arithmetic" do
+        expect((20 | "°C") + (9 | "°F")).to eql 25 | "°C"
+        expect((25 | "°C") - (9 | "°F")).to eql 20 | "°C"
+      end
+
+      it "works in a unit block" do
+        unit("°C") do
+          expect((20 | "°C") + (9 | "°F")).to eql 25 | "°C"
+          expect((25 | "°C") - (9 | "°F")).to eql 20 | "°C"
+        end
+      end
+    end
   end
 
   it "can be compared" do
@@ -71,10 +112,6 @@
     expect((0 | "W")..(10 | "W")).to cover(10 | "W")
   end
 
-  it "normalizes units in complex expression" do
-    expect(((23 | "°C") | "°F") - (70 | "°F")).to be < 4 | "°F"
-  end
-
   describe "comparisons" do
     let(:ten_c) { QuantityType.new("10 °C") }
     let(:five_c) { QuantityType.new("5 °C") }

spec/openhab/dsl_spec.rb

@@ -244,22 +244,43 @@
     it "converts all units and numbers to specific unit for all operations" do
       c = 23 | "°C"
       f = 70 | "°F"
+      # f.to_unit(SIUnits::CELSIUS) = 21.11 °C
+      # f.to_unit_relative(SIUnits::CELSIUS) = 38.89 °C
       unit("°F") do
         expect(c - f < 4).to be true
-        expect(c - (24 | "°C") < 4).to be true
-        expect(QuantityType.new("24 °C") - c < 4).to be true
+        expect(c - (24 | "°C") < 32).to be true
+        expect(QuantityType.new("24 °C") - c < 34).to be true
       end
 
       unit("°C") do
         expect(f - (20 | "°C") < 2).to be true
         expect((f - 2).format("%.1f %unit%")).to eq "19.1 °C"
-        expect((c + f).format("%.1f %unit%")).to eq "44.1 °C"
+        expect((c + f).format("%.1f %unit%")).to eq "61.9 °C"
         expect(f - 2 < 20).to be true
+        expect(40 - f < 2).to be true
         expect(2 + c == 25).to be true
-        expect(2 * c == 46).to be true
-        expect((2 * (f + c) / 2) < 45).to be true
+        expect(c + 2 == 25).to be true
         expect([c, f, 2].min).to be 2
       end
+
+      # The behavior of Multiplications and Divisions with non zero-based units such as °C and °F
+      # (as opposed to Kelvin) is different between OH 4.1 and previous versions.
+      # See https://github.com/openhab/openhab-core/pull/3792
+      # Use a zero-based unit to have a consistent result across OH versions.
+      w = 5 | "W"
+      kw = 5 | "kW"
+      unit("W") do
+        # numeric rhs
+        expect(w * 2 == 10).to be true
+        expect((kw * 2).format("%.0f %unit%")).to eq "10000 W"
+        expect(w / 5).to eql 1 | "W"
+        # numeric lhs
+        expect(2 * w).to eql 10 | "W"
+        expect((2 * kw).to_i).to eq 10_000
+        expect(2 * w == 10).to be true
+        expect(5 / w).to eql 1 | "W"
+        expect((2 * w / 2)).to eql w
+      end
     end
 
     it "supports setting multiple dimensions at once" do