Differential D24257

PHD2 Enhancements
ClosedPublic
Actions

Authored by lancaster on Sep 27 2019, 4:08 AM.

Details

Reviewers
mutlaqja
alexcherney
TallFurryMan
wreissenberger
Commits
R321:728a55f6e227: PHD2 Enhancements
Summary

This update is intended to fix several PHD2 problems and provide several PHD2 enhancements.

  1. Fixes an issue where if the user hits the stop button while guiding in the PHD2 interface, then the user restarts guiding from PHD2, Ekos did not pick up the guide state change since no settling occurred and the user didn't click guide in Ekos.
  2. Fixes an issue where the guiding data was not put into the graphs if the user did not enter a focal length into PHD2. Formerly the solution was to put a warning in the Guide log, but users ignored that. Now this tries to set the currentCCD, so the backup method can work the way it was intended to work.

Note-This still won't work if it is a PHD2-only camera and the user doesn't enter a focal length, so the warning still prints.

  1. Fixes an issue when the user had external guide frames selected, they did not appear in the guide view, and instead loaded in the fits viewer.
  2. Enables the loop and capture buttons for PHD2 along with the corresponding functions to make them work. Works well if the camera is connected to Ekos, also works if its not, but it will not display in Ekos. A message prints to the log letting the user know why.
  3. Disables the binning combo box which was enabled accidentally.
  4. If the user has external guide frames enabled, this enables the tracking box so that the lock position in the image can be reported to the user, and it also enables the user to be able to click to change the lock position for PHD2.
  5. This improves Ekos's ability to recognize the guide camera by using PHD2's report of what guide camera it is using. Then Ekos can use this information to disable or enable the receipt of image frames as well as the option to use the SubFrame checkbox.
  6. Improves the communication about the various options and status for cameras connected to PHD2 and Ekos.
  7. Enables the SubFrame method for PHD2 cameras so that they can switch quickly back and forth between the Guide Star image and full frame external guide frames at the push of a button.
  8. After I cleaned up some of the old and no longer needed methods/code, Ekos now connects to PHD2 much faster.
  9. I added the option for the user to select autostar, or to have PHD2 use the star lock position chosen by the user.
  10. Removes the External Guide Frames option since this makes it obsolete
  11. Saves the subframe option between KStars sessions and sets Ekos to receive External Guide frames or not based on it.
  12. Removes all the setting and unsetting of external blobs for Linguider, which doesn't support INDI cameras anyway, so it was pointless.
Test Plan
  1. Try having external guide frames enabled and guide with PHD2
  2. Try having external guide frames disabled and guide with PHD2
  3. Try starting and stopping the guiding from PHD2 instead of Ekos
  4. Try not entering a focal length into PHD2 so it records the error in pixels
  5. Try using the tracking box to change the lock position during or before guiding with PHD2 using Ekos

Note: A known issue: PHD2 doesn't always decide to use your lock position when selected before you hit guide. To get around this, you can also change the star after it starts guiding.

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.
lancaster requested review of this revision.Sep 27 2019, 4:08 AM
lancaster created this revision.
lancaster retitled this revision from Fixing an RA/DEC swap issue to PHD2 Enhancements.Sep 27 2019, 4:32 AM
lancaster edited the summary of this revision. (Show Details)
lancaster edited the test plan for this revision. (Show Details)
lancaster added reviewers: mutlaqja, alexcherney, TallFurryMan.
mutlaqja added a subscriber: wreissenberger.Sep 27 2019, 5:21 AM

Awesome work Robert! I hope this can fix any remaining regressions with PHD2.

@wreissenberger Any comments as well?

TallFurryMan added a comment.Sep 27 2019, 5:45 AM

Thanks a million for this Robert. I'm immediately feeding this diff to the observatory build chain for the next clear night.

TallFurryMan added a comment.Sep 27 2019, 6:01 AM

I have a small request : when phd2 is not started and Ekos starts, Ekos will keep the guider module disconnected as expected. But when Ekos requests the guider module to start guiding, I think no attempt to connect again is made. Could this be added in? This would improve robustness. Well that can be another diff of course.

kstars/ekos/guide/guide.cpp
552

