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

Port code to INDI 2.0.2 and update CPM #3269

Merged
merged 17 commits into from
Nov 9, 2024
Merged

Conversation

paolostivanin
Copy link
Contributor

@paolostivanin paolostivanin commented Jun 13, 2023

This PR is based on the work done by some people at #2891. In particular:

  • patch created by @panicgh was applied as is, since it worked fine
  • the code created by @ckuethe was updated to work with v2.0 since it was not compiling

What was tested:

  • compilation using the OS' INDI 2.0.2 works fine
  • compilation using the CPM's INDI 2.0.2 works fine
  • the TelescopeControl plugin could be loaded and connected to a INDIserver instance using Simulators.

What was not tested:

  • connection to real hardware
  • real hardware working properly

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding your first pull request to Stellarium. If you have questions, please do not hesitate to contact us.

@gzotti
Copy link
Member

gzotti commented Jun 13, 2023

Looks interesting, thank you! Anybody with real hardware willing to test?

@paolostivanin
Copy link
Contributor Author

for openSUSE TW users: I've created a test repo with Stellarium compiled using INDI 2.0. You can find it here: https://build.opensuse.org/package/show/home:polslinux:branches:Education/stellarium

@10110111
Copy link
Contributor

the TelescopeControl plugin could be loaded and connected to a INDIserver instance using Simulators

IIRC there were some problems only present in Qt6 but not Qt5. Did you check this?

@paolostivanin
Copy link
Contributor Author

@10110111 the tests I did were with Stellarium compiled using QT6

@alex-w
Copy link
Member

alex-w commented Jun 13, 2023

macOS:

[  4%] Building CXX object plugins/TelescopeControl/src/INDI/CMakeFiles/TelescopeControl_INDI.dir/TelescopeControl_INDI_autogen/mocs_compilation.cpp.o
In file included from /Users/aw/stellarium/builds/macosx/plugins/TelescopeControl/src/INDI/TelescopeControl_INDI_autogen/mocs_compilation.cpp:2:
In file included from /Users/aw/stellarium/builds/macosx/plugins/TelescopeControl/src/INDI/TelescopeControl_INDI_autogen/EWIEGA46WW/moc_INDIConnection.cpp:9:
/Users/aw/stellarium/builds/macosx/plugins/TelescopeControl/src/INDI/TelescopeControl_INDI_autogen/EWIEGA46WW/../../../../../../../../plugins/TelescopeControl/src/INDI/INDIConnection.hpp:23:10: fatal error: 'libindi/baseclient.h' file not found
#include "libindi/baseclient.h"
         ^~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[2]: *** [plugins/TelescopeControl/src/INDI/CMakeFiles/TelescopeControl_INDI.dir/TelescopeControl_INDI_autogen/mocs_compilation.cpp.o] Error 1
make[1]: *** [plugins/TelescopeControl/src/INDI/CMakeFiles/TelescopeControl_INDI.dir/all] Error 2
make: *** [all] Error 2
Stellarium.app: No such file or directory

@paolostivanin
Copy link
Contributor Author

@alex-w thanks for checking macos! I'm gonna look into it

@alex-w
Copy link
Member

alex-w commented Jun 13, 2023

@alex-w thanks for checking macos! I'm gonna look into it

