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

Merged
merged 7 commits into from
Sep 7, 2024

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
@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 c853b9b to 76e8028 Compare March 5, 2023 17:03
@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.

This comment was marked as resolved.

@github-actions github-actions bot added the has conflicts The pull request has conflicts label Sep 5, 2024
@gzotti gzotti force-pushed the fix/abortTelescopeSlew_LX200_NexStar branch from 14fd6a2 to 6a9809b Compare September 5, 2024 22:58

This comment was marked as resolved.

@github-actions github-actions bot removed the has conflicts The pull request has conflicts label Sep 5, 2024
@gzotti
Copy link
Member Author

gzotti commented Sep 5, 2024

Almost two years, and almost no response. Does nobody have an LX200 or NexStar telescope to test this? Or is this feature of no interest after all?

- free deleted pointees
- questions to original dev...
@gzotti
Copy link
Member Author

gzotti commented Sep 7, 2024

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

@alex-w It seems there is regular communication from LX200/NexStar to retrieve instrument position in update().

This plugin is so amazingly entangled, so underdocumented, it is a pain to go through without having an actual telescope to try out... I see no input from any testers with instruments, and I guess we can wait for 5 more years before anyone would show up. We could just put that in 24.3 and wait for response by users.

And while we are in this, I could disable ASCOM support for Qt6. I can confirm it is still broken with the ASCOM simulator. Just task-switch to other programs a few times, and any time seconds to minutes later debugger finds us in a library which we cannot change.
The problem is in function Qt6OpenGL!QOpenGL2PaintEngineEx::setState()
grafik

Then we should move on from ASCOM (Win only) to Alpaca (all platforms, TCP/IP communication), maybe as early as 24.4.

@gzotti gzotti self-assigned this Sep 7, 2024
@gzotti gzotti marked this pull request as ready for review September 7, 2024 12:25
@github-actions github-actions bot requested a review from 10110111 September 7, 2024 12:25
@alex-w alex-w added this to the 24.3 milestone Sep 7, 2024
@gzotti gzotti merged commit f653b90 into master Sep 7, 2024
28 of 29 checks passed
@gzotti gzotti deleted the fix/abortTelescopeSlew_LX200_NexStar branch September 7, 2024 12:29
@alex-w alex-w added the state: published The fix has been published for testing in weekly binary package label Sep 8, 2024
Copy link

github-actions bot commented Sep 8, 2024

Hello @gzotti!

Please check the fresh version (development snapshot) of Stellarium:
https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot

@alex-w alex-w removed the state: published The fix has been published for testing in weekly binary package label Sep 22, 2024
Copy link

Hello @gzotti!

Please check the latest stable version of Stellarium:
https://github.com/Stellarium/stellarium/releases/latest

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
Development

Successfully merging this pull request may close these issues.

4 participants