openhab / openhab-addons

Add-ons for openHAB
https://www.openhab.org/
Eclipse Public License 2.0
1.86k stars 3.57k forks source link

[astro] Positional job not starting #1223

Closed ghost closed 7 years ago

ghost commented 7 years ago

Although issue #1125 as closed I still can reproduce the problem in build 447 and 469. Sometimes (maybe one out of 20 restarts) the positional job is started but in most of the cases the positional job is not started

ghost commented 7 years ago

I have the same issue with the positional job not starting after a restart of openHAB 2.

The only way I can currently get it to work is to uninstall and reinstall the astro binding after a restart of openHAB2.

I am running build 469 on Windows 7.

lolodomo commented 7 years ago

This job is started only if you have certain channels linked and if channelLinked is called. I remember now that during my tests I was surprised that channelLinked was not always triggered at startup. @kaikreuzer: can you explain in which cases channelLinked is called at dtartup and when it is not ?

kaikreuzer commented 7 years ago

Since https://github.com/eclipse/smarthome/pull/1890, channelLinked should always be called after the handler has been initialized.

lolodomo commented 7 years ago

Ok maybe I noticed that with a version without the fix you mention. Will check again.

One thing a little weird with this binding implemention is that each call to channelLinked will trigger a restart of all jobs. If you have for example 10 channels linked, it will be done 11 times in a very short time at startup, one time when the handler initializes and one time for each channel.

kaikreuzer commented 7 years ago

That's definitely not good and should indeed be changed.

lolodomo commented 7 years ago

In fact, restartJobs is called several times times but normally the astro jobs are finally really started only one time as it is done in a delayed job that is first cancelled at each call to restartJobs. Will check that again to be sure. So the current implementation looks not so bad finally ;)

lolodomo commented 7 years ago

But this can be improved:

lolodomo commented 7 years ago

Ok, I can confirm the issue and its reason. Not always but often, at startup, channelLinked is not called for channels. It happens apparently when at startup the astro thing is initialized and then immediately disposed and then initialized again. In this case, channelLinked seems to be never called. As the binding starts the position job only if one of the relative channels are linked to items (channelLinked called), the position job is not started.

I already noticed this weird intialization sequence at startup (initialize => dispose => initialize) by the thing manager for several bindings. @kaikreuzer : what is the reason ?

By the way, I thing a new isuue relative to channelLinked not being called in this case should be opened in the smarthome repo.

kaikreuzer commented 7 years ago

@kaikreuzer : what is the reason ?

Don't know, but https://github.com/eclipse/smarthome/pull/2087 might fix that.

lolodomo commented 7 years ago

Merged PR #1237 does not fix this issue but avoid some potential problems and reduce the number of calls to restartJobs. By the way, this issue is not inside the astro binding and the isuue will be solved automatically when the other following issue in ESH core will be fixed: https://github.com/eclipse/smarthome/issues/2183 As Kai mentioned, maybe PR 2087 will help.

lolodomo commented 7 years ago

@dobe77 @mdilger : @kaikreuzer closed what was the root cause of this issue. Could you confirm that you don't reproduce this issue now?

ghost commented 7 years ago

I can confirm that it does not occur anymore with Build #595.

Dominik

lolodomo commented 7 years ago

Please close the issue in this case.