[snmp] Set thing only online on valid response ()

* [snmp] Set thing only online on valid response

Otherwise it will toggles between online and offline when the call always times-out, which can happen when the device is unreachable (or a wrong ip address configured).

Signed-off-by: Hilbrand Bouwkamp <hilbrand@h72.nl>
This commit is contained in:
Hilbrand Bouwkamp 2020-10-25 16:21:37 +01:00 committed by GitHub
parent 72bf43cfa0
commit c7b86c09d9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 37 additions and 9 deletions
bundles/org.openhab.binding.snmp/src
main/java/org/openhab/binding/snmp/internal
test/java/org/openhab/binding/snmp/internal

@ -40,6 +40,7 @@ import org.openhab.core.thing.Thing;
import org.openhab.core.thing.ThingStatus;
import org.openhab.core.thing.ThingStatusDetail;
import org.openhab.core.thing.binding.BaseThingHandler;
import org.openhab.core.thing.util.ThingHandlerHelper;
import org.openhab.core.types.Command;
import org.openhab.core.types.RefreshType;
import org.openhab.core.types.State;
@ -200,6 +201,9 @@ public class SnmpTargetHandler extends BaseThingHandler implements ResponseListe
return;
}
timeoutCounter = 0;
if (ThingHandlerHelper.isHandlerInitialized(this)) {
updateStatus(ThingStatus.ONLINE);
}
logger.trace("{} received {}", thing.getUID(), response);
response.getVariableBindings().forEach(variable -> {
@ -423,7 +427,6 @@ public class SnmpTargetHandler extends BaseThingHandler implements ResponseListe
try {
target.setAddress(new UdpAddress(InetAddress.getByName(config.hostname), config.port));
targetAddressString = ((UdpAddress) target.getAddress()).getInetAddress().getHostAddress();
updateStatus(ThingStatus.ONLINE, ThingStatusDetail.NONE);
return true;
} catch (UnknownHostException e) {
target.setAddress(null);

@ -23,6 +23,7 @@ import java.util.HashMap;
import java.util.Map;
import java.util.Vector;
import org.junit.jupiter.api.AfterEach;
import org.mockito.ArgumentCaptor;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
@ -64,6 +65,12 @@ public abstract class AbstractSnmpTargetHandlerTest extends JavaTest {
protected Thing thing;
protected SnmpTargetHandler thingHandler;
private AutoCloseable mocks;
@AfterEach
public void after() throws Exception {
mocks.close();
}
protected VariableBinding handleCommandSwitchChannel(SnmpDatatype datatype, Command command, String onValue,
String offValue, boolean refresh) throws IOException {
@ -134,7 +141,7 @@ public abstract class AbstractSnmpTargetHandlerTest extends JavaTest {
protected void refresh(SnmpChannelMode channelMode, boolean refresh) throws IOException {
setup(SnmpBindingConstants.CHANNEL_TYPE_UID_STRING, channelMode);
waitForAssert(() -> assertEquals(ThingStatus.ONLINE, thingHandler.getThing().getStatusInfo().getStatus()));
verifyStatus(ThingStatus.UNKNOWN);
verify(snmpService).addCommandResponder(any());
if (refresh) {
@ -165,7 +172,7 @@ public abstract class AbstractSnmpTargetHandlerTest extends JavaTest {
String onValue, String offValue, String exceptionValue) {
Map<String, Object> channelConfig = new HashMap<>();
Map<String, Object> thingConfig = new HashMap<>();
MockitoAnnotations.initMocks(this);
mocks = MockitoAnnotations.openMocks(this);
thingConfig.put("hostname", "localhost");
@ -206,6 +213,10 @@ public abstract class AbstractSnmpTargetHandlerTest extends JavaTest {
thingHandler.initialize();
waitForAssert(() -> assertEquals(ThingStatus.ONLINE, thingHandler.getThing().getStatusInfo().getStatus()));
verifyStatus(ThingStatus.UNKNOWN);
}
protected void verifyStatus(ThingStatus status) {
waitForAssert(() -> assertEquals(status, thingHandler.getThing().getStatusInfo().getStatus()));
}
}

@ -23,6 +23,7 @@ import org.junit.jupiter.api.Test;
import org.openhab.core.library.types.DecimalType;
import org.openhab.core.library.types.OnOffType;
import org.openhab.core.library.types.StringType;
import org.openhab.core.thing.ThingStatus;
import org.snmp4j.PDU;
import org.snmp4j.Snmp;
import org.snmp4j.event.ResponseEvent;
@ -62,6 +63,7 @@ public class SnmpTargetHandlerTest extends AbstractSnmpTargetHandlerTest {
new OctetString("on"), false));
assertNull(
onResponseSwitchChannel(SnmpChannelMode.TRAP, SnmpDatatype.INT32, "1", "2", new Integer32(2), false));
verifyStatus(ThingStatus.ONLINE);
}
@Test
@ -104,6 +106,7 @@ public class SnmpTargetHandlerTest extends AbstractSnmpTargetHandlerTest {
ResponseEvent event = new ResponseEvent("test", null, null, responsePDU, null);
thingHandler.onResponse(event);
verify(thingHandlerCallback, atLeast(1)).stateUpdated(eq(CHANNEL_UID), eq(new DecimalType("12.4")));
verifyStatus(ThingStatus.ONLINE);
}
@Test
@ -118,6 +121,7 @@ public class SnmpTargetHandlerTest extends AbstractSnmpTargetHandlerTest {
thingHandler.onResponse(event);
assertEquals(1, source.cancelCallCounter);
verifyStatus(ThingStatus.ONLINE);
}
class SnmpMock extends Snmp {

@ -22,6 +22,7 @@ import java.util.Collections;
import org.junit.jupiter.api.Test;
import org.openhab.core.library.types.DecimalType;
import org.openhab.core.library.types.StringType;
import org.openhab.core.thing.ThingStatus;
import org.snmp4j.PDU;
import org.snmp4j.event.ResponseEvent;
import org.snmp4j.smi.IpAddress;
@ -75,5 +76,6 @@ public class StringChannelTest extends AbstractSnmpTargetHandlerTest {
ResponseEvent event = new ResponseEvent("test", null, null, responsePDU, null);
thingHandler.onResponse(event);
verify(thingHandlerCallback, atLeast(1)).stateUpdated(eq(CHANNEL_UID), eq(new StringType("aa 11 bb")));
verifyStatus(ThingStatus.ONLINE);
}
}

@ -21,6 +21,8 @@ import java.util.Collections;
import org.junit.jupiter.api.Test;
import org.openhab.core.library.types.OnOffType;
import org.openhab.core.thing.ThingStatus;
import org.openhab.core.types.State;
import org.openhab.core.types.UnDefType;
import org.snmp4j.PDU;
import org.snmp4j.event.ResponseEvent;
@ -63,7 +65,7 @@ public class SwitchChannelTest extends AbstractSnmpTargetHandlerTest {
Collections.singletonList(new VariableBinding(new OID(TEST_OID), new OctetString("on"))));
ResponseEvent event = new ResponseEvent("test", null, null, responsePDU, null);
thingHandler.onResponse(event);
verify(thingHandlerCallback, atLeast(1)).stateUpdated(eq(CHANNEL_UID), eq(OnOffType.ON));
verifyResponse(OnOffType.ON);
}
@Test
@ -73,7 +75,7 @@ public class SwitchChannelTest extends AbstractSnmpTargetHandlerTest {
Collections.singletonList(new VariableBinding(new OID(TEST_OID), new Integer32(3))));
ResponseEvent event = new ResponseEvent("test", null, null, responsePDU, null);
thingHandler.onResponse(event);
verify(thingHandlerCallback, atLeast(1)).stateUpdated(eq(CHANNEL_UID), eq(OnOffType.OFF));
verifyResponse(OnOffType.OFF);
}
@Test
@ -84,7 +86,7 @@ public class SwitchChannelTest extends AbstractSnmpTargetHandlerTest {
.singletonList(new VariableBinding(new OID(TEST_OID), OctetString.fromHexStringPairs("aabb11"))));
ResponseEvent event = new ResponseEvent("test", null, null, responsePDU, null);
thingHandler.onResponse(event);
verify(thingHandlerCallback, atLeast(1)).stateUpdated(eq(CHANNEL_UID), eq(OnOffType.ON));
verifyResponse(OnOffType.ON);
}
@Test
@ -95,6 +97,7 @@ public class SwitchChannelTest extends AbstractSnmpTargetHandlerTest {
ResponseEvent event = new ResponseEvent("test", null, null, responsePDU, null);
thingHandler.onResponse(event);
verify(thingHandlerCallback, never()).stateUpdated(eq(CHANNEL_UID), any());
verifyStatus(ThingStatus.ONLINE);
}
@Test
@ -104,7 +107,7 @@ public class SwitchChannelTest extends AbstractSnmpTargetHandlerTest {
Collections.singletonList(new VariableBinding(new OID(TEST_OID), Null.noSuchInstance)));
ResponseEvent event = new ResponseEvent("test", null, null, responsePDU, null);
thingHandler.onResponse(event);
verify(thingHandlerCallback, atLeast(1)).stateUpdated(eq(CHANNEL_UID), eq(UnDefType.UNDEF));
verifyResponse(UnDefType.UNDEF);
}
@Test
@ -115,6 +118,11 @@ public class SwitchChannelTest extends AbstractSnmpTargetHandlerTest {
Collections.singletonList(new VariableBinding(new OID(TEST_OID), Null.noSuchInstance)));
ResponseEvent event = new ResponseEvent("test", null, null, responsePDU, null);
thingHandler.onResponse(event);
verify(thingHandlerCallback, atLeast(1)).stateUpdated(eq(CHANNEL_UID), eq(OnOffType.OFF));
verifyResponse(OnOffType.OFF);
}
private void verifyResponse(State expectedState) {
verify(thingHandlerCallback, atLeast(1)).stateUpdated(eq(CHANNEL_UID), eq(expectedState));
verifyStatus(ThingStatus.ONLINE);
}
}