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.

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

[shelly] Various channel types changed to system-defined ones, WD fix, BLU support optimized #15980

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jetty.websocket.servlet.ServletUpgradeRequest;
import org.eclipse.jetty.websocket.servlet.ServletUpgradeResponse;
Expand All @@ -49,7 +48,6 @@
*
* @author Markus Michels - Initial contribution
*/
@NonNullByDefault
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unexpected. Why remove the annotation ? Should be better to handle the warnings, if any.

@WebServlet(name = "ShellyEventServlet", urlPatterns = { SHELLY1_CALLBACK_URI, SHELLY2_CALLBACK_URI })
@Component(service = HttpServlet.class, configurationPolicy = ConfigurationPolicy.OPTIONAL)
public class ShellyEventServlet extends WebSocketServlet {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,8 +342,8 @@ private void fillWiFiSta(@Nullable Shelly2DeviceConfigSta from, ShellySettingsWi
private void checkSetWsCallback() throws ShellyApiException {
Shelly2ConfigParms wsConfig = apiRequest(SHELLYRPC_METHOD_WSGETCONFIG, null, Shelly2ConfigParms.class);
String url = "ws://" + config.localIp + ":" + config.localPort + "/shelly/wsevent";
if (!config.localIp.isEmpty() && !getBool(wsConfig.enable)
|| !url.equalsIgnoreCase(getString(wsConfig.server))) {
if (!config.localIp.isEmpty()
&& (!getBool(wsConfig.enable) || !url.equalsIgnoreCase(getString(wsConfig.server)))) {
logger.debug("{}: A battery device was detected without correct callback, fix it", thingName);
wsConfig.enable = true;
wsConfig.server = url;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import java.net.InetAddress;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
Expand Down Expand Up @@ -51,6 +52,7 @@
import org.openhab.binding.shelly.internal.config.ShellyThingConfiguration;
import org.openhab.binding.shelly.internal.discovery.ShellyThingCreator;
import org.openhab.binding.shelly.internal.provider.ShellyChannelDefinitions;
import org.openhab.binding.shelly.internal.provider.ShellyChannelDefinitions.ShellyChannel;
import org.openhab.binding.shelly.internal.provider.ShellyTranslationProvider;
import org.openhab.binding.shelly.internal.util.ShellyChannelCache;
import org.openhab.binding.shelly.internal.util.ShellyVersionDTO;
Expand All @@ -65,6 +67,8 @@
import org.openhab.core.thing.ThingStatusDetail;
import org.openhab.core.thing.ThingTypeUID;
import org.openhab.core.thing.binding.BaseThingHandler;
import org.openhab.core.thing.binding.ThingHandlerCallback;
import org.openhab.core.thing.binding.builder.ChannelBuilder;
import org.openhab.core.thing.binding.builder.ThingBuilder;
import org.openhab.core.thing.type.ChannelTypeUID;
import org.openhab.core.types.Command;
Expand Down Expand Up @@ -996,7 +1000,6 @@ protected void initializeThingConfig() {
config.localIp);
return;
}

if (!profile.isGen2 && config.userId.isEmpty() && !bindingConfig.defaultUserId.isEmpty()) {
// Gen2 has hard coded user "admin"
config.userId = bindingConfig.defaultUserId;
Expand Down Expand Up @@ -1291,29 +1294,69 @@ public void updateChannelDefinitions(Map<String, Channel> dynChannels) {
return; // already done
}

try {
// Get subset of those channels that currently do not exist
List<Channel> existingChannels = getThing().getChannels();
for (Channel channel : existingChannels) {
String id = channel.getUID().getId();
if (dynChannels.containsKey(id)) {
dynChannels.remove(id);
List<Channel> channelMap = new ArrayList<>(this.getThing().getChannels());
List<Channel> removeChannels = new ArrayList<>();
List<Channel> upgradeChannels = new ArrayList<>();

// Step 1: Replace existing channels with dynamically created ones
for (Map.Entry<String, Channel> channel : dynChannels.entrySet()) {
String id = channel.getKey();
channelMap.removeIf(c -> (c.getUID().getId().equals(id)));
channelMap.add(channel.getValue());
}

// Step 2: check channel definition s for all channels to be up-to-date
for (Channel channel : channelMap) {
try {
boolean upgrade = false;
String channelId = channel.getUID().getId();
ChannelTypeUID uid = channel.getChannelTypeUID();
if (!ShellyChannelDefinitions.hasDefinition(channelId) || uid == null) {
continue;
}
String typeId = uid.getBindingId().equals("system") ? uid.toString() : uid.getId();
ShellyChannel channelDef = ShellyChannelDefinitions.getDefinition(channelId);
ThingHandlerCallback callback = getCallback();
if (channelDef != null && getThing().getChannel(channel.getUID().getId()) != null && callback != null) {
ChannelBuilder channelBuilder = callback.editChannel(thing, channel.getUID());
if (!channelDef.typeId.equals(typeId)) {
channelBuilder.withType(channelDef.getChannelTypeUID());
upgrade = true;
}
String acceptedItemType = getString(channel.getAcceptedItemType());
if (!acceptedItemType.isEmpty() && !channelDef.itemType.equals(acceptedItemType)) {
channelBuilder.withAcceptedItemType(channelDef.itemType);
upgrade = true;
}
if (upgrade) {
logger.debug(
"{}: Upgrade channel definition for channel {} (typeId {} -> {}, acceptedItemType {} -> {})",
thingName, channelId, typeId, channelDef.typeId, acceptedItemType, channelDef.itemType);
removeChannels.add(channel);
upgradeChannels.add(channelBuilder.build());
}
}
} catch (IllegalArgumentException e) {
logger.debug("{}: Unable to upgrade definitions for channel {}", thingName, channel.getUID().toString(),
e);
}
}

if (!dynChannels.isEmpty()) {
logger.debug("{}: Updating channel definitions, {} channels", thingName, dynChannels.size());
ThingBuilder thingBuilder = editThing();
for (Map.Entry<String, Channel> channel : dynChannels.entrySet()) {
Channel c = channel.getValue();
logger.debug("{}: Adding channel {}", thingName, c.getUID().getId());
thingBuilder.withChannel(c);
}
updateThing(thingBuilder.build());
logger.debug("{}: Channel definitions updated", thingName);
// Step 3:
// - remove channels from channelMap, which gets replaced by the dynamic channels
// - add channels, which will get a new definition
channelMap.removeAll(removeChannels);
channelMap.addAll(upgradeChannels);

// Step 4: Update thing definition with new channel list+definitions
if (!channelMap.isEmpty()) {
if (!upgradeChannels.isEmpty()) {
logger.debug("{}: Updating channel definitions for {}/{} channels", thingName, upgradeChannels.size(),
channelMap.size());
}
} catch (IllegalArgumentException e) {
logger.debug("{}: Unable to update channel definitions", thingName, e);
ThingBuilder thingBuilder = editThing();
thingBuilder.withChannels(channelMap);
updateThing(thingBuilder.build());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,12 @@ public ShellyChannelDefinitions(@Reference ShellyTranslationProvider translation
.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_DEVST_GATEWAY, "gatewayDevice", ITEMT_STRING))
.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_DEVST_ITEMP, "system:indoor-temperature", ITEMT_TEMP))
Copy link
Contributor

Choose a reason for hiding this comment

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

All these changes you do to use system channel types concerns only channels created dynamically ?
No need for upgrade instructions ?

What about the old channel types ? If they are no more used, you should remove them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are only a very few channels, which are created statically on the device level (see device.xml), all others are created dynamically.

I tried to remove the old channel definitions, but that leads to the situation that the framework blocks starting the binding for 60sec trying to find the definitions somewhere. So, it's the hen and the egg. Old definitions are still required when starting the new binding version doing the channel upgrades and then they are no longer required on next startup. Again, including upgrade information for all channels for all groups and devices would create a mess with a ton of duplicate overhead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lolodomo could we mark this one as resolved?

Copy link
Contributor

Choose a reason for hiding this comment

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

@J-N-K could you advice how such a Thing upgrade case should be handled?

Copy link
Member

Choose a reason for hiding this comment

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

@J-N-K Any idea how to best deal with Thing upgrades in the case of dynamically created channels?

Copy link
Contributor

Choose a reason for hiding this comment

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

Except for this and the other upgrade comment htis PR is ready to merge. Woudl be nice if someone with more experience could comment if this is ok or not, hint: @J-N-K :-)

.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_DEVST_WAKEUP, "sensorWakeup", ITEMT_STRING))
.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_DEVST_ACCUWATTS, "meterAccuWatts", ITEMT_POWER))
.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_DEVST_ACCUTOTAL, "meterAccuTotal", ITEMT_ENERGY))
.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_DEVST_ACCURETURNED, "meterAccuReturned", ITEMT_ENERGY))
.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_DEVST_ACCUWATTS, "system:electric-power", ITEMT_POWER))
.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_DEVST_ACCUTOTAL, "system:electric-current", ITEMT_ENERGY))
.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_DEVST_ACCURETURNED, "system:electric-current",
ITEMT_ENERGY))
lsiepel marked this conversation as resolved.
Show resolved Hide resolved
.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_DEVST_RESETTOTAL, "meterResetTotals", ITEMT_SWITCH))
.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_DEVST_VOLTAGE, "supplyVoltage", ITEMT_VOLT))
.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_DEVST_VOLTAGE, "system:electric-voltage", ITEMT_VOLT))
.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_DEVST_CHARGER, "charger", ITEMT_SWITCH))
.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_LED_STATUS_DISABLE, "ledStatusDisable", ITEMT_SWITCH))
.add(new ShellyChannel(m, CHGR_DEVST, CHANNEL_LED_POWER_DISABLE, "ledPowerDisable", ITEMT_SWITCH))
Expand All @@ -157,7 +158,7 @@ public ShellyChannelDefinitions(@Reference ShellyTranslationProvider translation
.add(new ShellyChannel(m, CHGR_RELAY, CHANNEL_TIMER_ACTIVE, "timerActive", ITEMT_SWITCH))

// Dimmer
.add(new ShellyChannel(m, CHANNEL_GROUP_DIMMER_CONTROL, CHANNEL_BRIGHTNESS, "dimmerBrightness",
.add(new ShellyChannel(m, CHANNEL_GROUP_DIMMER_CONTROL, CHANNEL_BRIGHTNESS, "system:brightness",
ITEMT_DIMMER))

// Roller
Expand Down Expand Up @@ -193,16 +194,16 @@ public ShellyChannelDefinitions(@Reference ShellyTranslationProvider translation
.add(new ShellyChannel(m, CHGR_LIGHTCH, CHANNEL_TIMER_ACTIVE, "timerActive", ITEMT_SWITCH))

// Power Meter
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_METER_CURRENTWATTS, "meterWatts", ITEMT_POWER))
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_METER_TOTALKWH, "meterTotal", ITEMT_ENERGY))
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_METER_LASTMIN1, "lastPower1", ITEMT_POWER))
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_METER_CURRENTWATTS, "system:electric-power", ITEMT_POWER))
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_METER_TOTALKWH, "system:electric-current", ITEMT_ENERGY))
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_METER_LASTMIN1, "system:electric-power", ITEMT_POWER))
lsiepel marked this conversation as resolved.
Show resolved Hide resolved
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_LAST_UPDATE, "lastUpdate", ITEMT_DATETIME))

