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

Fix QuantityType arithmetic of mixed units #147

Merged
merged 3 commits into from
Sep 17, 2023

Conversation

jimtng
Copy link
Contributor

@jimtng jimtng commented Sep 16, 2023

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.

  1. 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
  1. 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
  1. +/- 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
  1. Fix failing specs due to core changes in Adjust QuantityType calculations for temperatures openhab-core#3792

@jimtng jimtng added the bug Something isn't working label Sep 16, 2023
@jimtng jimtng requested a review from ccutrer September 16, 2023 13:39
@jimtng jimtng force-pushed the quantity-arithmetics branch 2 times, most recently from a68cf1f to 20aaeae Compare September 17, 2023 01:39
Signed-off-by: Jimmy Tanagra <[email protected]>
@jimtng jimtng force-pushed the quantity-arithmetics branch 3 times, most recently from 1a7056a to fc29b57 Compare September 17, 2023 07:42
@jimtng jimtng force-pushed the quantity-arithmetics branch 2 times, most recently from 507a589 to 65b5507 Compare September 17, 2023 11:25
@jimtng jimtng marked this pull request as ready for review September 17, 2023 11:31
@jimtng
Copy link
Contributor Author

jimtng commented Sep 17, 2023

PS the last 2 commits are a bit intermixed, so perhaps just squash-merge them.

@jimtng jimtng force-pushed the quantity-arithmetics branch from 65b5507 to c81b9e1 Compare September 17, 2023 11:58
@ccutrer ccutrer merged commit 3624dfc into openhab:main Sep 17, 2023
@jimtng jimtng deleted the quantity-arithmetics branch September 18, 2023 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants