From 6ebe335ee00eb3cddfbcacd6b00ab5454106913e Mon Sep 17 00:00:00 2001 From: Alexander Falkenstern Date: Thu, 6 Jun 2024 20:57:41 +0200 Subject: [PATCH] [systeminfo] Reduce code complexity as well garbage collection (#16838) * [systeminfo] Avoid thing type change as well memory re-allocations Signed-off-by: Alexander Falkenstern --- .../internal/SystemInfoBindingConstants.java | 4 +- .../internal/SystemInfoHandlerFactory.java | 23 ++--- .../internal/SystemInfoThingTypeProvider.java | 85 +++++++------------ .../discovery/SystemInfoDiscoveryService.java | 21 +++-- .../internal/handler/SystemInfoHandler.java | 84 +++++++----------- .../main/resources/OH-INF/thing/computer.xml | 1 - 6 files changed, 86 insertions(+), 132 deletions(-) diff --git a/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SystemInfoBindingConstants.java b/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SystemInfoBindingConstants.java index 71a2e8df2e23e..5563c66fb01e7 100644 --- a/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SystemInfoBindingConstants.java +++ b/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SystemInfoBindingConstants.java @@ -28,8 +28,8 @@ public class SystemInfoBindingConstants { public static final String BINDING_ID = "systeminfo"; - public static final String THING_TYPE_COMPUTER_ID = "computer"; - public static final ThingTypeUID THING_TYPE_COMPUTER = new ThingTypeUID(BINDING_ID, THING_TYPE_COMPUTER_ID); + public static final ThingTypeUID THING_TYPE_COMPUTER = new ThingTypeUID(BINDING_ID, "computer"); + public static final ThingTypeUID THING_TYPE_COMPUTER_IMPL = new ThingTypeUID(BINDING_ID, "computer-impl"); // Thing properties /** diff --git a/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SystemInfoHandlerFactory.java b/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SystemInfoHandlerFactory.java index 0b4db4c74c75d..7f92c8d5bdc7f 100644 --- a/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SystemInfoHandlerFactory.java +++ b/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SystemInfoHandlerFactory.java @@ -14,6 +14,8 @@ import static org.openhab.binding.systeminfo.internal.SystemInfoBindingConstants.*; +import java.util.Set; + import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.binding.systeminfo.internal.handler.SystemInfoHandler; @@ -41,24 +43,25 @@ public class SystemInfoHandlerFactory extends BaseThingHandlerFactory { private @NonNullByDefault({}) SystemInfoInterface systeminfo; private @NonNullByDefault({}) SystemInfoThingTypeProvider thingTypeProvider; + private static final Set SUPPORTED_THING_TYPES_UIDS = Set.of(THING_TYPE_COMPUTER, + THING_TYPE_COMPUTER_IMPL); + @Override public boolean supportsThingType(ThingTypeUID thingTypeUID) { - return BINDING_ID.equals(thingTypeUID.getBindingId()) - && thingTypeUID.getId().startsWith(THING_TYPE_COMPUTER_ID); + return SUPPORTED_THING_TYPES_UIDS.contains(thingTypeUID); } @Override protected @Nullable ThingHandler createHandler(Thing thing) { - ThingTypeUID thingTypeUID = thing.getThingTypeUID(); - if (supportsThingType(thingTypeUID)) { - String extString = "-" + thing.getUID().getId(); - ThingTypeUID extThingTypeUID = new ThingTypeUID(BINDING_ID, THING_TYPE_COMPUTER_ID + extString); - if (thingTypeProvider.getThingType(extThingTypeUID, null) == null) { - thingTypeProvider.createThingType(extThingTypeUID); - thingTypeProvider.storeChannelsConfig(thing); // Save the current channels configs, will be restored - // after thing type change. + if (THING_TYPE_COMPUTER.equals(thing.getThingTypeUID())) { + if (thingTypeProvider.getThingType(THING_TYPE_COMPUTER_IMPL, null) == null) { + thingTypeProvider.createThingType(THING_TYPE_COMPUTER_IMPL); + // Save the current channels configs, will be restored after thing type change. + thingTypeProvider.storeChannelsConfig(thing); } return new SystemInfoHandler(thing, thingTypeProvider, systeminfo); + } else if (THING_TYPE_COMPUTER_IMPL.equals(thing.getThingTypeUID())) { + return new SystemInfoHandler(thing, thingTypeProvider, systeminfo); } return null; } diff --git a/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SystemInfoThingTypeProvider.java b/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SystemInfoThingTypeProvider.java index 93da1c148059a..0cd66df0e538f 100644 --- a/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SystemInfoThingTypeProvider.java +++ b/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/SystemInfoThingTypeProvider.java @@ -83,82 +83,54 @@ public SystemInfoThingTypeProvider(@Reference ThingTypeRegistry thingTypeRegistr * Create thing type with the provided typeUID and add it to the thing type registry. * * @param typeUID - * @return false if base type UID `systeminfo:computer` cannot be found in the thingTypeRegistry */ - public boolean createThingType(ThingTypeUID typeUID) { + public void createThingType(ThingTypeUID typeUID) { logger.trace("Creating thing type {}", typeUID); - return updateThingType(typeUID, getChannelGroupDefinitions(typeUID)); + updateThingType(typeUID, getChannelGroupDefinitions(typeUID)); } /** * Update `ThingType`with `typeUID`, replacing the channel group definitions with `groupDefs`. * * @param typeUID - * @param groupDefs - * @return false if `typeUID` or its base type UID `systeminfo:computer` cannot be found in the thingTypeRegistry + * @param channelGroupDefinitions */ - public boolean updateThingType(ThingTypeUID typeUID, List groupDefs) { + public void updateThingType(ThingTypeUID typeUID, List channelGroupDefinitions) { ThingType baseType = thingTypeRegistry.getThingType(typeUID); if (baseType == null) { baseType = thingTypeRegistry.getThingType(THING_TYPE_COMPUTER); if (baseType == null) { logger.warn("Could not find base thing type in registry."); - return false; + return; } } - ThingTypeBuilder builder = createThingTypeBuilder(typeUID, baseType.getUID()); - if (builder != null) { - logger.trace("Adding channel group definitions to thing type"); - ThingType type = builder.withChannelGroupDefinitions(groupDefs).build(); - putThingType(type); - return true; - } else { - logger.debug("Error adding channel groups"); - return false; - } - } - - /** - * Return a {@link ThingTypeBuilder} that can create an exact copy of the `ThingType` with `baseTypeUID`. - * Further build steps can be performed on the returned object before recreating the `ThingType` from the builder. - * - * @param newTypeUID - * @param baseTypeUID - * @return the ThingTypeBuilder, null if `baseTypeUID` cannot be found in the thingTypeRegistry - */ - private @Nullable ThingTypeBuilder createThingTypeBuilder(ThingTypeUID newTypeUID, ThingTypeUID baseTypeUID) { - ThingType type = thingTypeRegistry.getThingType(baseTypeUID); + final ThingTypeBuilder builder = ThingTypeBuilder.instance(THING_TYPE_COMPUTER_IMPL, baseType.getLabel()); + builder.withChannelGroupDefinitions(baseType.getChannelGroupDefinitions()); + builder.withChannelDefinitions(baseType.getChannelDefinitions()); + builder.withExtensibleChannelTypeIds(baseType.getExtensibleChannelTypeIds()); + builder.withSupportedBridgeTypeUIDs(baseType.getSupportedBridgeTypeUIDs()); + builder.withProperties(baseType.getProperties()).isListed(false); - if (type == null) { - return null; - } - - ThingTypeBuilder result = ThingTypeBuilder.instance(newTypeUID, type.getLabel()) - .withChannelGroupDefinitions(type.getChannelGroupDefinitions()) - .withChannelDefinitions(type.getChannelDefinitions()) - .withExtensibleChannelTypeIds(type.getExtensibleChannelTypeIds()) - .withSupportedBridgeTypeUIDs(type.getSupportedBridgeTypeUIDs()).withProperties(type.getProperties()) - .isListed(false); - - String representationProperty = type.getRepresentationProperty(); + final String representationProperty = baseType.getRepresentationProperty(); if (representationProperty != null) { - result = result.withRepresentationProperty(representationProperty); + builder.withRepresentationProperty(representationProperty); } - URI configDescriptionURI = type.getConfigDescriptionURI(); + final URI configDescriptionURI = baseType.getConfigDescriptionURI(); if (configDescriptionURI != null) { - result = result.withConfigDescriptionURI(configDescriptionURI); + builder.withConfigDescriptionURI(configDescriptionURI); } - String category = type.getCategory(); + final String category = baseType.getCategory(); if (category != null) { - result = result.withCategory(category); + builder.withCategory(category); } - String description = type.getDescription(); + final String description = baseType.getDescription(); if (description != null) { - result = result.withDescription(description); + builder.withDescription(description); } - return result; + logger.trace("Adding channel group definitions to thing type"); + putThingType(builder.withChannelGroupDefinitions(channelGroupDefinitions).build()); } /** @@ -224,18 +196,19 @@ public List getChannelGroupDefinitions(ThingTypeUID type channelTypeUID != null ? channelTypeUID.getId() : "null"); return null; } - ThingUID thingUID = thing.getUID(); + String index = String.valueOf(i); - ChannelUID channelUID = new ChannelUID(thingUID, channelID + index); - ChannelBuilder builder = ChannelBuilder.create(channelUID).withType(channelTypeUID) - .withConfiguration(baseChannel.getConfiguration()); + ChannelUID channelUID = new ChannelUID(thing.getUID(), channelID + index); + ChannelBuilder builder = ChannelBuilder.create(channelUID).withType(channelTypeUID); + builder.withConfiguration(baseChannel.getConfiguration()); builder.withLabel(channelType.getLabel() + " " + index); builder.withDefaultTags(channelType.getTags()); - String description = channelType.getDescription(); + + final String description = channelType.getDescription(); if (description != null) { builder.withDescription(description); } - String itemType = channelType.getItemType(); + final String itemType = channelType.getItemType(); if (itemType != null) { builder.withAcceptedItemType(itemType); } @@ -252,7 +225,7 @@ public List getChannelGroupDefinitions(ThingTypeUID type */ public void storeChannelsConfig(Thing thing) { Map channelsConfig = thing.getChannels().stream() - .collect(Collectors.toMap(c -> c.getUID().getId(), c -> c.getConfiguration())); + .collect(Collectors.toMap(c -> c.getUID().getId(), Channel::getConfiguration)); thingChannelsConfig.put(thing.getUID(), channelsConfig); } diff --git a/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/discovery/SystemInfoDiscoveryService.java b/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/discovery/SystemInfoDiscoveryService.java index 93a73f40e4636..5c3f313d3b31f 100644 --- a/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/discovery/SystemInfoDiscoveryService.java +++ b/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/discovery/SystemInfoDiscoveryService.java @@ -48,7 +48,7 @@ public class SystemInfoDiscoveryService extends AbstractDiscoveryService { private static final int DISCOVERY_TIME_SECONDS = 30; private static final String THING_UID_VALID_CHARS = "A-Za-z0-9_-"; - private static final String HOST_NAME_SEPERATOR = "_"; + private static final String HOST_NAME_SEPARATOR = "_"; public SystemInfoDiscoveryService() { super(SUPPORTED_THING_TYPES_UIDS, DISCOVERY_TIME_SECONDS); @@ -57,28 +57,31 @@ public SystemInfoDiscoveryService() { @Override protected void startScan() { logger.debug("Starting system information discovery !"); - String hostname; + String hostname; try { hostname = getHostName(); if (hostname.isEmpty()) { throw new UnknownHostException(); } - if (!hostname.matches("[" + THING_UID_VALID_CHARS + "]*")) { - hostname = hostname.replaceAll("[^" + THING_UID_VALID_CHARS + "]", HOST_NAME_SEPERATOR); - } } catch (UnknownHostException ex) { hostname = DEFAULT_THING_ID; logger.info("Hostname can not be resolved. Computer name will be set to the default one: {}", DEFAULT_THING_ID); } - ThingUID computer = new ThingUID(THING_TYPE_COMPUTER, hostname); - thingDiscovered(DiscoveryResultBuilder.create(computer).withLabel(DEFAULT_THING_LABEL).build()); + String thingId = hostname; + if (!thingId.matches("[" + THING_UID_VALID_CHARS + "]*")) { + thingId = thingId.replaceAll("[^" + THING_UID_VALID_CHARS + "]", HOST_NAME_SEPARATOR); + } + + final ThingUID computer = new ThingUID(THING_TYPE_COMPUTER, thingId); + final DiscoveryResultBuilder builder = DiscoveryResultBuilder.create(computer); + builder.withLabel(DEFAULT_THING_LABEL); + thingDiscovered(builder.build()); } protected String getHostName() throws UnknownHostException { - InetAddress addr = InetAddress.getLocalHost(); - return addr.getHostName(); + return InetAddress.getLocalHost().getHostName(); } } diff --git a/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/handler/SystemInfoHandler.java b/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/handler/SystemInfoHandler.java index 366ee9405c817..71a21a9d64148 100644 --- a/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/handler/SystemInfoHandler.java +++ b/bundles/org.openhab.binding.systeminfo/src/main/java/org/openhab/binding/systeminfo/internal/handler/SystemInfoHandler.java @@ -16,14 +16,12 @@ import java.math.BigDecimal; import java.util.ArrayList; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -43,7 +41,6 @@ import org.openhab.core.thing.ThingStatus; import org.openhab.core.thing.ThingStatusDetail; import org.openhab.core.thing.ThingTypeUID; -import org.openhab.core.thing.ThingUID; import org.openhab.core.thing.binding.BaseThingHandler; import org.openhab.core.thing.binding.builder.ThingBuilder; import org.openhab.core.thing.type.ChannelGroupDefinition; @@ -103,12 +100,6 @@ public class SystemInfoHandler extends BaseThingHandler { */ public static final int WAIT_TIME_CHANNEL_ITEM_LINK_INIT = 1; - /** - * String used to extend thingUID and channelGroupTypeUID for thing definition with added dynamic channels and - * extended channels. It is set in the constructor and unique to the thing. - */ - public final String idExtString; - public final SystemInfoThingTypeProvider thingTypeProvider; private SystemInfoInterface systeminfo; @@ -135,8 +126,6 @@ public SystemInfoHandler(Thing thing, SystemInfoThingTypeProvider thingTypeProvi super(thing); this.thingTypeProvider = thingTypeProvider; this.systeminfo = systeminfo; - - idExtString = "-" + thing.getUID().getId(); } @Override @@ -206,6 +195,7 @@ private boolean updateProperties() { properties.put(PROPERTY_OS_FAMILY, systeminfo.getOsFamily().toString()); properties.put(PROPERTY_OS_MANUFACTURER, systeminfo.getOsManufacturer().toString()); properties.put(PROPERTY_OS_VERSION, systeminfo.getOsVersion().toString()); + updateProperties(properties); logger.debug("Properties updated!"); return true; @@ -222,43 +212,36 @@ private boolean updateProperties() { * and are equal to the channel groups and channels with index 0. If there is only one entity in a group, do not add * a channels group and channels with index 0. *

- * If channel groups are added, the thing type will change to systeminfo:computer-Ext, with Ext equal to the thing - * id. A new handler will be created and initialization restarted. Therefore further initialization of the current - * handler can be aborted if the method returns true. + * If channel groups are added, the thing type will change to + * {@link org.openhab.binding.systeminfo.internal.SystemInfoBindingConstants#THING_TYPE_COMPUTER_IMPL + * computer-impl}. A new handler will be created and initialization restarted. + * Therefore further initialization of the current handler can be aborted if the method returns true. * * @return true if channel groups where added */ private boolean addDynamicChannels() { - ThingUID thingUID = thing.getUID(); - List newChannelGroups = new ArrayList<>(); - newChannelGroups.addAll(createChannelGroups(thingUID, CHANNEL_GROUP_STORAGE, CHANNEL_GROUP_TYPE_STORAGE, - systeminfo.getFileOSStoreCount())); - newChannelGroups.addAll(createChannelGroups(thingUID, CHANNEL_GROUP_DRIVE, CHANNEL_GROUP_TYPE_DRIVE, - systeminfo.getDriveCount())); - newChannelGroups.addAll(createChannelGroups(thingUID, CHANNEL_GROUP_DISPLAY, CHANNEL_GROUP_TYPE_DISPLAY, - systeminfo.getDisplayCount())); - newChannelGroups.addAll(createChannelGroups(thingUID, CHANNEL_GROUP_BATTERY, CHANNEL_GROUP_TYPE_BATTERY, - systeminfo.getPowerSourceCount())); - newChannelGroups.addAll(createChannelGroups(thingUID, CHANNEL_GROUP_NETWORK, CHANNEL_GROUP_TYPE_NETWORK, - systeminfo.getNetworkIFCount())); + addChannelGroups(CHANNEL_GROUP_STORAGE, CHANNEL_GROUP_TYPE_STORAGE, systeminfo.getFileOSStoreCount(), + newChannelGroups); + addChannelGroups(CHANNEL_GROUP_DRIVE, CHANNEL_GROUP_TYPE_DRIVE, systeminfo.getDriveCount(), newChannelGroups); + addChannelGroups(CHANNEL_GROUP_DISPLAY, CHANNEL_GROUP_TYPE_DISPLAY, systeminfo.getDisplayCount(), + newChannelGroups); + addChannelGroups(CHANNEL_GROUP_BATTERY, CHANNEL_GROUP_TYPE_BATTERY, systeminfo.getPowerSourceCount(), + newChannelGroups); + addChannelGroups(CHANNEL_GROUP_NETWORK, CHANNEL_GROUP_TYPE_NETWORK, systeminfo.getNetworkIFCount(), + newChannelGroups); if (!newChannelGroups.isEmpty()) { logger.debug("Creating additional channel groups"); newChannelGroups.addAll(0, thingTypeProvider.getChannelGroupDefinitions(thing.getThingTypeUID())); - ThingTypeUID thingTypeUID = new ThingTypeUID(BINDING_ID, THING_TYPE_COMPUTER_ID + idExtString); - if (thingTypeProvider.updateThingType(thingTypeUID, newChannelGroups)) { - logger.trace("Channel groups were added, changing the thing type"); - changeThingType(thingTypeUID, thing.getConfiguration()); - } else { - updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.HANDLER_INITIALIZING_ERROR, - "@text/offline.cannot-initialize"); - } + thingTypeProvider.updateThingType(THING_TYPE_COMPUTER_IMPL, newChannelGroups); + logger.trace("Channel groups were added, changing the thing type"); + changeThingType(THING_TYPE_COMPUTER_IMPL, thing.getConfiguration()); return true; } List newChannels = new ArrayList<>(); - newChannels.addAll(createChannels(thingUID, CHANNEL_SENSORS_FAN_SPEED, systeminfo.getFanCount())); - newChannels.addAll(createChannels(thingUID, CHANNEL_CPU_FREQ, systeminfo.getCpuLogicalCores().intValue())); + addChannels(CHANNEL_SENSORS_FAN_SPEED, systeminfo.getFanCount(), newChannels); + addChannels(CHANNEL_CPU_FREQ, systeminfo.getCpuLogicalCores().intValue(), newChannels); if (!newChannels.isEmpty()) { logger.debug("Creating additional channels"); newChannels.addAll(0, thing.getChannels()); @@ -270,47 +253,38 @@ private boolean addDynamicChannels() { return false; } - private List createChannelGroups(ThingUID thingUID, String channelGroupID, - String channelGroupTypeID, int count) { + private void addChannelGroups(String channelGroupID, String channelGroupTypeID, int count, + List groups) { if (count <= 1) { - return Collections.emptyList(); + return; } List channelGroups = thingTypeProvider.getChannelGroupDefinitions(thing.getThingTypeUID()).stream() - .map(ChannelGroupDefinition::getId).collect(Collectors.toList()); + .map(ChannelGroupDefinition::getId).toList(); - List newChannelGroups = new ArrayList<>(); for (int i = 0; i < count; i++) { String index = String.valueOf(i); ChannelGroupDefinition channelGroupDef = thingTypeProvider .createChannelGroupDefinitionWithIndex(channelGroupID, channelGroupTypeID, i); if (!(channelGroupDef == null || channelGroups.contains(channelGroupID + index))) { logger.trace("Adding channel group {}", channelGroupID + index); - newChannelGroups.add(channelGroupDef); + groups.add(channelGroupDef); } } - return newChannelGroups; } - private List createChannels(ThingUID thingUID, String channelID, int count) { + private void addChannels(String channelID, int count, List channels) { if (count <= 1) { - return Collections.emptyList(); + return; } - List newChannels = new ArrayList<>(); for (int i = 0; i < count; i++) { Channel channel = thingTypeProvider.createChannelWithIndex(thing, channelID, i); if (channel != null && thing.getChannel(channel.getUID()) == null) { logger.trace("Creating channel {}", channel.getUID().getId()); - newChannels.add(channel); + channels.add(channel); } } - return newChannels; - } - - private void storeChannelsConfig() { - logger.trace("Storing channel configurations"); - thingTypeProvider.storeChannelsConfig(thing); } private void restoreChannelsConfig() { @@ -802,9 +776,11 @@ private void handleChannelConfigurationChange(Channel channel, Configuration new publishDataForChannel(channel.getUID()); } + // Don't remove this override. If absent channels will not be populated properly @Override protected void changeThingType(ThingTypeUID thingTypeUID, Configuration configuration) { - storeChannelsConfig(); + logger.trace("Storing channel configurations"); + thingTypeProvider.storeChannelsConfig(thing); super.changeThingType(thingTypeUID, configuration); } diff --git a/bundles/org.openhab.binding.systeminfo/src/main/resources/OH-INF/thing/computer.xml b/bundles/org.openhab.binding.systeminfo/src/main/resources/OH-INF/thing/computer.xml index 3ebafc35a7f7d..c3f47a01cb99f 100644 --- a/bundles/org.openhab.binding.systeminfo/src/main/resources/OH-INF/thing/computer.xml +++ b/bundles/org.openhab.binding.systeminfo/src/main/resources/OH-INF/thing/computer.xml @@ -27,7 +27,6 @@ 1 - Not available Not available Not available