Refactored CULHandler lifecycle and configuration. #3885

tarioch · 2016-01-24T13:02:29Z

Signed-off-by: Patrick Ruckstuhl patrick@ch.tario.org (github: tarioch)

tarioch · 2016-02-06T22:21:39Z

Also fixes #3910

tarioch · 2016-02-26T15:09:27Z

Any chance of getting this merged? @Gugiman was able to test it also successfully in #3910 and I would like to fix #3987 and maybe have a look at #4035

watou · 2016-02-26T23:48:26Z

...nding/org.openhab.binding.fht/src/main/java/org/openhab/binding/fht/internal/FHTBinding.java

@@ -270,6 +239,7 @@ protected void removeBindingProvider(FHTBindingProvider bindingProvider) {
     */
    @Override
    public void updated(Dictionary<String, ?> config) throws ConfigurationException {
+        setProperlyConfigured(false);


I haven't looked if it's true for AbstractBindings, but for AbstractActiveBindings it has side effects to setProperlyConfigured(false) and then setProperlyConfigured(true). A different pattern could be:

if (config != null) { boolean properlyConfigured = false; try { // parse and throw as needed... properlyConfigured = true; } finally { setProperlyConfigured(properlyConfigured); } }

watou · 2016-02-27T00:02:44Z

This looks like an impressive bit of refactoring and code size reduction. Thanks! Because the changes are so expansive, is it being heavily tested by other CUL users? Does it work on both 1.8 and 2.0 runtimes? Is it fully compatible with existing item and openhab.cfg configurations and the wiki docs? Sorry I'm not up on the subject myself. Will you leave this "in progress" to consider the other issues you mentioned above? Otherwise your changes look good.

tarioch · 2016-02-27T08:52:27Z

Thanks for the feedback.
I will try on the forum to hopefully get some more testers, right now I tested it on 1.8 and 2.0 and @Gugiman tested it on 1.8. -> https://community.openhab.org/t/testing-help-needed-for-cul-refactoring/8037
Config should be backwards compatible
Wiki, I wanted to only update it after it get's merged as otherwise people could get confused
I would like to get this merged and then do the other changes for the other bugs on top of it (I have at least one of them fixed locally, but don't want to make this pr even bigger)

tarioch · 2016-02-27T08:53:37Z

and I will adjust the code like you mentioned it, I somehow missed this, thanks

Fixes openhab#3118, openhab#2358 Signed-off-by: Patrick Ruckstuhl <patrick@ch.tario.org> (github: tarioch)

tarioch · 2016-02-27T09:03:10Z

@watou updated setting the configured flag

watou · 2016-02-27T09:06:26Z

Thanks @tarioch, sounds good on all points. As much testing as you can recruit with different configurations will obviously help guard against regressions, but your changes seem like good improvements all around. @teichsta, how do you feel about merging this?

dennisausbremen · 2016-02-29T22:22:18Z

Any hints on getting the intertechno bindings to work in 2.0b2?
Skimmed through all of the PRs, tried using your 1.9 snapshot builds to no avail.
Any help would be greatly appreciated.

tarioch · 2016-03-01T06:14:30Z

I have it running with openhab 2. Are you using a version that has the feature for it or not? If you do it manually, there are some more dependencies.

tarioch · 2016-03-01T06:16:17Z

See https://github.com/openhab/openhab/blob/master/features/openhab-addons/src/main/feature/feature.xml

dennisausbremen · 2016-03-01T11:06:52Z

I was using the version you linked here: (1.9.0-pre1)
https://community.openhab.org/t/testing-help-needed-for-cul-refactoring/8037
and installed "openhab-runtime-compat1x" and "openhab-transport-cul" but did not get any further.

The version that can be installed via paper ui did not work.

To quickly tell you about my setup:
rPi2 - Jessie-Lite - Openhab 2.0-b2 (latest snapshot) - homegear 0.6.0-1244
1 nanoCul 868MHz /dev/ttyUSB0 -> homegear (works like a treat)
1 nanoCul 433MHz /dev/ttyUSB1 -> supposed to be used with openhab and intertechno-binding (works via screen /dev/ttyUSB1 38400 -> Sending is000000000FFF/F0)

When openhab starts it sometimes does so just starting the "CULIntertechno Refresh Service" and sometimes with:

11:57:59.501 [ERROR] [port.cul.internal.AbstractCULHandler] - Can't write report command to CUL
java.io.IOException: Stream closed
    at java.io.BufferedWriter.ensureOpen(BufferedWriter.java:116)[:1.8.0_65]
    at java.io.BufferedWriter.write(BufferedWriter.java:221)[:1.8.0_65]
    at java.io.Writer.write(Writer.java:157)[:1.8.0_65]
    at org.openhab.io.transport.cul.internal.AbstractCULHandler.requestCreditReport(AbstractCULHandler.java:296)[158:org.openhab.io.transport.cul:1.9.0.201602280212]
    at org.openhab.io.transport.cul.internal.AbstractCULHandler.writeMessage(AbstractCULHandler.java:322)[158:org.openhab.io.transport.cul:1.9.0.201602280212]
    at org.openhab.io.transport.cul.internal.AbstractCULHandler.access$0(AbstractCULHandler.java:309)[158:org.openhab.io.transport.cul:1.9.0.201602280212]
    at org.openhab.io.transport.cul.internal.AbstractCULHandler$SendThread.run(AbstractCULHandler.java:62)[158:org.openhab.io.transport.cul:1.9.0.201602280212]
11:57:59.548 [ERROR] [port.cul.internal.AbstractCULHandler] - Can't write to CUL /dev/ttyUSB1
java.io.IOException: Stream closed
    at java.io.BufferedWriter.ensureOpen(BufferedWriter.java:116)[:1.8.0_65]
    at java.io.BufferedWriter.write(BufferedWriter.java:221)[:1.8.0_65]
    at java.io.Writer.write(Writer.java:157)[:1.8.0_65]
    at org.openhab.io.transport.cul.internal.AbstractCULHandler.writeMessage(AbstractCULHandler.java:316)[158:org.openhab.io.transport.cul:1.9.0.201602280212]
    at org.openhab.io.transport.cul.internal.AbstractCULHandler.access$0(AbstractCULHandler.java:309)[158:org.openhab.io.transport.cul:1.9.0.201602280212]
    at org.openhab.io.transport.cul.internal.AbstractCULHandler$SendThread.run(AbstractCULHandler.java:62)[158:org.openhab.io.transport.cul:1.9.0.201602280212]

Even when it starts correctly i always get this message when trying to trigger a switch:

11:58:54.161 [WARN ] [org.apache.karaf.services.eventadmin] -
EventAdmin: Exception during event dispatch [org.osgi.service.event.Event [topic=openhab/command/Steckdose] | {org.osgi.service.event.EventHandler, org.osgi.service.cm.ManagedService}={event.topics=openhab/command/*, service.pid=org.openhab.culintertechno, component.name=org.openhab.binding.intertechno.binding, component.id=0, service.id=256, service.bundleid=10, service.scope=bundle} | Bundle(org.openhab.binding.intertechno_1.9.0.201601171404 [10])]
java.lang.NullPointerException
    at org.openhab.binding.intertechno.internal.CULIntertechnoBinding.internalReceiveCommand(CULIntertechnoBinding.java:160)[10:org.openhab.binding.intertechno:1.9.0.201601171404]
    at org.openhab.core.binding.AbstractBinding.receiveCommand(AbstractBinding.java:97)[151:org.openhab.core.compat1x:2.0.0.201602231517]
    at org.openhab.core.events.AbstractEventSubscriber.handleEvent(AbstractEventSubscriber.java:42)[151:org.openhab.core.compat1x:2.0.0.201602231517]
    at org.apache.felix.eventadmin.impl.handler.EventHandlerProxy.sendEvent(EventHandlerProxy.java:415)[3:org.apache.karaf.services.eventadmin:4.0.3]
    at org.apache.felix.eventadmin.impl.tasks.HandlerTask.runWithoutBlacklistTiming(HandlerTask.java:102)[3:org.apache.karaf.services.eventadmin:4.0.3]
    at org.apache.felix.eventadmin.impl.tasks.SyncDeliverTasks.execute(SyncDeliverTasks.java:104)[3:org.apache.karaf.services.eventadmin:4.0.3]
    at org.apache.felix.eventadmin.impl.tasks.AsyncDeliverTasks$TaskExecuter.run(AsyncDeliverTasks.java:163)[3:org.apache.karaf.services.eventadmin:4.0.3]
    at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)[:1.8.0_65]
    at java.util.concurrent.FutureTask.run(FutureTask.java:266)[:1.8.0_65]
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)[:1.8.0_65]
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)[:1.8.0_65]
    at java.lang.Thread.run(Thread.java:745)[:1.8.0_65]
11:58:54.179 [INFO ] [marthome.event.ItemStateChangedEvent] - Steckdose changed from NULL to OFF
11:58:59.504 [INFO ] [b.core.service.AbstractActiveService] - CULIntertechno Refresh Service has been shut down

What am i doing wrong? -.-*

tarioch · 2016-03-01T19:46:35Z

Looks like you actually mixed two things. My refactoring requires both intertechno and cul to be taken from the jars. So it should be

Features

UNINSTALL openhab-transport-cul
INSTALL openhab-runtime-compat1x
INSTALL openhab-transport-serial

Jars

org.openhab.io.transport.cul-1.9.0-SNAPSHOT.jar
org.openhab.binding.intertechno-1.9.0-SNAPSHOT.jar

Config for your stick should then be a file called conf/services/culintertechno.cfg with

device=serial:/dev/ttyUSB1
baudrate=38400
parity=NONE

Please also be aware that there seems to be an issue for some on the raspberry pi, see
https://community.openhab.org/t/replace-nrjavaserial-with-rxtxcomm/5867/4 and #3257 and actual the root issue being NeuronRobotics/nrjavaserial#60

dennisausbremen · 2016-03-01T22:50:45Z

Okay, thanks for your support.
Will wait for the final fixes of NeuronRobotics/nrjavaserial#60 then.
Hope that it gets incorporated quickly into org.openhab.io.transport.serial.

Any ideas where the the sources went? It seems like they're gone and i'm not really into this whole Maven stuff.

tarioch · 2016-03-22T21:23:25Z

Any chance of getting this merged?

dennisausbremen · 2016-03-29T21:21:10Z

Okay to get some pace here again and to focus more on the openhab side of things.

I'm now using the modified io.transport.serial mentioned in NeuronRobotics/nrjavaserial#60 and except the screen issue to get this whole thing started it runs smoothly. I am not using your JARs ATM.

tarioch · 2016-03-29T21:37:04Z

Can you try my JARs + modified nrjavaserial. With this hopefully everything should be working without a need for screen.

dirkclemens · 2016-03-30T06:01:44Z

where do I find the latest *.jars for further tests? The ones you've send me, didn't work in my setting, but I changed several things and will give it another try.

tarioch · 2016-03-30T18:40:11Z

As @watou suggested here https://community.openhab.org/t/pull-requests-and-time-to-get-in/8341/4 trying to summarize the current testing that went into this

@Gugiman

successfully tested culintertechno on linux mint => Can't run CUL Transport Binding with Baudrate of 38400 #3910
fails with raspberry because of nrjavaserial issue

@dirkclemens

successfully tested fht, fs20, s300th with 1.8.1 https://community.openhab.org/t/testing-help-needed-for-cul-refactoring/8037
fails for openhab 2, looks like an installation issue and again the nrjavaserial issue

@dennisausbremen

has nrjavaserial issue as well, hopefully be able to test it with prerelease of fixed nrjavaserial

myself

tested intertechnocul successfully for 1.8.1 and 2.0

Risks/changes

a bigger amount of bundles adjusted
adjustment was mostly removing duplicated logic from bundles and instead do it once in the cul transport bundle
the changes are related to how to fetch a connection, not about how to send/receive messages
it should be a lot easier to fix issues for retrieving the connection after this as it's simpler and all bundles reuse the same

dennisausbremen · 2016-03-30T20:45:12Z

@tarioch :

Setup 1st Try:

OH 2 SNAPSHOT

Bindings in /addon folder

org.openhab.binding.intertechno-1.9.0-SNAPSHOT.jar (by @tarioch)
org.openhab.io.transport.cul-1.9.0-SNAPSHOT.jar (by @tarioch)
org.openhab.io.transport.serial_3.12.0.a1.jar (by @mnlipp)

Bindings installed via feature:install

NONE

DOES NOT WORK
NullPointer from the intertechno-binding:
22:38:54.286 [WARN ] [org.apache.karaf.services.eventadmin] - EventAdmin: Exception during event dispatch [org.osgi.service.event.Event [topic=openhab/command/IT_Bogenlampe] | {org.osgi.service.event.EventHandler, org.osgi.service.cm.ManagedService}={event.topics=openhab/command/*, service.pid=org.openhab.culintertechno, component.name=org.openhab.binding.intertechno.binding, component.id=167, service.id=276, service.bundleid=164, service.scope=bundle} | Bundle(org.openhab.binding.intertechno_1.9.0.201601171404 [164])] java.lang.NullPointerException at org.openhab.binding.intertechno.internal.CULIntertechnoBinding.internalReceiveCommand(CULIntertechnoBinding.java:160)[164:org.openhab.binding.intertechno:1.9.0.201601171404] at org.openhab.core.binding.AbstractBinding.receiveCommand(AbstractBinding.java:97)[151:org.openhab.core.compat1x:2.0.0.201603280103] at org.openhab.core.events.AbstractEventSubscriber.handleEvent(AbstractEventSubscriber.java:42)[151:org.openhab.core.compat1x:2.0.0.201603280103] at org.apache.felix.eventadmin.impl.handler.EventHandlerProxy.sendEvent(EventHandlerProxy.java:415)[3:org.apache.karaf.services.eventadmin:4.0.3] at org.apache.felix.eventadmin.impl.tasks.HandlerTask.run(HandlerTask.java:90)[3:org.apache.karaf.services.eventadmin:4.0.3] at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)[:1.8.0_65] at java.util.concurrent.FutureTask.run(FutureTask.java:266)[:1.8.0_65] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)[:1.8.0_65] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)[:1.8.0_65] at java.lang.Thread.run(Thread.java:745)[:1.8.0_65]

Setup 2nd Try:

OH 2 SNAPSHOT

Bindings in /addon folder

org.openhab.io.transport.cul-1.9.0-SNAPSHOT.jar (by @tarioch)
org.openhab.io.transport.serial_3.12.0.a1.jar (by @mnlipp)

Bindings installed via feature:install

openhab-binding-intertechno (1.9.0.SNAPSHOT)

WORKS

tarioch · 2016-03-31T18:23:59Z

Hm, something seems off in the first try, the line number of that error only matches in the old code (without my changes). Could there maybe be some caching or something (I don't know what) that still parts of the original jar is somehow picked up?

dennisausbremen · 2016-04-01T07:31:19Z

I will try to look into this. But i won't have time until sunday. :-(
This in strange indeed. But i'll have a look.

tarioch · 2016-04-15T21:28:20Z

@watou @teichsta Is there anything I can do to get this merged?

watou · 2016-04-15T22:17:51Z

@tarioch, it looked like this PR is still awaiting tester feedback? Does its working over serial require changes that aren't part of openHAB yet? Will the affected JARs continue to work on openHAB 1.8? Is all the relevant user documentation consistent with this PR? I've looked for answers but I'm sorry I'm not deep enough in the code and history to piece it together. Any others you could enlist to give any critical feedback?

tarioch · 2016-04-16T00:38:36Z

@watou

the serial problems are unrelated to this, that's already broken without my changes and only affects raspberry pi (that's where most of the testers are hanging)
affected jars work fine in openhab 1.8 (I'm running this now for a couple of months and some other testers as well)
affected jars also work in openhab 2 (me and some others had some successful testing)
documentation: I did not want to update the wiki pages before this is merged, but if it makes it easier I can also update the documentation first
others that could give feedback: I don't known, to be honest it looks to me like the cul related bindings are a bit abandoned, that's why I wanted to get some stuff fixed and hopefully also easier to maintain

watou · 2016-04-16T05:56:33Z

Thank you @tarioch. This PR then lgtm. @teichsta, do you concur/merge?

watou · 2016-04-20T09:24:43Z

Thank you, @tarioch, for all of your wiki updates!

teichsta added bug enhancement labels Jan 28, 2016

teichsta added this to the 1.9.0 milestone Jan 28, 2016

teichsta added the awaiting-feedback label Jan 28, 2016

tarioch force-pushed the 2358_configurable_serial_settings_for_cul branch 2 times, most recently from 7a9d418 to f0e5a86 Compare February 2, 2016 08:19

This was referenced Feb 6, 2016

Intertechno seems not to work with OpenHab 2 #3986

Closed

Can't run CUL Transport Binding with Baudrate of 38400 #3910

Closed

tarioch mentioned this pull request Feb 6, 2016

Do not send intertechno sending parameters per default #3987

Closed

watou reviewed Feb 26, 2016
View reviewed changes

Refactored CULHandler lifecycle and configuration.

ad2a321

Fixes openhab#3118, openhab#2358 Signed-off-by: Patrick Ruckstuhl <patrick@ch.tario.org> (github: tarioch)

tarioch force-pushed the 2358_configurable_serial_settings_for_cul branch from f0e5a86 to ad2a321 Compare February 27, 2016 09:02

tarioch mentioned this pull request Mar 29, 2016

Use proper lock directory NeuronRobotics/nrjavaserial#60

Closed

teichsta merged commit 4a91720 into openhab:master Apr 18, 2016

teichsta removed the awaiting-feedback label Apr 18, 2016

tarioch deleted the 2358_configurable_serial_settings_for_cul branch April 19, 2016 19:33

tarioch mentioned this pull request Apr 19, 2016

Changes to MaxCULBinding in 1.7.0 break FS20 baudrate settings #2358

Closed

watou mentioned this pull request May 3, 2016

CUL network connection does not get restarted on broken connection #4035

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactored CULHandler lifecycle and configuration. #3885

Refactored CULHandler lifecycle and configuration. #3885

tarioch commented Jan 24, 2016

tarioch commented Feb 6, 2016

tarioch commented Feb 26, 2016

watou Feb 26, 2016

watou commented Feb 27, 2016

tarioch commented Feb 27, 2016

tarioch commented Feb 27, 2016

tarioch commented Feb 27, 2016

watou commented Feb 27, 2016

dennisausbremen commented Feb 29, 2016

tarioch commented Mar 1, 2016

tarioch commented Mar 1, 2016

dennisausbremen commented Mar 1, 2016

tarioch commented Mar 1, 2016

dennisausbremen commented Mar 1, 2016

tarioch commented Mar 22, 2016

dennisausbremen commented Mar 29, 2016

tarioch commented Mar 29, 2016

dirkclemens commented Mar 30, 2016

tarioch commented Mar 30, 2016

dennisausbremen commented Mar 30, 2016

tarioch commented Mar 31, 2016

dennisausbremen commented Apr 1, 2016

tarioch commented Apr 15, 2016

watou commented Apr 15, 2016

tarioch commented Apr 16, 2016

watou commented Apr 16, 2016

watou commented Apr 20, 2016

Refactored CULHandler lifecycle and configuration. #3885

Refactored CULHandler lifecycle and configuration. #3885

Conversation

tarioch commented Jan 24, 2016

tarioch commented Feb 6, 2016

tarioch commented Feb 26, 2016

watou Feb 26, 2016

Choose a reason for hiding this comment

watou commented Feb 27, 2016

tarioch commented Feb 27, 2016

tarioch commented Feb 27, 2016

tarioch commented Feb 27, 2016

watou commented Feb 27, 2016

dennisausbremen commented Feb 29, 2016

tarioch commented Mar 1, 2016

tarioch commented Mar 1, 2016

dennisausbremen commented Mar 1, 2016

tarioch commented Mar 1, 2016

dennisausbremen commented Mar 1, 2016

tarioch commented Mar 22, 2016

dennisausbremen commented Mar 29, 2016

tarioch commented Mar 29, 2016

dirkclemens commented Mar 30, 2016

tarioch commented Mar 30, 2016

dennisausbremen commented Mar 30, 2016

Setup 1st Try:

Setup 2nd Try:

tarioch commented Mar 31, 2016

dennisausbremen commented Apr 1, 2016

tarioch commented Apr 15, 2016

watou commented Apr 15, 2016

tarioch commented Apr 16, 2016

watou commented Apr 16, 2016

watou commented Apr 20, 2016