- User Since
- Apr 13 2018, 9:37 PM (116 w, 1 d)
May 31 2020
Wolfgang, I sent you a Google Meet invite to help you out with this.
May 20 2020
Excellent, thanks for this proposal! I have nothing but minor points, looks good to me.
May 11 2020
This is excellent. Sorry to arrive late to the party, two comments from me, one about the test mode of QtTest, and the other about build failures with versions older than Qt 5.9.
May 7 2020
Actually, the fix for sigpipe would potentially be in INDI's baseclient.cpp...
@mutlaqja I debugged the last connect/disconnect issue of BaseClient to be a sigpipe exception. I'll file the fix with this differential.
May 4 2020
This differential is not finished yet. @mutlaqja please use it to test your changes with regards to INDI property usage.
My initial mail was blocked for moderation, sorry for this weird reply:
Ah there will be a problem with your email being disclosed in the thread, so don't paste the commit log, sorry.
Tell us if your email matches your profile, in terms of committer or author.
Patrick, could you share the result of "git log HEAD" (without the patch
part, only the header so we see the committer and the author) ? My KDE user
is "TallFurryMan" but my profile name there is "Eric Dejouhanet". More
importantly I think, my email is the same in my commits and in my profile.
May 3 2020
Updated UI tests to properly set kstarsui.rc.
Thanks for this, I hadn't returned to it yet.
May 2 2020
Well, we could redo this from the beginning.
You locate the commits that contain your changes and fixes (not the merges, let's leave those). You write down the commit identifiers.
You check kde/master out, and you derive a new branch from that with git checkout -tb new_bahtinov_attempt (-b will fork, -t will track).
You then sequentially apply your changes with git cherry-pick <commit-id>, resolving conflicts as they appear.
Then you push your changes with arc diff --update D29325 kde/master.
May 1 2020
Yes you need the same committer name (not necessarily author name) in your tree and in phab.
You may change your user name by editing ~/.gitconfig.
@mutlaqja This is great and good to go.
Apr 25 2020
Apr 24 2020
One small answer on the concurrent use of m_Status. Just for theory.
Apr 23 2020
Also, the summary will appear as commit in the kstars history, so you may want to rewrite it.
I'm sorry if I sounded harsh in my comments. The last thing I want is a flamewar between C# and C++ ;)
Apr 22 2020
This is complete and ready to review.
- Add a longer exposure test, add a delay based on required exposure frames, check stop button immediately.
- Remove remaining reference to BZ398192, adjust timeouts.
- Merge branch 'master' into improve__determine_star_detection_end
Apr 21 2020
A few (pedantic) comments from me. Overall, this seems nice although I wasn't able to test. I am under the impression that something is missing for the new options, but I may be wrong.
The most annoying point for me is the broken test after stopping to track on HA limit. And also the system time instead of the simulation time, which will make things difficult to test.
Apr 17 2020
Apr 16 2020
I will certainly not block a differential on an issue with source centers which we do not use afterwards :) Sorry for forgetting the "Accept Revision" flag.
- Merge branch 'master' into improve__userdb_tests
The Focus test is still quite long because there is currently no way to determine the star detection is finished.
And actually KStars sometimes crashes when a new capture is taken while the detection is still in progress. Another test to be built!
Should be fixed under the hour, I'm currently working on the diff.
But when I look closely at your pictures, it seems the circles are offset to the right and bottom in both cases. I'm not referring to the width, I'm really talking about the center. Note that the pencil I was using had a width of 2, so it might be the reason? A pencil graphics code drawing stuff with all lines widths extended only to the right and bottom? That's entirely possible (and quite hateful).
About my concerns on rendering red circles, my point was that when you use a center and a radius to draw an ellipse, the center is set to a pixel. Therefore you risk to offset your ellipses when you should be using a subpixel as center. Agreed, this is only visible on very small stars, but it was annoying for me at the time, and made me incorrectly think this could be the reason my focus wasn't good or my solver was failing (newbie thoughts of course, but very real).
Apr 12 2020
Also would it be possible to report on the performance improvement with numbers? I'm curious!
That's excellent! I have several points in the comments. I would also like you to add more FITS fixtures captured with the CCD Simulator or real pictures, with various FWHM values (use the focuser simulator) to verify your improvements.
Apr 10 2020
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.
Apr 8 2020
A few things I wanted to add but will delay:
- UI does not reflect the use of SEP and Centroid for initial detection, nor for full-field analysis.
- Due to the first point, the tests are not properly verifying the Threshold and Gradient algorithms because the full-field analysis option is set!
- UI does a weird toggle on the capture button when preparing the screen, and re-enables said button *before* the star extraction completes (that code block is tricky to refactor).
- Due to the first point, the tests need to wait an arbitrary duration to verify the detection results.
Apr 6 2020
@murveit this should help you chase regressions. There's no obvious possibility to test the result of QBENCHMARK, but you just have to write the original duration down and evaluate your optimizations.
@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.
- Reworked merged verification of BZ398192.
Apr 5 2020
Verification of BZ398192 is done with this simple test in test_ekos_simulator.cpp:
- Merged verification of BZ398192 in.
Also I should rewrite a bit the differential log, not exact.
@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.
Not that difficult in the end.
Now the headache work: merging master changes in.
- 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.
I'd like to see a unitary test for this. A simple thing that loads a star cluster fits generated with the CCD simulator, and runs a star detection using SEP. My differential uses UI tests, which are an entirely different method, possibly maybe useful for changes spanning multiple modules.
Apr 3 2020
I'm OK, except for the change from Centroid to SEP. SEP requires more cpu resources than Centroid, so should maybe not be the default. My position is arguable, of course. I agree there are not that many stars detected by the Centroid algorithm in the general real-life situation, so maybe we need to fix Centroid instead?
Mar 30 2020
Still a WIP, but progressing fairly well I think.
These additional changes build up on tests, and move detection methods out of the Focus class.
Mar 25 2020
Spotted KTRY_EKOS_CLICK that I forgot to refactor in the same vein as KTRY_FOCUS_CLICK.
Mar 21 2020
Mar 18 2020
Mar 4 2020
Mar 2 2020
So, compatible with Extragear build as of now. As far as I understand, additional (empty) tests will be added to the test step automatically...?
- Revert to multiple test executables, fix copyrights.
Looking at the current Extragear build for SuSE, I see that:
Next I'll be looking into adding those tests to the build.
- Fix missing copyright.
Feb 29 2020
Yes, as it now, it's ok to me.
Feb 27 2020
Updated. @yurchor there were apparently no issues with newlines at the end of files.
- Fix copyrights.
Feb 26 2020
- Fix formatting.
I'm writing that for kstars classes, with empty test plans. I'll see what I can do for UI tests, although that's less easy as the source of a UI test is a use case.
Please don't test this on your own setup, because it may change your userdb (it doesn't right now, but will certainly).
I need a way to use another database, so that options are all default.
Feb 23 2020
Feb 20 2020
Feb 15 2020
- Fix rogue include.
Feb 14 2020
- Update for rebase.
Jan 30 2020
In this diff, quite a few blocks could be refactored: finding gadgets, clicking things, managing dialogs...
Currently, the test runs as fast as it can by using timeouts on states instead of sleeping (except for QTimer::singleShot calls), so when tests all pass, the duration is ~5 seconds on my laptop (most of which is KStars loading).
Fixtures should be integrated in order to test many more situations with a single action flow.
Also, I did not investigate the lack of menu and toolbar in KStars.
The way the project was implemented originally is to test the end-user side of manipulations, using UI as much as possible.
Effectively, this is final integration test which could probably be difficult to maintain and automate.
Running tests with a full stack, from star map down to drivers, could also have a long duration, making it less suitable to automation (imagine scheduler tests over three days).
Jan 29 2020
Jan 8 2020
I cannot test this, so I offer pedantic comments ;)
Dec 9 2019
(I originally posted this comment in the wrong diff, sorry)
Dec 4 2019
EDIT: wrong ticket......
Nov 29 2019
I think you fixed that long ago. I kept it to reinsert logs.
Sorry I'm overbooked, I can't return to this yet.
Sorry I'm overbooked, I can't return to this yet.
Nov 2 2019
Oct 3 2019
Oh I totally forgot about this one. Actually, it's interesting in the sense it can be used to ensure a mean ADU over the frame. That is, calculate the ideal exposure duration. But there are things to fix, as seen in the comments. I keep this in my list and I'll have a look just because it's cool.