openhab / openhab-js

openHAB JavaScript Library for JavaScript Scripting Automation
https://www.openhab.org/addons/automation/jsscripting/
Eclipse Public License 2.0
38 stars 31 forks source link

Add `js-quantities` for units of measurement handling #201

Closed florian-h05 closed 1 year ago

florian-h05 commented 1 year ago

Replaced by #206.

Fixes #102. Closes #200. Fixes https://github.com/openhab/openhab-webui/issues/836.

Description

This PR adds the fantastic js-quantities library to provide easy QuantityType handling and unit conversions.

Given the openHAB UoM Docs, I have created unit tests to check compatibility of all base units from openHAB with the js-quantities library.

After polyfilling support for some additional unit notations (e.g. 'Ws' is W*s) and four units (see https://github.com/florian-h05/openhab-js/blob/js-quantities/quantities.js, the required math is linked), the new Quantity namespace is able to handle all units from openHAB except octet for data amount and mired for Temperature.

FYI: The addition of this increases the bundle site from 857KB to 932KB, which is minimal given the advantage of having the library and it's functionality. js-quantities has no dependencies.

I will add the docs in a follow-up PR.

Testing

The additional units I've added are tested by unit tests as well to verify that they work mathematically correct.

/ping @stefan-hoehn @rkoshak @digitaldan

@digitaldan @jpg0 I would like to get your opinion on the addition of such a library and possibly also some review, there is not much to review. The real code is just 60 lines.

florian-h05 commented 1 year ago

@digitaldan Thanks for your positive feedback in the issue #200.

Can you please have a look at the code (the actual important code is just 60 lines: quantities.js)? FYI: I have provided links to all the math but the units test check whether the math is correct. Please add your approval if you are happy, I will merge then.

The major part of changes is just package-lock and unit tests.

jpg0 commented 1 year ago

@florian-h05 I think that it's a great addition to incorporate unit handling. The main question I have is why it's not possible to use the native openHAB logic to do this? Presumably it also needs to handling parsing strings into unit representations? It seems weird to have the core handle this, but also include additional logic in a JS library to do the same thing (or similar, which isn't great). It also means that other scripting languages don't benefit.

I'm guessing that there are fundamental reasons why this cannot be handled in core, but was interested to know why?

florian-h05 commented 1 year ago

there needs to be a section in the readme describing its use and a couple of common examples (temp and power especially)

Yes, this is something for a follow-up PR.

@digitaldan Have you read @jpg0 comments on the issue #200? What do you think regarding his question?

florian-h05 commented 1 year ago

@digitaldan I'll also add some conversion logic to the addon like you did for JS-Joda so that our JS Quantity is translated to a QuantityType when "send" to an openHAB API.

florian-h05 commented 1 year ago

@digitaldan Note that this has been replaced by #206. I‘ll close this here!