[rfxcom] unknown subtypes and exceptions

I got the first problem, don’t know if it is new but I don’t like the error:

2016-12-23 14:42:48.108 [ERROR] [g.rfxcom.handler.RFXComBridgeHandler] - Error occured during packet receiving, data: 0913000045DD99018870, cause: java.lang.reflect.InvocationTargetException

I see its a new one, thanks to the extensive use of reflection the exception is wrapped. I think that I want to unwrap it and at least include the stacktrace and some debug level of what is the cause.

Receiving of unsupported command values in this case for Lighting4.

@martinvw, Lightning4 message have some issues and I already made some work on my repo, but didn’t make PR because of pulse length issue.

Yes, the problem is not in the missing support but in the unclear error, without details about the actual problem.

I’m aware of lighting4 discussion see my last messages in the related topic:

Sorry, I didn’t read you previous post. You are testing your changes where you have removed all “unknown” enum values, which now cause the problem?

@Jamstah and me had an idea about a more explicit handling of not yet supported values. In the old situations some exceptions were caught which might better be prevented instead of caught.

@pauli_anttila what is your opinion about these changes? The goal is to make the code more robust and handle unsupported messages in a standardized way.

Well, reason why I made unknown values was that I didn’t have rfxcom SDK document at all, so I didn’t know all possible values. Another reason was, that from user perspective unknown sub types didn’t prevent to receive data (at least on that time there was new sub types introduces very often). If there is nowadays differencies between different sub types how data should be interpreted, I totally agree that unsupported messages should not be handled at all.

You should handle those new exceptions on RFXComMessageFactory createMessage procedure to print more user friendly error messages.

Without documentation it gets a lot harder, I got the SDK spec but I’m not allowed to share… If you would email the guy you can most likely also have it.

I just met a new Security1 message in my logs as well, so maybe your way is still the best. Maybe we should decide separately for each package-type.

I meant to say that when I did first version of the rfxcom binding, I didn’t have the SDK. I definitely have it now. In fact, Rfxcom have sponsored me additional transceiver for development purposes as well :blush:

I could use one extra for my OH2 env as well :wink:

Great work Martin, thanks for carrying it on. I agree with Pauli that we can capture and handle the error in createMessage. Perhaps if it’s a reflection error, just re-throw the cause instead, which should be the right exception.

I did but i don’t like the first code result:

try {
    String className = messageClasses.get(packetType);
    Class<?> cl = Class.forName(classUrl + className);
    Constructor<?> c = cl.getConstructor(byte[].class);
    return (RFXComMessage) c.newInstance(packet);
} catch (ClassNotFoundException e) {
    throw new RFXComMessageNotImplementedException("Message " + packetType + " not implemented, exception: ",
            e);
} catch (InvocationTargetException e){
    if (e.getCause() instanceof RFXComException){
        throw (RFXComException)e.getCause();
    } else {
        throw new RFXComException(e);
    }
// Java 7 - multi catch please
} catch (NoSuchMethodException e) {
    throw new RFXComException(e);
} catch (IllegalAccessException e) {
    throw new RFXComException(e);
} catch (InstantiationException e) {
    throw new RFXComException(e);
}

PS: what is source version for OH2? Can I use Java 7 things, Java 8?

I changed some of the logging, this is the current state (also checked in):

2016-12-23 20:50:35.205 [ERROR] [g.rfxcom.handler.RFXComBridgeHandler] - Error occured during packet receiving, data: 0913000045DD99018570
org.openhab.binding.rfxcom.internal.exceptions.RFXComUnsupportedValueException: Unsupported value '9' for Commands
    at org.openhab.binding.rfxcom.internal.messages.RFXComLighting4Message$Commands.fromByte(RFXComLighting4Message.java:99)
    at org.openhab.binding.rfxcom.internal.messages.RFXComLighting4Message.encodeMessage(RFXComLighting4Message.java:145)[243:org.openhab.binding.rfxcom:2.0.0.201612231933]
    at org.openhab.binding.rfxcom.internal.messages.RFXComLighting4Message.<init>(RFXComLighting4Message.java:120)[243:org.openhab.binding.rfxcom:2.0.0.201612231933]
    at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)[:1.8.0_111]
    at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)[:1.8.0_111]
    at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)[:1.8.0_111]
    at java.lang.reflect.Constructor.newInstance(Constructor.java:423)[:1.8.0_111]
    at org.openhab.binding.rfxcom.internal.messages.RFXComMessageFactory.createMessage(RFXComMessageFactory.java:128)[243:org.openhab.binding.rfxcom:2.0.0.201612231933]
    at org.openhab.binding.rfxcom.handler.RFXComBridgeHandler$MessageListener.packetReceived(RFXComBridgeHandler.java:371)[243:org.openhab.binding.rfxcom:2.0.0.201612231933]
    at org.openhab.binding.rfxcom.internal.connector.RFXComBaseConnector.sendMsgToListeners(RFXComBaseConnector.java:49)[243:org.openhab.binding.rfxcom:2.0.0.201612231933]
    at org.openhab.binding.rfxcom.internal.connector.RFXComStreamReader.run(RFXComStreamReader.java:103)[243:org.openhab.binding.rfxcom:2.0.0.201612231933]

I’m comfortable with this, do you (@Jamstah & @pauli_anttila ) also think it too be a proper reporting?

Hello @Jamstah

Happy new year, I invited you for my repo so you can collaborate on the branch, maybe we should use separate commits for now otherwise it will go wrong.

If the Java source version allows we should use a multi catch and I think it is more or less complete after that, what do you think?

Changes look great, some really good ideas. Have stuck a bunch of comments on for my own reference, I’m putting together a commit.

Yes, multi catch is fine in SE-7 and I think openhab2 is Java 8.

OK, I have a commit ready which addresses all of my comments. Just need to get it to commit to your branch, which is proving less easy than I expected. Off for lunch but will get it there soon I’m sure.

I went with the PR approach and just merged it into your branch. Take a look, and if you’re happy, feel free to squash it into your commit.

Its looking good, might be time for a pull request. I will sleep over it and hope to take a fresh look again next week.

I just started a new assignment for a new customer so I’m way busier than I was in december :frowning:

Sounds good, what are you working on?

Internal systems for https://www.ripe.net/