Differential D29836

Code refactoring unifying capture initialization for new sequences and continuing of sequences
AbandonedPublic
Actions

Authored by wreissenberger on May 31 2020, 12:00 PM.

Details

Reviewers
mutlaqja
chrisrowland
TallFurryMan
murveit
Summary

The capture module had two entry points for doing pre-capture checks: one when a new sequence is started and one for continuing to capture within a sequence. This creates several problems and unexpected behaviour. For example, focusing and dithering did only work for continuing a sequence.

With this diff, both starting a new sequence and continuing a sequence runs through the same check before starting to capture a new frame.

All changes are related to light frames only, capturing darks, bias and flat frames is (hopefully) untouched.

Test Plan

This change touches the core of light frames capturing and requires intense testing. The tricky thing here is that the capture module handles by far more than pure frame capturing: it interacts with guiding and focusing, it starts dithering and interaction with the meridian flip, capturing may be started standalone or from the scheduler. At least the following needs to be tested:

General setup: mount is tracking, execute all tests for multiple Lum and 1xLRGB to ensure, that both in-sequence and new sequence.

Scenario 1: standalone capturing, no meridian flip

  • simple capture sequence with filter changes
  • abort capturing
  • capture with guiding and dithering
  • capture with guiding, star lost --> capture should abort after timeout
  • capture with focusing
  • capture with focusing, no stars found during focusing --> capture should abort after timeout

Scenario 2: standalone capturing, mount crossing the meridian

  • all tests of scenario 1
  • capture with aligning
  • capture with aligning, no stars found during aligning --> capture should abort
  • abort capture during meridian flip waiting --> meridian flip should start after capture aborted
  • abort guiding during meridian flip waiting --> guiding should be restarted after meridian flip

Scenario 3: capturing from a schedule

  • all tests of scenario 1, check for restart attempts when problems occured
  • all tests of scenario 2, check for restart attempts when problems occured

Diff Detail

Repository
R321 KStars
Branch
capture_refactor_start_new_image
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27090
Build 27108: arc lint + arc unit
Restricted Application added a project: KDE Edu. · View Herald TranscriptMay 31 2020, 12:00 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
wreissenberger requested review of this revision.May 31 2020, 12:00 PM
mutlaqja added a comment.May 31 2020, 1:29 PM

Great work Wolfgang, this indeed requires quite a bit of testing. Can you submit it directly to Gitlab? https://invent.kde.org/education/kstars

wreissenberger added a comment.May 31 2020, 2:33 PM

Great work Wolfgang, this indeed requires quite a bit of testing. Can you submit it directly to Gitlab? https://invent.kde.org/education/kstars

I need a little bit of support how to create a new merge request on gitlab. I've imported my kstars clone from Github, but it does not know its upstream repository.

Any hints?

yurchor added a subscriber: yurchor.May 31 2020, 2:49 PM
In D29836#674265, @wreissenberger wrote:

Great work Wolfgang, this indeed requires quite a bit of testing. Can you submit it directly to Gitlab? https://invent.kde.org/education/kstars

I need a little bit of support how to create a new merge request on gitlab. I've imported my kstars clone from Github, but it does not know its upstream repository.

Any hints?

Generally, it can be done as follows

  1. Fork the repo on gitlab with the "Fork" button.
  2. Clone the repo.
  3. Create a remote for the upstream for the future syncing: git remote add upstream https://invent.kde.org/education/kstars.git
  4. Sync the upstream: git fetch upstream && git merge upstream/master
  5. Create the feature branch: git checkout -b feature_branch
  6. Apply patch from Phabricator: git am D29835.patch
  7. Push the branch: git push -u origin feature_branch
  8. Copy and paste the link for the merge request from console to the browser.
  9. Allow squashing, changing, and deleting the branch after merging.
  10. Should something need to be changed, it is enough just to push the changes into the branch to update the merge request.

It is also helpful to star KStars repo on gitlab (that star button on the main repo page) and watch it to review the incoming merge requests (the bell button on the main page -> Watch).

wreissenberger added a comment.May 31 2020, 2:59 PM

Many thanks for your explanation, Yuri. But if I get it right, this leads to a new clone of KStars. I just tried to import my GitHub project to Gitlab. This worked fine, but it has no linkage to its new upstream master on Gitlab. Is there a solution for this?

Otherwise my preferred way to do it is:

  1. create a new KStars clone on Gitlab
  2. create a new remote repository link to my gitlab clone
  3. push my changes to my new gitlab clone
  4. create a merge request from these changes

Does that make sense?

TallFurryMan added a comment.May 31 2020, 4:23 PM

Wolfgang, I sent you a Google Meet invite to help you out with this.

wreissenberger abandoned this revision.May 31 2020, 7:02 PM

Created a merge-request on invent.kde.org, this one is obsolete. Thanks for your support, @TallFurryMan !!

chrisrowland added a comment.May 31 2020, 9:53 PM

I'm sorry but I don't think I am qualified to review a refactor of the
capture module because I do not have enough understanding of the capture
process to be able to contribute.

I see no benefit in my nit picking about the code style.

I should also point out that I've asked Jasem to revert the meridian
flip changes I made.  It looks as if there have been too many side
effects in other parts of the system and in other mount control systems
to make the changes viable for production code.  I don't know if
reverting that will affect this.

Chris

Revision Contents
Changeset List

Diff 83185

View Options

kstars/ekos/capture/capture.h

Loading...
View Options

kstars/ekos/capture/capture.cpp

Loading...
View Options

kstars/ekos/focus/focus.cpp

Loading...
View Options

kstars/ekos/mount/mount.cpp

Loading...