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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

paolostivanin
Copy link

@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

panicgh and others added 2 commits June 13, 2023 08:44
based on work of Chris Kuethe <chris.kuethe@gmail.com>
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
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
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
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
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
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

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
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
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
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
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 Jul 30, 2023

Any news? It seems other projects are expecting Stellarium finally moving to INDI 2.* 'really soon'.

@gzotti
Copy link
Member

gzotti commented Sep 12, 2023

Any news here?

@hmartinez82
Copy link
Contributor

hmartinez82 commented Oct 3, 2023

I'm working on getting Stellarium running on Windows on ARM. I already got it working without CalcMySky and without the Telescope plugin (due to INDI). But I just packaged CalcMySky and I'm going to package libindiclient, but it will have to be the latest INDI (2.0.4 as of this writing). Support for INDI 2.x should happen soon™️ .

@alex-w
Copy link
Member

alex-w commented Oct 18, 2023

Any progress?

@hmartinez82
Copy link
Contributor

hmartinez82 commented Oct 18, 2023

Well since I got Stellarium building for Windows on Arm with the current INDI I did not pursue this any further. It was not my main objective.

I don't know this library nor how it's used by Stellarium so someone who knows this stuff should do this upgrade

@panicgh
Copy link

panicgh commented Oct 18, 2023

As already mentioned four months ago: if we didn't try to build INDI from Stellarium but treat it like any other dependency, the issue would have been long solved. Some distros, e.g. NixOS, apply the patch for new INDI support and just use the system-provided INDI.

@gzotti
Copy link
Member

gzotti commented Oct 18, 2023

If we can depend on optionally system-installed stock INDI2 (build yourself where required), this would be the best solution IMO. When users don't need telescope control, there is no need for INDI either. That would make it a "recommended" package for apt-get or similar systems. We still need to know what "our own copy" has changed, and a maintainer who occasionally cares for INDI updates as years go by.

@github-actions github-actions bot added the has conflicts The pull request has conflicts label Nov 28, 2023
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@luzpaz
Copy link
Contributor

luzpaz commented Dec 23, 2023

What's the verdict here ?

@gzotti
Copy link
Member

gzotti commented Dec 23, 2023

  • the pull request has conflicts
  • From @panicgh 's wording ("we") I had expected final changes, build and usage instructions to get that working on all platforms, etc., for people not familiar with the internals of INDI. This must come from someone with a goto telescope which then must be sure to work.
  • seems too late for 23.4, but let's target it for 24.1

@gzotti gzotti added this to the 24.1 milestone Dec 23, 2023
@gzotti
Copy link
Member

gzotti commented Mar 6, 2024

Anybody using INDI and interested to move forward?

@alex-w alex-w removed this from the 24.1 milestone Mar 11, 2024
@gzotti
Copy link
Member

gzotti commented Apr 29, 2024

Is anybody out there using Stellarium with INDI interested in going forward?
Is anybody out there on Windows using Stellarium with some INDI server on the local network?
Is there a header package for Windows available from INDI?
Should we ask in some INDI forum for multiplatform guidance?

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has conflicts The pull request has conflicts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants