Skip to content

Commit 156e691

Browse files
ccutrerccutrer
authored
[mqtt.homeassistant] fix multi-speed fans (openhab#17813)
* fix step math so that the state description represents the step scaled to 0-100% Signed-off-by: Cody Cutrer <[email protected]>
1 parent d0ea14f commit 156e691

File tree

11 files changed

+122
-30
lines changed

11 files changed

+122
-30
lines changed

‎bundles/org.openhab.binding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/values/PercentageValue.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,10 @@ public class PercentageValue extends Value {
5454
private final BigDecimal stepPercent;
5555
private final @Nullable String onValue;
5656
private final @Nullable String offValue;
57+
private final @Nullable String formatOverride;
5758

5859
public PercentageValue(@Nullable BigDecimal min, @Nullable BigDecimal max, @Nullable BigDecimal step,
59-
@Nullable String onValue, @Nullable String offValue) {
60+
@Nullable String onValue, @Nullable String offValue, @Nullable String formatOverride) {
6061
super(CoreItemFactory.DIMMER, List.of(DecimalType.class, QuantityType.class, IncreaseDecreaseType.class,
6162
OnOffType.class, UpDownType.class, StringType.class));
6263
this.onValue = onValue;
@@ -69,6 +70,7 @@ public PercentageValue(@Nullable BigDecimal min, @Nullable BigDecimal max, @Null
6970
this.span = this.max.subtract(this.min);
7071
this.step = step == null ? BigDecimal.ONE : step;
7172
this.stepPercent = this.step.multiply(HUNDRED).divide(this.span, MathContext.DECIMAL128);
73+
this.formatOverride = formatOverride;
7274
}
7375

7476
@Override
@@ -135,7 +137,10 @@ public Command parseCommand(Command command) throws IllegalArgumentException {
135137

136138
@Override
137139
public String getMQTTpublishValue(Command command, @Nullable String pattern) {
138-
String formatPattern = pattern;
140+
String formatPattern = this.formatOverride;
141+
if (formatPattern == null) {
142+
formatPattern = pattern;
143+
}
139144
if (formatPattern == null) {
140145
formatPattern = "%s";
141146
}
@@ -170,7 +175,7 @@ public String getMQTTpublishValue(Command command, @Nullable String pattern) {
170175

171176
@Override
172177
public StateDescriptionFragmentBuilder createStateDescription(boolean readOnly) {
173-
return super.createStateDescription(readOnly).withMaximum(HUNDRED).withMinimum(BigDecimal.ZERO).withStep(step)
174-
.withPattern("%.0f %%");
178+
return super.createStateDescription(readOnly).withMaximum(HUNDRED).withMinimum(BigDecimal.ZERO)
179+
.withStep(stepPercent).withPattern("%.0f %%");
175180
}
176181
}

‎bundles/org.openhab.binding.mqtt.generic/src/main/java/org/openhab/binding/mqtt/generic/values/ValueFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public static Value createValueState(ChannelConfig config, String channelTypeID)
5454
value = new NumberValue(config.min, config.max, config.step, UnitUtils.parseUnit(config.unit));
5555
break;
5656
case MqttBindingConstants.DIMMER:
57-
value = new PercentageValue(config.min, config.max, config.step, config.on, config.off);
57+
value = new PercentageValue(config.min, config.max, config.step, config.on, config.off, null);
5858
break;
5959
case MqttBindingConstants.COLOR_HSB:
6060
value = new ColorValue(ColorMode.HSB, config.on, config.off, config.onBrightness);

‎bundles/org.openhab.binding.mqtt.generic/src/test/java/org/openhab/binding/mqtt/generic/ChannelStateTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ public void receiveDecimalAsPercentageUnitTest() {
267267
@Test
268268
public void receivePercentageTest() {
269269
PercentageValue value = new PercentageValue(new BigDecimal(-100), new BigDecimal(100), new BigDecimal(10), null,
270-
null);
270+
null, null);
271271
ChannelState c = spy(new ChannelState(config, channelUIDMock, value, channelStateUpdateListenerMock));
272272
c.start(connectionMock, mock(ScheduledExecutorService.class), 100);
273273

‎bundles/org.openhab.binding.mqtt.generic/src/test/java/org/openhab/binding/mqtt/generic/values/ValueTests.java

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import static org.junit.jupiter.api.Assertions.*;
1818

1919
import java.math.BigDecimal;
20+
import java.math.MathContext;
2021
import java.util.Objects;
2122

2223
import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -99,7 +100,7 @@ public void illegalNumberCommand() {
99100

100101
@Test
101102
public void illegalPercentCommand() {
102-
PercentageValue v = new PercentageValue(null, null, null, null, null);
103+
PercentageValue v = new PercentageValue(null, null, null, null, null, null);
103104
assertThrows(IllegalStateException.class, () -> v.parseCommand(new StringType("demo")));
104105
}
105106

@@ -111,7 +112,7 @@ public void illegalOnOffCommand() {
111112

112113
@Test
113114
public void illegalPercentUpdate() {
114-
PercentageValue v = new PercentageValue(null, null, null, null, null);
115+
PercentageValue v = new PercentageValue(null, null, null, null, null, null);
115116
assertThrows(IllegalArgumentException.class, () -> v.parseCommand(new DecimalType(101.0)));
116117
}
117118

@@ -304,7 +305,9 @@ public void rollershutterUpdateWithOutStrings() {
304305
@Test
305306
public void percentCalc() {
306307
PercentageValue v = new PercentageValue(new BigDecimal(10.0), new BigDecimal(110.0), new BigDecimal(1.0), null,
307-
null);
308+
null, null);
309+
assertThat(v.createStateDescription(false).build().getStep(), is(new BigDecimal(1)));
310+
308311
assertThat(v.parseCommand(new DecimalType("110.0")), is(PercentType.HUNDRED));
309312
assertThat(v.getMQTTpublishValue(PercentType.HUNDRED, null), is("110"));
310313
assertThat(v.parseCommand(new DecimalType(10.0)), is(PercentType.ZERO));
@@ -316,9 +319,20 @@ public void percentCalc() {
316319
assertThat(v.getMQTTpublishValue(OnOffType.OFF, null), is("10"));
317320
}
318321

322+
@Test
323+
public void percentFormatOverride() {
324+
PercentageValue v = new PercentageValue(BigDecimal.ZERO, new BigDecimal(3.0), null, null, null, "%.0f");
325+
assertThat(v.createStateDescription(false).build().getStep(),
326+
is(new BigDecimal(100).divide(new BigDecimal(3), MathContext.DECIMAL128)));
327+
assertThat(v.getMQTTpublishValue(PercentType.HUNDRED, null), is("3"));
328+
assertThat(v.getMQTTpublishValue(PercentType.valueOf("67"), null), is("2"));
329+
assertThat(v.getMQTTpublishValue(PercentType.valueOf("33"), null), is("1"));
330+
assertThat(v.getMQTTpublishValue(PercentType.ZERO, null), is("0"));
331+
}
332+
319333
@Test
320334
public void percentMQTTValue() {
321-
PercentageValue v = new PercentageValue(null, null, null, null, null);
335+
PercentageValue v = new PercentageValue(null, null, null, null, null, null);
322336
assertThat(v.parseCommand(new DecimalType("10.10000")), is(new PercentType("10.1")));
323337
assertThat(v.getMQTTpublishValue(new PercentType("10.1"), null), is("10.1"));
324338
Command command;
@@ -333,7 +347,7 @@ public void percentMQTTValue() {
333347
@Test
334348
public void percentCustomOnOff() {
335349
PercentageValue v = new PercentageValue(new BigDecimal("0.0"), new BigDecimal("100.0"), new BigDecimal("1.0"),
336-
"on", "off");
350+
"on", "off", null);
337351
assertThat(v.parseCommand(new StringType("on")), is(OnOffType.ON));
338352
assertThat(v.getMQTTpublishValue(OnOffType.ON, "%s"), is("on"));
339353
assertThat(v.parseCommand(new StringType("off")), is(OnOffType.OFF));
@@ -343,7 +357,7 @@ public void percentCustomOnOff() {
343357
@Test
344358
public void decimalCalc() {
345359
PercentageValue v = new PercentageValue(new BigDecimal("0.1"), new BigDecimal("1.0"), new BigDecimal("0.1"),
346-
null, null);
360+
null, null, null);
347361
assertThat(v.parseCommand(new DecimalType(1.0)), is(PercentType.HUNDRED));
348362
assertThat(v.parseCommand(new DecimalType(0.1)), is(PercentType.ZERO));
349363
PercentType command = (PercentType) v.parseCommand(new DecimalType(0.2));
@@ -353,7 +367,7 @@ public void decimalCalc() {
353367
@Test
354368
public void increaseDecreaseCalc() {
355369
PercentageValue v = new PercentageValue(new BigDecimal("1.0"), new BigDecimal("11.0"), new BigDecimal("0.5"),
356-
null, null);
370+
null, null, null);
357371

358372
// Normal operation.
359373
PercentType command = (PercentType) v.parseCommand(new DecimalType("6.0"));
@@ -382,7 +396,7 @@ public void increaseDecreaseCalc() {
382396
@Test
383397
public void upDownCalc() {
384398
PercentageValue v = new PercentageValue(new BigDecimal("1.0"), new BigDecimal("11.0"), new BigDecimal("0.5"),
385-
null, null);
399+
null, null, null);
386400

387401
// Normal operation.
388402
PercentType command = (PercentType) v.parseCommand(new DecimalType("6.0"));
@@ -411,7 +425,7 @@ public void upDownCalc() {
411425
@Test
412426
public void percentCalcInvalid() {
413427
PercentageValue v = new PercentageValue(new BigDecimal(10.0), new BigDecimal(110.0), new BigDecimal(1.0), null,
414-
null);
428+
null, null);
415429
assertThrows(IllegalArgumentException.class, () -> v.parseCommand(new DecimalType(9.0)));
416430
}
417431

‎bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/Fan.java

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ public class Fan extends AbstractComponent<Fan.ChannelConfiguration> implements
5050
public static final String OSCILLATION_CHANNEL_ID = "oscillation";
5151
public static final String DIRECTION_CHANNEL_ID = "direction";
5252

53+
private static final BigDecimal BIG_DECIMAL_HUNDRED = new BigDecimal(100);
54+
private static final String FORMAT_INTEGER = "%.0f";
55+
5356
/**
5457
* Configuration class for MQTT component
5558
*/
@@ -60,6 +63,8 @@ static class ChannelConfiguration extends AbstractChannelConfiguration {
6063

6164
protected @Nullable Boolean optimistic;
6265

66+
@SerializedName("state_value_template")
67+
protected @Nullable String stateValueTemplate;
6368
@SerializedName("state_topic")
6469
protected @Nullable String stateTopic;
6570
@SerializedName("command_template")
@@ -136,18 +141,17 @@ public Fan(ComponentFactory.ComponentConfiguration componentConfiguration, boole
136141
: this;
137142
onOffChannel = buildChannel(newStyleChannels ? SWITCH_CHANNEL_ID : SWITCH_CHANNEL_ID_DEPRECATED,
138143
ComponentChannelType.SWITCH, onOffValue, "On/Off State", onOffListener)
139-
.stateTopic(channelConfiguration.stateTopic, channelConfiguration.getValueTemplate())
144+
.stateTopic(channelConfiguration.stateTopic, channelConfiguration.stateValueTemplate)
140145
.commandTopic(channelConfiguration.commandTopic, channelConfiguration.isRetain(),
141146
channelConfiguration.getQos(), channelConfiguration.commandTemplate)
142147
.inferOptimistic(channelConfiguration.optimistic)
143148
.build(channelConfiguration.percentageCommandTopic == null);
144149

145150
rawSpeedState = UnDefType.NULL;
146151

147-
int speeds = Math.min(channelConfiguration.speedRangeMax, 100) - Math.max(channelConfiguration.speedRangeMin, 1)
148-
+ 1;
149-
speedValue = new PercentageValue(BigDecimal.ZERO, BigDecimal.valueOf(100), BigDecimal.valueOf(100.0d / speeds),
150-
channelConfiguration.payloadOn, channelConfiguration.payloadOff);
152+
speedValue = new PercentageValue(BigDecimal.valueOf(channelConfiguration.speedRangeMin - 1),
153+
BigDecimal.valueOf(channelConfiguration.speedRangeMax), null, channelConfiguration.payloadOn,
154+
channelConfiguration.payloadOff, FORMAT_INTEGER);
151155

152156
if (channelConfiguration.percentageCommandTopic != null) {
153157
hiddenChannels.add(onOffChannel);

‎bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/Light.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ public abstract class Light extends AbstractComponent<Light.ChannelConfiguration
7373
protected static final String ON_COMMAND_TYPE_BRIGHTNESS = "brightness";
7474
protected static final String ON_COMMAND_TYPE_LAST = "last";
7575

76+
protected static final String FORMAT_INTEGER = "%.0f";
77+
7678
/**
7779
* Configuration class for MQTT component
7880
*/
@@ -276,7 +278,7 @@ protected Light(ComponentFactory.ComponentConfiguration builder, boolean newStyl
276278

277279
onOffValue = new OnOffValue(channelConfiguration.payloadOn, channelConfiguration.payloadOff);
278280
brightnessValue = new PercentageValue(null, new BigDecimal(channelConfiguration.brightnessScale), null, null,
279-
null);
281+
null, FORMAT_INTEGER);
280282
@Nullable
281283
List<String> effectList = channelConfiguration.effectList;
282284
if (effectList != null) {

‎bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/TemplateSchemaLight.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ protected void buildChannels() {
8484
}
8585

8686
onOffValue = new OnOffValue("on", "off");
87-
brightnessValue = new PercentageValue(null, new BigDecimal(255), null, null, null);
87+
brightnessValue = new PercentageValue(null, new BigDecimal(255), null, null, null, FORMAT_INTEGER);
8888

8989
if (channelConfiguration.redTemplate != null && channelConfiguration.greenTemplate != null
9090
&& channelConfiguration.blueTemplate != null) {

‎bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/Vacuum.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ ComponentChannelType.STRING, new TextValue(), updateListener, null,
173173
if (supportedFeatures.contains(FEATURE_BATTERY)) {
174174
buildOptionalChannel(newStyleChannels ? BATTERY_LEVEL_CH_ID : BATTERY_LEVEL_CH_ID_DEPRECATED,
175175
ComponentChannelType.DIMMER,
176-
new PercentageValue(BigDecimal.ZERO, BigDecimal.valueOf(100), BigDecimal.ONE, null, null),
176+
new PercentageValue(BigDecimal.ZERO, BigDecimal.valueOf(100), BigDecimal.ONE, null, null, null),
177177
updateListener, null, null, "{{ value_json.battery_level }}", channelConfiguration.stateTopic);
178178
}
179179
}

‎bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/component/Valve.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ public class Valve extends AbstractComponent<Valve.ChannelConfiguration> impleme
6161
private static final String POSITION_KEY = "position";
6262
private static final String STATE_KEY = "state";
6363

64+
private static final String FORMAT_INTEGER = "%.0f";
65+
6466
private final Logger logger = LoggerFactory.getLogger(Valve.class);
6567

6668
/**
@@ -121,7 +123,7 @@ public Valve(ComponentFactory.ComponentConfiguration componentConfiguration, boo
121123
onOffValue = new OnOffValue(channelConfiguration.stateOpen, channelConfiguration.stateClosed,
122124
channelConfiguration.payloadOpen, channelConfiguration.payloadClose);
123125
positionValue = new PercentageValue(BigDecimal.valueOf(channelConfiguration.positionClosed),
124-
BigDecimal.valueOf(channelConfiguration.positionOpen), null, null, null);
126+
BigDecimal.valueOf(channelConfiguration.positionOpen), null, null, null, FORMAT_INTEGER);
125127

126128
if (channelConfiguration.reportsPosition) {
127129
buildChannel(VALVE_CHANNEL_ID, ComponentChannelType.DIMMER, positionValue, getName(), this)

0 commit comments

Comments
 (0)