×

INDI Library v2.0.7 is Released (01 Apr 2024)

Bi-monthly release with minor bug fixes and improvements

Partitioning the Scheduler code

  • Posts: 85
  • Thank you received: 40
The length of the scheduler.cpp file alone implies it needs to be broken in pieces.
The class has 98 member functions and is 7310 lines long :pinch:

I'd like to see a sorting of each of these member functions into the proposed new classes, like this :
"scheduler.cpp", which would contain observatory startup and shutdown
 
    Scheduler::Scheduler()
    QString Scheduler::getCurrentJobName()
    void Scheduler::toggleScheduler()
    void Scheduler::stop()
    void Scheduler::start()
    void Scheduler::pause()
    void Scheduler::wakeUpScheduler()
    bool Scheduler::checkEkosState()
    bool Scheduler::isINDIConnected()
    bool Scheduler::checkINDIState()
    bool Scheduler::checkStartupState()
    bool Scheduler::checkShutdownState()
    bool Scheduler::checkParkWaitState()
    bool Scheduler::manageConnectionLoss()
    void Scheduler::disconnectINDI()
    void Scheduler::stopEkos()
    void Scheduler::parkMount()
    void Scheduler::unParkMount()
    void Scheduler::checkMountParkingStatus()
    bool Scheduler::isMountParked()
    void Scheduler::parkDome()
    void Scheduler::unParkDome()
    void Scheduler::checkDomeParkingStatus()
    bool Scheduler::isDomeParked()
    void Scheduler::parkCap()
    void Scheduler::unParkCap()
    void Scheduler::checkCapParkingStatus()
    void Scheduler::checkStartupProcedure()
    void Scheduler::runStartupProcedure()
    void Scheduler::checkShutdownProcedure()
    void Scheduler::runShutdownProcedure()
 
"scheduler_execjob.cpp", which would contain job runtime execution code
    ...
"scheduler_plan.cpp", which would contain calculations scheduling jobs
    ...
"scheduler_serialize.cpp", which would contain job and sequence loading and saving to disk
    ...
"scheduler_ui.cpp", which would contain UI manipulations
    ...
"schedulerjob.cpp"
    ...
"mosaic.cpp"
    ...
With that we could migrate piece-by-piece while making clear interfaces.
Also as sterne-jaeger said we could indeed create the new scheduler next to the existing one, users can then select one or the other during this migration.
And about 'Having a separate UI controller that handles all the UI interactions' is a good idea because that allows for other controllers (DBus anyone ? Or a nice REST API ?) as well as testing.

@dokeeffe I looked briefly at adding back the shutdown-on-bad-weather functionality, but decided controlling this from the outside was faster to achieve by writing ekos_sentinel.py which should remain useful in the future as fall-back safety mechanism for when the scheduler does have the needed features itself

@ChrisRowland: I agree with the safety monitor remark. Have a look at indilib.org/forum/ekos/5065-full-automat....html?start=12#38799 for an external process.

-- Hans
The following user(s) said Thank You: Eric
4 years 11 months ago #38836

Please Log in or Create an account to join the conversation.

  • Posts: 456
  • Thank you received: 76
A fresh start could be the best way to go alright. I'm not a cpp developer so I'm not an expert. It would be great to somehow work on the testing side from the start, enable TDD and the best design will then emerge from the TDD process. I've seen this produce better built software than big up front designs.

Could I also suggest that the 'weather' tickbox be hidden/removed from the current UI to prevent any misunderstanding? I'm really lucky it didn't rain when I discovered the function of this checkbox changed from monitoring weather to an initial check only. If not then maybe some help text next to it or something.

Derek
4 years 11 months ago #38843

Please Log in or Create an account to join the conversation.

  • Posts: 1185
  • Thank you received: 370
OK, that should be easy. I'll remove it and submit a diff.

- Wolfgang
4 years 11 months ago #38854

Please Log in or Create an account to join the conversation.

  • Posts: 1185
  • Thank you received: 370
Maybe we do not need to remove it. It seems like there is a D-Bus problem with the weather devices. I'll try to figure it out, maybe we could repair it.

- Wolfgang
The following user(s) said Thank You: Eric
4 years 11 months ago #38857

Please Log in or Create an account to join the conversation.

I don't think weather should be removed from scheduler. Hans is interested in improving the weather support, but maybe this would be part of the partitioning plan as well.

