Summary: Connect Scheduler sleep timer to Simulation Clock scale change.
ClosedPublic
Actions

Authored by TallFurryMan on Sep 22 2019, 8:00 PM.

Details

Reviewers

wreissenberger
mutlaqja
alexcherney

Commits

R321:0ffd9c6ec0fb: Summary: Connect Scheduler sleep timer to Simulation Clock scale change.

Summary

First, this differential fixes the SimClock scale change so that the change signal is emitted before actual change, providing the new scale as argument.
This enables the slot function to properly estimate the change affecting durations.

Then, it connects the scale change to the Scheduler, so that the sleep timer is recalculated when the SimClock scale updates.
Scheduler sleep duration is then computed based on the SimClock scale, and updated whenever that scale changes.
This allows to debug situations where the Scheduler is set to sleep for a long time in the future.

Warning: pausing or manually advancing the SimClock currently does not impact the sleep timer.
In that case, Scheduler will wake up at an unexpected time.

The change does not impact any other timer than the sleep timer.
The change does not scale the estimation of job duration neither, as it would cause further observations to be scheduled incorrectly far away.
However, the change has all Ekos logs use the current simulation time instead of the system time.

Test Plan

Create a scheduled observation starting in two hours from now.
Start the Scheduler, verify it sets to sleep for two hours.
Change the simulation clock scale, verify a message is logged indicating the rescaled remaining time.
Verify Scheduler wakes up a the proper simulation time.

In that process, verify Ekos logs properly use the current simulation time.

Diff Detail

Repository

R321 KStars

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

TallFurryMan created this revision.Sep 22 2019, 8:00 PM

Restricted Application added a project: KDE Edu. · View Herald TranscriptSep 22 2019, 8:00 PM

Restricted Application added a subscriber: kde-edu. · View Herald Transcript

TallFurryMan requested review of this revision.Sep 22 2019, 8:00 PM

Harbormaster completed remote builds in B16892: Diff 66631.Sep 22 2019, 8:00 PM

mutlaqja accepted this revision.Sep 23 2019, 5:11 AM

mutlaqja added inline comments.

kstars/time/simclock.cpp
185	Why was this moved here? Was it tested for any regressions in KStars outside the scheduler?

This revision is now accepted and ready to land.Sep 23 2019, 5:11 AM

TallFurryMan added inline comments.Sep 23 2019, 5:57 AM

kstars/time/simclock.cpp
185	Because sending the scale update to slots after setting the scale update in SimClock defeats the purpose of sending the scale update. Clients should be able to restore their "real" time values by de-scaling before using the new scale value, like I did in simClockScaleChanged. Else, clients need to store the current scale in order to restore their time values before using the new scale, which is cumbersome. I verified no code was using the old version of the signal before fixing. Probably this small problem would have been fixed earlier had this signal been actually used in the code.

I can update that differential to use the SimClock time in all Ekos logs if you want.

TallFurryMan mentioned this in D22446: Adding error handling strategy control to Scheduler.Sep 23 2019, 6:05 AM

Sure, good way to stay consistent. This should be across all Ekos then, not just scheduler.

TallFurryMan planned changes to this revision.Sep 23 2019, 9:02 AM

Changed all Ekos logs to use the simulation time as timestamp.

This revision is now accepted and ready to land.Sep 25 2019, 7:22 PM

Harbormaster completed remote builds in B17029: Diff 66854.Sep 25 2019, 7:22 PM

TallFurryMan edited the summary of this revision. (Show Details)Sep 25 2019, 7:23 PM

TallFurryMan edited the test plan for this revision. (Show Details)

Great job Eric, I'll land this and hopefully it won't cause any regressions.

Closed by commit R321:0ffd9c6ec0fb: Summary: Connect Scheduler sleep timer to Simulation Clock scale change. (authored by TallFurryMan, committed by mutlaqja). · Explain WhySep 25 2019, 8:46 PM

This revision was automatically updated to reflect the committed changes.

After playing with this diff, I think I will add a warning pop-up when the simulation clock is too far from the system clock. That would be a timed pop-up defaulting to start the schedule. But this pop-up could also reset the simulation time instead. If we go for full-dbus, there should be a way for the remote user to intercept such requests. Opinions?

I do not think it is super necessary. A user who is using KStars for astrophotography normally does not play around with the simulation clock. A message in the log should be a good compromise.

Actually the use case I stumbled upon was:

launch Ekos in the afternoon
change the simclock to see how the sky is at 22:00
configure schedule with objects available at that time
start scheduler, expecting it to fall asleep until 22:00

In that situation, scheduler starts straight away because the simclock is still at 22h. Note that if the simclock is paused, nothing will happen because there is currently a bug that makes scheduler start one minute after the scheduled time... So scheduler will wait endlessly. But the simclock might not be paused... And if the end-user is using D-Bus, we add another layer of abstraction.

Regression crash found when SimClock is scaled while no job is current.

Revision Contents
Changeset List

			Path	Packages
M			kstars/ekos/align/align.cpp (2 lines)
M			kstars/ekos/capture/capture.cpp (2 lines)
M			kstars/ekos/focus/focus.cpp (7 lines)
M			kstars/ekos/guide/guide.cpp (2 lines)
M			kstars/ekos/manager.cpp (2 lines)
M			kstars/ekos/mount/mount.cpp (2 lines)
M			kstars/ekos/observatory/observatory.cpp (4 lines)
M			kstars/ekos/scheduler/scheduler.h (2 lines)
M			kstars/ekos/scheduler/scheduler.cpp (27 lines)
M			kstars/time/simclock.cpp (2 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	66631	76aba5f		Sep 22 2019, 8:00 PM	★	★
Diff 2	66854	76aba5f	Changed all Ekos logs to use the simulation time as timestamp.	Sep 25 2019, 7:22 PM	★	★
Diff 3	66856	234a1d2	R321:0ffd9c6ec0fba2cb12aa20c02154d8ac073a0ca0	Sep 25 2019, 8:46 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
3b59aa1b4b18	aa5e11981ce2	53f45f166011	Eric Dejouhanet	Summary: Connect Scheduler sleep timer to Simulation Clock scale change. (Show More…)	Sep 22 2019, 7:47 PM
53f45f166011	32330c932bfa	76aba5fa3e57	Eric Dejouhanet	Fix simclock scale change so that change signal is emitted before actual change.	Sep 22 2019, 7:46 PM