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

Migrate to INDI v2.0.0+ #1089

Merged
merged 2 commits into from
Aug 27, 2023
Merged

Migrate to INDI v2.0.0+ #1089

merged 2 commits into from
Aug 27, 2023

Conversation

knro
Copy link
Contributor

@knro knro commented Aug 11, 2023

Migrate PHD2 to utilize INDI v2

  • Make libnova required on Linux/MacOS since it is required by INDI. Missing libnova would break some functionality like Calibration Assistant due to missing LST.
  • Use INDI v2.0.3 as base for bundled INDI.
  • Need to produce Windows library
  • Needs testing on MacOS

@agalasso
Copy link
Contributor

this looks great so far @knro ! I have PHD2 build/test environments setup for Mac, Windows and Linux -- please let me know if there is any testing you'd like me to do before we merge this.

@knro
Copy link
Contributor Author

knro commented Aug 13, 2023

Thanks @agalasso I wasn't able to test this on Windows so far, so if you can that would be great. I tried a Windows VM but that didn't go as I planned so I ordered a MiniPC for Windows :D until I receive it, it would be great to know if there are any issues that can be resolved. On Linux it works great, but should probably get more testing.

@pchev
Copy link
Contributor

pchev commented Aug 13, 2023

I tested on Linux and it work fine by using my INDI 2.0.3 local install.

But using the bundled INDI fail because the file is a zip but thirdparty.cmake try to use a tar.gz

I have all the VM setup for testing on Windows but no build setup. @agalasso I am interested if you can build a Windows installer and send me for testing.

@agalasso
Copy link
Contributor

build a Windows installer and send me for testing.

@pchev here you go: https://openphdguiding.org/phd2-2.6.11dev6-indi2-a-installer.exe

(I have not tested it at all yet, will try to test later today)

@pchev
Copy link
Contributor

pchev commented Aug 13, 2023

I test on a Windows 11 virtual machine, connect to a INDI server 2.0.3 running on Linux with the simulator drivers.
Everything work as expected, camera, telescope, calibration, guiding and control panel.

@knro knro marked this pull request as ready for review August 17, 2023 08:06
@bwdev01
Copy link
Contributor

bwdev01 commented Aug 26, 2023

We've just gotten this comment in a long-running support thread:
https://groups.google.com/g/open-phd-guiding/c/1rqgqY2Yx5s

The OP now claims that your PR will "fix" his problem although he has never tried using it. The problem is that the local sidereal time returned by the driver was wrong, as was the local time on the computer. When I questioned his assertion, he referred me to you and may have been parroting something you said. Is he just confused? If not, can you explain how a version change to the Indi framework could have anything to do with this and if it does, why it hasn't affected every Indi mount driver that's been used with PHD2. Thanks.
Bruce

@knro
Copy link
Contributor Author

knro commented Aug 27, 2023

INDI mount drivers do not report LST. PHD2 relies on INDI compiled with libnova to get LST. However, PHD2 was not compiled with libnova, so the value from libindi for LST was always zero. With this PR, PHD2 is required to use libnova when compiled against INDI and therefore the LST returned is always valid.

@agalasso
Copy link
Contributor

@knro thanks for the PR! sorry for the delay merging it.. I have been traveling

@agalasso agalasso merged commit 9ff645f into OpenPHDGuiding:master Aug 27, 2023
@bwdev01
Copy link
Contributor

bwdev01 commented Aug 28, 2023 via email

@sn3ik
Copy link

sn3ik commented Sep 4, 2023

Hi all, I've been trying to compile this release for several days, but I get a lot of errors.

First: the thirdparty.cmake file tries to find a tar.gz archive, I changed to the zip file and it extracts fine.

Second: cmake throws errors because file paths are changed to INDI > 2 so you have to change all library paths

