{{ message }}
Repository navigation
Conversation
micahnyc
marked this pull request as draft
August 19, 2026 02:46
Contributor
Author
alexanderbock
requested changes
Aug 19, 2026
alexanderbock
left a comment
alexanderbock
left a comment
Member
There was a problem hiding this comment.
I'm not sure why we'd need two sets of functions that basicallyt do the same thing. I think the globebrowsing functions could move into the spicemanager_lua and become the only functions we'll need
Comment on lines
+881
to
+889
| * \\param lat_deg Latitude in degrees | ||
| * \\param lon_deg Longitude in degrees | ||
| * \\param utc_date UTC date as a string (e.g., "2024-01-01") | ||
| * \\param observer The observing body (e.g., "EARTH") | ||
| * \\param isSunrise If true, computes sunrise time; if false, computes sunset time | ||
| * \\return The ephemeris time (ET seconds past J2000) of the solar event | ||
| * | ||
| * \\throw SpiceException If no sunrise/sunset found for the given date/location | ||
| * (e.g., polar day or night) |
Member
There was a problem hiding this comment.
Suggested change
| * \\param lat_deg Latitude in degrees | |
| * \\param lon_deg Longitude in degrees | |
| * \\param utc_date UTC date as a string (e.g., "2024-01-01") | |
| * \\param observer The observing body (e.g., "EARTH") | |
| * \\param isSunrise If true, computes sunrise time; if false, computes sunset time | |
| * \\return The ephemeris time (ET seconds past J2000) of the solar event | |
| * | |
| * \\throw SpiceException If no sunrise/sunset found for the given date/location | |
| * (e.g., polar day or night) | |
| * \param latitude Latitude in degrees | |
| * \param longitude Longitude in degrees | |
| * \param date UTC date as a string (e.g., "2024-01-01") | |
| * \param observer The observing body (e.g., "EARTH") | |
| * \param isSunrise If true, computes sunrise time; if false, computes sunset time | |
| * \return The ephemeris time (ET seconds past J2000) of the solar event | |
| * | |
| * \throw SpiceException If no sunrise/sunset found for the given date/location | |
| * (e.g., polar day or night) |
Comment on lines
+891
to
+892
| double solarEventTime(double lat_deg, double lon_deg, const std::string& utc_date, | ||
| const std::string& observer, bool isSunrise) const; |
Member
There was a problem hiding this comment.
Suggested change
| double solarEventTime(double lat_deg, double lon_deg, const std::string& utc_date, | |
| const std::string& observer, bool isSunrise) const; | |
| double solarEventTime(double latitude, double longitude, const std::string& date, | |
| const std::string& observer, bool isSunrise) const; |
Comment on lines
+602
to
+606
| std::string_view date = global::timeManager->time().ISO8601(); | ||
| std::string_view dateTrim = date.substr(0, date.find('T')); | ||
| std::string dateStr(dateTrim); | ||
| std::string globe = global::navigationHandler->orbitalNavigator().anchorNode()->identifier(); | ||
| return SpiceManager::ref().solarEventTime(lon, lat, dateStr.c_str(), globe, false); |
Member
There was a problem hiding this comment.
Suggested change
| std::string_view date = global::timeManager->time().ISO8601(); | |
| std::string_view dateTrim = date.substr(0, date.find('T')); | |
| std::string dateStr(dateTrim); | |
| std::string globe = global::navigationHandler->orbitalNavigator().anchorNode()->identifier(); | |
| return SpiceManager::ref().solarEventTime(lon, lat, dateStr.c_str(), globe, false); | |
| std::string date = global::timeManager->time().ISO8601(); | |
| std::string dateTrim = date.substr(0, date.find('T')); | |
| std::string globe = global::navigationHandler->orbitalNavigator().anchorNode()->identifier(); | |
| constexpr bool IsSunrise = true; | |
| return SpiceManager::ref().solarEventTime(lon, lat, dateTrim, globe, IsSunrise); |
Comment on lines
+611
to
+615
| std::string_view date = global::timeManager->time().ISO8601(); | ||
| std::string_view dateTrim = date.substr(0, date.find('T')); | ||
| std::string dateStr(dateTrim); | ||
| std::string globe = global::navigationHandler->orbitalNavigator().anchorNode()->identifier(); | ||
| return SpiceManager::ref().solarEventTime(lon, lat, dateStr.c_str(), globe, true); |
Member
There was a problem hiding this comment.
Suggested change
| std::string_view date = global::timeManager->time().ISO8601(); | |
| std::string_view dateTrim = date.substr(0, date.find('T')); | |
| std::string dateStr(dateTrim); | |
| std::string globe = global::navigationHandler->orbitalNavigator().anchorNode()->identifier(); | |
| return SpiceManager::ref().solarEventTime(lon, lat, dateStr.c_str(), globe, true); | |
| std::string date = global::timeManager->time().ISO8601(); | |
| std::string dateTrim = date.substr(0, date.find('T')); | |
| std::string globe = global::navigationHandler->orbitalNavigator().anchorNode()->identifier(); | |
| constexpr bool IsSunrise = false; | |
| return SpiceManager::ref().solarEventTime(lon, lat, dateTrim, globe, IsSunrise); |
| return res; | ||
| } | ||
|
|
||
| [[codegen::luawrap]] double sunriseTimeForSimulation() { |
Member
There was a problem hiding this comment.
The functions would become more useful if they'd have an optional parameter that is the date, and only if the parameter is not specified we'd use the current simulation time. Ofc the ForSimulation would be removed from the function name
|
|
||
| if (!found) { | ||
| std::string eventType = isSunrise ? "sunrise" : "sunset"; | ||
| throwSpiceError("No " + eventType + " found for the given date/location"); |
Member
There was a problem hiding this comment.
Suggested change
| throwSpiceError("No " + eventType + " found for the given date/location"); | |
| throwSpiceError(std::format( | |
| "No {} found for the given date/location", | |
| eventType | |
| )); |
| } | ||
|
|
||
| if (!found) { | ||
| std::string eventType = isSunrise ? "sunrise" : "sunset"; |
Member
There was a problem hiding this comment.
Suggested change
| std::string eventType = isSunrise ? "sunrise" : "sunset"; | |
| const std::string_view eventType = isSunrise ? "sunrise" : "sunset"; |
| while (b - a > tol) { | ||
| double mid = 0.5 * (a + b); | ||
| double fm = solarElevation(mid); | ||
| if (isSunrise ? (fm < 0.0) : (fm >= 0.0)) a = mid; else b = mid; |
Member
There was a problem hiding this comment.
Suggested change
| if (isSunrise ? (fm < 0.0) : (fm >= 0.0)) a = mid; else b = mid; | |
| if (isSunrise ? (fm < 0.0) : (fm >= 0.0)) { | |
| a = mid; | |
| } | |
| else { | |
| b = mid; | |
| } |
Comment on lines
+473
to
+476
| /** | ||
| * Helper function to compute solar event times (sunrise/sunset). | ||
| * Both sunriseTime and sunsetTime delegate to SpiceManager::solarEventTime. | ||
| */ |
Member
There was a problem hiding this comment.
This will currently become the documentation for the sunriseTime function
Comment on lines
+478
to
+486
| [[codegen::luawrap]] double sunriseTime(double lat_deg, double lon_deg, | ||
| std::string utc_date, std::string observer) { | ||
| return SpiceManager::ref().solarEventTime(lat_deg, lon_deg, utc_date, observer, true); | ||
| } | ||
|
|
||
| [[codegen::luawrap]] double sunsetTime(double lat_deg, double lon_deg, | ||
| std::string utc_date, std::string observer) { | ||
| return SpiceManager::ref().solarEventTime(lat_deg, lon_deg, utc_date, observer, false); | ||
| } |
Member
There was a problem hiding this comment.
These functions should get their own proper documentation
alexanderbock
requested changes
Aug 20, 2026
Comment on lines
+878
to
+903
| /** | ||
| * Computes the time of a solar event (sunrise or sunset) for a given location | ||
| * and date using bisection search on the solar elevation angle. | ||
| * | ||
| * \\param latitude The latitude in degrees (range: -90 to 90) | ||
| * \\param longitude The longitude in degrees (range: -180 to 180) | ||
| * \\param altitude The observer's altitude above the surface in meters | ||
| * (default: 0.0 for sea level) | ||
| * \\param date UTC date as a string (e.g., "2024-01-01") | ||
| * \\param observer The SPICE name of the observing body (e.g., "EARTH") | ||
| * \\param isSunrise If \\p true, computes sunrise; if \\p false, computes sunset | ||
| * \\return The ephemeris time (ET seconds past J2000) of the requested solar | ||
| * event | ||
| * | ||
| * \\throw SpiceException If no sunrise/sunset is found for the given | ||
| * date/location (e.g., polar day or night) | ||
| * \\throw SpiceException If required SPICE kernels are not loaded or if the | ||
| * observer is not a valid SPICE object | ||
| * \\pre \\p latitude must be between -90 and 90 | ||
| * \\pre \\p longitude must be between -180 and 180 | ||
| * \\pre \\p date must be a valid UTC date string | ||
| * \\pre \\p observer must be a valid SPICE body name | ||
| */ | ||
| double solarEventTime(double latitude, double longitude, double altitude, | ||
| const std::string& date, const std::string& observer, | ||
| bool isSunrise) const; |
Member
There was a problem hiding this comment.
Suggested change
| /** | |
| * Computes the time of a solar event (sunrise or sunset) for a given location | |
| * and date using bisection search on the solar elevation angle. | |
| * | |
| * \\param latitude The latitude in degrees (range: -90 to 90) | |
| * \\param longitude The longitude in degrees (range: -180 to 180) | |
| * \\param altitude The observer's altitude above the surface in meters | |
| * (default: 0.0 for sea level) | |
| * \\param date UTC date as a string (e.g., "2024-01-01") | |
| * \\param observer The SPICE name of the observing body (e.g., "EARTH") | |
| * \\param isSunrise If \\p true, computes sunrise; if \\p false, computes sunset | |
| * \\return The ephemeris time (ET seconds past J2000) of the requested solar | |
| * event | |
| * | |
| * \\throw SpiceException If no sunrise/sunset is found for the given | |
| * date/location (e.g., polar day or night) | |
| * \\throw SpiceException If required SPICE kernels are not loaded or if the | |
| * observer is not a valid SPICE object | |
| * \\pre \\p latitude must be between -90 and 90 | |
| * \\pre \\p longitude must be between -180 and 180 | |
| * \\pre \\p date must be a valid UTC date string | |
| * \\pre \\p observer must be a valid SPICE body name | |
| */ | |
| double solarEventTime(double latitude, double longitude, double altitude, | |
| const std::string& date, const std::string& observer, | |
| bool isSunrise) const; | |
| /** | |
| * Computes the time of a solar event (sunrise or sunset) for a given location and | |
| * date using bisection search on the solar elevation angle. | |
| * | |
| * \param latitude The latitude in degrees (range: -90 to 90) | |
| * \param longitude The longitude in degrees (range: -180 to 180) | |
| * \param altitude The observer's altitude above the surface in meters (default: 0.0 | |
| for sea level) | |
| * \param date UTC date as a string (e.g., "2024-01-01") | |
| * \param observer The SPICE name of the observing body (e.g., "EARTH") | |
| * \param isSunrise If \\p true, computes sunrise; if \\p false, computes sunset | |
| * \return The ephemeris time (ET seconds past J2000) of the requested solar event | |
| * | |
| * \throw SpiceException If no sunrise/sunset is found for the given date/location | |
| * (e.g., polar day or night) | |
| * \throw SpiceException If required SPICE kernels are not loaded or if the observer | |
| * is not a valid SPICE object | |
| * \pre \p latitude must be between -90 and 90 | |
| * \pre \p longitude must be between -180 and 180 | |
| * \pre \p date must be a valid UTC date string | |
| * \pre \p observer must be a valid SPICE body name | |
| */ | |
| double solarEventTime(double latitude, double longitude, double altitude, | |
| const std::string& date, const std::string& observer, bool isSunrise) const; |
Comment on lines
+31
to
+32
| #include <openspace/util/timemanager.h> | ||
| #include <openspace/util/spicemanager.h> |
Member
There was a problem hiding this comment.
Suggested change
| #include <openspace/util/timemanager.h> | |
| #include <openspace/util/spicemanager.h> | |
| #include <openspace/util/spicemanager.h> | |
| #include <openspace/util/timemanager.h> |
Comment on lines
+601
to
+602
| * Return the ephemeris time (ET seconds past J2000) of sunrise for the current | ||
| * camera position |
Member
There was a problem hiding this comment.
Suggested change
| * Return the ephemeris time (ET seconds past J2000) of sunrise for the current | |
| * camera position | |
| * Return the ephemeris time (ET seconds past J2000) of sunrise for the current camera | |
| * position. |
Comment on lines
+613
to
+623
| [[codegen::luawrap]] double sunriseTime(bool useCameraAltitude = true, | ||
| std::string date = "") { | ||
| auto [lat, lon, alt] = geoPositionForCamera(); | ||
| if (date.empty()) { | ||
| date = global::timeManager->time().ISO8601(); | ||
| } | ||
| std::string dateTrim = date.substr(0, date.find('T')); | ||
| std::string globe = global::navigationHandler->orbitalNavigator().anchorNode()->identifier(); | ||
| double eventAlt = useCameraAltitude ? alt : 0.0; | ||
| constexpr bool IsSunrise = true; | ||
| return SpiceManager::ref().solarEventTime(lat, lon, eventAlt, dateTrim, globe, IsSunrise); |
Member
There was a problem hiding this comment.
Suggested change
| [[codegen::luawrap]] double sunriseTime(bool useCameraAltitude = true, | |
| std::string date = "") { | |
| auto [lat, lon, alt] = geoPositionForCamera(); | |
| if (date.empty()) { | |
| date = global::timeManager->time().ISO8601(); | |
| } | |
| std::string dateTrim = date.substr(0, date.find('T')); | |
| std::string globe = global::navigationHandler->orbitalNavigator().anchorNode()->identifier(); | |
| double eventAlt = useCameraAltitude ? alt : 0.0; | |
| constexpr bool IsSunrise = true; | |
| return SpiceManager::ref().solarEventTime(lat, lon, eventAlt, dateTrim, globe, IsSunrise); | |
| [[codegen::luawrap]] double sunriseTime(bool useCameraAltitude = true, | |
| std::string date = "") | |
| { | |
| auto [lat, lon, alt] = geoPositionForCamera(); | |
| if (date.empty()) { | |
| date = global::timeManager->time().ISO8601(); | |
| } | |
| std::string dateTrim = date.substr(0, date.find('T')); | |
| std::string globe = | |
| global::navigationHandler->orbitalNavigator().anchorNode()->identifier(); | |
| alt = useCameraAltitude ? alt : 0.0; | |
| constexpr bool IsSunrise = true; | |
| return SpiceManager::ref().solarEventTime(lat, lon, alt, dateTrim, globe, IsSunrise); |
Comment on lines
+639
to
+649
| [[codegen::luawrap]] double sunsetTime(bool useCameraAltitude = true, | ||
| std::string date = "") { | ||
| auto [lat, lon, alt] = geoPositionForCamera(); | ||
| if (date.empty()) { | ||
| date = global::timeManager->time().ISO8601(); | ||
| } | ||
| std::string dateTrim = date.substr(0, date.find('T')); | ||
| std::string globe = global::navigationHandler->orbitalNavigator().anchorNode()->identifier(); | ||
| double eventAlt = useCameraAltitude ? alt : 0.0; | ||
| constexpr bool IsSunrise = false; | ||
| return SpiceManager::ref().solarEventTime(lat, lon, eventAlt, dateTrim, globe, IsSunrise); |
Member
There was a problem hiding this comment.
Suggested change
| [[codegen::luawrap]] double sunsetTime(bool useCameraAltitude = true, | |
| std::string date = "") { | |
| auto [lat, lon, alt] = geoPositionForCamera(); | |
| if (date.empty()) { | |
| date = global::timeManager->time().ISO8601(); | |
| } | |
| std::string dateTrim = date.substr(0, date.find('T')); | |
| std::string globe = global::navigationHandler->orbitalNavigator().anchorNode()->identifier(); | |
| double eventAlt = useCameraAltitude ? alt : 0.0; | |
| constexpr bool IsSunrise = false; | |
| return SpiceManager::ref().solarEventTime(lat, lon, eventAlt, dateTrim, globe, IsSunrise); | |
| [[codegen::luawrap]] double sunsetTime(bool useCameraAltitude = true, | |
| std::string date = "") | |
| { | |
| auto [lat, lon, alt] = geoPositionForCamera(); | |
| if (date.empty()) { | |
| date = global::timeManager->time().ISO8601(); | |
| } | |
| std::string dateTrim = date.substr(0, date.find('T')); | |
| std::string globe = | |
| global::navigationHandler->orbitalNavigator().anchorNode()->identifier(); | |
| alt = useCameraAltitude ? alt : 0.0; | |
| constexpr bool IsSunrise = false; | |
| return SpiceManager::ref().solarEventTime(lat, lon, alt, dateTrim, globe, IsSunrise); |
Comment on lines
+1670
to
+1671
| georec_c(lon, lat, altitudeKm, | ||
| equatorialRadius, flattening, observerPosition); |
Member
There was a problem hiding this comment.
Suggested change
| georec_c(lon, lat, altitudeKm, | |
| equatorialRadius, flattening, observerPosition); | |
| georec_c( | |
| lon, | |
| lat, | |
| altitudeKm, | |
| equatorialRadius, | |
| flattening, | |
| observerPosition | |
| ); |
| }; | ||
|
|
||
| // 3. Bisection search to find the solar event time | ||
| const double tolerance = 1.0; // seconds |
Member
There was a problem hiding this comment.
Suggested change
| const double tolerance = 1.0; // seconds | |
| constexpr double Tolerance = 1.0; // seconds |
| } | ||
|
|
||
| // Refine the bracket using bisection search | ||
| while (bracketsHigh - bracketsLow > tolerance) { |
Member
There was a problem hiding this comment.
Suggested change
| while (bracketsHigh - bracketsLow > tolerance) { | |
| while (bracketsHigh - bracketsLow > Tolerance) { |
| (elevationAtMidpoint >= -horizonDipAngle); | ||
| if (shouldUpdateLow) { | ||
| bracketsLow = midpoint; | ||
| } else { |
Member
There was a problem hiding this comment.
Suggested change
| } else { | |
| } | |
| else { |
| return 0.5 * (bracketsLow + bracketsHigh); // ET seconds past J2000 | ||
| } | ||
|
|
||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
These are the functions for getting the sunset/rise times. In separate PR I will add elements to the Night Sky panel that will let users see sunrise/set without having to script.
2026-08-18.18-46-12.mp4