Add INDI Rotator support

[comptonizing]

Good day,

First of all, thank you for your great work.

I took the liberty of adding support for INDI rotators. Large portions of this code have been adapted from the INDI scope and camera implementation.

I am aware of #1089 but am not sure how to proceed with regards to it. In case this PR is useful and being considered for merge, maybe some comments from the maintainers could help. Of course I'd be very happy with someone else adapting it to INDI 2, in line with the changes by Jasem.

Would address #1041

Thank you, Philipp

[agalasso]

@comptonizing I wanted to also mention that #1089 has merged to master, so we'll need to make sure this code is in sync with those changes, thanks!

[comptonizing]

@comptonizing I wanted to also mention that #1089 has merged to master, so we'll need to make sure this code is in sync with those changes, thanks!

Thank you for this, I'll be addressing these comments in the next days. I pulled from the master but now have trouble building the project. I see that cmake expects a .tar.gz file, but the bundled INDI comes as .zip. I could work around that by renaming the file. But now cmake fails to find certain INDI files, see the attached log I created on a fresh clone of phd2. What am I doing wrong?

cmake.log

[comptonizing]

@agalasso I think I resolved the things pointed out above, thanks for the comments. I also saw that the merge of the new INDI version was reverted, so my changes are currently in sync with the master branch. I'm not sure what Jasem's plans are for migrating the entire phd2 project to the new INDI version, so I would like to leave this PR at your discretion.

[agalasso]

@comptonizing it took us a while to get here, but once #1129 merges that should un-block this pr. I plan to merge #1129 today or tomorrow.

[comptonizing]

@agalasso thanks a lot! I already started to get it in sync again, but PHD2 compiled from master constantly segfaults when connecting to INDI in the setup wizard (at least for me on Ubuntu 22.04). I'll first have to figure out what's going on there.

[deletio]

Greetings everyone. It has been a while without updates here, so I gave it a try myself.

I locally merged the source branch into the current master and used #1129 as reference to refactor rotator_indi.cpp. The build succeeds and I am able to connect from PHD2 to my Wanderer Rotator Mini V2.

Attaching the log with a quick connection/disconnection of the rotator for your review. PHD2_DebugLog_2024-03-05_015115.txt

Please let me know if there are any specific test cases you would like me to run. Otherwise please advise whether I should try to commit into @comptonizing's branch to enhance this current PR or raise my own. I would rather keep this PR, since it has a good history, but will need write access to the source branch.

upd. Uploaded to a fresh fork, the changes are here: https://github.com/deletio/phd2-indi-rotator/commit/e674291c946c15033ca577e28253e1859dc0cb25

[comptonizing]

Greetings everyone. It has been a while without updates here, so I gave it a try myself.

I locally merged the source branch into the current master and used #1129 as reference to refactor rotator_indi.cpp. The build succeeds and I am able to connect from PHD2 to my Wanderer Rotator Mini V2.

Attaching the log with a quick connection/disconnection of the rotator for your review. PHD2_DebugLog_2024-03-05_015115.txt

Please let me know if there are any specific test cases you would like me to run. Otherwise please advise whether I should try to commit into @comptonizing's branch to enhance this current PR or raise my own. I would rather keep this PR, since it has a good history, but will need write access to the source branch.

upd. Uploaded to a fresh fork, the changes are here: deletio@e674291

Thanks a lot for this! For a while I couldn't rebase against master because I always ran into segfaults and wanted to work on this again this week, but if you've done the work already there really isn't any reason to do so again. I granted you access to my repository if that helps in any way.

Of course the last word lies with @agalasso

[deletio]

Greetings everyone. It has been a while without updates here, so I gave it a try myself. I locally merged the source branch into the current master and used #1129 as reference to refactor rotator_indi.cpp. The build succeeds and I am able to connect from PHD2 to my Wanderer Rotator Mini V2.

Attaching the log with a quick connection/disconnection of the rotator for your review. PHD2_DebugLog_2024-03-05_015115.txt Please let me know if there are any specific test cases you would like me to run. Otherwise please advise whether I should try to commit into @comptonizing's branch to enhance this current PR or raise my own. I would rather keep this PR, since it has a good history, but will need write access to the source branch. upd. Uploaded to a fresh fork, the changes are here: deletio@e674291

Thanks a lot for this! For a while I couldn't rebase against master because I always ran into segfaults and wanted to work on this again this week, but if you've done the work already there really isn't any reason to do so again. I granted you access to my repository if that helps in any way.

Of course the last word lies with @agalasso

I was hesitant to force-push the rebase, so for now I have pushed it as a new branch https://github.com/comptonizing/phd2/tree/indi-rotator-refactored. Please see if it looks good to you and maybe check if it still segfaults as your previous attempts did. Will happily force-push onto the current branch (indi-rotator) at your first signal.

@agalasso's comments have been addressed in the last commit.

[deletio]

I ended up updating the source branch of this PR, so it is now in the final state with all comments addressed.

[comptonizing]

I ended up updating the source branch of this PR, so it is now in the final state with all comments addressed.

Thank you, seems to be working fine, with one minor problem: The rotator angle in PHD2 is shown as UKNOWN until the rotator has actually changed position once. This is problematic because it forces a recalibration upon this first rotator movement. It appears as updating the angle upon connecting to the driver is not working.

[deletio]

Thank you, seems to be working fine, with one minor problem: The rotator angle in PHD2 is shown as UKNOWN until the rotator has actually changed position once. This is problematic because it forces a recalibration upon this first rotator movement. It appears as updating the angle upon connecting to the driver is not working.

I also see that in the log as GetDouble("/profile/1/scope/calibration/rotatorAngle", -888.000000) returns -888.000000

looking into this

[comptonizing]

Thank you, seems to be working fine, with one minor problem: The rotator angle in PHD2 is shown as UKNOWN until the rotator has actually changed position once. This is problematic because it forces a recalibration upon this first rotator movement. It appears as updating the angle upon connecting to the driver is not working.

I also see that in the log as GetDouble("/profile/1/scope/calibration/rotatorAngle", -888.000000) returns -888.000000

looking into this

I think I was working correctly with my previous version before the introduction of INDI 2.0 if that helps in any way

[deletio]

I also see that in the log as GetDouble("/profile/1/scope/calibration/rotatorAngle", -888.000000) returns -888.000000 looking into this

I think I was working correctly with my previous version before the introduction of INDI 2.0 if that helps in any way

Of course it helps 👍

The log line I observed was actually not related: it was merely indicating that the calibration being restored did not have a rotator angle in it.

But indeed the angle was not updated on connection, which should be fixed now. Would be great if you could confirm, because I am not sure where exactly you were observing the UNKNOWN angle.

Also, I think I have an idea what could be causing segfaults. Calling RotatorINDI::updateAngle() without prior initialization of the angle_prop pointer could do that.

[comptonizing]

I also see that in the log as GetDouble("/profile/1/scope/calibration/rotatorAngle", -888.000000) returns -888.000000 looking into this

I think I was working correctly with my previous version before the introduction of INDI 2.0 if that helps in any way

Of course it helps 👍

The log line I observed was actually not related: it was merely indicating that the calibration being restored did not have a rotator angle in it.

But indeed the angle was not updated on connection, which should be fixed now. Would be great if you could confirm, because I am not sure where exactly you were observing the UNKNOWN angle.

The "UNKNOWN" showed up in the statistics pane of the GUI. I just tested your changes, they're working perfectly!

Also, I think I have an idea what could be causing segfaults. Calling RotatorINDI::updateAngle() without prior initialization of the angle_prop pointer could do that.

Sorry, it appears I didn't explain properly. After the INDI 2.0 stuff was merged, the master branch itself got me segfaults, at least here on Ubuntu 22.04 with INDI 2.0.6. It was completely unrelated to my changes and I couldn't find the problem quickly.

(If you want to you can add your name to the copyright section of the new files)

[deletio]

Done, thank you!

[comptonizing]

@agalasso I think this is looking good now from a functional point of view and is in line with INDI 2.0. We'd be happy if you could have a look

[deletio]

@comptonizing Since you drew my attention to the guide stats pane which I never used before, here is one more minor update that makes sure the rotator angle line updates not only when the mount is [dis]connected, but also when the rotator is [dis]connected or actually rotated.

[comptonizing]

@comptonizing Since you drew my attention to the guide stats pane which I never used before, here is one more minor update that makes sure the rotator angle line updates not only when the mount is [dis]connected, but also when the rotator is [dis]connected or actually rotated.

Awesome, thanks! I wanted to do this initially but since the status updated during calibration and guiding anyway I didn't deem it necessary. It's pretty nice now, you get immediate feedback

[agalasso]

(If you want to you can add your name to the copyright section of the new files)

re: copyright: if you do not mind, I'd prefer to say:

Copyright (C) 2024 openphdguiding.org

Having individual developers copyright individual source files was a pattern that got established early on and I would like to get away from it. It is unwieldy due to the BSD license 2d clause requirement that all the copyright statements be reproduced.

I would prefer give both of you credit by adding your names to the contributors list (Help > About) as a followup PR after this is merged, and not have to add an additional copyright statement as required by the 2d clause of the license.

[deletio]

Updated rotator_indi.* as requested. @agalasso should we also update files with one author?

[comptonizing]

@comptonizing is this ready to merge?

Yes, I had a look, tested it again, and everything seems to work like a charm.

Sorry, I got sick last week.