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.

Sign up for GitHub

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

[expiry] [expire] Expiry to ignore commands #2954

Merged
merged 4 commits into from
May 11, 2022
Merged

[expiry] [expire] Expiry to ignore commands #2954

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c349027
[expiry] [expire] bool configuration parameters parsing as util method
ssalonen May 8, 2022
74f2088
[expiry] [expire] extend ExpiryManager to allow ignoring commands
ssalonen May 8, 2022
fa94936
[expire] [expiry] Code cleaning
ssalonen May 9, 2022
fc41525
[expire] [expiry] code cleanup + spotless
ssalonen May 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 64 additions & 36 deletions bundles/org.openhab.core/src/main/java/org/openhab/core/internal/items/ExpireManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.openhab.core.items.MetadataRegistry;
import org.openhab.core.items.events.GroupItemStateChangedEvent;
import org.openhab.core.items.events.ItemCommandEvent;
import org.openhab.core.items.events.ItemEvent;
import org.openhab.core.items.events.ItemEventFactory;
import org.openhab.core.items.events.ItemStateChangedEvent;
import org.openhab.core.items.events.ItemStateEvent;
Expand Down Expand Up @@ -141,29 +142,23 @@ protected void deactivate() {
itemExpireMap.clear();
}

private void processEvent(String itemName, Type type, boolean isStateUpdate) {
logger.trace("Received '{}' for item {}, state update = {}", type, itemName, isStateUpdate);
ExpireConfig expireConfig = getExpireConfig(itemName);
if (expireConfig != null) {
if (isStateUpdate && expireConfig.ignoreStateUpdates) {
return;
}
Command expireCommand = expireConfig.expireCommand;
State expireState = expireConfig.expireState;

if ((expireCommand != null && expireCommand.equals(type))
|| (expireState != null && expireState.equals(type))) {
// New event is expired command or state -> no further action needed
itemExpireMap.remove(itemName); // remove expire trigger until next update or command
logger.debug("Item {} received '{}'; stopping any future expiration.", itemName, type);
} else {
// New event is not the expired command or state, so add the trigger to the map
Duration duration = expireConfig.duration;
itemExpireMap.put(itemName, Instant.now().plus(duration));
logger.debug("Item {} will expire (with '{}' {}) in {} ms", itemName,
expireCommand == null ? expireState : expireCommand,
expireCommand == null ? "state" : "command", duration);
}
private void processEvent(String itemName, Type stateOrCommand, ExpireConfig expireConfig, Class<?> eventClz) {
logger.trace("Received '{}' for item {}, event type= {}", stateOrCommand, itemName, eventClz.getSimpleName());
Command expireCommand = expireConfig.expireCommand;
State expireState = expireConfig.expireState;

if ((expireCommand != null && expireCommand.equals(stateOrCommand))
|| (expireState != null && expireState.equals(stateOrCommand))) {
// New event is expired command or state -> no further action needed
itemExpireMap.remove(itemName); // remove expire trigger until next update or command
logger.debug("Item {} received '{}'; stopping any future expiration.", itemName, stateOrCommand);
} else {
// New event is not the expired command or state, so add the trigger to the map
Duration duration = expireConfig.duration;
itemExpireMap.put(itemName, Instant.now().plus(duration));
logger.debug("Item {} will expire (with '{}' {}) in {} ms", itemName,
expireCommand == null ? expireState : expireCommand, expireCommand == null ? "state" : "command",
duration);
}
}

Expand Down Expand Up @@ -242,15 +237,30 @@ public void receive(Event event) {
if (!enabled) {
return;
}
if (!(event instanceof ItemEvent)) {
return;
}

ItemEvent itemEvent = (ItemEvent) event;
String itemName = itemEvent.getItemName();
ExpireConfig expireConfig = getExpireConfig(itemName);
if (expireConfig == null) {
return;
}

if (event instanceof ItemStateEvent) {
ItemStateEvent isEvent = (ItemStateEvent) event;
processEvent(isEvent.getItemName(), isEvent.getItemState(), true);
if (!expireConfig.ignoreStateUpdates) {
processEvent(itemName, isEvent.getItemState(), expireConfig, event.getClass());
}
} else if (event instanceof ItemCommandEvent) {
ItemCommandEvent icEvent = (ItemCommandEvent) event;
processEvent(icEvent.getItemName(), icEvent.getItemCommand(), false);
if (!expireConfig.ignoreCommands) {
processEvent(itemName, icEvent.getItemCommand(), expireConfig, event.getClass());
}
} else if (event instanceof ItemStateChangedEvent) {
ItemStateChangedEvent icEvent = (ItemStateChangedEvent) event;
processEvent(icEvent.getItemName(), icEvent.getItemState(), false);
processEvent(itemName, icEvent.getItemState(), expireConfig, event.getClass());
}
}

Expand Down Expand Up @@ -289,6 +299,7 @@ public void updated(Metadata oldElement, Metadata element) {

static class ExpireConfig {
static final String CONFIG_IGNORE_STATE_UPDATES = "ignoreStateUpdates";
static final String CONFIG_IGNORE_COMMANDS = "ignoreCommands";

private static final StringType STRING_TYPE_NULL_HYPHEN = new StringType("'NULL'");
private static final StringType STRING_TYPE_NULL = new StringType("NULL");
Expand All @@ -306,13 +317,14 @@ static class ExpireConfig {
final String durationString;
final Duration duration;
final boolean ignoreStateUpdates;
final boolean ignoreCommands;

/**
* Construct an ExpireConfig from the config string.
*
* Valid syntax:
*
* {@code &lt;duration&gt;[,(state=|command=|)&lt;stateOrCommand&gt;][,ignoreStateUpdates]}<br>
* {@code &lt;duration&gt;[,(state=|command=|)&lt;stateOrCommand&gt;][,ignoreStateUpdates][,ignoreCommands]}<br>
* if neither state= or command= is present, assume state
*
* @param item the item to which we are bound
Expand All @@ -330,14 +342,8 @@ public ExpireConfig(Item item, String configString, Map<String, Object> configur
? configString.substring(commaPos + 1).trim()
: null;

Object ignoreStateUpdatesConfigObject = configuration.get(CONFIG_IGNORE_STATE_UPDATES);
if (ignoreStateUpdatesConfigObject instanceof String) {
ignoreStateUpdates = Boolean.parseBoolean((String) ignoreStateUpdatesConfigObject);
} else if (ignoreStateUpdatesConfigObject instanceof Boolean) {
ignoreStateUpdates = (Boolean) ignoreStateUpdatesConfigObject;
} else {
ignoreStateUpdates = false;
}
ignoreStateUpdates = getBooleanConfigValue(configuration, CONFIG_IGNORE_STATE_UPDATES);
ssalonen marked this conversation as resolved.
Show resolved Hide resolved
ignoreCommands = getBooleanConfigValue(configuration, CONFIG_IGNORE_COMMANDS);

if ((stateOrCommand != null) && (stateOrCommand.length() > 0)) {
if (stateOrCommand.startsWith(COMMAND_PREFIX)) {
Expand Down Expand Up @@ -375,6 +381,27 @@ public ExpireConfig(Item item, String configString, Map<String, Object> configur
}
}

/**
* Parse configuration value as primitive boolean. Supports parsing of String and Boolean values.
*
* @param configuration map of configuration keys and values
* @param configKey configuration key to lookup configuration map
* @return configuration value parsed as boolean. Defaults to false when configKey is not present in
* configuration
*/
private boolean getBooleanConfigValue(Map<String, Object> configuration, String configKey) {
boolean configValue;
Object configValueObject = configuration.get(configKey);
if (configValueObject instanceof String) {
configValue = Boolean.parseBoolean((String) configValueObject);
} else if (configValueObject instanceof Boolean) {
configValue = (Boolean) configValueObject;
} else {
configValue = false;
}
return configValue;
}

private Duration parseDuration(String durationString) throws IllegalArgumentException {
Matcher m = DURATION_PATTERN.matcher(durationString);
if (!m.matches() || (m.group(1) == null && m.group(2) == null && m.group(3) == null)) {
Expand All @@ -398,7 +425,8 @@ private Duration parseDuration(String durationString) throws IllegalArgumentExce
@Override
public String toString() {
return "duration='" + durationString + "', s=" + duration.toSeconds() + ", state='" + expireState
+ "', command='" + expireCommand + "', ignoreStateUpdates=" + ignoreStateUpdates;
+ "', command='" + expireCommand + "', ignoreStateUpdates=" + ignoreStateUpdates
+ ", ignoreCommands=" + ignoreCommands;
}
}
}
74 changes: 74 additions & 0 deletions ...les/org.openhab.core/src/test/java/org/openhab/core/internal/items/ExpireManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,58 @@ void testIgnoreStateUpdateDoesNotExtendExpiryOnStateUpdate() throws InterruptedE
verify(eventPublisherMock, times(1)).post(any());
}

@Test
void testIgnoreCommandsExtendsExpiryOnStateChange() throws InterruptedException, ItemNotFoundException {
Item testItem = new NumberItem(ITEMNAME);
when(itemRegistryMock.getItem(ITEMNAME)).thenReturn(testItem);
when(metadataRegistryMock.get(METADATA_KEY))
.thenReturn(config("2s", Map.of(ExpireConfig.CONFIG_IGNORE_COMMANDS, true)));

Event event = ItemEventFactory.createStateEvent(ITEMNAME, new DecimalType(1));
expireManager.receive(event);
Thread.sleep(1500L);
event = ItemEventFactory.createStateChangedEvent(ITEMNAME, new DecimalType(2), new DecimalType(1));
expireManager.receive(event);
Thread.sleep(1500L);
verify(eventPublisherMock, never()).post(any());
Thread.sleep(2000L);
verify(eventPublisherMock, times(1)).post(any());
}

@Test
void testIgnoreCommandsExtendsExpiryOnStateUpdate() throws InterruptedException, ItemNotFoundException {
Item testItem = new NumberItem(ITEMNAME);
when(itemRegistryMock.getItem(ITEMNAME)).thenReturn(testItem);
when(metadataRegistryMock.get(METADATA_KEY))
.thenReturn(config("2s", Map.of(ExpireConfig.CONFIG_IGNORE_COMMANDS, true)));

Event event = ItemEventFactory.createStateEvent(ITEMNAME, new DecimalType(1));
expireManager.receive(event);
Thread.sleep(1500L);
event = ItemEventFactory.createStateEvent(ITEMNAME, new DecimalType(1));
expireManager.receive(event);
Thread.sleep(1500L);
verify(eventPublisherMock, never()).post(any());
Thread.sleep(2000L);
verify(eventPublisherMock, times(1)).post(any());
}

@Test
void testIgnoreCommandDoesNotExtendExpiryOnStateUpdate() throws InterruptedException, ItemNotFoundException {
Item testItem = new NumberItem(ITEMNAME);
when(itemRegistryMock.getItem(ITEMNAME)).thenReturn(testItem);
when(metadataRegistryMock.get(METADATA_KEY))
.thenReturn(config("2s", Map.of(ExpireConfig.CONFIG_IGNORE_COMMANDS, true)));

Event event = ItemEventFactory.createStateEvent(ITEMNAME, new DecimalType(1));
expireManager.receive(event);
Thread.sleep(1500L);
event = ItemEventFactory.createCommandEvent(ITEMNAME, new DecimalType(1));
expireManager.receive(event);
Thread.sleep(1500L);
verify(eventPublisherMock, times(1)).post(any());
}

@Test
void testMetadataChange() throws InterruptedException, ItemNotFoundException {
Metadata md = new Metadata(METADATA_KEY, "1s", null);
Expand Down Expand Up @@ -214,44 +266,66 @@ void testExpireConfig() {
assertEquals(UnDefType.UNDEF, cfg.expireState);
assertNull(cfg.expireCommand);
assertFalse(cfg.ignoreStateUpdates);
assertFalse(cfg.ignoreCommands);

cfg = new ExpireManager.ExpireConfig(testItem, "5h 3m 2s", Map.of());
assertEquals(Duration.ofHours(5).plusMinutes(3).plusSeconds(2), cfg.duration);
assertEquals(UnDefType.UNDEF, cfg.expireState);
assertNull(cfg.expireCommand);
assertFalse(cfg.ignoreStateUpdates);
assertFalse(cfg.ignoreCommands);

cfg = new ExpireManager.ExpireConfig(testItem, "1h,OFF", Map.of());
assertEquals(Duration.ofHours(1), cfg.duration);
assertEquals(OnOffType.OFF, cfg.expireState);
assertNull(cfg.expireCommand);
assertFalse(cfg.ignoreStateUpdates);
assertFalse(cfg.ignoreCommands);

cfg = new ExpireManager.ExpireConfig(testItem, "1h,state=OFF", Map.of());
assertEquals(Duration.ofHours(1), cfg.duration);
assertEquals(OnOffType.OFF, cfg.expireState);
assertNull(cfg.expireCommand);
assertFalse(cfg.ignoreStateUpdates);
assertFalse(cfg.ignoreCommands);

cfg = new ExpireManager.ExpireConfig(testItem, "1h,command=OFF", Map.of());
assertEquals(Duration.ofHours(1), cfg.duration);
assertNull(cfg.expireState);
assertEquals(OnOffType.OFF, cfg.expireCommand);
assertFalse(cfg.ignoreStateUpdates);
assertFalse(cfg.ignoreCommands);

cfg = new ExpireManager.ExpireConfig(testItem, "1h,command=OFF",
Map.of(ExpireConfig.CONFIG_IGNORE_STATE_UPDATES, true));
assertEquals(Duration.ofHours(1), cfg.duration);
assertNull(cfg.expireState);
assertEquals(OnOffType.OFF, cfg.expireCommand);
assertTrue(cfg.ignoreStateUpdates);
assertFalse(cfg.ignoreCommands);

cfg = new ExpireManager.ExpireConfig(testItem, "5h 3m 2s",
Map.of(ExpireConfig.CONFIG_IGNORE_STATE_UPDATES, "true"));
assertEquals(Duration.ofHours(5).plusMinutes(3).plusSeconds(2), cfg.duration);
assertEquals(UnDefType.UNDEF, cfg.expireState);
assertNull(cfg.expireCommand);
assertTrue(cfg.ignoreStateUpdates);
assertFalse(cfg.ignoreCommands);

cfg = new ExpireManager.ExpireConfig(testItem, "1h,command=OFF",
Map.of(ExpireConfig.CONFIG_IGNORE_COMMANDS, true));
assertEquals(Duration.ofHours(1), cfg.duration);
assertNull(cfg.expireState);
assertEquals(OnOffType.OFF, cfg.expireCommand);
assertFalse(cfg.ignoreStateUpdates);
assertTrue(cfg.ignoreCommands);

cfg = new ExpireManager.ExpireConfig(testItem, "5h 3m 2s", Map.of(ExpireConfig.CONFIG_IGNORE_COMMANDS, "true"));
assertEquals(Duration.ofHours(5).plusMinutes(3).plusSeconds(2), cfg.duration);
assertEquals(UnDefType.UNDEF, cfg.expireState);
assertNull(cfg.expireCommand);
assertFalse(cfg.ignoreStateUpdates);
assertTrue(cfg.ignoreCommands);

try {
cfg = new ExpireManager.ExpireConfig(testItem, "1h,command=OPEN", Map.of());
Expand Down