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

Native support for Player One cameras #1276

Merged
merged 9 commits into from
Jan 1, 2025

Conversation

EthanChappel
Copy link
Contributor

This pull request is for adding native support for cameras from Player One Astronomy, which I've seen brought up in #1238 in August but haven't seen signs of movement on since. Based on the code for supporting ZWO cameras in PHD2 since the SDKs for both manufacturers are similar.

Tested cameras
Xena-M
Uranus-C

Tested Platforms:
Windows 11 x64
Linux ARM32 (Raspberry Pi 2)
Linux ARM64 (Raspberry Pi 5)

Camera controls for exposure, gain, bit depth, and binning work as expected on tested cameras and platforms. Can test cooling control soon with a Uranus-C Pro or Poseiden-M Pro.

Builds successfully on x86 and x64 Linux but not tested yet. Currently facing issues building PHD2 on macOS Sequoia on an Intel Mac, so may need help testing if I am unable to get it to succeed.

image

@agalasso
Copy link
Contributor

@EthanChappel thanks for taking this on! I actually had started working on this as well but stalled due to lack of time.
I'd be happy to discard my work and take this pr. I can also help get it going on Mac as I already did some testing with Mac (Sequoia).
I have a couple suggestions on the pr overall:

  • going forward we are no longer copyrighting individual files to individual developers. Please do credit yourself if you like with a comment like "Created by Ethan Chappel based on cam_zwo.cpp" or similar, but have the copyright statement be "Copyright 2024 PHD2 Developers". We will also credit you in the Help > About window in a followup PR before the release.
  • please move the functions in cameras/ConvFuncs.h into cam_poa.cpp. Since these functions are POA-specific and dependent on the definitions in the POA SDK, I do not think we can frame them as general camera convenience functions as that will potentially be confusing for future developers. If there are functions that we want to generalize, we can look into that with a followup pr.

After those changes please add me as a reviewer on the pr and I'll review it again and we can get it merged. Thanks!

@EthanChappel EthanChappel marked this pull request as ready for review January 1, 2025 10:49
@EthanChappel EthanChappel changed the title WIP: Native support for Player One cameras Native support for Player One cameras Jan 1, 2025
@EthanChappel
Copy link
Contributor Author

Have made the requested changes. Seem to be unable to add reviewers to the PR. Think I would need write permissions to the repo to add you.

Should be able to perform a real-world test under the stars very soon with my mini PC running Windows.

Copy link
Contributor

@agalasso agalasso left a comment

Choose a reason for hiding this comment

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

nice work, thanks for the contribution!
could you please make the minor changes suggested and we'll get this merged.

@agalasso agalasso merged commit 6b2676b into OpenPHDGuiding:master Jan 1, 2025
@EthanChappel
Copy link
Contributor Author

Ceres 462M guide camera worked great in a real-world test last night through its ST4 port, with the mount's ASCOM driver as the aux mount.

@agalasso
Copy link
Contributor

agalasso commented Jan 3, 2025

Ceres 462M guide camera worked great in a real-world test last night through its ST4 port, with the mount's ASCOM driver as the aux mount.

@EthanChappel thanks for the update. If you have a chance, could you try building and testing the code from #1278 (branch andy/playerone-camera-support-added) which includes your code from this PR plus some additional changes for Mac? I was able to do some testing with #1278's branch using a Neptune on Windows and Mac, but I have not tested Linux. (we can continue the discussion over on #1278 if you find any issues)

Eyeke2 pushed a commit to Eyeke2/phd2.planetary that referenced this pull request Jan 23, 2025
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.

2 participants