please see our build bots also - looks like cmake has missing files + source code need to be update (check include# directives) for INDI

@paolostivanin
Copy link
Contributor Author

That's weird though, I can't reproduce that issue locally 🤔

@alex-w
Copy link
Member

alex-w commented Jun 14, 2023

That's weird though, I can't reproduce that issue locally 🤔

Probably you have locally installed INDI

@paolostivanin
Copy link
Contributor Author

yep I do have INDI installed, but I'm running cmake with -DPREFER_SYSTEM_INDILIB=OFF and cmake is executing the CPM action while generating stuff.
I'll try on a vm

@panicgh
Copy link
Contributor

panicgh commented Jun 14, 2023

Thanks for following this up. I am super busy at the moment and got stuck in the CMake porting part.

I wonder if it is really necessary to build INDI through CPMAddPackage and then repeating parts of INDI's CMakeFile (including version, so-version, dependencies between some files etc.) and only building selected parts of INDI. Wouldn't it be simpler to either build full INDI as CMake external project or not build it at all and simply require a system-provided INDI? It appears to me that the current approach makes it unnecessarily hard to port to new INDI versions.

@paolostivanin
Copy link
Contributor Author

I agree that bundling INDI is, IMHO, not the best solution.
I also think that we should treat INDI like any other dep: is it installed? Yes, then build the plugin. No? Then don't build the plugin.

@gzotti
Copy link
Member

gzotti commented Jun 14, 2023

If the existence of compatible versions is guaranteed by simple dependencies, and we don't change anything in the INDI code, this would be OK for me.

@10110111
Copy link
Contributor

Wouldn't this go against the practice used for QXlsx and CalcMySky, which were also once optional but are now CPM-required?

@gzotti
Copy link
Member

gzotti commented Jun 14, 2023

If INDI is available as standard package from default repositories just like Qt stuff, and people may have installed them already for other programs, it would be preferred over CPM and private compilations. But I had understood our first adoption of INDI had some non-standard edits in the INDI 0.* sources, so required a private build. The other packages for QXlsx and CMS are also still optional, but there are no apt packages from Debian, right?

@10110111
Copy link
Contributor

10110111 commented Jun 14, 2023

there are no apt packages from Debian, right?

Right.

BTW I just noticed that in Ubuntu, even in 23.04 and in the development version, Stellarium is still 0.22.2. Do you happen to know why it's so old?

@gzotti
Copy link
Member

gzotti commented Jun 14, 2023

No, I don't know how the official Ubuntu maintainers upgrade their packages. We provide an Ubuntu repo with the latest version, see Guide 2.3.3. If INDI does the same, it should be fine for us. But CPM may be the solution if INDI is not available everywhere.

@paolostivanin
Copy link
Contributor Author

paolostivanin commented Jun 15, 2023

Speaking of Linux, most of the widely used distros have INDI 2:

  • Fedora >= 38
  • openSUSE Tumbleweed
  • Archlinux
  • Gentoo
  • Ubuntu has a PPA directly from the INDI dev

For MacOS, there may be a port or whatever. Still, compiling there it's a pretty trivial task. There are instructions on the INDI website.

For Windows, I don't think it's gonna be an issue. There they have ASCOM, official software from hardware makers and imaging software that integrates pretty well with Stellarium.

@paolostivanin
Copy link
Contributor Author

Hello 😄 do we have a final decision on what to do with INDI? Treat it as a dep or bundle it?

@alex-w
Copy link
Member

alex-w commented Jun 21, 2023

INDI client within Telescope Control plugin expected to all platforms, so, e.g. Windows user can connect direct to INDI server onto Linux. The current branch has few #include directives to building the INDI-client, but when INDI is not installed locally, then headers are missing (INDI2 has other locations for these files) - the code of Telescope Control plugin should be updates to reflect the changes paths in INDI.

@paolostivanin
Copy link
Contributor Author

I guess we better keep CPM then. Otherwise Windows support could be an issue 🤔
I'll update the PR

@gzotti
Copy link
Member

gzotti commented Sep 23, 2024

Yes, there are people who want to USE it. Are there users who want to use this part of the program and are also capable of MAKING it work? Or, given that it somehow works, still improve what's wrong with the current situation. If INDI is unsupported in Windows (which I don't know) we can also exclude it on this platform.

@alex-w alex-w added the subsystem: plugins The issue is related to plugins of planetarium... label Oct 13, 2024
@github-actions github-actions bot removed the has conflicts The pull request has conflicts label Oct 29, 2024
Copy link

Conflicts have been resolved. A maintainer will review the pull request shortly.

@alex-w
Copy link
Member

alex-w commented Oct 29, 2024

Tried testing on INDI mount simulator - it can be connected/disconnected, but not targeting

@alex-w
Copy link
Member

alex-w commented Nov 8, 2024

OK, so, 4 subtasks are open at the moment:

  • restoring unit testing
  • building INDIclient in FreeBSD
  • restoring speed chooser for directional movements in INDI
  • movement of telescope' circles in the sky.

@alex-w
Copy link
Member

alex-w commented Nov 8, 2024

Well, the PR is ready to review and merge almost - I don’t understand why marker of telescope is not showed a targeting to the selected object or position. Is it problem for INDI only or for any telescope?

@alex-w
Copy link
Member

alex-w commented Nov 8, 2024

Important note: I didn’t tested connection to INDIGO server

@alex-w alex-w self-assigned this Nov 8, 2024
@alex-w alex-w removed their request for review November 8, 2024 14:58
@alex-w alex-w added this to the 24.4 milestone Nov 8, 2024
@gzotti
Copy link
Member

gzotti commented Nov 8, 2024

Sounds good! However, I cannot comment on it or test it.

@alex-w
Copy link
Member

alex-w commented Nov 8, 2024

Well, the PR is ready to review and merge almost - I don’t understand why marker of telescope is not showed a targeting to the selected object or position. Is it problem for INDI only or for any telescope?

Hmm… I can check the problem in master with INDI 1.8.5 (why I didn’t think about this possibility before?)

@alex-w
Copy link
Member

alex-w commented Nov 9, 2024

OK, the problem in current port of INDI

@alex-w alex-w merged commit 701d10c into Stellarium:master Nov 9, 2024
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
subsystem: plugins The issue is related to plugins of planetarium...
Development

Successfully merging this pull request may close these issues.

8 participants