Substantial refactoring of the InsteonPLM binding

[berndpfrommer]

Incorporated lessons learned from the first version:

• binding now largely ignores category/subcategory entries in the modem link database
• no more device querying (too fragile, rarely successful, not really needed)
• binding can now handle updates to the item binding configuration while running
• new approach for deleting modem database entries allows for selective deleting of devices
• split DeviceFeature class into new classes MessageHandler, CommandHandler, MessageDispatcher, PollHandler
• did away with device descriptors, categories, subcategories as they are no longer needed
• replaced categories.xml by much simpler device_types.xml file
• added configuration option to add new device types via local device_types.xml file
• more code documentation

[buildhive]

openhab » openhab #1360 SUCCESS
This pull request looks good
(what's this?)

[teichsta]

Hi @berndpfrommer, thanks a lot for contributing this huge refactoring. Before merging this into master … can anybody (except you ;-)) confirm this refactoring to be working? Thanks, Thomas E.-E.

[berndpfrommer]

I'm not sure how easy it will be to find somebody to test this since the user base is still pretty small (estimated to be no more than 10).
I'll put a request out on the openHAB group to see if somebody can vouch for the sanity of this version.
I usually always compile and test in my production environment (about 110 devices) for several days before I file any pull requests. This version is no exception.

However there may be some state left on my machine that covers up bugs and I may have broken some other devices out there, so yes, I agree it's a good idea.

[jrbenito]

I did a quick test with 4 devices over bench and it works very well.

Will put to work on the house during weekend.

[CrackerStealth]

I ran through a simple test of my devices last night.
[2477D Light Switch Dimmer] [2334-222 8 Buttom Keypad Dimmer] [2472DWH Dimmable Wall Receptical]

• Turn on from OpenHAB: Success
• Turn off from OpenHAB: Success
• Turn on from switch and see state update: Success
• Turn off from switch and see state update: Success
• Set Dimmer Level from OpenHAB: Success

[2843-222 TriggerLinc Door/Window Sensor] [2845-222 Hidden Door Sensor]

• Open door causes state to change: Success
• Close door causes state to change: Success

[2852-222 Leak Sensor]

• Press button to change state: Success

Something of note. I believe that the InsteonPLM binding may not be releasing it's lock on the serial port under certain cases. I have a linux init.d script hat lets me manage OpenHAB as a service (using "service openhab start" and "service openhab stop"), but it stops the OpenHAB process using kill -9. I have always found that when I stop the OpenHAB software using these service scripts, the Insteon devices would not work until I reboot the system.

In this new version, all items entirely refuse to load and won't until I reboot the system, including non-insteon ones. My web app screen becomes completed void of any items, but still loads all the groups.

Another thing of note. I have a script on the change state of the water leak sensor that doesn't seem to fire when the leak sensor changes state to use Pushover to send a message. Haven't gone too deeply into checking into it, so not sure if my script or something with InsteonPLM binding or Pushover binding.

[berndpfrommer]

First: thanks for testing!

I did notice the locks not getting released on exit.

That is supposed to be done by "somebody" else: either a signal handler in the native part of the nrjavaserial library (probably), or the OS (unlikely). I didn't change the code that deals with the serial port, so that behavior is unlikely to be new.
However, my binding starts up just fine, even when the locks are taken, as long as the user is the same. When running in the IDE there are some error messages on the console saying "overriding lock" or the like. That's what's supposed to be happening.

Could you post the debug log that you get when it fails to start (the second time around).
Not sure if we want to debug this here. It may not be related to the refactoring.

[berndpfrommer]

@teichsta, is the testing done by @CrackerStealth and @jrbenito sufficient to make the pull request go forward?
I think the issues raised by @CrackerStealth above are likely to be unrelated to the InsteonPLM binding in particular, and should be addressed separately.
Please advise, thanks!

[CrackerStealth]

The startup problem I have is definitely not related to the refactoring (other then the symptoms being slightly different), so I would say it shouldn't be considered an issue. I also won't get to looking more closely at that for a few days.

As far as locks being taken over, I don't run OpenHab as an administrator/root. It runs as it's own dedicated user with fairly strict permissions.

[berndpfrommer]

Tried to send you my startup script etc as email, but it posted it here instead. Deleted it as that is too far off topic.

[jrbenito]

I put the new bind into my production Insteon Network, it is not much larger than bench test but it is on production for a week and I did not notice any kind of issue I did not had before, the refactory so far worked great.

My network:

3 DIN Rail On/Off switch
2 DIN Dimmer
1 Micro Dimmer
1 Motion Sensor (wireless) (presence)
1 Hidden Door sensor (wireless)
1 Open/Close sensor (magnet) (wireless)

All of the above are talking with InsteonPLM bind very well. I still have two mini remotes linked direct to DIN Rail Dimmers and that will be changed as soon as I get chance to reconfigure both. They are also working with DIN Dimmers parallels to InsteonPLM for now.

@teichsta, since the number of users active with InsteonPLM is very small and even smaller are the users playing with 1.6 beta version, in my humble opinion the best option is to integrate this so any new user willing to try 1.6 version will also be a beta tester of new binding without the hassle of compile it at home. This would have the benefit of any bug that might be hidden from our tests until now brought to light. Please consider taking this pull request.

All of you thanks for your hard work!

[CrackerStealth]

@teichsta I agree with @jrbenito . The pull request should be safe as neither of us had any issues we didn't have before, and my leak sensor action issue was related to #1585.

[berndpfrommer]

@teichsta, there are issues with the binding that this rewrite fixes. There are users asking for support to fix these problems:
https://groups.google.com/forum/#!topic/openhab/SvqA9IDzbk8
and there is a request to add devices (pull request #1560) which also will be facilitated by this pull request.
Please approve of this pull request, thanks!

[CCCmarine]

It would be most helpful to get that extra device support!
I can test as well in my small Insteon network as well. How would I proceed?

[berndpfrommer]

Instructions for build on linux:
0) install maven and java SDK

1. download the source tree:
   git clone https://github.com/berndpfrommer/openhab.git
   git checkout insteonplm
2. export JAVA_HOME=your_java_home_path_here
3. mvn clean install

If all goes well it will build the entire source tree. Don't know what the exact instructions are to transfer the generated .zip files to your runtime directory. I have a bash script that does the copying etc.

[teichsta]

Hi @berndpfrommer sorry, for the late replay. Thanks again for this contribution and all others for testing and commenting! Keep up the great work :-) Best, Thomas E.-E.