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

Astro: Fix applying earliest and latest config on channel #4061

Merged
merged 1 commit into from Aug 16, 2017

Conversation

Projects
None yet
4 participants
@triller-telekom
Copy link
Contributor

commented Aug 16, 2017

Fixes #4059

Signed-off-by: Stefan Triller stefan.triller@telekom.de

Astro: Fix applying earliest and latest config on channel
Fixes #4059

Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
@sjka

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2017

Why did you add the "configAlreadyApplied" parameter at all? If I'm not mistaken, the only place where "true" is passed is here - and that even was wrong... Shoudln't we remove it now again then?

@triller-telekom

This comment has been minimized.

Copy link
Contributor Author

commented Aug 16, 2017

That "configAlreadyApplied" was because of this bug #3882 and (wrongly) fixed in pr #3938.

The problem is that applying the config to every single event is too late because then you cannot compare the times of 2 events afterwards. And in order to not apply the config twice I introduced the variable.

@sjka

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2017

I see, thanks for the explanation. Looks good to me then.

@sjka

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2017

I re-triggered travis.

@sjka sjka merged commit 941e3d1 into eclipse:master Aug 16, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
ip-validation
Details

@sjka sjka added the Binding-Astro label Aug 16, 2017

@triller-telekom triller-telekom deleted the triller-telekom:astroEarliestFix branch Aug 16, 2017

@kaikreuzer kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017

@kaikreuzer kaikreuzer added bug and removed bug labels Dec 15, 2017

@binderth

This comment has been minimized.

Copy link

commented Feb 7, 2018

I'm not into release management and github... so excuse me for asking: This should be in the stable release at the latest of 2.2?
I still experience related issues while using latest and earliest in the same configuration (on stable 2.2): https://community.openhab.org/t/astro-binding-trigger-latest-earliest-daylight-night/37703/

in short: defining earliest AND latest in one config results in events firing regardless the configuration times.

@triller-telekom

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2018

From the code:

AstroChannelConfig config = channel.getConfiguration().as(AstroChannelConfig.class);
Calendar configStart = applyConfig(start, config);
Calendar configEnd = applyConfig(end, config);
if (truncatedEquals(configStart, configEnd, SECOND)) {
scheduleEvent(thingUID, astroHandler, configStart, asList(EVENT_START, EVENT_END), channelId, true);
} else {
scheduleEvent(thingUID, astroHandler, configStart, EVENT_START, channelId, true);
scheduleEvent(thingUID, astroHandler, configEnd, EVENT_END, channelId, true);
}

I see that both values are applied and the jobs are scheduled with the values inside the config. Do you have a small minimal example where you can reproduce that it doesn't work?

@binderth

This comment has been minimized.

Copy link

commented Feb 8, 2018

see forum entry, but basically this is my thing-configuration:

astro:sun:home   [ geolocation="44.4444, 11.1111", interval=300 ] {
	Channels:
		Type rangeEvent : night#start [
			earliest="21:30",
			latest="22:30"
		]
		Type rangeEvent : night#end [
			earliest="06:00",
			latest="06:30"
		]
		Type rangeEvent : daylight#start [
			earliest="06:00",
			latest="07:30"
		]
		Type rangeEvent : daylight#end [
			earliest="16:00",
			latest="21:30"
		]
}

so I would expect the Events to start and end accordingly in at least v2.2 stable.
what they do:

2017-12-29 18:25:00.006 [vent.ChannelTriggeredEvent] - astro:sun:home:night#event triggered START
2017-12-29 16:26:00.007 [vent.ChannelTriggeredEvent] - astro:sun:home:set#event triggered START

just yesterday evening I switched the earliest and latest lines in the Thing. Same Outcome:

2018-02-08 07:41:00.044 [vent.ChannelTriggeredEvent] - astro:sun:home:daylight#event triggered START

but, I find this one interesting (at 00:30 at night?):
the times are calculated accordingly to the config - as I can see, but the events are fired as if there was no config?

2018-02-08 00:00:30.133 [vent.ItemStateChangedEvent] - Astro_MoonPhase changed from THIRD_QUARTER to WANING_CRESCENT
2018-02-08 00:00:30.162 [vent.ItemStateChangedEvent] - Astro_SunriseTime changed from 2018-02-07T07:42:00.000+0100 to 2018-02-08T07:41:00.000+0100
2018-02-08 00:00:30.177 [vent.ItemStateChangedEvent] - Astro_MoonAzimuth changed from 93.34 to 93.70
2018-02-08 00:00:30.197 [vent.ItemStateChangedEvent] - Astro_MoonElevation changed from -15.94 to -15.63
2018-02-08 00:00:30.209 [vent.ItemStateChangedEvent] - Astro_SunsetTime changed from 2018-02-07T17:25:00.000+0100 to 2018-02-08T17:27:00.000+0100
2018-02-08 00:00:30.222 [vent.ItemStateChangedEvent] - Astro_NightStartTime changed from 2018-02-07T21:30:00.000+0100 to 2018-02-08T21:30:00.000+0100
2018-02-08 00:00:30.266 [vent.ChannelTriggeredEvent] - astro:sun:home:morningNight#event triggered START
2018-02-08 00:00:30.275 [vent.ItemStateChangedEvent] - Astro_NightEndTime changed from 2018-02-08T06:00:00.000+0100 to 2018-02-09T06:00:00.000+0100
2018-02-08 00:00:30.299 [vent.ItemStateChangedEvent] - Astro_DaylightStart changed from 2018-02-07T07:30:00.000+0100 to 2018-02-08T07:30:00.000+0100
2018-02-08 00:00:30.317 [vent.ItemStateChangedEvent] - Astro_DaylightEnde changed from 2018-02-07T17:22:00.000+0100 to 2018-02-08T17:24:00.000+0100
2018-02-08 00:00:30.323 [vent.ItemStateChangedEvent] - Astro_SunAzimuth changed from 345.78 to 346.57
2018-02-08 00:00:30.327 [vent.ItemStateChangedEvent] - Astro_SunElevation changed from -56.10 to -56.17
@triller-telekom

