[shelly] Fix resource leak, BLU script installation, TRV init, NPE on IPv6 mDNS discovery (#15798)

* Fix resource leak when discovery handling failed and NPE when a IPv6
address is reported.
* remove callApi(request/innerRequest from Shelly1HttpApi, which changes
to callApi/httpRequest in ShellyHttpCLient.

Signed-off-by: Markus Michels <markus7017@gmail.com>
This commit is contained in:
Markus Michels 2023-11-13 19:37:04 +01:00 committed by GitHub
parent c15f1344c3
commit beade55ca6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 71 additions and 132 deletions

View File

@ -333,7 +333,7 @@ public class ShellyBindingConstants {
public static final int UPDATE_MIN_DELAY = 15;// update every x triggers or when a key was pressed
public static final int UPDATE_SETTINGS_INTERVAL_SECONDS = 60; // check for updates every x sec
public static final int HEALTH_CHECK_INTERVAL_SEC = 300; // Health check interval, 5min
public static final int VIBRATION_FILTER_SEC = 5; // Absore duplicate vibration events for xx sec
public static final int VIBRATION_FILTER_SEC = 5; // Absorb duplicate vibration events for xx sec
public static final String BUNDLE_RESOURCE_SNIPLETS = "sniplets"; // where to find code sniplets in the bundle
public static final String BUNDLE_RESOURCE_SCRIPTS = "scripts"; // where to find scrips in the bundle

View File

@ -196,7 +196,8 @@ public class ShellyDeviceProfile {
return;
}
isBlu = thingType.startsWith("shellyblu"); // e.g. SBBT for BU Button
isGen2 = isGeneration2(thingType);
isBlu = isBluSeries(thingType); // e.g. SBBT for BLU Button
isDimmer = deviceType.equalsIgnoreCase(SHELLYDT_DIMMER) || deviceType.equalsIgnoreCase(SHELLYDT_DIMMER2)
|| deviceType.equalsIgnoreCase(SHELLYDT_PLUSDIMMERUS)
@ -397,6 +398,14 @@ public class ShellyDeviceProfile {
return "";
}
public static boolean isGeneration2(String thingType) {
return thingType.startsWith("shellyplus") || thingType.startsWith("shellypro") || isBluSeries(thingType);
}
public static boolean isBluSeries(String thingType) {
return thingType.startsWith("shellyblu");
}
public boolean coiotEnabled() {
if ((settings.coiot != null) && (settings.coiot.enabled != null)) {
return settings.coiot.enabled;

View File

@ -113,6 +113,8 @@ public class ShellyHttpClient {
while (retries > 0) {
try {
apiResult = innerRequest(HttpMethod.GET, uri, null, "");
// If call doesn't throw an exception the device is reachable == no timeout
if (timeout) {
logger.debug("{}: API timeout #{}/{} recovered ({})", thingName, timeoutErrors, timeoutsRecovered,
apiResult.getUrl());
@ -128,9 +130,10 @@ public class ShellyHttpClient {
}
timeout = true;
retries--;
timeoutErrors++; // count the retries
logger.debug("{}: API Timeout, retry #{} ({})", thingName, timeoutErrors, e.toString());
retries--;
}
}
throw new ShellyApiException("API Timeout or inconsistent result"); // successful
@ -174,7 +177,7 @@ public class ShellyHttpClient {
}
}
fillPostData(request, data);
logger.trace("{}: HTTP {} for {} {}\n{}", thingName, method, url, data, request.getHeaders());
logger.trace("{}: HTTP {} {}\n{}\n{}", thingName, method, url, request.getHeaders(), data);
// Do request and get response
ContentResponse contentResponse = request.send();

View File

@ -126,6 +126,9 @@ public class Shelly1CoIoTVersion2 extends Shelly1CoIoTProtocol implements Shelly
thingHandler.requestUpdates(1, false);
}
break;
case "3122": // boost mode, Type=S, Range=0/1
updateChannel(updates, CHANNEL_GROUP_CONTROL, CHANNEL_CONTROL_BCONTROL, getOnOff(value > 0));
break;
default:
processed = false;
}

View File

@ -182,7 +182,7 @@ public class Shelly1CoapHandler implements Shelly1CoapListener {
for (Option opt : options) {
if (opt.getNumber() == COIOT_OPTION_GLOBAL_DEVID) {
String devid = opt.getStringValue();
if (devid.contains("#")) {
if (devid.contains("#") && profile.mac != null) {
// Format: <device type>#<mac address>#<coap version>
String macid = substringBetween(devid, "#", "#");
if (profile.mac.toUpperCase().contains(macid.toUpperCase())) {

View File

@ -20,20 +20,11 @@ import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpStatus;
import org.openhab.binding.shelly.internal.api.ShellyApiException;
import org.openhab.binding.shelly.internal.api.ShellyApiInterface;
import org.openhab.binding.shelly.internal.api.ShellyApiResult;
import org.openhab.binding.shelly.internal.api.ShellyDeviceProfile;
import org.openhab.binding.shelly.internal.api.ShellyHttpClient;
import org.openhab.binding.shelly.internal.api1.Shelly1ApiJsonDTO.ShellyOtaCheckResult;
@ -258,7 +249,7 @@ public class Shelly1HttpApi extends ShellyHttpClient implements ShellyApiInterfa
@Override
public void setValveTemperature(int valveId, int value) throws ShellyApiException {
request("/thermostat/" + valveId + "?target_t_enabled=1&target_t=" + value);
httpRequest("/thermostat/" + valveId + "?target_t_enabled=1&target_t=" + value);
}
@Override
@ -273,17 +264,17 @@ public class Shelly1HttpApi extends ShellyHttpClient implements ShellyApiInterfa
@Override
public void setValveProfile(int valveId, int value) throws ShellyApiException {
String uri = "/settings/thermostat/" + valveId + "?";
request(uri + (value == 0 ? "schedule=0" : "schedule=1&schedule_profile=" + value));
httpRequest(uri + (value == 0 ? "schedule=0" : "schedule=1&schedule_profile=" + value));
}
@Override
public void setValvePosition(int valveId, double value) throws ShellyApiException {
request("/thermostat/" + valveId + "?pos=" + value); // percentage to open the valve
httpRequest("/thermostat/" + valveId + "?pos=" + value); // percentage to open the valve
}
@Override
public void setValveBoostTime(int valveId, int value) throws ShellyApiException {
request("/settings/thermostat/" + valveId + "?boost_minutes=" + value);
httpRequest("/settings/thermostat/" + valveId + "?boost_minutes=" + value);
}
@Override
@ -641,86 +632,6 @@ public class Shelly1HttpApi extends ShellyHttpClient implements ShellyApiInterfa
return eventType + SHELLY_EVENTURL_SUFFIX;
}
/**
* Submit GET request and return response, check for invalid responses
*
* @param uri: URI (e.g. "/settings")
*/
@Override
public <T> T callApi(String uri, Class<T> classOfT) throws ShellyApiException {
String json = request(uri);
return fromJson(gson, json, classOfT);
}
private String request(String uri) throws ShellyApiException {
ShellyApiResult apiResult = new ShellyApiResult();
int retries = 3;
boolean timeout = false;
while (retries > 0) {
try {
apiResult = innerRequest(HttpMethod.GET, uri);
if (timeout) {
logger.debug("{}: API timeout #{}/{} recovered ({})", thingName, timeoutErrors, timeoutsRecovered,
apiResult.getUrl());
timeoutsRecovered++;
}
return apiResult.response; // successful
} catch (ShellyApiException e) {
if ((!e.isTimeout() && !apiResult.isHttpServerError()) || profile.hasBattery || (retries == 0)) {
// Sensor in sleep mode or API exception for non-battery device or retry counter expired
throw e; // non-timeout exception
}
timeout = true;
retries--;
timeoutErrors++; // count the retries
logger.debug("{}: API Timeout, retry #{} ({})", thingName, timeoutErrors, e.toString());
}
}
throw new ShellyApiException("API Timeout or inconsistent result"); // successful
}
private ShellyApiResult innerRequest(HttpMethod method, String uri) throws ShellyApiException {
Request request = null;
String url = "http://" + config.deviceIp + uri;
ShellyApiResult apiResult = new ShellyApiResult(method.toString(), url);
try {
request = httpClient.newRequest(url).method(method.toString()).timeout(SHELLY_API_TIMEOUT_MS,
TimeUnit.MILLISECONDS);
if (!config.userId.isEmpty()) {
String value = config.userId + ":" + config.password;
request.header(HTTP_HEADER_AUTH,
HTTP_AUTH_TYPE_BASIC + " " + Base64.getEncoder().encodeToString(value.getBytes()));
}
request.header(HttpHeader.ACCEPT, CONTENT_TYPE_JSON);
logger.trace("{}: HTTP {} for {}", thingName, method, url);
// Do request and get response
ContentResponse contentResponse = request.send();
apiResult = new ShellyApiResult(contentResponse);
String response = contentResponse.getContentAsString().replace("\t", "").replace("\r\n", "").trim();
logger.trace("{}: HTTP Response {}: {}", thingName, contentResponse.getStatus(), response);
// validate response, API errors are reported as Json
if (contentResponse.getStatus() != HttpStatus.OK_200) {
throw new ShellyApiException(apiResult);
}
if (response.isEmpty() || !response.startsWith("{") && !response.startsWith("[") && !url.contains("/debug/")
&& !url.contains("/sta_cache_reset")) {
throw new ShellyApiException("Unexpected response: " + response);
}
} catch (ExecutionException | InterruptedException | TimeoutException | IllegalArgumentException e) {
ShellyApiException ex = new ShellyApiException(apiResult, e);
if (!ex.isTimeout()) { // will be handled by the caller
logger.trace("{}: API call returned exception", thingName, ex);
}
throw ex;
}
return apiResult;
}
@Override
public String getControlUriPrefix(Integer id) {
String uri = "";

View File

@ -319,14 +319,16 @@ public class Shelly2ApiRpc extends Shelly2ApiClient implements ShellyApiInterfac
asyncApiRequest(SHELLYRPC_METHOD_GETSTATUS); // request periodic status updates from device
try {
if (config.enableBluGateway != null) {
if (profile.alwaysOn && config.enableBluGateway != null) {
logger.debug("{}: BLU Gateway support is {} for this device", thingName,
config.enableBluGateway ? "enabled" : "disabled");
boolean bluetooth = getBool(profile.settings.bluetooth);
if (config.enableBluGateway && !bluetooth) {
logger.info("{}: Bluetooth needs to be enabled to activate BLU Gateway mode", thingName);
if (config.enableBluGateway) {
boolean bluetooth = getBool(profile.settings.bluetooth);
if (config.enableBluGateway && !bluetooth) {
logger.info("{}: Bluetooth needs to be enabled to activate BLU Gateway mode", thingName);
}
installScript(SHELLY2_BLU_GWSCRIPT, config.enableBluGateway && bluetooth);
}
installScript(SHELLY2_BLU_GWSCRIPT, config.enableBluGateway && bluetooth);
}
} catch (ShellyApiException e) {
logger.debug("{}: Device config failed", thingName, e);
@ -1023,7 +1025,7 @@ public class Shelly2ApiRpc extends Shelly2ApiClient implements ShellyApiInterfac
Shelly2RpcRequestParams params = new Shelly2RpcRequestParams();
if (prod || beta) {
params.stage = prod || beta ? "stable" : "beta";
params.stage = prod ? "stable" : "beta";
} else {
params.url = fwurl;
}

View File

@ -13,10 +13,11 @@
package org.openhab.binding.shelly.internal.discovery;
import static org.openhab.binding.shelly.internal.ShellyBindingConstants.*;
import static org.openhab.binding.shelly.internal.util.ShellyUtils.substringBeforeLast;
import static org.openhab.binding.shelly.internal.util.ShellyUtils.*;
import static org.openhab.core.thing.Thing.PROPERTY_MODEL_ID;
import java.io.IOException;
import java.net.Inet4Address;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
@ -117,9 +118,9 @@ public class ShellyDiscoveryParticipant implements MDNSDiscoveryParticipant {
Map<String, Object> properties = new TreeMap<>();
name = service.getName().toLowerCase();
String[] hostAddresses = service.getHostAddresses();
Inet4Address[] hostAddresses = service.getInet4Addresses();
if ((hostAddresses != null) && (hostAddresses.length > 0)) {
address = hostAddresses[0];
address = substringAfter(hostAddresses[0].toString(), "/");
}
if (address.isEmpty()) {
logger.trace("{}: Shelly device discovered with empty IP address (service-name={})", name, service);
@ -142,12 +143,11 @@ public class ShellyDiscoveryParticipant implements MDNSDiscoveryParticipant {
config.password = bindingConfig.defaultPassword;
boolean gen2 = "2".equals(service.getPropertyString("gen"));
ShellyApiInterface api = null;
try {
ShellyApiInterface api = gen2 ? new Shelly2ApiRpc(name, config, httpClient)
: new Shelly1HttpApi(name, config, httpClient);
api = gen2 ? new Shelly2ApiRpc(name, config, httpClient) : new Shelly1HttpApi(name, config, httpClient);
api.initialize();
profile = api.getDeviceProfile(thingType);
api.close();
logger.debug("{}: Shelly settings : {}", name, profile.settingsJson);
deviceName = profile.name;
model = profile.deviceType;
@ -170,6 +170,10 @@ public class ShellyDiscoveryParticipant implements MDNSDiscoveryParticipant {
}
} catch (IllegalArgumentException e) { // maybe some format description was buggy
logger.debug("{}: Discovery failed!", name, e);
} finally {
if (api != null) {
api.close();
}
}
if (thingUID != null) {
@ -185,7 +189,7 @@ public class ShellyDiscoveryParticipant implements MDNSDiscoveryParticipant {
String thingLabel = deviceName.isEmpty() ? name + " - " + address
: deviceName + " (" + name + "@" + address + ")";
return DiscoveryResultBuilder.create(thingUID).withProperties(properties).withLabel(thingLabel)
.withRepresentationProperty(PROPERTY_DEV_NAME).build();
.withRepresentationProperty(PROPERTY_SERVICE_NAME).build();
}
} catch (IOException | NullPointerException e) {
// maybe some format description was buggy

View File

@ -150,11 +150,8 @@ public abstract class ShellyBaseHandler extends BaseThingHandler
Map<String, String> properties = thing.getProperties();
String gen = getString(properties.get(PROPERTY_DEV_GEN));
String thingType = getThingType();
if (gen.isEmpty() && thingType.startsWith("shellyplus") || thingType.startsWith("shellypro")) {
gen = "2";
}
gen2 = "2".equals(gen);
blu = thingType.startsWith("shellyblu");
gen2 = "2".equals(gen) || ShellyDeviceProfile.isGeneration2(thingType);
blu = ShellyDeviceProfile.isBluSeries(thingType);
this.api = !blu ? !gen2 ? new Shelly1HttpApi(thingName, this) : new Shelly2ApiRpc(thingName, thingTable, this)
: new ShellyBluApi(thingName, thingTable, this);
if (gen2) {
@ -569,7 +566,9 @@ public abstract class ShellyBaseHandler extends BaseThingHandler
status = "offline.conf-error-access-denied";
} else if (isWatchdogStarted()) {
if (!isWatchdogExpired()) {
logger.debug("{}: Ignore API Timeout on {} {}, retry later", thingName, res.method, res.url);
if (profile.alwaysOn) { // suppress for battery powered sensors
logger.debug("{}: Ignore API Timeout on {} {}, retry later", thingName, res.method, res.url);
}
} else {
if (isThingOnline()) {
status = "offline.status-error-watchdog";
@ -799,7 +798,7 @@ public abstract class ShellyBaseHandler extends BaseThingHandler
private boolean checkRestarted(ShellySettingsStatus status) {
if (profile.isInitialized() && profile.alwaysOn /* exclude battery powered devices */
&& (status.uptime != null && status.uptime < stats.lastUptime
|| (!profile.status.update.oldVersion.isEmpty()
|| (profile.status.update != null && !getString(profile.status.update.oldVersion).isEmpty()
&& !status.update.oldVersion.equals(profile.status.update.oldVersion)))) {
logger.debug("{}: Device has been restarted, uptime={}/{}, firmware={}/{}", thingName, stats.lastUptime,
getLong(status.uptime), profile.status.update.oldVersion, status.update.oldVersion);
@ -1022,10 +1021,14 @@ public abstract class ShellyBaseHandler extends BaseThingHandler
config.serviceName = getString(properties.get(PROPERTY_SERVICE_NAME));
config.localIp = bindingConfig.localIP;
config.localPort = String.valueOf(bindingConfig.httpPort);
if (config.userId.isEmpty() && !bindingConfig.defaultUserId.isEmpty()) {
if (!profile.isGen2 && config.userId.isEmpty() && !bindingConfig.defaultUserId.isEmpty()) {
// Gen2 has hard coded user "admin"
config.userId = bindingConfig.defaultUserId;
logger.debug("{}: Using default userId {} from binding config", thingName, config.userId);
}
if (config.password.isEmpty() && !bindingConfig.defaultPassword.isEmpty()) {
config.password = bindingConfig.defaultPassword;
logger.debug("{}: Using userId {} from bindingConfig", thingName, config.userId);
logger.debug("{}: Using default password from bindingConfig (userId={})", thingName, config.userId);
}
if (config.updateInterval == 0) {
config.updateInterval = UPDATE_STATUS_INTERVAL_SECONDS * UPDATE_SKIP_COUNT;
@ -1052,6 +1055,11 @@ public abstract class ShellyBaseHandler extends BaseThingHandler
private void checkVersion(ShellyDeviceProfile prf, ShellySettingsStatus status) {
try {
if (prf.fwVersion.isEmpty()) {
// no fw version available (e.g. BLU device)
return;
}
ShellyVersionDTO version = new ShellyVersionDTO();
if (version.checkBeta(getString(prf.fwVersion))) {
logger.info("{}: {}", prf.hostname, messages.get("versioncheck.beta", prf.fwVersion, prf.fwDate));

View File

@ -435,7 +435,10 @@ public class ShellyComponents {
if (t.tmp != null) {
updated |= updateTempChannel(thingHandler, CHANNEL_GROUP_SENSOR, CHANNEL_SENSOR_TEMP,
t.tmp.value, t.tmp.units);
updated |= updateTempChannel(thingHandler, CHANNEL_GROUP_SENSOR, CHANNEL_CONTROL_SETTEMP,
if (t.targetTemp.unit == null) {
t.targetTemp.unit = t.tmp.units;
}
updated |= updateTempChannel(thingHandler, CHANNEL_GROUP_CONTROL, CHANNEL_CONTROL_SETTEMP,
t.targetTemp.value, t.targetTemp.unit);
}
if (t.pos != null) {

View File

@ -519,7 +519,7 @@ public class ShellyChannelDefinitions {
CHANNEL_SENSOR_VIBRATION);
}
// Create tilt for DW/DW2, for BLU DW create channel even tilt is currently not reported
if (sdata.accel != null || (profile.isBlu && sdata.lux != null)) {
if (sdata.accel != null || (profile.isBlu && profile.isDW && sdata.lux != null)) {
addChannel(thing, newChannels, sdata.accel.tilt != null, CHANNEL_GROUP_SENSOR, CHANNEL_SENSOR_TILT);
}
@ -670,7 +670,7 @@ public class ShellyChannelDefinitions {
}
description = getText(PREFIX_CHANNEL + typeId + ".description");
if (description.contains(PREFIX_CHANNEL)) {
description = "";
description = ""; // no resource found
}
}

View File

@ -96,10 +96,6 @@
<description>@text/thing-type.config.shelly.deviceIp.description</description>
<context>network-address</context>
</parameter>
<parameter name="userId" type="text" required="false">
<label>@text/thing-type.config.shelly.userId.label</label>
<description>@text/thing-type.config.shelly.userId.description</description>
</parameter>
<parameter name="password" type="text" required="false">
<label>@text/thing-type.config.shelly.password.label</label>
<description>@text/thing-type.config.shelly.password.description</description>

View File

@ -7,7 +7,7 @@
<config-description uri="thing-type:shelly:blubattery">
<parameter name="deviceAddress" type="text" required="true">
<label>@text/thing-type.config.shelly.deviceAddress.label</label>
<description>@text/thing-type.config.shelly.@text/thing-type.config.shelly.deviceAddress.label.description</description>
<description>@text/thing-type.config.shelly.deviceAddress.description</description>
</parameter>
<parameter name="lowBattery" type="integer" required="false">
<label>@text/thing-type.config.shelly.battery.lowBattery.label</label>

View File

@ -42,7 +42,7 @@ message.event.triggered = Event triggered: {0}
message.event.filtered = Event filtered: {0}
message.coap.init.failed = Unable to start CoIoT: {0}
message.discovery.disabled = Device is marked as non-discoverable, will be skipped
message.discovery.protected = Device {0} reported 'Access defined' (missing userid/password or incorrect).
message.discovery.protected = Device {0} is protected and reports 'Access denied', check userId/password
message.discovery.failed = Device discovery of device with address {0} failed: {1}
message.roller.calibrating = Device is not calibrated, use Shelly App to perform initial roller calibration.
message.roller.favmissing = Roller position favorites are not supported by installed firmware or not configured in the Shelly App
@ -101,7 +101,7 @@ thing-type.shelly.shellyplussmoke.description = Shelly Plus Smoke - Smoke Detect
thing-type.shelly.shellypluswdus.description = Shelly Wall Dimmer US Device
# Wall displays
thing-type.shelly.shellywalldisplay.description = Shelly Plus Wall Display with sensors and input/output
thing-type.shelly.shellywalldisplay.description = Shelly Wall Display with sensors and input/output
# Plus Mini Devices
thing-type.shelly.shellyplusmini1.description = Shelly Plus Mini 1 - Single Relay Switch
@ -120,7 +120,7 @@ thing-type.shelly.shellyproem50.description = Shelly Pro EM-50 - 2xPower Meter +
thing-type.shelly.shellypro4pm.description = Shelly Pro 4PM - 4xRelay Switch with Power Meter
# BLU devices
thing-type.shelly.shellypblubutton.description = Shelly BLU Button
thing-type.shelly.shellyblubutton.description = Shelly BLU Button
thing-type.shelly.shellybludw.description = Shelly BLU Door/Window Sensor
# Wall Displays

View File

@ -32,7 +32,7 @@
<tr><td>Device Mode</td><td>${deviceMode}</td></tr>
<tr><td>Firmware Version</td><td>${firmwareVersion}</td></tr>
<tr><td>Network Name</td><td>${serviceName}</td></tr>
<tr><td>MAC Address</td><td>${macAddress}</td></tr>
<tr><td>Device Address</td><td>${macAddress}</td></tr>
<tr><td>Discoverable</td><td>${discoverable}</td></tr>
<tr><td>WiFi Auto Recovery</td><td>${wifiAutoRecovery}</td></tr>
<tr><td>WiFi AP Roaming</td><td>${apRoamingMode}</td></tr>