[digiplex] Fix exception handling in sender and receiver threads

[rmichalak]

Improves error handling in sender and receiver threads so that the threads do not occasionally stop when an unexpected exception occurs.

Closes #17828

[jlaur]

Improves error handling in sender and receiver threads so that the threads do not occasionally stop when an unexpected exception occurs.

Which unexpected exception have you observed?

[rmichalak]

Improves error handling in sender and receiver threads so that the threads do not occasionally stop when an unexpected exception occurs.

Which unexpected exception have you observed?

Honestly, I don't remember now. I'm the original author of this binding and I had this fix applied in my environment for a long time (just forgot to push it to remote). As far as I remember, my alarm system is sending some rubbish over RS232 from time to time and then parsing may fail in many unexpected ways. that's why I have this handleCommunicationError function which reinitializes everything.

[jlaur]

I'm the original author of this binding and I had this fix applied in my environment for a long time (just forgot to push it to remote). As far as I remember, my alarm system is sending some rubbish over RS232 from time to time and then parsing may fail in many unexpected ways

It seems that this fix will potentially hide parser bugs, for example StringIndexOutOfBoundsException here:

openhab-addons/bundles/org.openhab.binding.digiplex/src/main/java/org/openhab/binding/digiplex/internal/communication/DigiplexResponseResolver.java

Line 60 in 388dc6b

return ZoneLabelResponse.success(zoneNo, message.substring(5).trim());

when message.length() is 4:

openhab-addons/bundles/org.openhab.binding.digiplex/src/main/java/org/openhab/binding/digiplex/internal/communication/DigiplexResponseResolver.java

Line 42 in 388dc6b

if (message.length() < 4) { // sanity check: try to filter out malformed responses

If you cannot fix that now, perhaps you could catch it closer to where it is expected to happen:

openhab-addons/bundles/org.openhab.binding.digiplex/src/main/java/org/openhab/binding/digiplex/internal/handler/DigiplexBridgeHandler.java

Line 190 in 388dc6b

DigiplexResponse response = DigiplexResponseResolver.resolveResponse(message);

and log it so that "evidence" could be collected for further analysis and fixing. For example, something like this:

        DigiplexResponse response;
        try {
            response = DigiplexResponseResolver.resolveResponse(message);
        } catch (Exception e) {
            logger.warn("Failed to parse response. This is a bug.", e);
            return;
        }

Maybe you also need to call handleCommunicationError() before returning, if indeed the issue with parsing is caused by RS232 rubbish as you wrote.

WDYT?

[jlaur]

It seems that this fix will potentially hide parser bugs

I fixed those parser bugs in #17864 and added test coverage and logging.

Maybe you also need to call handleCommunicationError() before returning, if indeed the issue with parsing is caused by RS232 rubbish as you wrote.

I don't think this is relevant after all. The parser already returned UnknownResponse when message was too short, which did not trigger any communication error and reinitialization. If that was not needed, it should not be needed for other malformed messages either. Hopefully the logging can help exposing such messages, but of course it would require you to enable debug logging probably for a longer period.