[lcn] Add LCN binding

[fwolter]

Migrates the Local Control Network Binding from OH1 to OH2.

Closes #108

Signed-off-by: Fabian Wolter [email protected]

[TravisBuddy]

Travis tests were successful

Hey @fwolter,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[TravisBuddy]

Travis tests were successful

Hey @fwolter,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[TravisBuddy]

Travis tests were successful

Hey @fwolter,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[kaikreuzer]

Wow, a huge work @fwolter, thanks!
Before going into a review, can you give some hints on whether you have written everything from scratch or if you have ported code over from the 1.x version?

[fwolter]

Thanks, the following classes are based on the OH1 implementation. Most of them are DTOs and constant definitions. The rest is written from scratch.

• SendDataPlainText
• SendData
• SendDataPck
• RequestStatus
• ModInfo
• ConnectionSettings
• ConnectionCallback
• Variable
• VariableValue
• ReverseNumber
• PckGenerator

[TravisBuddy]

Travis tests were successful

Hey @fwolter,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[TravisBuddy]

Travis tests have failed

Hey @fwolter,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

[TravisBuddy]

Travis tests have failed

Hey @fwolter,
please read the following log in order to understand the failure reason. There might also be some helpful tips along the way.
It will be awesome if you fix what is wrong and commit the changes.

[TravisBuddy]

Travis tests were successful

Hey @fwolter,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[kaikreuzer]

Awesome work, @fwolter!
Really very clean and professional code and I see that you dived deep into the openHAB concepts already by using advanced stuff like profiles, vetos, etc.
My review comments should all be fairly simple, there's nothing that should mean any huge effort. Please see below and let me know, if you have any questions!

[kaikreuzer]

Please note that your binding is expected to quickly provide state updates after receiving commands, if you go for a veto here.
Is that really the case for all the channels where you have put "veto"?

[fwolter]

I removed the vetos for these states which must be polled after changing and left the veto for those, which are updated event-based (~100ms after the change).

[cpmeister]

This is amazing work you have done here. Nice job!

[cpmeister]

Why not have each LcnChannelGroup enum instance include a AbstractLcnModuleSubHandler factory lambda method for creating class instances? That way you wouldn't need to use reflection here.

[fwolter]

I really like this idea! Refactored.

[fwolter]

Thank you very much for your positive feedback and your time spent! Really appreciate it! I implemented your remarks and commented on your questions. During testing, I found two bugs, I fixed.

[TravisBuddy]

Travis tests were successful

Hey @fwolter,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[cpmeister]

I think these are my final changes. Thanks for keeping up 😄

[TravisBuddy]

Travis tests were successful

Hey @fwolter,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[TravisBuddy]

Travis tests were successful

Hey @fwolter,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

[cpmeister]

Is it necessary to add all these generics? Shouldn't the new state factory function just be a simple Function< AbstractStateMachine, AbstractState>?

[fwolter]

I tried several approaches, but couldn't get rid of the generics. Any suggestions?

[cpmeister]

Not sure what to say. As an example, this code compiles just fine:

    public interface State {

    }

    public interface StateMachine {
        public void nextState(Function<StateMachine, State> stateFactory);
    }

    public class StateImpl implements State {

        private StateMachine context;

        public StateImpl(StateMachine context) {
            this.context = context;
        }

        public void changeState() {
            context.nextState(StateImpl::new);
        }
    }

[fwolter]

The problems arise when storing data in the context like the connection object. These data cannot be accessed from within StateImpl, because the reference to the context is of the interface's type. I used generics to be able to access the concrete type.

The state machine and state abstraction layer could be removed (and so the generics), but then the code wouldn't be reusable.

Another approach is to replace the generics by casts. If I see correctly only a single cast would be necessary.

As always, there are several solutions with their pros and cons. Maybe I hadn't the bright idea, yet.

[cpmeister]

Well, I took that as a challenge and spent way too much time trying to find a better solution, but I can't say I really found one. I think the generics are irreducibly complex, so they have to stay.
With that resolved I don't really have anything else to suggest. Nice work.

[fwolter]

Well, I took that as a challenge and spent way too much time trying to find a better solution, but I can't say I really found one.

Me too :) Thanks!

[kaikreuzer]

Many thanks again, @fwolter!
Also for your patience with the numerous reviews by @cpmeister. 🤣

Let's get that in openHAB 2.5.6!

[fwolter]

Thanks for merging and thank you very much for the highly professional review feedback. I integrated your proposals very gladly!

[timbms]

Thanks for this great enhancement
Sorry my mistake. I issued the wrong command. It just works great

[fwolter]

Can you please open an issue? I will take a look into it.

[timbms]

Was about to do so. No reason to do so. Sorry for the confusion.