Where would this weather handling exactly resides? Well, this is up for discussion now.
The following user(s) said Thank You: Eric
4 years 11 months ago #38864

Please Log in or Create an account to join the conversation.

  • Posts: 1185
  • Thank you received: 370
Ok, absolutely fine, but it seems like the Scheduler does not connect to weather devices right now. Any idea?
4 years 11 months ago #38865

Please Log in or Create an account to join the conversation.

It does not connect directly, weather devices are handled by INDI::Weather Ekos::Weather. the latter has a signal newStatus _should_ be connected to the scheduler. I know used this to work but with all the changes to the scheduler, it might have been broken, I'll look into it.

But if Hans has plans to implement a more through implementation for the weather handling then I'll just follow up with him.
4 years 11 months ago #38866

Please Log in or Create an account to join the conversation.

  • Posts: 85
  • Thank you received: 40
I found that some weather related code blobs were commented-out, or completely missing (checkWeather() no longer exists).

That will be part of this scheduler repartitioning project. Not something we could do today like hopefully re-enabling what is there to shut down an observatory on bad weather.

-- Hans
4 years 11 months ago #38874

Please Log in or Create an account to join the conversation.

Great thanks!

I just pushed a commit that fixes the scheduler shutdown due to severe weather bug. Now I tested it and it works OK. I plan to release a maintenance KStars release (v3.2.2) this weekend and I think after that we'll start merging any scheduler partitioning work for the next release.
The following user(s) said Thank You: Wolfgang Reissenberger
4 years 11 months ago #38875

Please Log in or Create an account to join the conversation.

  • Posts: 1185
  • Thank you received: 370
Hi Jasem,
looks good, my tests with the simulator went fine. But it's not so easy to understand, how it works. Maybe this is another candidate for isolation.
- Wolfgang
4 years 11 months ago #38882

Please Log in or Create an account to join the conversation.

  • Posts: 1029
  • Thank you received: 301
From what I see, we should probably separate the observatory management from the Scheduler. That new tab with a shiny obsy icon would:

- Take care of running customized scripts, eventually simplify what was requested last year in terms of ssh access to external raspberries to control electricity.
- Control dome unparking and parking, eventually mount parking too (that's at the bottom of Scheduler currently).
- Report possible manual interactions inside the observatory (for observation logging purpose, or handle a panic button).
- Manage the CCD temperature in a less weird way as now (temperature setting is in capture, warmup is in scheduler).
- Manage and consolidate reports from weather devices (safety of course, and also outside temperature having an effect on CCD temperature).
- And there could be a good place for monitoring cameras too (controlled by INDI drivers of course) and observatory security as was discussed in the past.

To me it makes sense to at least move the whole "Ekos init/shutdown", that is, dome/mount parking, out of the scheduler to a new tab, thus to a new independent module.
That module should have the same status reporting as other modules, and Scheduler should handle interruptions just like Capture handles deviations from Guide.

If someone is up to this task, we could move code out. But we should probably define "good practices" so that all modules are structured more or less the same.
The following user(s) said Thank You: Hans, Wolfgang Reissenberger
4 years 11 months ago #38884

Please Log in or Create an account to join the conversation.

  • Posts: 1029
  • Thank you received: 301
(I separate this post from the previous one to mark the digression)

We need to confirm what is the role of Scheduler: currently it is the top-level controller of automated observation.
If we continue this way, on one hand other modules must be "slave" modules, and in return Scheduler must guarantee perfect robustness (yeah tests I see you).
When not being automated, "slave" modules are simply providing their capabilities through their UI.
Having a clearer and simpler header interface that doesn't involve UI will certainly help testing (UI testing is inherently hard).

To me the design of Ekos, and of the Scheduler in particular, is really neat.
Well, it is too much flexible for its own good, but the planning, the execution and the recovery, and the exchanges with other modules, make sense.
Obviously it should be modularized even more to ease maintenance, as we see in the case of the Scheduler.

Also there's something I want to discuss later on: INDI connection, as that could be separated too. But I just edited out that (further) digressing paragraph in my post, we'll see to this later.
The following user(s) said Thank You: Derek, Hans
4 years 11 months ago #38885

Please Log in or Create an account to join the conversation.

Time to create page: 0.477 seconds