Switch case might have been appropriate now that all cases are distinct.

627

As a side note, this will be the case in my obs, so that branch will tested in my environment. I think the option "receive external guide frames" is confusing and should be renamed (no action from your part, warning is OK).

hansl added a subscriber: hansl.Sep 27 2019, 10:24 AM

These changes look awesome and promise to fix a few ekos-phd2 things i've been struggling with !

lancaster added a comment.Sep 27 2019, 12:28 PM

One of the major fixes in this diff, the case when PHD2 starts guiding remotely and it wasn't changing the state to guiding in Ekos, used to work perfectly about a year ago after my first big round of PHD2 changes. I'm not sure exactly when it broke, but somebody must have made a change (me or somebody else) in the meantime that broke it.

lancaster added a comment.Sep 27 2019, 12:30 PM

But whoever made the change that made Ekos not change the state to guiding, should check to make sure that my change to make that work again doesn't break whatever that person was trying to fix. . .unfortunately I don't know who changed it, when it changed, or why it was changed. . .

lancaster added a comment.Sep 27 2019, 12:39 PM

Adding comments

kstars/ekos/guide/guide.cpp
552

Well my hope is to get rid of the if statement that now only happens for Linguider entirely. But I don't have Linguider to try to make the same changes that I did for PHD2. Nor do I have any experience with it.

627

So I partially agree. If you have seen my comments before, I have definitely advocated using PHD2-only connected guiders rather than using INDI connected guiders that are also connected to Ekos. In fact this is what I have been doing on my system the last couple of years.

I would like to see this option of switching back and forth between receiving the full guide frames and the profile image become an option similar to and as easy to use as the "use subframe" checkboxes in Ekos. This is like my first step on that road. It still requires reconnecting or restarting. I haven't yet figured out how to make it switch perfectly. The issue is that the guide camera getting blobs enabled and disabled doesn't always seem to work except for the first time I call it. So a reconnection or restart is needed? it needs further analysis. But for now, the message just says restart Ekos and that works.

TallFurryMan added inline comments.Sep 27 2019, 1:27 PM
kstars/ekos/guide/guide.cpp
627

(What I meant was that my obs has two indiservers indeed, one for the guider ccd, one for Ekos. But configuring phd2 to capture from one and send corrections to the other is tricky.)

wreissenberger added a comment.Sep 27 2019, 6:40 PM

Looks good! There is one warning that seems to have a problem:
PHD2's current camera: Simulator(I18N_ARGUMENT_MISSING), is NOT connected to Ekos. It does not need to be connected, the PHD2 Guide Star Image should still be received.

But I'm not through with testing.

lancaster updated this revision to Diff 66967.Sep 27 2019, 8:07 PM

Fixing i18n

lancaster added inline comments.Sep 27 2019, 8:08 PM
kstars/ekos/guide/guide.cpp
627

Do you mean one INDI Server for the Mount and one for the CCD?

I'm not sure your configuration would cause any issues. With my new code, the only thing Ekos cares about for this option is what PHD2 is using for the Camera, not for the mount or aux mount. And basically if Ekos has a connection to the same camera, then it should get the name from PHD2, identify that they are the same camera and treat that as the "guide" camera. Then it should receive the full guide frames if the user wants that, or the guide star image instead. If the same camera isn't connected to Ekos, then all you can do is get the guide star image.

wreissenberger added a comment.Sep 27 2019, 11:24 PM

One behavior that seems to be new: if PHD2 is not connected to EKOS and PHD2 is looping, the images of the guiding camera are displayed in the FITS viewer. Is there a reason behind or is it simply a bug?

wreissenberger added a comment.Sep 27 2019, 11:30 PM

Fixing i18n

Great, thanks! But I now do not understand why this happens:
2019-09-28T01:24:39 PHD2's current camera: INDI Camera [Guide Simulator], is NOT connected to Ekos. It does not need to be connected, the PHD2 Guide Star Image should still be received.
This message always comes when I press "Connect" in the EKOS guiding module.

wreissenberger added inline comments.Sep 27 2019, 11:55 PM
kstars/ekos/guide/guide.cpp
618

At least when using the INDI Guiding simulator, ccd->getDeviceName() returns "Guide Simulator", and phd2Guider->getCurrentCamera() returns "INDI Camera [Guide Simulator]". As a result, this loop does not find the matching CCD.

lancaster added a comment.Sep 28 2019, 1:43 AM
In D24257#538989, @wreissenberger wrote:

One behavior that seems to be new: if PHD2 is not connected to EKOS and PHD2 is looping, the images of the guiding camera are displayed in the FITS viewer. Is there a reason behind or is it simply a bug?

So part of my goal is to identify the actual camera that is connected to Ekos so that we can enable/disable that specific camera when PHD2 gets connected. If the camera is connected to Ekos and PHD has not connected and told it not to receive guide frames, then it is basically a camera that is connected to Ekos and the frames should be received. We don't really want to be disabling cameras that are connected to Ekos unless they are actually being used in PHD2 as a guide camera I would think? The images are received and displayed in fits viewer because PHD2 hasn't said that that is the guide camera yet.

Another part of my goal is that we should be able to enable and disable the guide camera's remote images receipt in Ekos at will. The old method discovers cameras when Ekos starts up and so at that point PHD2 is not connected. I would like to be able to check and uncheck a box to enable remote images when I want to, so that I can get the full image, select a guide star, setup guiding, and then disable them so that I just get the guide star image from PHD2. It's not quite there yet, but this is the first step.

lancaster added inline comments.Sep 28 2019, 1:44 AM
kstars/ekos/guide/guide.cpp
618

Odd, for me, both returned "Guide Simulator". Well I could use the contains method to see if the PHD2 String contains the string from the CCD device name.

lancaster updated this revision to Diff 66977.Sep 28 2019, 1:47 AM

Changing to contains method

lancaster added inline comments.Sep 28 2019, 1:48 AM
kstars/ekos/guide/guide.cpp
618

Try it again now with the change I just made.

wreissenberger accepted this revision.Sep 28 2019, 10:06 AM

Changing to contains method

Fine, that solved the problem. I am using the latest PHD2 version 2.6.6dev2. Maybe they changed something.

From my side, everything works fine - accepted.

This revision is now accepted and ready to land.Sep 28 2019, 10:06 AM
wreissenberger added a comment.Sep 28 2019, 10:09 AM

We don't really want to be disabling cameras that are connected to Ekos unless they are actually being used in PHD2 as a guide camera I would think? The images are received and displayed in fits viewer because PHD2 hasn't said that that is the guide camera yet.

Personally, I do not like it when the FITS viewer pops up with the image from the guiding camera - no matter in what state Ekos and PHD2 are. I am not fully aware of all the consequences what happens if it is turned off.

But since I know now what do do, I can live with it.

TallFurryMan accepted this revision.Sep 28 2019, 1:14 PM

Not tested yet due to weather. But changes seem OK to me from code review.

lancaster updated this revision to Diff 67012.Sep 29 2019, 4:13 AM
Fixed the external guide frame enable/disable issue that was requiring restart.
lancaster added a comment.Sep 29 2019, 4:17 AM

So I figured out why the restart was required (at least for my code) when switching between full external guide frames and the guide star image from PHD2. It was a matter of two methods with very similar names but different parameters. So I had given it the wrong parameter. I think it works now. I'm pretty sure we can now make a button to click back and forth between full external guide frames and the guide star image. It would be just like going to a subframe, except it isn't really. Thoughts?

lancaster updated this revision to Diff 67048.Sep 30 2019, 3:58 AM

Fixing Linguider issues, Making SubFrame Button work, cleaning up methods, and fixing disabling external guider when not connected.

lancaster added a comment.Sep 30 2019, 4:01 AM

"Personally, I do not like it when the FITS viewer pops up with the image from the guiding camera - no matter in what state Ekos and PHD2 are. I am not fully aware of all the consequences what happens if it is turned off."

I think I just fixed this. The method that was being used to disable PHD2 and Linguider before can be used to just turn off PHD2 cameras, until the camera is properly configured by my method. So I think this meets what you were looking for. Please check to see if it works better.

lancaster added a comment.Sep 30 2019, 4:02 AM

I also successfully managed to get my idea of SubFrame implemented. It seems to work perfectly now, and lets you click to subframe to the Guide Star Image when you like, and go back too.

lancaster marked 9 inline comments as done.Sep 30 2019, 4:07 AM

Please test again to see if I fixed all the issues. I also reduced the message spam from the configure PHD2 camera method.

kstars/ekos/guide/guide.cpp
552

I went back to the old if structure here because it makes sense to disable PHD2 cameras using the old method until they get recognized by my configure PHD2 camera method and get enabled by the user

lancaster edited the summary of this revision. (Show Details)Sep 30 2019, 4:10 AM
lancaster marked 2 inline comments as done.
lancaster edited the summary of this revision. (Show Details)Sep 30 2019, 4:17 AM
lancaster updated this revision to Diff 67050.Sep 30 2019, 6:21 AM
I found a way to enforce the user's star lock position when guiding is started.
lancaster edited the summary of this revision. (Show Details)Sep 30 2019, 6:22 AM
mutlaqja requested changes to this revision.Sep 30 2019, 7:56 AM

Ok I quickly tested with PHD2. Subframe and Autostar buttons were checked. Slewed to a star and clicked guide.

  1. PHD2 calibration failed. I saw the guide frame displayed in KStars. However PHD2 window was over-saturated for some reason:

  1. When I click on "Capture", KStars crashes with this: https://pastebin.com/0LP5R2LU
  2. In options, it says "receive external guide frames (linguider)". However linguider does not support INDI cameras. Also, I'm not sure how we can disable receiving frames _completely_ from PHD2 now? If it is the "subframe" checkbox then it is not clear.
This revision now requires changes to proceed.Sep 30 2019, 7:56 AM
wreissenberger requested changes to this revision.Sep 30 2019, 1:31 PM

With "receive external guide frames" selected:

  1. pushing the "Capture" button in the Guide module creates an assertion failure in PHD2::captureSingleFrame().
  2. pushing the "Loop" button shows the warning "PHD2 Error: could not start looping", but PHD2 starts looping.
kstars/ekos/guide/externalguide/phd2.cpp
70

Has to be defined here as methodResults[] member, otherwise sendPHD2Request() has an assertion failure when called from PHD2::captureSingleFrame().

lancaster updated this revision to Diff 67081.Sep 30 2019, 6:43 PM
lancaster edited the summary of this revision. (Show Details)

Removing option for external guide frames, fixing missing method, saving subFrame setting for PHD2, improving logic.

lancaster edited the summary of this revision. (Show Details)Sep 30 2019, 6:45 PM
lancaster edited the summary of this revision. (Show Details)
lancaster marked an inline comment as done.Sep 30 2019, 6:47 PM
lancaster edited the summary of this revision. (Show Details)
lancaster added a comment.Sep 30 2019, 10:50 PM

Ok I think I resolved all those issues. It seems to be working better now. Please test again.

lancaster updated this revision to Diff 67093.Sep 30 2019, 11:22 PM
Two more checks that PHD2 is configured just to make sure.
lancaster updated this revision to Diff 67095.Oct 1 2019, 12:54 AM
Disconnecting currentCCD if the user switches to another camera.
mutlaqja accepted this revision.Oct 1 2019, 4:44 AM
mutlaqja added a comment.Oct 1 2019, 3:43 PM

@wreissenberger I checked and all the issues are resolved now. Do you have any further concerns?

This revision was not accepted when it landed; it landed in state Needs Review.Oct 2 2019, 7:21 AM
Closed by commit R321:728a55f6e227: PHD2 Enhancements (authored by lancaster, committed by mutlaqja). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 67164

View Options

kstars/ekos/guide/externalguide/phd2.h

Loading...
View Options

kstars/ekos/guide/externalguide/phd2.cpp

Loading...
View Options

kstars/ekos/guide/guide.h

Loading...
View Options

kstars/ekos/guide/guide.cpp

Loading...
View Options

kstars/ekos/guide/guide.ui

Loading...
View Options

kstars/ekos/guide/guideinterface.h

Loading...
View Options

kstars/ekos/guide/opsguide.cpp

Loading...
View Options

kstars/ekos/guide/opsguide.ui

Loading...
View Options

kstars/kstars.kcfg

Loading...