// EMeter
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_EMETER_TOTALRET, "meterReturned", ITEMT_ENERGY))
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_EMETER_REACTWATTS, "meterReactive", ITEMT_POWER))
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_EMETER_VOLTAGE, "meterVoltage", ITEMT_VOLT))
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_EMETER_CURRENT, "meterCurrent", ITEMT_AMP))
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_EMETER_TOTALRET, "system:electric-current", ITEMT_ENERGY))
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_EMETER_REACTWATTS, "system:electric-power", ITEMT_POWER))
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_EMETER_VOLTAGE, "system:electric-voltage", ITEMT_VOLT))
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_EMETER_CURRENT, "system:electric-current", ITEMT_AMP))
kaikreuzer marked this conversation as resolved.
Show resolved Hide resolved
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_EMETER_PFACTOR, "meterPowerFactor", ITEMT_NUMBER))
.add(new ShellyChannel(m, CHGR_METER, CHANNEL_EMETER_RESETTOTAL, "meterResetTotals", ITEMT_SWITCH))

Expand All @@ -218,11 +219,11 @@ public ShellyChannelDefinitions(@Reference ShellyTranslationProvider translation
ITEMT_PERCENT))
.add(new ShellyChannel(m, CHGR_SENSOR, CHANNEL_SENSOR_LUX, "sensorLux", ITEMT_LUX))
.add(new ShellyChannel(m, CHGR_SENSOR, CHANNEL_SENSOR_ILLUM, "sensorIllumination", ITEMT_STRING))
.add(new ShellyChannel(m, CHGR_SENSOR, CHANNEL_SENSOR_VOLTAGE, "sensorADC", ITEMT_VOLT))
.add(new ShellyChannel(m, CHGR_SENSOR, CHANNEL_SENSOR_VOLTAGE, "system:electric-voltage", ITEMT_VOLT))
.add(new ShellyChannel(m, CHGR_SENSOR, CHANNEL_SENSOR_STATE, "sensorContact", ITEMT_CONTACT))
.add(new ShellyChannel(m, CHGR_SENSOR, CHANNEL_SENSOR_SSTATE, "sensorState", ITEMT_STRING))
.add(new ShellyChannel(m, CHGR_SENSOR, CHANNEL_SENSOR_TILT, "sensorTilt", ITEMT_ANGLE))
.add(new ShellyChannel(m, CHGR_SENSOR, CHANNEL_SENSOR_MOTION, "sensorMotion", ITEMT_SWITCH))
.add(new ShellyChannel(m, CHGR_SENSOR, CHANNEL_SENSOR_MOTION, "system:motion", ITEMT_SWITCH))
.add(new ShellyChannel(m, CHGR_SENSOR, CHANNEL_SENSOR_MOTION_TS, "motionTimestamp", ITEMT_DATETIME))
.add(new ShellyChannel(m, CHGR_SENSOR, CHANNEL_SENSOR_MOTION_ACT, "motionActive", ITEMT_SWITCH))
.add(new ShellyChannel(m, CHGR_SENSOR, CHANNEL_SENSOR_VIBRATION, "sensorVibration", ITEMT_SWITCH))
Expand Down Expand Up @@ -274,8 +275,21 @@ public ShellyChannelDefinitions(@Reference ShellyTranslationProvider translation
}

