Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Homematic #3

Closed
wants to merge 18 commits into from
Closed

Homematic #3

wants to merge 18 commits into from

Conversation

falkena
Copy link

@falkena falkena commented May 7, 2017

Hi,

it seems, that i got the error we speak about:
https://community.openhab.org/t/homematic-binding-channel-not-found-for-datapoint-errors-for-definitely-existing-channels/26209/19

Please, take a look. May be it's ok for you.

gerrieg and others added 17 commits March 25, 2017 19:57
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
…ixes openhab#1437) (openhab#2221)

Signed-off-by: Alexander Falkenstern <alexander.falkenstern@gmail.com> (github: falkena)
# Conflicts:
#	addons/binding/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/handler/HomematicThingHandler.java
…s on slower computers

Signed-off-by: Gerhard Riegler <gerhard.riegler@gmail.com>
# Conflicts:
#	addons/binding/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/handler/HomematicBridgeHandler.java
#	addons/binding/org.openhab.binding.homematic/src/main/java/org/openhab/binding/homematic/handler/HomematicThingHandler.java
@gerrieg
Copy link
Owner

gerrieg commented May 7, 2017

Thank you for this PR, can you please rebase your branch, because there are so many older commits.

@falkena
Copy link
Author

falkena commented May 7, 2017

It's better now? Further rebasing produces a lot of conflicts.

updateStatus(ThingStatus.ONLINE);

try {
isInitializedLock.lock();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know for what the ReentrantLock is good for. I do not think that's the right approach, because all bindings would have to add such a lock.
Have you seen, that the BrideHandler.initialize() method is called twice?

Copy link
Author

@falkena falkena May 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the problem comes, since "onConnectionResumed" and "initialize" sets state of bridge to ONLINE. If "onConnectionResumed" will be called first, thing goes online and framework will not call "initialize", since already ONLINE. Reenetrant lock is "good old scholl" mutex, someting like syncronized, only for any peace of code. Not objects only. No i have not seen, that "initialize" was called twice, on my PC it was called once only. Do we have next race condition here? May be it's better solution to use getThing().getStatusInfo() instead of using "isInitialized" member.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nearly the same problem is in HomematicThingHandler: take a look into "updateDatapointState" and "initialize" methods calls. They will be called from different threads also, like "initialize" and "onConnectionResumed" in HomematicBridgeHandler. What ever the implementation is, we should wait with further threads processing until things are initialized.

if (hmThing != null) {
HomematicThingHandler thingHandler = (HomematicThingHandler) hmThing.getHandler();
thingHandler.updateDatapointState(dp);
final HmChannel hmChannel = dp.getChannel();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to do this in the AbstractHomematicGateway.eventReceived(). Good hint!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got NPE here :-)

@gerrieg
Copy link
Owner

gerrieg commented May 8, 2017

Something is wrong with this PR, it removes some changes i did recently.

@gerrieg
Copy link
Owner

gerrieg commented May 11, 2017

@falkena
Because i can not merge your PR, i will close it now.

But i have implemented your changes and also found another solution without the lock. So thank you for your help and hints!

@gerrieg gerrieg closed this May 11, 2017
@falkena
Copy link
Author

falkena commented May 12, 2017

@gerrieg Sure. It helped to solve the Problem. So, i would say: "Mission complete" :-)

@falkena falkena deleted the homematic branch May 12, 2017 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants