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

Add units to Illuminance and Relative Humidity, fix missing units #765

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add units to Illuminance and Relative Humidity, fix missing units #765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4afeea3
Use Dimensionless unit for Relative Humidity + add reporting config
lucasec Jun 5, 2022
3623c2b
Fix dynamically defined channels missing units of measure
lucasec Jun 6, 2022
72c2a6e
Add unit of measure to Illuminance channel
lucasec Jun 6, 2022
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
16 changes: 8 additions & 8 deletions org.openhab.binding.zigbee/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,24 +390,24 @@ The following channels are supported -:
| ias_vibration | `IAS_ZONE` (0x0500) | Switch | |
| ias_tamper | `IAS_ZONE` (0x0500) | Switch | |
| ias_tamper | `IAS_ZONE` (0x0500) | Switch | |
| measurement_illuminance | `ILLUMINANCE_MEASUREMENT` (0x0400) | Number | |
| measurement_illuminance | `ILLUMINANCE_MEASUREMENT` (0x0400) | Number:Illuminance | |
| measurement_pressure | `PRESSURE_MEASUREMENT` (0x0403) | Number:Pressure | |
| measurement_relativehumidity | `RELATIVE_HUMIDITY_MEASUREMENT` (0x0405) | Number | |
| measurement_relativehumidity | `RELATIVE_HUMIDITY_MEASUREMENT` (0x0405) | Number:Dimensionless | |
| measurement_temperature | `TEMPERATURE_MEASUREMENT` (0x0402) | Number:Temperature | |
| metering_instantdemand | `METERING` (0x0702) | Number | |
| metering_sumdelivered | `METERING` (0x0702) | Number | |
| metering_sumreceived | `METERING` (0x0702) | Number | |
| sensor_occupancy | `OCCUPANCY_SENSING` (0x0406) | Switch | |
| switch_dimmer | `LEVEL_CONTROL` (0x0008) | Dimmer | |
| switch_onoff | `ON_OFF` (0x0006) | Switch | |
| thermostat_localtemp | `THERMOSTAT` (0x0201) | Number | |
| thermostat_occupiedcooling | `THERMOSTAT` (0x0201) | Number | |
| thermostat_occupiedheating | `THERMOSTAT` (0x0201) | Number | |
| thermostat_outdoortemp | `THERMOSTAT` (0x0201) | Number | |
| thermostat_localtemp | `THERMOSTAT` (0x0201) | Number:Temperature | |
| thermostat_occupiedcooling | `THERMOSTAT` (0x0201) | Number:Temperature | |
| thermostat_occupiedheating | `THERMOSTAT` (0x0201) | Number:Temperature | |
| thermostat_outdoortemp | `THERMOSTAT` (0x0201) | Number:Temperature | |
| thermostat_runningmode | `THERMOSTAT` (0x0201) | Number | |
| thermostat_systemmode | `THERMOSTAT` (0x0201) | Number | |
| thermostat_unoccupiedcooling | `THERMOSTAT` (0x0201) | Number | |
| thermostat_unoccupiedheating | `THERMOSTAT` (0x0201) | Number | |
| thermostat_unoccupiedcooling | `THERMOSTAT` (0x0201) | Number:Temperature | |
| thermostat_unoccupiedheating | `THERMOSTAT` (0x0201) | Number:Temperature | |
| thermostat_heatingdemand | `THERMOSTAT` (0x0201) | Number:Dimensionless | |
| thermostat_coolingdemand | `THERMOSTAT` (0x0201) | Number:Dimensionless | |
| warning_device | `IAS_WD` (0x0502) | String | |
Expand Down
5 changes: 5 additions & 0 deletions ...enhab.binding.zigbee/src/main/java/org/openhab/binding/zigbee/ZigBeeBindingConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,11 @@ public class ZigBeeBindingConstants {
public static final String ITEM_TYPE_CONTACT = "Contact";
public static final String ITEM_TYPE_DIMMER = "Dimmer";
public static final String ITEM_TYPE_NUMBER = "Number";
public static final String ITEM_TYPE_NUMBER_DIMENSIONLESS = "Number:Dimensionless";
public static final String ITEM_TYPE_NUMBER_ELECTRICCURRENT = "Number:ElectricCurrent";
public static final String ITEM_TYPE_NUMBER_ELECTRICPOTENTIAL = "Number:ElectricPotential";
public static final String ITEM_TYPE_NUMBER_ILLUMINANCE = "Number:Illuminance";
public static final String ITEM_TYPE_NUMBER_POWER = "Number:Power";
public static final String ITEM_TYPE_NUMBER_PRESSURE = "Number:Pressure";
public static final String ITEM_TYPE_NUMBER_TEMPERATURE = "Number:Temperature";
public static final String ITEM_TYPE_ROLLERSHUTTER = "Rollershutter";
Expand Down
6 changes: 4 additions & 2 deletions ...c/main/java/org/openhab/binding/zigbee/internal/converter/ZigBeeConverterIlluminance.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import org.openhab.binding.zigbee.internal.converter.config.ZclReportingConfig;
import org.openhab.core.config.core.Configuration;
import org.openhab.core.library.types.DecimalType;
import org.openhab.core.library.types.QuantityType;
import org.openhab.core.library.unit.Units;
import org.openhab.core.thing.Channel;
import org.openhab.core.thing.ThingUID;
import org.openhab.core.thing.binding.builder.ChannelBuilder;
Expand Down Expand Up @@ -163,7 +165,7 @@ public Channel getChannel(ThingUID thingUID, ZigBeeEndpoint endpoint) {
}
return ChannelBuilder
.create(createChannelUID(thingUID, endpoint, ZigBeeBindingConstants.CHANNEL_NAME_ILLUMINANCE_VALUE),
ZigBeeBindingConstants.ITEM_TYPE_NUMBER)
ZigBeeBindingConstants.ITEM_TYPE_NUMBER_ILLUMINANCE)
.withType(ZigBeeBindingConstants.CHANNEL_ILLUMINANCE_VALUE)
.withLabel(ZigBeeBindingConstants.CHANNEL_LABEL_ILLUMINANCE_VALUE)
.withProperties(createProperties(endpoint)).build();
Expand All @@ -174,7 +176,7 @@ public void attributeUpdated(ZclAttribute attribute, Object val) {
logger.debug("{}: ZigBee attribute reports {}", endpoint.getIeeeAddress(), attribute);
if (attribute.getClusterType() == ZclClusterType.ILLUMINANCE_MEASUREMENT
&& attribute.getId() == ZclIlluminanceMeasurementCluster.ATTR_MEASUREDVALUE) {
updateChannelState(new DecimalType(Math.pow(10.0, (Integer) val / 10000.0) - 1));
updateChannelState(new QuantityType<>(new DecimalType(Math.pow(10.0, (Integer) val / 10000.0) - 1), Units.LUX));
}
}
}
2 changes: 1 addition & 1 deletion ...n/java/org/openhab/binding/zigbee/internal/converter/ZigBeeConverterMeasurementPower.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public Channel getChannel(ThingUID thingUID, ZigBeeEndpoint endpoint) {
return ChannelBuilder
.create(createChannelUID(thingUID, endpoint,
ZigBeeBindingConstants.CHANNEL_NAME_ELECTRICAL_ACTIVEPOWER),
ZigBeeBindingConstants.ITEM_TYPE_NUMBER)
ZigBeeBindingConstants.ITEM_TYPE_NUMBER_POWER)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm assuming when we dynamically generate channels, e.g. for a generic device without a thing XML definition, we ignore the contents of channels.xml completely? Thus without changing the type here we were not actually getting the correct channel type?

Copy link
Contributor

Choose a reason for hiding this comment

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

No - the channels XML file is not ignored. The channel type definitions are still required. When you define a channel (either programatically or in the thing XML file) you have to specify the channel-type, but the channel can also override certain aspects of this. I'm pretty sure that if you try and define a channel that doesn't have a channel type defined, it will error.

.withType(ZigBeeBindingConstants.CHANNEL_ELECTRICAL_ACTIVEPOWER)
.withLabel(ZigBeeBindingConstants.CHANNEL_LABEL_ELECTRICAL_ACTIVEPOWER)
.withProperties(createProperties(endpoint)).build();
Expand Down
2 changes: 1 addition & 1 deletion ...a/org/openhab/binding/zigbee/internal/converter/ZigBeeConverterMeasurementRmsCurrent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public Channel getChannel(ThingUID thingUID, ZigBeeEndpoint endpoint) {

return ChannelBuilder
.create(createChannelUID(thingUID, endpoint, ZigBeeBindingConstants.CHANNEL_NAME_ELECTRICAL_RMSCURRENT),
ZigBeeBindingConstants.ITEM_TYPE_NUMBER)
ZigBeeBindingConstants.ITEM_TYPE_NUMBER_ELECTRICCURRENT)
.withType(ZigBeeBindingConstants.CHANNEL_ELECTRICAL_RMSCURRENT)
.withLabel(ZigBeeBindingConstants.CHANNEL_LABEL_ELECTRICAL_RMSCURRENT)
.withProperties(createProperties(endpoint)).build();
Expand Down
2 changes: 1 addition & 1 deletion ...a/org/openhab/binding/zigbee/internal/converter/ZigBeeConverterMeasurementRmsVoltage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public Channel getChannel(ThingUID thingUID, ZigBeeEndpoint endpoint) {

return ChannelBuilder
.create(createChannelUID(thingUID, endpoint, ZigBeeBindingConstants.CHANNEL_NAME_ELECTRICAL_RMSVOLTAGE),
ZigBeeBindingConstants.ITEM_TYPE_NUMBER)
ZigBeeBindingConstants.ITEM_TYPE_NUMBER_ELECTRICPOTENTIAL)
.withType(ZigBeeBindingConstants.CHANNEL_ELECTRICAL_RMSVOLTAGE)
.withLabel(ZigBeeBindingConstants.CHANNEL_LABEL_ELECTRICAL_RMSVOLTAGE)
.withProperties(createProperties(endpoint)).build();
Expand Down
51 changes: 45 additions & 6 deletions ...n/java/org/openhab/binding/zigbee/internal/converter/ZigBeeConverterRelativeHumidity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,18 @@
package org.openhab.binding.zigbee.internal.converter;

