Refactoring the Focus module.
ClosedPublic

Authored by TallFurryMan on Mar 25 2020, 11:08 PM.

Details

Summary

This differential relates to tests and code structure for the Focus module.
Tests show that:

  • There is no way to determine when the star detection procedure is done. The capture button should re-enable when everything is finished.
  • The CCD Simulator needs improvement for some detection mechanisms to work properly.
  • It is difficult to create fixtures with deterministic verifications, although better controlling the simulation time should help.
  • Syncing the mount to object coordinates is a good idea to speed up tests.
  • When not tracking, the CCD Simulator doesn't render trails, it could be a nice improvement.
  • Because kstars_ui_tests is becoming larger, it needs to be divided into multiple smaller tests for efficiency.
  • QTest, because failing a test means returning from the test function, is macro hell.
  • QComboBox is difficult to control programmatically because of its internal signal management.

We then refactor star detection for clarity and consistency in Focus and attempt to reduce side-effects in FITSData:

  • Make FITSData buffer read-only, except for DarkLibrary.
  • Move Threshold, Gradient and Centroid detections out of FITSData.
  • Move SEP detection out of FITSData.
  • Docs, copyrights and adjustments.
  • Removing expected failure on Gradient/Threshold.
  • Adjust KTRY_EKOS_CLICK to use KTRY_EKOS_GADGET.
  • Extract star search block to simplify setCaptureComplete.
  • Reset KStars time after starting Ekos.
  • Fix object fixture builder.
  • Slight delay to overcome the weird toggling of the capture button.
  • Separate/simplify annulus and threshold tests.
  • Delay target chip retrieval when capture is complete.
  • Test frame average, extract code averaging HFRs.
Test Plan

Run kstars_ui_tests.

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.Mar 25 2020, 11:08 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptMar 25 2020, 11:08 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
TallFurryMan requested review of this revision.Mar 25 2020, 11:08 PM
TallFurryMan planned changes to this revision.Mar 25 2020, 11:08 PM

Spotted KTRY_EKOS_CLICK that I forgot to refactor in the same vein as KTRY_FOCUS_CLICK.

TallFurryMan updated this revision to Diff 78929.EditedMar 30 2020, 8:47 PM

These additional changes build up on tests, and move detection methods out of the Focus class.

  • Make FITSData buffer read-only, except for DarkLibrary.
  • Move Threshold, Gradient and Centroid detections out of FITSData.
  • Move SEP detection out of FITSData.
  • Docs, copyrights and adjustments.
  • Removing expected failure on Gradient/Threshold.
  • Adjust KTRY_EKOS_CLICK to use KTRY_EKOS_GADGET.
  • Extract star search block to simplify setCaptureComplete.
TallFurryMan planned changes to this revision.Mar 30 2020, 8:48 PM

Still a WIP, but progressing fairly well I think.

TallFurryMan retitled this revision from Added tests for the Focus module. to Refactoring the Focus module..Mar 30 2020, 8:51 PM
TallFurryMan edited the summary of this revision. (Show Details)

That's a much needed update!! Focus module needs to be broken down even further to manageable pieces. All these huge functions with ifs need to be broken down so that they can be tested.

  • Reset KStars time after starting Ekos.
  • Fix object fixture builder.
  • Slight delay to overcome the weird toggling of the capture button.
  • Separate/simplify annulus and threshold tests.
  • Delay target chip retrieval when capture is complete.
  • Test frame average, extract code averaging HFRs, fix HFR unexpected reset.
TallFurryMan planned changes to this revision.Apr 5 2020, 11:20 AM

Now the headache work: merging master changes in.

Not that difficult in the end.

  • Merge branch 'master' into improve__concurrent_star_detector

@murveit the SEP code has been moved to fitsdatasepdetector.cpp.
If we merge your changes before mine, I can take care of merging.
If we merge my changes before yours, you will need to move your changes there.
I'll be creating a test for the detection code purely in another differential.
I need to verify the BZ bug about INDI disconnection hanging Ekos first.

Also I should rewrite a bit the differential log, not exact.

TallFurryMan edited the summary of this revision. (Show Details)Apr 5 2020, 12:39 PM
TallFurryMan edited the summary of this revision. (Show Details)
  • Merged verification of BZ398192 in.

Verification of BZ398192 is done with this simple test in test_ekos_simulator.cpp:

void TestEkosSimulator::testConnectDisconnect()
{
    // Tests BZ398192
    QBENCHMARK {
        KTRY_EKOS_STOP_SIMULATORS();
        KTRY_EKOS_START_SIMULATORS();
    }
}

The test init/cleanup is starting the Simulators profile. So this test does a benchmark on stopping/starting. I'll probably change this to a loop, because QBENCHMARK thinks one iteration is enough.

  • Reworked merged verification of BZ398192.

@murveit I added differential D28635 with a very simple test class for the FITS viewer. I simply picked a branch that I had created while waiting for more formalism to transpire from https://indilib.org/forum/wish-list/6591-collimation-circles.html, hence the name.

mutlaqja accepted this revision.Apr 8 2020, 5:44 AM

This is good to go with limited testing.

This revision is now accepted and ready to land.Apr 8 2020, 5:44 AM
This revision was automatically updated to reflect the committed changes.

Hey that's cheating, it's a build that began 3 days ago :) Joking, thanks for reporting @bcooksley, test_ekos_focus should not be built when there is no INDI installed.

15:12:30  -- The following OPTIONAL packages have not been found:
15:12:30  
15:12:30   * INDI (required version >= 1.7.1), Astronomical instrumentation control, <https://www.indilib.org>
15:12:30     Support for controlling astronomical devices on Linux with KStars.
15:12:30   * WCSLIB, World Coordinate System library, <https://www.atnf.csiro.au/people/mcalabre/WCS>
15:12:30     WCS enables KStars to read and process world coordinate systems in FITS header.

I'll issue a fix asap.

Any news on a fix for this?

Should be fixed under the hour, I'm currently working on the diff.

@bcooksley The fix for the non-INDI build is in D28890.