This comment has been minimized.

Copy link
Contributor Author

commented Feb 8, 2018

Alright, I found the bug in your configuration:

You are configuring Type rangeEvent : night#start whereas you have to configure the TRIGGER channel which is Type rangeEvent : night#event.

astro:sun:home   [ geolocation="44.4444, 11.1111", interval=300 ] {
	Channels:
		Type rangeEvent : night#event [
			earliest="21:30",
			latest="22:30"
		]
		Type rangeEvent : night#event [
			earliest="06:00",
			latest="06:30"
		]
		Type rangeEvent : daylight#event [
			earliest="06:00",
			latest="07:30"
		]
		Type rangeEvent : daylight#event [
			earliest="16:00",
			latest="21:30"
		]
}

If you would have added the thing via paperUI so you could have also configured it via paperUI you would have seen that it works, because there the name is correct :)

@binderth

This comment has been minimized.

Copy link

commented Feb 9, 2018

oh - I'm afraid that's not the solution yet! ;)
I did have a look into the PaperUI configuration and noticed the missing "Event"-piece, so my actual configuration since end of December is this:

astro:sun:home   [ geolocation="44.4444, 11.1111", interval=300 ] {
	Channels:
		Type rangeEvent : night#start [
			latest="22:30",
			earliest="21:30"
		]
		Type rangeEvent : night#end [
			latest="06:30",
			earliest="06:00"
		]
		Type rangeEvent : night#event [
			latest="21:31",
			earliest="06:01"
		]
		Type rangeEvent : daylight#start [
			latest="07:30",
			earliest="06:00"
		]
		Type rangeEvent : daylight#end [
			latest="21:30",
			earliest="16:00"
		]
		Type rangeEvent : daylight#event [
			latest="21:32",
			earliest="06:02"
		]
}

but still, from the log the last 24h:

2018-02-08 17:24:00.032 [vent.ChannelTriggeredEvent] - astro:sun:home:daylight#event triggered END
2018-02-08 19:13:00.007 [vent.ChannelTriggeredEvent] - astro:sun:home:night#event triggered START
2018-02-09 07:39:00.010 [vent.ChannelTriggeredEvent] - astro:sun:home:daylight#event triggered START

(no night#event END as of now yet?)

@triller-telekom

This comment has been minimized.

Copy link
Contributor Author

commented Feb 9, 2018

Please have a look at the full example in the README

Type rangeEvent : night#start [ is not a "rangeEvent it is Type start...

I hope that fixes your configuration problems.

@binderth

This comment has been minimized.

Copy link

commented Feb 9, 2018

Ok, I'll try that one - thanks for pointing me!

nevertheless, I'm a bit confused, because with my above config, I see this in PaperUI:
image

changing the Thing-file didn't change this as of now - but I'll see this evening - Keep you updated!

@binderth

This comment has been minimized.

Copy link

commented Feb 10, 2018

please tell me, I didn't catch it, because it's still not working...

my things

astro:sun:home   [ geolocation="44.4444, 11.1111", interval=300 ] {
	Channels:
		Type start : night#start [
			latest="22:30",
			earliest="21:30"
		]
		Type end : night#end [
			latest="06:30",
			earliest="06:00"
		]
		Type rangeEvent : night#event [
			latest="21:31",
			earliest="06:01"
		]
		Type start : daylight#start [
			latest="07:30",
			earliest="06:00"
		]
		Type end : daylight#end [
			latest="21:30",
			earliest="16:00"
		]
		Type rangeEvent : daylight#event [
			latest="21:32",
			earliest="06:02"
		]
}

Events.log

2018-02-09 17:25:00.050 [vent.ChannelTriggeredEvent] - astro:sun:home:daylight#event triggered END
2018-02-09 19:15:00.050 [vent.ChannelTriggeredEvent] - astro:sun:home:night#event triggered START
2018-02-10 07:37:00.036 [vent.ChannelTriggeredEvent] - astro:sun:home:daylight#event triggered START

strangely enough, still no astro:sun:home:night#event triggered END ?

if it helps:

  • openHAB 2.2.0-1 (Release Build)
  • PaperUI says "binding-astro - 2.2.0"
  • Console says 204 │ Active │ 80 │ 0.10.0.b1 │ Astro Binding
@triller-telekom

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2018

I am sorry, but I cannot reproduce this issue here. If you still think that is is not working, please minify your setup, i.e. only keep the 2 night channels, see if the problem persists and if it does, post a new issue with it.

This is a closed PR and thus you should continue this in a new issue if it still persists with a minimal example. You can link the discussion from here in the new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.