Implemented Bahtinov mask focus assistant, fix compiler warnings (in fpackutil.c and modcalcvizequinox.cpp)
ClosedPublic

Authored by astrorunner on May 1 2020, 12:47 PM.

Details

Summary

Implemented Bahtinov mask focus assistant, fix compiler warnings (in fpackutil.c and modcalcvizequinox.cpp)
Added unit test for bahtinov focus assistent
Fix compiler error (comparison between int and double)
Fix compiler error after merge

  1. Conflicts:
  2. kstars/fitsviewer/fitsview.cpp
Test Plan

Tested manually by installing software on Raspberry Pi and connect it to my telescope, pointing to a bright star and check if bahtinov mask is overlaying the diffraction pattern correctly.

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.
astrorunner created this revision.May 1 2020, 12:47 PM
Restricted Application added a project: KDE Edu. · View Herald TranscriptMay 1 2020, 12:47 PM
Restricted Application added a subscriber: kde-edu. · View Herald Transcript
astrorunner requested review of this revision.May 1 2020, 12:47 PM
  • Fix build error
TallFurryMan accepted this revision.May 1 2020, 1:02 PM
TallFurryMan added a subscriber: mutlaqja.

@mutlaqja This is great and good to go.

This revision is now accepted and ready to land.May 1 2020, 1:02 PM
mutlaqja accepted this revision.May 1 2020, 1:45 PM

Looks like merge failed due to "Non-full name" error since there is "Patrick" only. Is there a quick way to fix this?

Looks like merge failed due to "Non-full name" error since there is "Patrick" only. Is there a quick way to fix this?

It could be because in Git I use 'Patrick' as name for committing, but here in Phabricator I use the name 'astrorunner'. Will it help if I change my name in Git to astrorunner?
I don't know what else it could be. I don't see the error in my Phabricator web interface. Can I see the error with an 'arc' command and maybe fix it with that?

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.

Check this informative post on github: https://help.github.com/en/github/using-git/changing-author-info.

...
Check this informative post on github: https://help.github.com/en/github/using-git/changing-author-info.

I have followed the steps from the suggested site and changed my .gitconfig.
Afterwards I updated my branch I worked on yesterday (the branch from where we created this review). I checked the git log and saw that the author was changed. Then I tried to update this review using 'arc diff --update D29325 kde/master'. That first gave a notification that it was a very large commit. When I continued I got the message that 2 files were not UTF-8 format (Tests/fitsviewer/m47_sim_stars.fits and Tests/fitsviewer/ngc4535-autofocus2.fits). When I continued I stumbled upon an error:

" Exception
Command failed with error #128!
COMMAND
git ls-tree '043526fe49fef98fee0473a981ae3d1fad7ced7d' -- ''

STDOUT
(empty)

STDERR
fatal: empty string is not a valid pathspec. please use . instead if you meant to match all paths

(Run with --trace for a full exception trace.)"

I don't know how to recover from that. Can you give me some advice on this?

TallFurryMan added a comment.EditedMay 2 2020, 8:24 AM

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.

astrorunner updated this revision to Diff 81747.May 2 2020, 5:25 PM

Updated committer and author to fix merge failure due to Non-full name error.

I have updated the changelist according to the suggestion from TallFurryMan. I hope it will work properly now.
In my quest to update this revision I accidentally created a new revision with number D29364. That can be deleted, but I could not find how to do that.
Let me know if something else needs to be done.

Been getting 0 HFR for CCD Simulator, any ideas why?

Now I'm getting this:

remote: Audit failure - Commit 5843bfcb28bddc838c9ef2893e97bf9643a27084 - Non-full name: astrorunner
remote: FATAL: VREF/kde-update-checks: helper program exit status 256
remote: Push declined - commits failed audit

Been getting 0 HFR for CCD Simulator, any ideas why?

I have tried playing with all settings, but I can't reproduce the 0 HFR for the CCD Simulator. Does the focus preview show 4 lines and 2 circles in the selection rectangle? If it shows nothing, then no lines have been detected, or the lines does not cross each other. I am not sure if that will result in a 0 HFR though.
If you can sent me a reproduction scenario and the debug logging of the focus module and fits module, then I can take a closer look to find out what goes wrong.

Now I'm getting this:

remote: Audit failure - Commit 5843bfcb28bddc838c9ef2893e97bf9643a27084 - Non-full name: astrorunner
remote: FATAL: VREF/kde-update-checks: helper program exit status 256
remote: Push declined - commits failed audit

I am sorry it keeps getting this error. A am new to working with Phabricator and Arc and probably don't have my system setup properly yet.

What are the criteria for the audit to fail on Non-full name?
Can you indicate what I should fill in for the committer and what for the author?
Should committer be the same as the username I login with in Phabircator?
And should author be my real first and last name?

Currently I have both author and committer as my Phabricator login name.

I hope I can get it right the next time. Sorry for the inconvenience.

It still doesn't work now, I think you need to commit again for the commit to have your full name :(

This sucks I know, but we're moving to Gitlab soon so hopefully this would be over soon.

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.

Le lun. 4 mai 2020 à 00:46, Jasem Mutlaq <noreply@phabricator.kde.org> a
écrit :

mutlaqja added a comment. View Revision
https://phabricator.kde.org/D29325

It still doesn't work now, I think you need to commit again for the commit
to have your full name :(

This sucks I know, but we're moving to Gitlab soon so hopefully this would
be over soon.

*REPOSITORY*
R321 KStars

*BRANCH*
new_bahtinov_attempt (branched from master)

*REVISION DETAIL*
https://phabricator.kde.org/D29325

*To: *astrorunner, KStars, TallFurryMan, mutlaqja
*Cc: *mutlaqja, TallFurryMan, kde-edu, narvaez, apol

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.

My initial mail was blocked for moderation, sorry for this weird reply:

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.

This revision was automatically updated to reflect the committed changes.

Hi Eric,

The e-mail address for my github and phabricator account are all the same. I have matched my committer and author name to match the phabricator too, but that doesn't seem to work.

Here the result of 'git log --format=full HEAD'

$ git log --format=full HEAD
commit f75ee7357e1ac3cd7b7cfbf68468d8b5b36f4513 (HEAD -> new_bahtinov_attempt)
Author: astrorunner <pr_molenaar@hotmail.com>
Commit: astrorunner <pr_molenaar@hotmail.com>

Fix compiler error (comparison between int and double)

Summary:
Implemented Bahtinov mask focus assistant, fix compiler warnings (in fpackutil.c and modcalcvizequinox.cpp)

Updated copyright information

Added unit test for bahtinov focus assistent

Fix build error

Subscribers: kde-edu

Tags: #kde_edu

Differential Revision: https://phabricator.kde.org/D29364

commit 57682009ba84871f733626a2a1e4ad96fd76fcbb
Author: astrorunner <pr_molenaar@hotmail.com>
Commit: astrorunner <pr_molenaar@hotmail.com>

Added unit test for bahtinov focus assistent

commit e60e9050eb5c15e1b58d9f8973c4448ba0c05b9e
Author: astrorunner <pr_molenaar@hotmail.com>
Commit: astrorunner <pr_molenaar@hotmail.com>

Updated copyright information

commit bb0532cab26b215219c21b7bd252ceec14bc503c
Author: astrorunner <pr_molenaar@hotmail.com>
Commit: astrorunner <pr_molenaar@hotmail.com>

Implemented Bahtinov mask focus assistant, fix compiler warnings (in fpackutil.c and modcalcvizequinox.cpp)

commit 343311c4b033e90c06453b3b49caeb50b64aced3
Author: astrorunner <pr_molenaar@hotmail.com>
Commit: astrorunner <pr_molenaar@hotmail.com>

Fix compiler error (comparison between int and double)

commit 3d738d3a6ea07d362cdb579c74052b29ab659157 (kde/master, master)
Author: Hy Murveit <hy-2@murveit.com>
Commit: Jasem Mutlaq <mutlaqja@ikarustech.com>

Comment out a couple wordy debug log messages

Test Plan: Check that no code changed, outside of 2 fewer log lines.

Reviewers: mutlaqja, chrisrowland

Reviewed By: mutlaqja

Subscribers: kde-edu

Tags: #kde_edu

Differential Revision: https://phabricator.kde.org/D29317

I see in the logging that I should use my full name as committer and author. I will change the git log (as described in an earlier e-mail) and then commit it again. Hopefully that will solve the issue.

Kind regards,

Patrick


Van: Eric Dejouhanet <noreply@phabricator.kde.org>
Verzonden: maandag 4 mei 2020 10:59
Aan: pr_molenaar@hotmail.com <pr_molenaar@hotmail.com>; jarno.paananen@gmail.com <jarno.paananen@gmail.com>; luigi.toscano@tiscali.it <luigi.toscano@tiscali.it>; rlancaste@gmail.com <rlancaste@gmail.com>; akarsh.simha@kdemail.net <akarsh.simha@kdemail.net>; valentin@boettcher.cf <valentin@boettcher.cf>; mutlaqja@ikarustech.com <mutlaqja@ikarustech.com>; prasun.code@gmail.com <prasun.code@gmail.com>; zyziuk@gmail.com <zyziuk@gmail.com>; alex@navigheaza.ro <alex@navigheaza.ro>; steffen_moeller@gmx.de <steffen_moeller@gmx.de>
CC: kde-edu@kde.org <kde-edu@kde.org>; david.narvaez@computer.org <david.narvaez@computer.org>; aleixpol@kde.org <aleixpol@kde.org>
Onderwerp: D29325: Implemented Bahtinov mask focus assistant, fix compiler warnings (in fpackutil.c and modcalcvizequinox.cpp)

View Revisionhttps://phabricator.kde.org/D29325
TallFurryMan added a comment.

My initial mail was blocked for moderation, sorry for this weird reply:

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.

REPOSITORY
R321 KStars

BRANCH
new_bahtinov_attempt (branched from master)

REVISION DETAIL
https://phabricator.kde.org/D29325

To: astrorunner, KStars, TallFurryMan, mutlaqja
Cc: lancaster, astrorunner, mutlaqja, TallFurryMan, kde-edu, narvaez, apol

No need to do anything now, I merged it in master now after editing the git commit.

No need to do anything now, I merged it in master now after editing the git commit.

Thanks a lot. If you need anything more, just let me know.