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

Add Camera Mosaic plugin #4005

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

Conversation

jmeyers314
Copy link

Description

Hello Stellarium developers!

As a long-time Stellarium fan and user as both an amateur and professional astronomer, I'm excited to submit a new plugin for your consideration. This plugin overlays the sensor outlines of mosaic cameras onto Stellarium's celestial sphere. While I do realize that both the oculars and telescope control plugins share some similarities, I hope you can appreciate the added detail that the mosaic provides. I've found the plugin quite valuable in my recent work commissioning the brand new Vera C. Rubin Observatory. Here are a couple quick screenshots of what the plugin provides:

Screenshot 2024-12-12 at 4 00 40 PM Screenshot 2024-12-12 at 4 11 31 PM

Some key features are:

  1. Includes several pre-configured cameras (LSSTCam, DECam, HSC, and MegaPrime)
  2. Allows users to add custom cameras via JSON
  3. Adjustable pointing and rotation either through plugin configuration or TCP messages

The plugin has garnered interest among my colleagues, already prompting a few of them to compile from my fork. A pre-compiled binary (i.e., distribution with the mainline Stellarium) would likely increase adoption significantly.

As this is my first Qt project, I welcome feedback on adherence to Qt and Stellarium conventions. I'm particularly interested in whether integration with the Stellarium remote control plugin would be an alternative to setting up a new TCP server for the plugin.

I'm grateful for Stellarium's impact on the astronomical community and hope this contribution aligns with your vision for the software.

Thanks,
Josh Meyers

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • This change requires a documentation update
  • Housekeeping

How Has This Been Tested?

While there are no formal unit tests, I've used the plugin interactively for several weeks now, including as an observing aide while commissioning the Vera C. Rubin Observatory.

Test Configuration:

  • Operating system: <MacOS, 14.7 Sonoma>
  • Graphics Card: < Apple M3 Pro Integrated Graphics>

Checklist:

  • My code follows the code style of this project.
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (header file)
  • I have updated the respective chapter in the Stellarium User Guide
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Note: I will work on the above checklist imminently, but thought it would be a good idea to initiate this PR in the meantime.

Copy link

Great PR! Please pay attention to the following items before merging:

Files matching src/**/*.cpp:

  • Are possibly unused includes removed?

This is an automatically generated QA checklist based on modified files.

@alex-w
Copy link
Member

alex-w commented Dec 13, 2024

Thank you very much for your plugin, but at the moment compilation is failed with Qt5 and Qt6 - please fix it.

Just after seeing the code: why TCP server is required in plugin and what will happen when TCP message will have wrong format?

@alex-w alex-w added feature Entirely new feature subsystem: plugins The issue is related to plugins of planetarium... labels Dec 13, 2024
Copy link

Hello @jmeyers314!

Thank you for proposing of the feature.

@alex-w alex-w added this to the 25.1 milestone Dec 13, 2024
@jmeyers314
Copy link
Author

Thank you very much for your plugin, but at the moment compilation is failed with Qt5 and Qt6 - please fix it.

Thanks, hopefully this is now addressed...

Just after seeing the code: why TCP server is required in plugin and what will happen when TCP message will have wrong format?

I wanted there to be a way for an external program to set the camera mosaic ra/dec/rotation, and this was the first option I was able to get work. I am very curious however if there's a way to integrate with the Remote Control plugin. I couldn't figure it out at first glance.

Nothing will currently happen if the format is wrong. You can experiment with, e.g.,

echo "LSSTCam,-2.0,1.0,12.0" | nc localhost 5772

which will set the LSSTCam overlay to ra=-2 deg, dec=1 deg, rotation = 12 deg.

On the other hand,

echo "LSSTCam,-2.0,1.0,12.0,blah" | nc localhost 5772

will currently be silently ignored.

@alex-w
Copy link
Member

alex-w commented Dec 13, 2024

The comma char is delimiter in numbers in some languages ;)

@alex-w
Copy link
Member

alex-w commented Dec 13, 2024

Please check CodeFactor report

@alex-w
Copy link
Member

alex-w commented Dec 13, 2024

  1. Allows users to add custom cameras via JSON

But right now it's developers defined cameras - I guess this is a planning option.

I think the version of plugin should not be 0.0.0 :)

@gzotti
Copy link
Member

gzotti commented Dec 13, 2024

Wow, that's quite a thing, Stellarium used for Vera Rubin etc... Not sure how many people would actually use the plugin, though. But it's also a great thing for outreach demos in the amateur communities.
Technically, it could be integrated as yet another instrument type for the Oculars plugin. However, that latter has some long overdue things to fix first, so let's stay with a separate plugin at least for now.

I have not deeply looked at the code yet. You can indeed use the RemoteControl plugin to send mini scripts which just include one "setter" command or other "public slot" which are automatically exposed as scripting commands. Connecting to APT or my Unity bridge works like that. For this to work properly, settable variables must be exposed as PROPERTY. See other plugins for countless examples.

