Add some requested new units : J/m², gr/ft³,gr #4467

clinique · 2024-12-03T10:20:26Z

Partially solves #4294 and #4143
Resolves #4307
Resolves #4346

Signed-off-by: Gaël L'hopital <[email protected]>

This reverts commit cdfceb1.

Signed-off-by: Gaël L'hopital <[email protected]>

Signed-off-by: Gaël L'hopital <[email protected]> Signed-off-by: Gaël L'hopital <[email protected]>

Signed-off-by: Gaël L'hopital <[email protected]>

jlaur · 2024-12-03T21:59:44Z

bundles/org.openhab.core/src/main/java/org/openhab/core/library/dimension/EnergyDensity.java

+import org.eclipse.jdt.annotation.NonNullByDefault;
+
+/**
+ * The {@link EnergyDensity} defines the dimension for prices


Suggested change

* The {@link EnergyDensity} defines the dimension for prices

* The {@link EnergyDensity} defines the dimension for energy density

jlaur · 2024-12-03T22:03:04Z

bundles/org.openhab.core/src/main/java/org/openhab/core/library/unit/Units.java

@@ -91,13 +92,15 @@ public final class Units extends CustomUnits {
    public static final Unit<AmountOfSubstance> MOLE = addUnit(tech.units.indriya.unit.Units.MOLE);
    public static final Unit<Volume> LITRE = addUnit(tech.units.indriya.unit.Units.LITRE);
    @SuppressWarnings("unchecked")


Is this suppression still needed?

Tried to get rid of them but tests fails then.

jlaur · 2024-12-03T22:06:25Z

bundles/org.openhab.core/src/main/java/org/openhab/core/library/dimension/EnergyDensity.java

+import org.eclipse.jdt.annotation.NonNullByDefault;
+
+/**
+ * The {@link EnergyDensity} defines the dimension for prices


Perhaps a link could be added like for some of the other dimensions? E.g. https://en.wikipedia.org/wiki/Energy_density

Reading that, is this dimension correct for e.g. Wh/m²? That seems to be something like Wh/L, i.e. per volume:

Energy density refers to the quantity of energy in a material per unit of volume.

whereas Wh/m² is more like unit of energy in relation to surface area?

I don't know the answers yet, only the questions. 😄

Please see: #4346

Yes, you're correct; it needs to be reworded. Made some more searches and it should be named Surface Energy (surprise :-))

Hmm, both surface energy and radiant exposure can be expressed in unit J/m², but I'm wondering if https://en.wikipedia.org/wiki/Radiant_exposure is closer to the requested dimension in the linked issue? I guess it depends on the context. A quick ChatGTP session:

Radiant exposure (denoted as 𝐻) is the energy received per unit area of a surface from radiation over a given time period.

Surface energy (𝛾) is the amount of energy per unit area required to create a new surface. It quantifies the cohesive forces between molecules at a material's surface.

Yes, agreed. These notions use the same measure and we can understand these are the two sides of the same coin. You're correct that in an home automation context, we're more likely to talk about radiant exposure than surface energy (even if yesterday I did even not know these :-P)

Signed-off-by: Gaël L'hopital <[email protected]>

jlaur · 2024-12-04T10:16:41Z

bundles/org.openhab.core/src/main/java/org/openhab/core/internal/i18n/I18nProviderImpl.java

@@ -433,6 +434,7 @@ public static Map<Class<? extends Quantity<?>>, Map<SystemOfUnits, Unit<? extend
        addDefaultUnit(dimensionMap, Radioactivity.class, Units.BECQUEREL);
        addDefaultUnit(dimensionMap, SolidAngle.class, Units.STERADIAN);
        addDefaultUnit(dimensionMap, Speed.class, SIUnits.KILOMETRE_PER_HOUR, ImperialUnits.MILES_PER_HOUR);
+        addDefaultUnit(dimensionMap, SurfaceEnergy.class, Units.WATT_HOUR_PER_SQUARE_METRE);


Consider if default unit should be J/m²? I'm not sure what is most common, but that seems to be the SI unit.

You're correct, the default unit should rather be J/m2. Added this one and changed accordingly.

Signed-off-by: Gaël L'hopital <[email protected]>

lsiepel · 2024-12-04T19:44:20Z

This PR would also 'require' updates to other repo's:

Documenation: https://github.com/openhab/openhab-docs/blob/main/concepts/units-of-measurement.md
MainUI: https://github.com/openhab/openhab-webui/blob/main/bundles/org.openhab.ui/web/src/assets/units.js
other ui?
Addons: bindings currenlty using Indriya library to create the custom units need to change to core provided units.

clinique · 2024-12-05T16:50:52Z

@lsiepel : PR created for documentation

lsiepel · 2024-12-05T17:25:20Z

bundles/org.openhab.core/src/main/java/org/openhab/core/library/unit/Units.java

@@ -91,13 +92,15 @@ public final class Units extends CustomUnits {
    public static final Unit<AmountOfSubstance> MOLE = addUnit(tech.units.indriya.unit.Units.MOLE);
    public static final Unit<Volume> LITRE = addUnit(tech.units.indriya.unit.Units.LITRE);
    @SuppressWarnings("unchecked")
-    public static final Unit<AmountOfSubstance> DEUTSCHE_HAERTE = addUnit(new TransformedUnit<>("°dH",
-            (Unit<AmountOfSubstance>) MetricPrefix.MILLI(Units.MOLE).divide(Units.LITRE), MultiplyConverter.of(5.6)));
+    public static final Unit<Dimensionless> DEUTSCHE_HAERTE = addUnit((Unit<Dimensionless>) new TransformedUnit<>("°dH",


Might be perfectly sane, but wondering why is this changed to Dimensionless?

See this comment

jimtng · 2024-12-06T02:28:20Z

Suggestion: make the PR title list all the new units added, or at least hint at what type of units were added. It would make for a much more informative release log

clinique · 2024-12-06T10:12:30Z

Suggestion: make the PR title list all the new units added, or at least hint at what type of units were added. It would make for a much more informative release log

Done

holgerfriedrich

Overall, LGTM.
Could you please cover JOULE_PER_SQUARE_METRE in the tests, or did I overlook it?

holgerfriedrich · 2024-12-08T11:56:56Z

@lsiepel should we delay this (i.e., shift to 5.0) because of further implications to other parts of OH?
#4467 (comment)

lsiepel · 2024-12-08T12:17:25Z

@lsiepel should we delay this (i.e., shift to 5.0) because of further implications to other parts of OH? #4467 (comment)

My first thought was to merge it so future addons would be compatible with 4.3.x core api’s but as 5.0 would be breaking anyway (Java 21) there is no point.
So I guess it is better to merge after 4.3.0 release

Signed-off-by: Gaël L'hopital <[email protected]>

clinique · 2024-12-09T09:53:31Z

Overall, LGTM. Could you please cover JOULE_PER_SQUARE_METRE in the tests, or did I overlook it?

Done

jlaur · 2024-12-09T20:43:00Z

So I guess it is better to merge after 4.3.0 release

Please note that openhab/openhab-docs#2420 was already merged, so if this PR is not merged, that PR should be reverted. If there is anything breaking in this PR (I don't see it?), perhaps the non-breaking changes could be extracted to a separate PR?

lsiepel

In that case maybe @holgerfriedrich can merge this. Not much risk.

holgerfriedrich · 2024-12-10T20:02:42Z

@florian-h05 @lsiepel Can you explain what is the required change to units.js mentioned above? Will you take care?

Refs openhab/openhab-core#4467. Signed-off-by: Florian Hotze <[email protected]>

florian-h05 · 2024-12-11T16:47:19Z

i have taken care: openhab/openhab-webui#2912
Those unit definitions are used for the Item editor to suggest dimensions and avalable units.

Refs openhab/openhab-core#4467. Signed-off-by: Florian Hotze <[email protected]>

lsiepel · 2024-12-11T16:58:19Z

Ping @magx2 for salus binding

magx2 · 2024-12-11T20:18:16Z

Ping @magx2 for salus binding

Thanks!

clinique requested a review from a team as a code owner December 3, 2024 10:20

clinique added 4 commits December 3, 2024 11:26

Added some convenient methods for DateTimeType

dda9ee5

Signed-off-by: Gaël L'hopital <[email protected]>

Revert "Added some convenient methods for DateTimeType"

b51181c

This reverts commit cdfceb1.

Adding some units requested in open issues

5237125

Signed-off-by: Gaël L'hopital <[email protected]>

Adding some units requested in open issues

8db6438

Signed-off-by: Gaël L'hopital <[email protected]> Signed-off-by: Gaël L'hopital <[email protected]>

clinique force-pushed the core_units branch from df0bd5e to 8db6438 Compare December 3, 2024 10:38

clinique added 4 commits December 3, 2024 11:39

Removing file

dd02767

Signed-off-by: Gaël L'hopital <[email protected]>

Add resolution of openhab#4307

2146ca5

Signed-off-by: Gaël L'hopital <[email protected]>

Adding Solar Irradiance

2695c3d

Signed-off-by: Gaël L'hopital <[email protected]>

Adding EnergyDensity to i18n

359806f

Signed-off-by: Gaël L'hopital <[email protected]>

jlaur reviewed Dec 3, 2024

View reviewed changes

clinique added 3 commits December 4, 2024 09:34

Some corrections

e14410a

Signed-off-by: Gaël L'hopital <[email protected]>

Fixing format issue

c53abe3

Signed-off-by: Gaël L'hopital <[email protected]>

Enable tests

4d778a9

Signed-off-by: Gaël L'hopital <[email protected]>

jlaur reviewed Dec 4, 2024

View reviewed changes

clinique added 4 commits December 4, 2024 13:50

Change default unit

822d3d2

Signed-off-by: Gaël L'hopital <[email protected]>

Switching from Surface Energy to Radiant Exposure

59a4d4c

Signed-off-by: Gaël L'hopital <[email protected]>

Correction of class doc

f9483cc

Signed-off-by: Gaël L'hopital <[email protected]>

Correction of alphabetical order

ddd248a

Signed-off-by: Gaël L'hopital <[email protected]>

holgerfriedrich added the enhancement An enhancement or new feature of the Core label Dec 4, 2024

clinique mentioned this pull request Dec 5, 2024

Add documentation for new units openhab/openhab-docs#2420

Merged

lsiepel reviewed Dec 5, 2024

View reviewed changes

clinique changed the title ~~Add some requested new units~~ Add some requested new units : J/m², gr/ft³,gr Dec 6, 2024

holgerfriedrich reviewed Dec 7, 2024

View reviewed changes

holgerfriedrich approved these changes Dec 8, 2024

View reviewed changes

Adds unit test for Joule per m2

9634390

Signed-off-by: Gaël L'hopital <[email protected]>

holgerfriedrich approved these changes Dec 9, 2024

View reviewed changes

lsiepel approved these changes Dec 9, 2024

View reviewed changes

holgerfriedrich merged commit 241e55f into openhab:main Dec 10, 2024
4 of 5 checks passed

holgerfriedrich added this to the 4.3 milestone Dec 10, 2024

florian-h05 added a commit to florian-h05/openhab-webui that referenced this pull request Dec 11, 2024

Update unit definitions

3b9730c

Refs openhab/openhab-core#4467. Signed-off-by: Florian Hotze <[email protected]>

florian-h05 mentioned this pull request Dec 11, 2024

Update unit definitions openhab/openhab-webui#2912

Merged

florian-h05 added a commit to openhab/openhab-webui that referenced this pull request Dec 11, 2024

Update unit definitions (#2912)

b623325

Refs openhab/openhab-core#4467. Signed-off-by: Florian Hotze <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add some requested new units : J/m², gr/ft³,gr #4467

Add some requested new units : J/m², gr/ft³,gr #4467

clinique commented Dec 3, 2024 •

edited

Loading

jlaur Dec 3, 2024

clinique Dec 4, 2024

jlaur Dec 3, 2024

clinique Dec 4, 2024 •

edited

Loading

jlaur Dec 3, 2024

jlaur Dec 3, 2024

lsiepel Dec 3, 2024 •

edited

Loading

clinique Dec 4, 2024 •

edited

Loading

jlaur Dec 4, 2024

clinique Dec 4, 2024

jlaur Dec 4, 2024

clinique Dec 4, 2024

lsiepel commented Dec 4, 2024 •

edited

Loading

clinique commented Dec 5, 2024

lsiepel Dec 5, 2024 •

edited

Loading

clinique Dec 6, 2024

jimtng commented Dec 6, 2024 •

edited

Loading

clinique commented Dec 6, 2024

holgerfriedrich left a comment

holgerfriedrich commented Dec 8, 2024

lsiepel commented Dec 8, 2024

clinique commented Dec 9, 2024

jlaur commented Dec 9, 2024 •

edited

Loading

lsiepel left a comment

holgerfriedrich commented Dec 10, 2024

florian-h05 commented Dec 11, 2024

lsiepel commented Dec 11, 2024 •

edited

Loading

magx2 commented Dec 11, 2024

	* The {@link EnergyDensity} defines the dimension for prices
	* The {@link EnergyDensity} defines the dimension for energy density

Add some requested new units : J/m², gr/ft³,gr #4467

Add some requested new units : J/m², gr/ft³,gr #4467

Conversation

clinique commented Dec 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clinique Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lsiepel Dec 3, 2024 • edited Loading

Choose a reason for hiding this comment

clinique Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lsiepel commented Dec 4, 2024 • edited Loading

clinique commented Dec 5, 2024

lsiepel Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jimtng commented Dec 6, 2024 • edited Loading

clinique commented Dec 6, 2024

holgerfriedrich left a comment

Choose a reason for hiding this comment

holgerfriedrich commented Dec 8, 2024

lsiepel commented Dec 8, 2024

clinique commented Dec 9, 2024

jlaur commented Dec 9, 2024 • edited Loading

lsiepel left a comment

Choose a reason for hiding this comment

holgerfriedrich commented Dec 10, 2024

florian-h05 commented Dec 11, 2024

lsiepel commented Dec 11, 2024 • edited Loading

magx2 commented Dec 11, 2024

clinique commented Dec 3, 2024 •

edited

Loading

clinique Dec 4, 2024 •

edited

Loading

lsiepel Dec 3, 2024 •

edited

Loading

clinique Dec 4, 2024 •

edited

Loading

lsiepel commented Dec 4, 2024 •

edited

Loading

lsiepel Dec 5, 2024 •

edited

Loading

jimtng commented Dec 6, 2024 •

edited

Loading

jlaur commented Dec 9, 2024 •

edited

Loading

lsiepel commented Dec 11, 2024 •

edited

Loading