public static @Nullable ShellyChannel getDefinition(String channelName) throws IllegalArgumentException {
return CHANNEL_DEFINITIONS.get(getDefinitionKey(channelName));
}

public static boolean hasDefinition(String channelName) {
try {
CHANNEL_DEFINITIONS.get(getDefinitionKey(channelName)); // may throws exception
return true;
} catch (IllegalArgumentException e) {
}
return false;
}

private static String getDefinitionKey(String channelName) {
String group = substringBefore(channelName, "#");
String channel = substringAfter(channelName, "#");
String channel = channelName.contains("#") ? substringAfter(channelName, "#") : channelName;

if (group.startsWith(CHANNEL_GROUP_METER)) {
group = CHANNEL_GROUP_METER; // map meter1..n to meter
Expand All @@ -297,8 +311,7 @@ public ShellyChannelDefinitions(@Reference ShellyTranslationProvider translation
channel = CHANNEL_STATUS_EVENTCOUNT;
}

String channelId = group + "#" + channel;
return CHANNEL_DEFINITIONS.get(channelId);
return group.isEmpty() ? channel : group + "#" + channel;
}

/**
Expand Down Expand Up @@ -680,6 +693,7 @@ public ShellyChannel(ShellyTranslationProvider messages, String group, String ch
this.itemType = itemType;
this.typeId = typeId;

// Auto-Naming if there are multiple instances of the same group, e.g. relay, meter...
groupLabel = getText(PREFIX_GROUP + group + ".label");
if (groupLabel.startsWith(PREFIX_GROUP)) {
groupLabel = "";
Expand All @@ -688,11 +702,21 @@ public ShellyChannel(ShellyTranslationProvider messages, String group, String ch
if (groupDescription.startsWith(PREFIX_GROUP)) {
groupDescription = "";
}
label = getText(PREFIX_CHANNEL + typeId.replace(':', '.') + ".label");

boolean isSystem = typeId.startsWith("system:");
label = PREFIX_CHANNEL + (isSystem ? channel : typeId) + ".label";
label = getText(label);
if (isSystem && label.startsWith(PREFIX_CHANNEL)) {
label = getText(PREFIX_CHANNEL + typeId.replace(":", ".") + ".label");
}
if (label.startsWith(PREFIX_CHANNEL)) {
label = "";
label = ""; // no resource found
}
description = PREFIX_CHANNEL + (isSystem ? channel : typeId) + ".description";
description = getText(description);
if (isSystem && description.startsWith(PREFIX_CHANNEL)) {
description = getText(PREFIX_CHANNEL + typeId.replace(":", ".") + ".description");
}
description = getText(PREFIX_CHANNEL + typeId + ".description");
if (description.startsWith(PREFIX_CHANNEL)) {
description = ""; // no resource found
}
Expand Down Expand Up @@ -763,6 +787,13 @@ public String getChannelAttribute(String attribute) {
private String getText(String key) {
return messages.get(key);
}

public ChannelTypeUID getChannelTypeUID() {
if (typeId.contains("system:")) {
return new ChannelTypeUID("system", substringAfter(typeId, ":"));
}
return new ChannelTypeUID(BINDING_ID, typeId);
}
}

public static class ChannelMap {
Expand Down