[systeminfo] Reduce code complexity as well garbage collection (#16838)

* [systeminfo] Avoid thing type change as well memory re-allocations

Signed-off-by: Alexander Falkenstern <alexander.falkenstern@gmail.com>
Signed-off-by: Ciprian Pascu <contact@ciprianpascu.ro>
This commit is contained in:
Alexander Falkenstern 2024-06-06 20:57:41 +02:00 committed by Ciprian Pascu
parent c2eb8af40e
commit bf22a64da1
6 changed files with 86 additions and 132 deletions

View File

@ -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
/**

View File

@ -14,6 +14,8 @@ package org.openhab.binding.systeminfo.internal;
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<ThingTypeUID> 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;
}

View File

@ -83,82 +83,54 @@ public class SystemInfoThingTypeProvider extends AbstractStorageBasedTypeProvide
* 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<ChannelGroupDefinition> groupDefs) {
public void updateThingType(ThingTypeUID typeUID, List<ChannelGroupDefinition> 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;
}
}
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);
/**
* 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);
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 class SystemInfoThingTypeProvider extends AbstractStorageBasedTypeProvide
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 class SystemInfoThingTypeProvider extends AbstractStorageBasedTypeProvide
*/
public void storeChannelsConfig(Thing thing) {
Map<String, Configuration> 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);
}

View File

@ -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 class SystemInfoDiscoveryService extends AbstractDiscoveryService {
@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();
}
}

View File

@ -16,14 +16,12 @@ import static org.openhab.binding.systeminfo.internal.SystemInfoBindingConstants
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.Thing;
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 class SystemInfoHandler extends BaseThingHandler {
super(thing);
this.thingTypeProvider = thingTypeProvider;
this.systeminfo = systeminfo;
idExtString = "-" + thing.getUID().getId();
}
@Override
@ -206,6 +195,7 @@ public class SystemInfoHandler extends BaseThingHandler {
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 @@ public class SystemInfoHandler extends BaseThingHandler {
* 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.
* <p>
* 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<ChannelGroupDefinition> 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<Channel> 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 @@ public class SystemInfoHandler extends BaseThingHandler {
return false;
}
private List<ChannelGroupDefinition> createChannelGroups(ThingUID thingUID, String channelGroupID,
String channelGroupTypeID, int count) {
private void addChannelGroups(String channelGroupID, String channelGroupTypeID, int count,
List<ChannelGroupDefinition> groups) {
if (count <= 1) {
return Collections.emptyList();
return;
}
List<String> channelGroups = thingTypeProvider.getChannelGroupDefinitions(thing.getThingTypeUID()).stream()
.map(ChannelGroupDefinition::getId).collect(Collectors.toList());
.map(ChannelGroupDefinition::getId).toList();
List<ChannelGroupDefinition> 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<Channel> createChannels(ThingUID thingUID, String channelID, int count) {
private void addChannels(String channelID, int count, List<Channel> channels) {
if (count <= 1) {
return Collections.emptyList();
return;
}
List<Channel> 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 @@ public class SystemInfoHandler extends BaseThingHandler {
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);
}

View File

@ -27,7 +27,6 @@
<properties>
<property name="thingTypeVersion">1</property>
<property name="CPU Logical Cores">Not available</property>
<property name="CPU Physical Cores">Not available</property>
<property name="OS Manufacturer">Not available</property>