`configure_file(
"${LIBINDI_DIRECTORY}/libs/indiclient/baseclient.h"
"${libindi_root_config}/libs/indiclient/baseclient.h"
COPY ONLY)
set(indiclient_INC
libs/indicore/indiapi.h
libs/indicore/indidevapi.h
libs/indicere/base64.h
libs/indicere/lilxml.h
libs/indicore/indicom.h
libs/eventloop/eventloop.h
libs/indibase/indidriver.h
libs/indidevice/indibase.h
libs/indidevice/indibasetypes.h
libs/indidevice/baseddevice.h
libs/indibase/defaultdevice.h
libs/indibase/indiccd.h
libs/indibase/indidetector.h
libs/indibase/indifilterwheel.h
libs/indibase/indifocuserinterface.h
libs/indibase/indifocuser.h
libs/indibase/inditelescope.h
libs/indibase/indiguiderinterface.h
libs/indibase/indifilterinterface.h
libs/indidevice/property/indiproperty.h
libs/indidevice/indistandardproperty.h
libs/indibase/indidome.h
libs/indibase/indigps.h
libs/indibase/indilightboxinterface.h
libs/indibase/indidustcapinterface.h
libs/indibase/indiweather.h
libs/indibase/indilogger.h
libs/indibase/indicontroller.h
libs/indibase/indiusbdevice.h
libs/hid/hidapi.h

   libs/indibase/connectionplugins/connectioninterface.h
   libs/indibase/connectionplugins/connectionserial.h
   libs/indibase/connectionplugins/connectiontcp.h
 )

 # include_directories( ${CMAKE_CURRENT_BINARY_DIR})
 ####include_directories( ${LIBINDI_DIRECTORY})
 ####include_directories( ${LIBINDI_DIRECTORY}/libs)
 ####include_directories( ${LIBINDI_DIRECTORY}/libs/indibase)
 ####include_directories( ${ZLIB_INCLUDE_DIR})
 ####include_directories( ${CFITSIO_INCLUDE_DIR})

 # default for libindi client
 option(INDI_FAST_BLOB "Build INDI with Fast BLOB support" ON)

 set(indiclient_C_SRC
     ${LIBINDI_DIRECTORY}/libs/indicore/lilxml.cpp
     ${LIBINDI_DIRECTORY}/libs/indicore/base64.c
     ${LIBINDI_DIRECTORY}/libs/indicore/indicom.c)

 set(indiclient_CXX_SRC
     ${LIBINDI_DIRECTORY}/libs/indidevice/basedevice.cpp
     ${LIBINDI_DIRECTORY}/libs/indiclient/baseclient.cpp
     ${LIBINDI_DIRECTORY}/libs/indidevice/property/indiproperty.cpp
     ${LIBINDI_DIRECTORY}/libs/indidevice/indistandardproperty.cpp)`

with these modifications it is possible to generate the Makefile

Third: launching make still causes other errors pointing to the libraries, I tried changing the path of the files again but I was not able to solve all the problems

Fourth: you need to rename the indiapi.h.in file to indiapi.h

Has anyone encountered these problems?

Thanks and sorry for my bad english

agalasso added a commit that referenced this pull request Sep 5, 2023
@agalasso
Copy link
Contributor

agalasso commented Sep 5, 2023

I'm not sure how we got into these compile issues as we did test builds on windows, mac and linux before we merged this. I am able to repro the compile errors on windows and Linux now, so I'm going to revert the merge of this pr into phd2master until we get these compile issues sorted out.

@knro would you mind putting up a new pr so we can fix the issues before re-mergig to phd2/master, thanks

agalasso added a commit that referenced this pull request Sep 5, 2023
agalasso added a commit that referenced this pull request Sep 5, 2023
@knro
Copy link
Contributor Author

knro commented Sep 7, 2023

@agalasso The issue is with compiling the bundled INDI zip file which is quite complex under Linux/MacOS due to servers, tools, drivers, and libraries that must be installed. Can't we just rely on system installed INDI by default?

@agalasso
Copy link
Contributor

agalasso commented Sep 7, 2023

I agree it makes sense to use the system-installed INDI libs for Linux builds -- let's do that.

For Windows we're pretty much forced to compile a bundled INDI zip file since virtually no Windows users will have INDI libs installed (and similar for Mac)

@pchev
Copy link
Contributor

pchev commented Sep 7, 2023

The problem is it not work on all current system that not include INDI 2.0, so almost all, even Ubuntu 23.04 only include 1.9.9.
This force user to install INDI from source and is very inconvenient to build in a PPA,

I try on Debian Buster and cmake with -DUSE_SYSTEM_LIBINDI=1 return:
-- Found libindi, version 1.7.5
-- INDI version 1.7.5 found in /usr/include/libindi, but at least version 2.0.0 is required
INDI not found. Please install INDI and try again.

@knro
Copy link
Contributor Author

knro commented Sep 18, 2023

I agree @pchev , I will see if there is an easy way to build libindi within the CMakeLists.txt but since the cmake build process was significantly changed after 2.0.0 this is going to take some time.

@agalasso agalasso mentioned this pull request Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants