Skip to content

Commit

Permalink
[onewire] Code improvements and bug fixes (#14400)
Browse files Browse the repository at this point in the history
Signed-off-by: Jan N. Klug <github@klug.nrw>
  • Loading branch information
J-N-K authored Feb 16, 2023
1 parent 9a05e9f commit 5ea0dcb
Show file tree
Hide file tree
Showing 28 changed files with 279 additions and 436 deletions.
2 changes: 1 addition & 1 deletion CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@
/bundles/org.openhab.binding.omnikinverter/ @hansbogert
/bundles/org.openhab.binding.omnilink/ @ecdye
/bundles/org.openhab.binding.onebusaway/ @sdwilsh
/bundles/org.openhab.binding.onewire/ @openhab/add-ons-maintainers
/bundles/org.openhab.binding.onewire/ @J-N-K
/bundles/org.openhab.binding.onewiregpio/ @aogorek
/bundles/org.openhab.binding.onkyo/ @pail23 @paulianttila
/bundles/org.openhab.binding.opengarage/ @psmedley
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,19 +87,16 @@ public DS2438Configuration(OwserverBridgeHandler bridgeHandler, SensorId sensorI
SensorId associatedSensorId = new SensorId(sensorId.getPath() + matcher.group(2));

switch (matcher.group(2).substring(0, 2)) {
case "26":
case "26" -> {
DS2438Configuration associatedDs2438Config = new DS2438Configuration(bridgeHandler,
associatedSensorId);
associatedSensors.put(associatedSensorId, associatedDs2438Config.getSensorSubType());
associatedSensors.putAll(associatedDs2438Config.getAssociatedSensors());
break;
case "28":
associatedSensors.put(associatedSensorId, OwSensorType.DS18B20);
break;
case "3A":
associatedSensors.put(associatedSensorId, OwSensorType.DS2413);
break;
default:
}
case "28" -> associatedSensors.put(associatedSensorId, OwSensorType.DS18B20);
case "3A" -> associatedSensors.put(associatedSensorId, OwSensorType.DS2413);
default -> {
}
}
}
}
Expand Down Expand Up @@ -129,7 +126,7 @@ public List<SensorId> getAssociatedSensorIds() {
* @return a list of OwDiscoveryItems
*/
public List<SensorId> getAssociatedSensorIds(OwSensorType sensorType) {
return associatedSensors.entrySet().stream().filter(s -> s.getValue() == sensorType).map(s -> s.getKey())
return associatedSensors.entrySet().stream().filter(s -> s.getValue() == sensorType).map(Map.Entry::getKey)
.collect(Collectors.toList());
}

Expand Down Expand Up @@ -198,18 +195,16 @@ public static OwSensorType getMultisensorType(OwSensorType mainsensorType,
List<OwSensorType> associatedSensorTypes) {
OwSensorType multisensorType = OwSensorType.UNKNOWN;
switch (associatedSensorTypes.size()) {
case 0:
multisensorType = mainsensorType;
break;
case 1:
case 0 -> multisensorType = mainsensorType;
case 1 -> {
if (mainsensorType == OwSensorType.MS_TH_S && associatedSensorTypes.contains(OwSensorType.DS18B20)) {
multisensorType = OwSensorType.BMS_S;
} else if (mainsensorType == OwSensorType.MS_TH
&& associatedSensorTypes.contains(OwSensorType.DS18B20)) {
multisensorType = OwSensorType.BMS;
}
break;
case 3:
}
case 3 -> {
if (mainsensorType == OwSensorType.MS_TH_S && associatedSensorTypes.contains(OwSensorType.MS_TV)
&& associatedSensorTypes.contains(OwSensorType.DS18B20)
&& associatedSensorTypes.contains(OwSensorType.DS2413)) {
Expand All @@ -221,8 +216,9 @@ public static OwSensorType getMultisensorType(OwSensorType mainsensorType,
// two DS2438 (first TH, second TV), DS18B20, DS2413
multisensorType = OwSensorType.AMS;
}
break;
default:
}
default -> {
}
}

return multisensorType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

import static org.openhab.binding.onewire.internal.OwBindingConstants.CHANNEL_DIGITAL;

import java.util.Arrays;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.openhab.binding.onewire.internal.owserver.OwserverDeviceParameter;
import org.openhab.core.library.types.DecimalType;
Expand Down Expand Up @@ -100,6 +98,6 @@ public DecimalType convertState(OnOffType command) {

@Override
public String toString() {
return String.format("path=%s, mode=%s, logic=%s", Arrays.asList(getParameter()), ioMode, ioLogic);
return String.format("path=%s, mode=%s, logic=%s", getParameter(), ioMode, ioLogic);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import org.openhab.core.thing.type.ChannelTypeUID;

/**
* The {@link OneWireBinding} class defines common constants, which are
* The {@link OwBindingConstants} class defines common constants, which are
* used across the whole binding.
*
* @author Jan N. Klug - Initial contribution
Expand Down Expand Up @@ -143,19 +143,17 @@ public class OwBindingConstants {
public static final Map<OwSensorType, String> THING_LABEL_MAP;
public static final Map<OwSensorType, Set<OwChannelConfig>> SENSOR_TYPE_CHANNEL_MAP;

public static final Map<String, String> ACCEPTED_ITEM_TYPES_MAP = Util
.readPropertiesFile("accepted_itemtypes.properties");

static {
Map<String, String> properties = Util.readPropertiesFile("sensor.properties");
THING_TYPE_MAP = properties.entrySet().stream().filter(e -> e.getKey().endsWith(".thingtype"))
.collect(Collectors.toConcurrentMap(e -> OwSensorType.valueOf(e.getKey().split("\\.")[0]),
e -> new ThingTypeUID(BINDING_ID, e.getValue())));
SENSOR_TYPE_CHANNEL_MAP = properties.entrySet().stream().filter(e -> e.getKey().endsWith(".channels"))
.collect(Collectors.toConcurrentMap(e -> OwSensorType.valueOf(e.getKey().split("\\.")[0]),
e -> !e.getValue().isEmpty() ? Stream.of(e.getValue().split(","))
.map(c -> OwChannelConfig.fromString(c)).collect(Collectors.toSet())
: new HashSet<>()));
.collect(
Collectors.toConcurrentMap(e -> OwSensorType.valueOf(e.getKey().split("\\.")[0]),
e -> !e.getValue().isEmpty() ? Stream.of(e.getValue().split(","))
.map(OwChannelConfig::fromString).collect(Collectors.toSet())
: new HashSet<>()));
THING_LABEL_MAP = properties.entrySet().stream().filter(e -> e.getKey().endsWith(".label")).collect(
Collectors.toConcurrentMap(e -> OwSensorType.valueOf(e.getKey().split("\\.")[0]), e -> e.getValue()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,23 @@

import static org.openhab.binding.onewire.internal.OwBindingConstants.SUPPORTED_THING_TYPES;

import java.util.HashMap;
import java.util.Hashtable;
import java.util.Map;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.onewire.internal.discovery.OwDiscoveryService;
import org.openhab.binding.onewire.internal.handler.AdvancedMultisensorThingHandler;
import org.openhab.binding.onewire.internal.handler.BAE091xSensorThingHandler;
import org.openhab.binding.onewire.internal.handler.BasicMultisensorThingHandler;
import org.openhab.binding.onewire.internal.handler.BasicThingHandler;
import org.openhab.binding.onewire.internal.handler.EDSSensorThingHandler;
import org.openhab.binding.onewire.internal.handler.OwserverBridgeHandler;
import org.openhab.core.config.discovery.DiscoveryService;
import org.openhab.core.thing.Bridge;
import org.openhab.core.thing.Thing;
import org.openhab.core.thing.ThingTypeUID;
import org.openhab.core.thing.ThingUID;
import org.openhab.core.thing.binding.BaseThingHandlerFactory;
import org.openhab.core.thing.binding.ThingHandler;
import org.openhab.core.thing.binding.ThingHandlerFactory;
import org.osgi.framework.ServiceRegistration;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* The {@link OwHandlerFactory} is responsible for creating things and thing
Expand All @@ -50,11 +41,12 @@
@NonNullByDefault
@Component(service = ThingHandlerFactory.class, configurationPid = "binding.onewire")
public class OwHandlerFactory extends BaseThingHandlerFactory {
Logger logger = LoggerFactory.getLogger(OwHandlerFactory.class);
private final Map<ThingUID, ServiceRegistration<?>> discoveryServiceRegs = new HashMap<>();
private final OwDynamicStateDescriptionProvider dynamicStateDescriptionProvider;

@NonNullByDefault({})
private OwDynamicStateDescriptionProvider dynamicStateDescriptionProvider;
@Activate
public OwHandlerFactory(@Reference OwDynamicStateDescriptionProvider dynamicStateDescriptionProvider) {
this.dynamicStateDescriptionProvider = dynamicStateDescriptionProvider;
}

@Override
public boolean supportsThingType(ThingTypeUID thingTypeUID) {
Expand All @@ -66,9 +58,7 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
ThingTypeUID thingTypeUID = thing.getThingTypeUID();

if (OwserverBridgeHandler.SUPPORTED_THING_TYPES.contains(thingTypeUID)) {
OwserverBridgeHandler owserverBridgeHandler = new OwserverBridgeHandler((Bridge) thing);
registerDiscoveryService(owserverBridgeHandler);
return owserverBridgeHandler;
return new OwserverBridgeHandler((Bridge) thing);
} else if (BasicMultisensorThingHandler.SUPPORTED_THING_TYPES.contains(thingTypeUID)) {
return new BasicMultisensorThingHandler(thing, dynamicStateDescriptionProvider);
} else if (AdvancedMultisensorThingHandler.SUPPORTED_THING_TYPES.contains(thingTypeUID)) {
Expand All @@ -83,41 +73,4 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {

return null;
}

@Override
public void unregisterHandler(Thing thing) {
super.unregisterHandler(thing);
logger.error("factory {} deleting thing {}", this, thing);
}

private synchronized void registerDiscoveryService(OwserverBridgeHandler owserverBridgeHandler) {
OwDiscoveryService owDiscoveryService = new OwDiscoveryService(owserverBridgeHandler);

this.discoveryServiceRegs.put(owserverBridgeHandler.getThing().getUID(),
bundleContext.registerService(DiscoveryService.class.getName(), owDiscoveryService, new Hashtable<>()));
}

@Override
protected synchronized void removeHandler(ThingHandler thingHandler) {
if (thingHandler instanceof OwserverBridgeHandler) {
// remove discovery service, if bridge handler is removed
ServiceRegistration<?> serviceReg = this.discoveryServiceRegs.remove(thingHandler.getThing().getUID());
if (serviceReg != null) {
OwDiscoveryService service = (OwDiscoveryService) bundleContext.getService(serviceReg.getReference());
serviceReg.unregister();
if (service != null) {
service.deactivate();
}
}
}
}

@Reference
protected void setDynamicStateDescriptionProvider(OwDynamicStateDescriptionProvider provider) {
this.dynamicStateDescriptionProvider = provider;
}

protected void unsetDynamicStateDescriptionProvider(OwDynamicStateDescriptionProvider provider) {
this.dynamicStateDescriptionProvider = null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
package org.openhab.binding.onewire.internal;

import java.io.IOException;
import java.net.URL;
import java.io.InputStream;
import java.util.HashMap;
import java.util.Map;
import java.util.Properties;
Expand Down Expand Up @@ -52,9 +52,9 @@ public static State calculateAbsoluteHumidity(QuantityType<Temperature> temperat
if (temperatureDegC == null) {
throw new IllegalArgumentException("could not change unit");
}
Double theta = temperatureDegC.doubleValue();
double theta = temperatureDegC.doubleValue();
// saturation vapor pressure in kg/(m s^2)
Double saturationVaporPressure = 611.2 * Math.exp(17.62 * theta / (243.12 + theta));
double saturationVaporPressure = 611.2 * Math.exp(17.62 * theta / (243.12 + theta));
// absolute humidity in kg/m^3
Double aH = relativeHumidity.doubleValue() / 100 * saturationVaporPressure / (461.52 * (273.15 + theta));
State absoluteHumidity = new QuantityType<>(aH, Units.KILOGRAM_PER_CUBICMETRE).toUnit("g/m³");
Expand All @@ -78,24 +78,32 @@ public static State calculateDewpoint(QuantityType<Temperature> temperature,
if (temperatureDegC == null) {
throw new IllegalArgumentException("could not change unit");
}
Double theta = temperatureDegC.doubleValue();
Double rH = relativeHumidity.doubleValue() / 100;
double theta = temperatureDegC.doubleValue();
double rH = relativeHumidity.doubleValue() / 100;
// dewpoint in °C
Double dP = 243.12 * (((17.62 * theta) / (243.12 + theta) + Math.log(rH))
/ (((17.62 * 243.12) / (243.12 + theta) - Math.log(rH))));
State dewPoint = new QuantityType<>(dP, SIUnits.CELSIUS);
return dewPoint;
return new QuantityType<>(dP, SIUnits.CELSIUS);
}

public static Map<String, String> readPropertiesFile(String filename) {
URL resource = Thread.currentThread().getContextClassLoader().getResource(filename);
ClassLoader classLoader = OwBindingConstants.class.getClassLoader();
if (classLoader == null) {
LOGGER.warn("Could not get classloader, binding will fail.");
return Map.of();
}
Properties properties = new Properties();
try {
properties.load(resource.openStream());

try (InputStream inputStream = classLoader.getResourceAsStream(filename)) {
if (inputStream == null) {
LOGGER.warn("Could not get input stream for resource file '{}', binding will fail.", filename);
return Map.of();
}
properties.load(inputStream);
return properties.entrySet().stream()
.collect(Collectors.toMap(e -> (String) e.getKey(), e -> (String) e.getValue()));
} catch (IOException e) {
LOGGER.warn("Could not read resource file {}, binding will probably fail: {}", filename, e.getMessage());
LOGGER.warn("Could not read resource file {}, binding will fail: {}", filename, e.getMessage());
return new HashMap<>();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import org.slf4j.LoggerFactory;

/**
* The {@link AbstractOwClass} class defines an abstract onewire device
* The {@link AbstractOwDevice} class defines an abstract onewire device
*
* @author Jan N. Klug - Initial contribution
*/
Expand Down Expand Up @@ -68,7 +68,7 @@ public AbstractOwDevice(SensorId sensorId, OwSensorType sensorType, OwBaseThingH
/**
* refresh this sensor
*
* @param bridgeHandler for sending requests
* @param owBridgeHandler for sending requests
* @param forcedRefresh post update even if state did not change
* @throws OwException in case of communication error
*/
Expand All @@ -80,9 +80,7 @@ public AbstractOwDevice(SensorId sensorId, OwSensorType sensorType, OwBaseThingH
* @param channelID the channels channelID
*/
public void enableChannel(String channelID) {
if (!enabledChannels.contains(channelID)) {
enabledChannels.add(channelID);
}
enabledChannels.add(channelID);
}

/**
Expand All @@ -91,9 +89,7 @@ public void enableChannel(String channelID) {
* @param channelID the channels channelID
*/
public void disableChannel(String channelID) {
if (enabledChannels.contains(channelID)) {
enabledChannels.remove(channelID);
}
enabledChannels.remove(channelID);
}

/**
Expand All @@ -108,10 +104,9 @@ public SensorId getSensorId() {
/**
* check sensor presence and update thing state
*
* @param owServerConnection
* @param bridgeHandler
* @return sensors presence state
*/

public Boolean checkPresence(OwserverBridgeHandler bridgeHandler) {
try {
State present = bridgeHandler.checkPresence(sensorId);
Expand All @@ -123,18 +118,4 @@ public Boolean checkPresence(OwserverBridgeHandler bridgeHandler) {
return false;
}
}

/**
* get this sensors type
*
* @param bridgeHandler bridge handler to request from if type formerly unknown
* @return this sensors type
* @throws OwException
*/
public OwSensorType getSensorType(OwserverBridgeHandler bridgeHandler) throws OwException {
if (sensorType == OwSensorType.UNKNOWN) {
sensorType = bridgeHandler.getType(sensorId);
}
return sensorType;
}
}
Loading

0 comments on commit 5ea0dcb

Please sign in to comment.