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.

Sign up for GitHub

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

Convert units for bounds when compiling full state descriptions #3132

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Convert units for bounds when compiling full state descriptions #3132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 86 additions & 1 deletion ...nhab.core/src/main/java/org/openhab/core/internal/types/StateDescriptionFragmentImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,21 @@
package org.openhab.core.internal.types;

import java.math.BigDecimal;
import java.math.RoundingMode;
import java.util.Collections;
import java.util.List;
import java.util.regex.Pattern;

import javax.measure.Unit;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.library.types.QuantityType;
import org.openhab.core.types.StateDescription;
import org.openhab.core.types.StateDescriptionFragment;
import org.openhab.core.types.StateDescriptionFragmentBuilder;
import org.openhab.core.types.StateOption;
import org.openhab.core.types.util.UnitUtils;

/**
* Data holder for StateDescriptionFragment creation.
Expand All @@ -30,6 +36,8 @@
*/
@NonNullByDefault
public class StateDescriptionFragmentImpl implements StateDescriptionFragment {
private static final Pattern PATTERN_PRECISION_PATTERN = Pattern.compile("%[-#+ ,\\(<0-9$]*.(\\d+)[eEf]");
private static final String INTEGER_FORMAT_TYPE = "d";

private class StateDescriptionImpl extends StateDescription {
StateDescriptionImpl(@Nullable BigDecimal minimum, @Nullable BigDecimal maximum, @Nullable BigDecimal step,
Expand Down Expand Up @@ -182,13 +190,90 @@ public void setOptions(List<StateOption> options) {
* @return this instance with the fields merged.
*/
public StateDescriptionFragment merge(StateDescriptionFragment fragment) {
String newPattern = fragment.getPattern();
// Do unit conversions if possible.
// Example:
// The GenericItemProvider sets a pattern of ° F, but no min, max, or step.
// The ChannelStateDescriptionProvider sets a pattern of ° C, min of 0, max of 100, step of 0.5
// The latter is lower priority, so gets merged into the former.
// We want to construct a final description with a pattern of ° F, min of 32, max of 212, and step of 0.9
//
// In other words, we keep the user's overridden unit, but convert the bounds provided by the
// channel (that is describing the bounds in terms of its unit) to the user's preferred unit.
boolean skipStep = false;
if (pattern != null && newPattern != null) {
Unit<?> oldUnit = UnitUtils.parseUnit(pattern);
Unit<?> newUnit = UnitUtils.parseUnit(newPattern);
if (oldUnit != null && newUnit != null && !oldUnit.equals(newUnit)
&& (oldUnit.isCompatible(newUnit) || oldUnit.inverse().isCompatible(newUnit))) {
BigDecimal newValue;
// when inverting, min and max will swap
if (oldUnit.inverse().isCompatible(newUnit)) {
// It's highly likely that an invertible unit conversion will end up with a very long decimal
// So use the format to round min/max to what we're going to display.
Integer scale = null;
var m = PATTERN_PRECISION_PATTERN.matcher(pattern);
if (m.find()) {
String precision = m.group(1);
if (precision != null) {
scale = Integer.valueOf(precision);
} else if (m.group(2).equals(INTEGER_FORMAT_TYPE)) {
scale = 0;
}
}

if (minimum == null && (newValue = fragment.getMaximum()) != null) {
minimum = new QuantityType(newValue, newUnit).toInvertibleUnit(oldUnit).toBigDecimal();
Comment on lines +224 to +226
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this works, I would prefer a more explicit style here like:

Suggested change
if (minimum == null && (newValue = fragment.getMaximum()) != null) {
minimum = new QuantityType(newValue, newUnit).toInvertibleUnit(oldUnit).toBigDecimal();
BigDecimal newMinimum = fragment.getMaximum();
if (minimum == null && newMinimum != null) {
minimum = new QuantityType(newMinimum, newUnit).toInvertibleUnit(oldUnit).toBigDecimal();

The same of course also applies to the other locations.

if (minimum.scale() > 0) {
minimum = minimum.stripTrailingZeros();
}
if (scale != null && minimum.scale() > scale) {
minimum = minimum.setScale(scale, RoundingMode.FLOOR);
}
}
if (maximum == null && (newValue = fragment.getMinimum()) != null) {
maximum = new QuantityType(newValue, newUnit).toInvertibleUnit(oldUnit).toBigDecimal();
if (maximum.scale() > 0) {
maximum = maximum.stripTrailingZeros();
}
if (scale != null && maximum.scale() > scale) {
maximum = maximum.setScale(scale, RoundingMode.CEILING);
}
}

// Invertible units cannot have a linear relationship, so just leave step blank.
// Make sure it doesn't get overwritten below with a non-sensical value
skipStep = true;
} else {
if (minimum == null && (newValue = fragment.getMinimum()) != null) {
minimum = new QuantityType(newValue, newUnit).toInvertibleUnit(oldUnit).toBigDecimal();
if (minimum.scale() > 0) {
minimum = minimum.stripTrailingZeros();
}
}
if (maximum == null && (newValue = fragment.getMaximum()) != null) {
maximum = new QuantityType(newValue, newUnit).toInvertibleUnit(oldUnit).toBigDecimal();
if (maximum.scale() > 0) {
maximum = maximum.stripTrailingZeros();
}
}
if (step == null && (newValue = fragment.getStep()) != null) {
step = new QuantityType(newValue, newUnit).toUnitRelative(oldUnit).toBigDecimal();
if (step.scale() > 0) {
step = step.stripTrailingZeros();
}
}
}
}
}

if (minimum == null) {
minimum = fragment.getMinimum();
}
if (maximum == null) {
maximum = fragment.getMaximum();
}
if (step == null) {
if (step == null && !skipStep) {
step = fragment.getStep();
}
if (pattern == null) {
Expand Down
37 changes: 37 additions & 0 deletions ....core/src/test/java/org/openhab/core/internal/types/StateDescriptionFragmentImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;

import java.math.BigDecimal;
import java.util.List;
Expand Down Expand Up @@ -74,6 +75,42 @@ public void mergeFragment() {
assertThat(fragment.getOptions(), is(not(sourceWithOptions.getOptions())));
}

@Test
public void mergeFragmentWithUnits() {
StateDescriptionFragmentImpl userFragment = new StateDescriptionFragmentImpl();
userFragment.setPattern("%0.0f °F");

StateDescriptionFragmentImpl channelFragment = new StateDescriptionFragmentImpl();
channelFragment.setPattern("%0.1f °C");
channelFragment.setMinimum(BigDecimal.ZERO);
channelFragment.setMaximum(new BigDecimal(100));
channelFragment.setStep(new BigDecimal(0.5));

userFragment.merge(channelFragment);
assertThat(userFragment.getPattern(), is("%0.0f °F"));
assertThat(userFragment.getMinimum(), is(new BigDecimal(32)));
assertThat(userFragment.getMaximum(), is(new BigDecimal(212)));
assertThat(userFragment.getStep(), is(new BigDecimal("0.9")));
}

@Test
public void mergeFragmentWithInvertibleUnits() {
StateDescriptionFragmentImpl userFragment = new StateDescriptionFragmentImpl();
userFragment.setPattern("%0.0f K");

StateDescriptionFragmentImpl channelFragment = new StateDescriptionFragmentImpl();
channelFragment.setPattern("%0.0f mired");
channelFragment.setMinimum(new BigDecimal(153));
channelFragment.setMaximum(new BigDecimal(400));
channelFragment.setStep(BigDecimal.ONE);

userFragment.merge(channelFragment);
assertThat(userFragment.getPattern(), is("%0.0f K"));
assertThat(userFragment.getMinimum(), is(new BigDecimal(2500)));
assertThat(userFragment.getMaximum(), is(new BigDecimal(6536)));
assertThat(userFragment.getStep(), is(nullValue()));
}

@Test
@SuppressWarnings("null")
public void toStateDescription() {
Expand Down