If your external application that communicates with Stellarium over RemoteControl needs to know Stellarium's state, there is a built-in mechanism to transmit, at first connection, the state of all StelProperty variables (which are basically the Qt PROPERTY entries), and later only transmit the small set of changes relevant to keep state in-sync. See scripting docs.

@jmeyers314
Copy link
Author

The comma char is delimiter in numbers in some languages ;)

Thanks, good point. I'll switch this to a space for the moment. Let me know if you have another suggestion though.

@jmeyers314
Copy link
Author

Wow, that's quite a thing, Stellarium used for Vera Rubin etc... Not sure how many people would actually use the plugin, though. But it's also a great thing for outreach demos in the amateur communities.

Yeah, potentially limited user interest did occur to me. I played around with making a dynamic plugin for a while, but couldn't get it to work. I think the outreach angle is real though.

Technically, it could be integrated as yet another instrument type for the Oculars plugin.

I did look into using an Ocular for monitoring the pointing before diving in to making my own plugin. It works okay, but I like having the individual sensor boundaries and also the ability to leave the boundaries fixed while I pan around the sky looking at what's nearby to the current pointing.

You can indeed use the RemoteControl plugin to send mini scripts which just include one "setter" command or other "public slot" which are automatically exposed as scripting commands.

Thanks, I'll dig deeper here. Not opening an additional port certainly seems preferable if it can be avoided.

@alex-w
Copy link
Member

alex-w commented Dec 16, 2024

Is it possible to add a short description for each cameras (file + GUI)?

@jmeyers314
Copy link
Author

Is it possible to add a short description for each cameras (file + GUI)?

Yes, will do.

@jmeyers314
Copy link
Author

Also, looks like the hooks are already in place for updating the plugin without the TCP server. Syntax is something like:

curl -d 'code=MosaicCamera.updateMosaic("LSSTCam",-2.0,1.0,12.0)' http://localhost:8090/api/scripts/direct

So I'll get to work on removing the TCP bits.

@gzotti
Copy link
Member

gzotti commented Dec 17, 2024

Yes, this is the mini-script API. However, even cleaner is the /api/stelproperty service. It would circumvent the scripting module and allow synchronizing connected instances of Stellarium (RemoteSync plugin). You only have to make properties out of your plugin's instance variables.

Hmm, I see you have your setters like "setRA(QString &cameraName, double RA)" and display several cameras at the same time. Then maybe the mini-script is the simplest to change to. Another path would be to have a "currentCamera" property onto which property setters would apply. (This could be propagated via RemoteSync.) Like:

MosaicCamera.setCurrentCamera("LSSTCam");
MosaicCamera.setCurrentRA(ra):
MosaicCamera.setCurrentDec(dec);
MosaicCamera.setCurrentRotation(rot);

RA/dec could also be constructed and manipulated together as Vec3d type. See various examples.

@jmeyers314
Copy link
Author

However, even cleaner is the /api/stelproperty service.

Thanks, I managed to figure this out today. Now the following works:

curl -d "id=MosaicCamera.currentCamera&value=LSSTCam" http://localhost:8090/api/stelproperty/set
curl -d "id=MosaicCamera.ra&value=-2.0" http://localhost:8090/api/stelproperty/set
curl -d "id=MosaicCamera.dec&value=1.0" http://localhost:8090/api/stelproperty/set
curl -d "id=MosaicCamera.rotation&value=12.0" http://localhost:8090/api/stelproperty/set

I kept the setRA(const QString& cameraName, double ra) style setters and getters for now alongside ones where you set currentCamera first, as I think they're more intuitive for maintaining multiple cameras. But let me know what you think.

I think I'm happy with the interface now, but still have some cleanup/documentation work to go through.

@gzotti
Copy link
Member

gzotti commented Dec 18, 2024

Yes, like that. Don't forget to document what angles ra/dec/rotation are: decimal degrees, decimal hours, radians, ...
No problem to keep the other setters like setRA(name, value) e.g. as higher-level scripting interface, as long as you then call the single property setters internally (to allow correct propagation).

@jmeyers314
Copy link
Author

While fixing up the cameras I've had another related plugin idea: outlining astronomical survey boundaries. For example, outlining the Hubble Ultra Deep Field, or the Dark Energy Survey footprint. User fields, e.g., "all the locations of my astrophotos" could be implemented too. In fact, user input here seems essential, as there are too many cameras and surveys to include all by default.

One challenge to consider is that larger regions might require more than a simple outline; some form of shading could be necessary to clearly distinguish between interior and exterior areas.

Bringing this up now because if this feature were to be included, renaming the plugin to something like "Footprints" or "Overlays" might be appropriate, as it would encompass both camera and survey boundaries. Thoughts appreciated.

@gzotti
Copy link
Member

gzotti commented Dec 19, 2024

Should these footprints not just be HiPS surveys? In any case, I don't see them in this plugin. In essence, it's like "footprint shapefiles" in GIS, we currently don't have such polygonal functionality.

@github-actions github-actions bot added the has conflicts The pull request has conflicts label Dec 25, 2024
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Entirely new feature has conflicts The pull request has conflicts subsystem: plugins The issue is related to plugins of planetarium...
Development

Successfully merging this pull request may close these issues.

3 participants