import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;

import org.eclipse.jdt.annotation.NonNull;
import org.openhab.binding.zigbee.ZigBeeBindingConstants;
import org.openhab.binding.zigbee.converter.ZigBeeBaseChannelConverter;
import org.openhab.binding.zigbee.handler.ZigBeeThingHandler;
import org.openhab.core.library.types.DecimalType;
import org.openhab.binding.zigbee.internal.converter.config.ZclReportingConfig;
import org.openhab.core.config.core.Configuration;
import org.openhab.core.thing.Channel;
import org.openhab.core.thing.ThingUID;
import org.openhab.core.thing.binding.builder.ChannelBuilder;
Expand All @@ -43,8 +47,13 @@
public class ZigBeeConverterRelativeHumidity extends ZigBeeBaseChannelConverter implements ZclAttributeListener {
private Logger logger = LoggerFactory.getLogger(ZigBeeConverterRelativeHumidity.class);

private static BigDecimal CHANGE_DEFAULT = new BigDecimal(50);
private static BigDecimal CHANGE_MIN = new BigDecimal(1);
private static BigDecimal CHANGE_MAX = new BigDecimal(10000);

private ZclRelativeHumidityMeasurementCluster cluster;
private ZclAttribute attribute;
private ZclReportingConfig configReporting;

@Override
public Set<Integer> getImplementedClientClusters() {
Expand All @@ -65,15 +74,17 @@ public boolean initializeDevice() {
return false;
}

ZclReportingConfig reporting = new ZclReportingConfig(channel);

try {
CommandResult bindResponse = bind(serverCluster).get();
if (bindResponse.isSuccess()) {
// Configure reporting
ZclAttribute attribute = serverCluster
.getAttribute(ZclRelativeHumidityMeasurementCluster.ATTR_MEASUREDVALUE);
CommandResult reportingResponse = attribute
.setReporting(REPORTING_PERIOD_DEFAULT_MIN, REPORTING_PERIOD_DEFAULT_MAX, 50).get();
handleReportingResponse(reportingResponse, POLLING_PERIOD_DEFAULT, REPORTING_PERIOD_DEFAULT_MAX);
CommandResult reportingResponse = attribute.setReporting(reporting.getReportingTimeMin(),
reporting.getReportingTimeMax(), reporting.getReportingChange()).get();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not exactly clear on the lifecycle order—would the CHANGE_DEFAULT configured on line 115 be picked up here? As in the converter is initialized first then when we create the reporting it reads the already defined default parameter values?

Copy link
Contributor

Choose a reason for hiding this comment

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

No - it won't - it will use whatever the default is in the ZclReportingConfig class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm assuming the device does not immediately get reconfigured later with the actual default either (unless the user goes in and changes/saves the channel options)?
So I probably need to refactor the pattern that was used in other converters somewhat to ensure it uses a reasonable default

handleReportingResponse(reportingResponse, POLLING_PERIOD_DEFAULT, reporting.getPollingPeriod());
}
} catch (InterruptedException | ExecutionException e) {
logger.error("{}: Exception setting reporting ", endpoint.getIeeeAddress(), e);
Expand All @@ -99,6 +110,12 @@ public boolean initializeConverter(ZigBeeThingHandler thing) {
return false;
}

// Add reporting configuration
configReporting = new ZclReportingConfig(channel);
configReporting.setAnalogue(CHANGE_DEFAULT, CHANGE_MIN, CHANGE_MAX);
configOptions = new ArrayList<>();
configOptions.addAll(configReporting.getConfiguration());

// Add a listener, then request the status
cluster.addAttributeListener(this);
return true;
Expand All @@ -109,11 +126,33 @@ public void disposeConverter() {
cluster.removeAttributeListener(this);
}

@Override
public int getPollingPeriod() {
return configReporting.getPollingPeriod();
}

@Override
public void handleRefresh() {
attribute.readValue(0);
}

@Override
public void updateConfiguration(@NonNull Configuration currentConfiguration,
Map<String, Object> updatedParameters) {
if (configReporting.updateConfiguration(currentConfiguration, updatedParameters)) {
try {
ZclAttribute attribute = cluster.getAttribute(ZclRelativeHumidityMeasurementCluster.ATTR_MEASUREDVALUE);
CommandResult reportingResponse;
reportingResponse = attribute.setReporting(configReporting.getReportingTimeMin(),
configReporting.getReportingTimeMax(), configReporting.getReportingChange()).get();
handleReportingResponse(reportingResponse, configReporting.getPollingPeriod(),
configReporting.getReportingTimeMax());
} catch (InterruptedException | ExecutionException e) {
logger.debug("{}: Relative humidity measurement exception setting reporting", endpoint.getIeeeAddress(), e);
}
}
}

@Override
public Channel getChannel(ThingUID thingUID, ZigBeeEndpoint endpoint) {
if (endpoint.getInputCluster(ZclRelativeHumidityMeasurementCluster.CLUSTER_ID) == null) {
Expand All @@ -123,7 +162,7 @@ public Channel getChannel(ThingUID thingUID, ZigBeeEndpoint endpoint) {

return ChannelBuilder
.create(createChannelUID(thingUID, endpoint, ZigBeeBindingConstants.CHANNEL_NAME_HUMIDITY_VALUE),
ZigBeeBindingConstants.ITEM_TYPE_NUMBER)
ZigBeeBindingConstants.ITEM_TYPE_NUMBER_DIMENSIONLESS)
.withType(ZigBeeBindingConstants.CHANNEL_HUMIDITY_VALUE)
.withLabel(ZigBeeBindingConstants.CHANNEL_LABEL_HUMIDITY_VALUE)
.withProperties(createProperties(endpoint)).build();
Expand All @@ -135,7 +174,7 @@ public void attributeUpdated(ZclAttribute attribute, Object val) {
if (attribute.getClusterType() == ZclClusterType.RELATIVE_HUMIDITY_MEASUREMENT
&& attribute.getId() == ZclRelativeHumidityMeasurementCluster.ATTR_MEASUREDVALUE) {
Integer value = (Integer) val;
updateChannelState(new DecimalType(BigDecimal.valueOf(value, 2)));
updateChannelState(valueToPercentDimensionless(BigDecimal.valueOf(value, 2)));
}
}
}
2 changes: 1 addition & 1 deletion ...g/openhab/binding/zigbee/internal/converter/ZigBeeConverterThermostatPiCoolingDemand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public Channel getChannel(ThingUID thingUID, ZigBeeEndpoint endpoint) {

return ChannelBuilder
.create(createChannelUID(thingUID, endpoint, ZigBeeBindingConstants.CHANNEL_NAME_THERMOSTAT_COOLING_DEMAND),
ZigBeeBindingConstants.ITEM_TYPE_NUMBER)
ZigBeeBindingConstants.ITEM_TYPE_NUMBER_DIMENSIONLESS)
.withType(ZigBeeBindingConstants.CHANNEL_THERMOSTAT_COOLING_DEMAND)
.withLabel(ZigBeeBindingConstants.CHANNEL_LABEL_THERMOSTAT_COOLING_DEMAND)
.withProperties(createProperties(endpoint)).build();
Expand Down
2 changes: 1 addition & 1 deletion ...g/openhab/binding/zigbee/internal/converter/ZigBeeConverterThermostatPiHeatingDemand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public Channel getChannel(ThingUID thingUID, ZigBeeEndpoint endpoint) {

return ChannelBuilder
.create(createChannelUID(thingUID, endpoint, ZigBeeBindingConstants.CHANNEL_NAME_THERMOSTAT_HEATING_DEMAND),
ZigBeeBindingConstants.ITEM_TYPE_NUMBER)
ZigBeeBindingConstants.ITEM_TYPE_NUMBER_DIMENSIONLESS)
.withType(ZigBeeBindingConstants.CHANNEL_THERMOSTAT_HEATING_DEMAND)
.withLabel(ZigBeeBindingConstants.CHANNEL_LABEL_THERMOSTAT_HEATING_DEMAND)
.withProperties(createProperties(endpoint)).build();
Expand Down
10 changes: 5 additions & 5 deletions org.openhab.binding.zigbee/src/main/resources/OH-INF/thing/channels.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,11 @@

<!-- Illuminance Channel -->
<channel-type id="measurement_illuminance">
<item-type>Number</item-type>
<item-type>Number:Illuminance</item-type>
<label>Illuminance</label>
<description>Indicates the current illuminance in lux</description>
<description>Indicates the current illuminance</description>
<category></category>
<state pattern="%.0f" readOnly="true" />
<state pattern="%.0f %unit%" readOnly="true" />
</channel-type>

<!-- Atmospheric Pressure Channel -->
Expand All @@ -215,11 +215,11 @@

<!-- Humidity Channel -->
<channel-type id="measurement_relativehumidity">
<item-type>Number</item-type>
<item-type>Number:Dimensionless</item-type>
<label>Humidity</label>
<description>Indicates the current relative humidity</description>
<category>Humidity</category>
<state pattern="%.1f" readOnly="true" />
<state pattern="%.1f %%" readOnly="true" />
</channel-type>

<!-- Temperature Channel -->
Expand Down