OpenPHDGuiding / phd2

PHD2 Guiding
https://openphdguiding.org
BSD 3-Clause "New" or "Revised" License
244 stars 111 forks source link

Rewrite event server code and add necessary features #1212

Closed AstroAir closed 2 weeks ago

AstroAir commented 2 weeks ago

Firstly, I apologize for the problems caused by my previous unprofessional answers. The goal of this issue is to rewrite event servers using the new features of c++11 and 14.

This is my preliminary idea. If feasible, I will submit a PR soon and hope to adopt it.

PS:I can also complete the function of adding a dark field library for shooting recently, as I have previously added relevant interfaces for PHD2.Another question is whether it is possible to add files with formatting rules such as. clang format to the project, which can effectively improve the code style.

ceterumnet commented 2 weeks ago

@AstroAir - FYI, I am going to be submitting a PR that also adds nlohmann::json support - I am using it to parse responses Alpaca based interfaces...

My fork that I'm working on is here: https://github.com/ceterumnet/phd2/tree/alpaca_support

In my fork, I've just included the single hpp file for nlohmann::json

AstroAir commented 2 weeks ago

@ceterumnet Thank you for your reply. I think using JSON.hpp to operate JSON was a good decision, and I have now started modifying the PHD2 server. Also, do you have any ideas for implementing a C++version of Alpyca?

ceterumnet commented 2 weeks ago

@AstroAir - I'm working on a more generic project focused on Alpaca. I currently have the repository set to private, but I will add you to it so you can see what I'm working on there. I plan on releasing it fully open source. You will be able to see what I am building there, and we can discuss!

AstroAir commented 2 weeks ago

@ceterumnet I have been working on a project recently, and my idea is to create a mobile astronomical shooting software that aims to replace Asiair. However, it is not only targeted at Linux platforms such as Raspberry Pi, but also Windows platforms. If you are interested, please take a look at my homepage.

bwdev01 commented 2 weeks ago

@AstroAir : I appreciate your desire to improve things but I don't think a rewrite of the existing EventServer code makes any sense at all. Even accepting that the newer style of handling JSON is probably better, I think the risk/reward ratio is completely upside-down. We have thousands of people indirectly using the API through NINA, SGP, and Voyager to say nothing of all the private amateur and professional observatory apps. Unfortunately, the proposed re-write provides zero benefit to any end-user. Even though I’m not an expert in C++, I have never had any trouble dealing with the EventServer code as-is, mostly because a lot of the JSON nitty-gritty has already been abstracted and there are always existing examples of what needs to be done. I think a complete re-write would require an exhaustive set of unit tests that could demonstrate that the new implementation exactly matches the old one (all functions, bugs and all, syntax errors and all) – AND – a beta test with the major session manager apps. This would involve a large time investment and doesn't make any sense given the expected benefits.

AstroAir commented 2 weeks ago

@bwdev01Thank you for your reply. It is obvious that your opinion is correct. The benefits of rewriting everything are not proportional to the time required. So I think it's better to put aside this idea for now and do something else. A year ago, I proposed the idea of cleaning up the entire phd2 directory. I'm not sure if it's still necessary now?

bwdev01 commented 2 weeks ago

@AstroAir: Thanks for understanding. Andy and I both have many decades of experience working with large-scale, business-critical software systems and we tend to manage PHD2 in the same way. Cheers.