Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add abort telescope slew signals to LX200 and NexStar telescopes #2754

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

gzotti
Copy link
Member

@gzotti gzotti commented Oct 17, 2022

Description

I found that most telescopes are missing the AbortSlew implementation. This is severely bad. This PR should add the signals at least for LX200 and NexStar.

However, I cannot test it, so this needs external help. I unassigned myself for now in the hope one of our active observers steps in. It may just take a rainy weekend or two. Of course, support for a Big Red Button (emergency switch, separate hardware) would also be nice!

Fixes # (issue)

  • Add AbortSlew to LX200
  • Add AbortSlew to NexStar
  • Add graceful recovery (put back marker) to LX200
  • Add graceful recovery (put back marker) to NexStar

Screenshots (if appropriate):

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update

How Has This Been Tested?

Test Configuration:

  • Operating system: Windows 10 or any other
  • Graphics Card: irrelevant
  • Tested AbortSlew for LX200
  • Tested AbortSlew for NexStar

Checklist:

  • My code follows the code style of this project.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (header file)
  • I have updated the respective chapter in the Stellarium User Guide
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@gzotti gzotti added enhancement Improve existing functionality help wanted We may not have the hardware or expertise importance: medium A bit annoying, minor miscalculation, but no crash labels Oct 17, 2022
@gzotti gzotti added this to Needs triage in Plugin: Telescope Control via automation Oct 17, 2022
@github-actions github-actions bot requested a review from alex-w October 17, 2022 13:37
@gzotti gzotti force-pushed the fix/abortTelescopeSlew_LX200_NexStar branch from 9ecf035 to addc637 Compare October 17, 2022 13:38
@alex-w
Copy link
Member

alex-w commented Oct 17, 2022

You should read the target coordinates from the mount after stopping slewing and place marker in these coordinates

@gzotti gzotti removed their assignment Oct 17, 2022
@gzotti
Copy link
Member Author

gzotti commented Mar 5, 2023

You should read the target coordinates from the mount after stopping slewing and place marker in these coordinates

If I knew how this positioning interpolation stuff works I could maybe do that. But while I can estimate that I should test with hasKnownPosition() and can likely retrieve pos with getJ2000EquatorialPos(), I know zilch how to set that. How do the InterpolatedPositions work? Is that a ring buffer? What is the purpose of Position.{server_micros, client_micros,status}? Dev docs are so annoyingly often missing or useless in this plugin! IDC where some struct had been defined 15 years ago and "now" moved over. I wish somebody could reverse-engineer a design document for this plugin!

@alex-w
Copy link
Member

alex-w commented Mar 5, 2023

InterpolatedPositions is own implementation of interpolation and this is does not touches NexStar/LX200 implementation - it's similar, not equal. LX200/NexStar protocols have methods for getting current coordinates from the mount and this parts of protocols are not implemented.

@alex-w
Copy link
Member

alex-w commented Mar 5, 2023

The Telescope Control plugin should be fully redesigned and rewritten and it should support asynchronous for connection to the devices. I full agree - we need experts for it.

@gzotti
Copy link
Member Author

gzotti commented Mar 5, 2023

I see that the TelescopeClientDirectLx200 has an interpolatedPosition that does something. But I don't dare touching this without fitting device and high enough personal interest in this plugin. Anyone out there?

@gzotti gzotti force-pushed the fix/abortTelescopeSlew_LX200_NexStar branch from 76e8028 to 14fd6a2 Compare July 4, 2023 14:34
@A-j-K
Copy link
Contributor

A-j-K commented Jul 4, 2023

The Telescope Control plugin should be fully redesigned and rewritten and it should support asynchronous for connection to the devices. I full agree - we need experts for it.

It's not async by design? Oh! Hmm, if I'm diving into this plugin then, as others said, it may need a rethink. And then I'd need to find folk with hardware who can test. I have a SkyWatcher mount but that's it, not the others.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improve existing functionality help wanted We may not have the hardware or expertise importance: medium A bit annoying, minor miscalculation, but no crash
Projects
Plugin: Telescope Control
  
Needs triage
Development

Successfully merging this pull request may close these issues.

None yet

3 participants