Optimization with Time Flag by Eesha-Jain · Pull Request #60 · UWCubeSat/found

Eesha-Jain · 2025-11-15T22:25:24Z

Description

Resolves #44

Adds a time flag request to found's distance command that marks when the image was taken (in nanoseconds). This allows for a more accurate understanding of the satellite's latitude and longitude. Currently outputs time into the outputted .found file.

This PR also modifies the output of output.cpp's GetEarthCoordinates function to be ECEF coordinates. This is the standard set for representing position by NASA (example: https://ntrs.nasa.gov/api/citations/20190002515/downloads/20190002515.pdf) and other space grade receivers.

Note that the nanoseconds since epoch is being stored as an unsigned 64-bit integer throughout the code. This can reliably store up until the year 2557.

Artifacts for PR #60 (DO NOT CHANGE)

… some tests

…void leaks

Copilot

Pull request overview

This pull request adds a --image-time flag to the distance command that records when an image was captured, enabling more accurate satellite position calculations by using the actual image capture time instead of the current system time. The timestamp is stored in the output .found file.

Adds new strtodatetime converter function to parse datetime strings in YYYY-MM-DD HH:MM:SS format
Replaces automatic timestamp generation with user-provided image capture time in position records
Updates all distance command tests and README documentation to include the new time parameter

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/providers/converters.hpp	Adds `strtodatetime` function to convert string timestamps to DateTime structures
src/command-line/parsing/options.hpp	Adds `image-time` option definition for the distance command
src/command-line/execution/executors.cpp	Modifies OutputResults to use provided imageTime instead of calling getUT1Time()
test/integration/integration-test.cpp	Updates integration tests to include the new --image-time parameter and removes obsolete test
test/command-line/parsing/parser-test.cpp	Adds parser tests for the new imageTime field
test/command-line/execution/executors-test.cpp	Updates executor test to include sample imageTime data
README.md	Updates documentation and examples to show usage of the new --image-time flag

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

nguy8tri

This is by no means comprehensive, but great job, just left some comments!

nguy8tri

Good job on the fixes. This time, my comments are more towards having you think critically about some code changes you've made. Please respond to my comments as appropriate. Also if it takes a long time for me to review your code, ping me on slack so I can make time for it.

j4lando · 2026-03-03T20:53:45Z

It might be better to replace celestial coordinate with equatorial coordinate since equatorial coordinates are a specific definition of celestial coordinates.

Eesha-Jain · 2026-03-24T19:48:31Z

It seems that you changed celestial to equatorial in your PR, so I won't implement that here to prevent merge conflicts in the future

nguy8tri

Good job, just a few more minor comments.

nguy8tri · 2026-03-28T01:13:09Z

-decimal getGreenwichMeanSiderealTime(std::time_t epochs) {
-    return 15 * (DECIMAL(18.697374558) + DECIMAL(24.06570982441908) * (getJulianDateTime(epochs) - DECIMAL(2451545.0)));
+decimal getGreenwichMeanSiderealTime(uint64_t epochs) {
+    return 15 * (DECIMAL(18.697374558) + DECIMAL(24.06570982441908) *


We should have constants for these 3

nguy8tri · 2026-03-28T01:16:22Z

 // This is preferable than continuing with invalid data and outputting an unintended
 // result.

+constexpr int days_in_month[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};


scope to the namespace

nguy8tri · 2026-03-28T01:16:47Z

+    // Convert to nanoseconds: seconds * NS_PER_SEC + nanoseconds
+    uint64_t epochs_ns = static_cast<uint64_t>(t) * NS_PER_SEC + nanosecond;
+
+    return {


Are we sure there isn't a library function that will do this for us? Please look into that

nguy8tri · 2026-03-28T01:17:44Z

-FOUND_CLI_OPTION("isdda-radius-loss-order" , int               , ISDDARadLossOrd , 4                          , atoi(optarg)               , kNoDefaultArgument, REQ_ASSIGN, "The Radius Loss Order ISSDA (even int)"                       ) \
-FOUND_CLI_OPTION("output-file"             , std::string       , outputFile      , ""                         , optarg                     , kNoDefaultArgument, REQ_ASSIGN, "The output file (*.found)"                                    ) \
+FOUND_CLI_OPTION("image"                   , found::Image      , image           , {}                          , found::strtoimage(optarg)   , kNoDefaultArgument, REQ_ASSIGN, "The image to process (JPG, PNG, TGA, BMP, PSD, GIF, HDR, PIC)") \
+FOUND_CLI_OPTION("image-time"              , found::DateTime   , imageTime       , {}                          , found::strtodatetime(optarg), kNoDefaultArgument, REQ_ASSIGN, "Image capture time (in UT1 in format YYYY-MM-DD HH:MM:SS.NS)" ) \


Its unclear how many nano seconds, or if its required. It may add value to be flexible about the format, maybe not. Usually libraries will parse a string into a date.

nguy8tri · 2026-03-28T01:18:44Z


+    // Test Time (epochs in nanoseconds: 1762889400 seconds * NS_PER_SEC)
+    DateTime expectedImageTime{1762889400000000000ULL, 2025, 11, 11, 19, 30, 0};
+    ASSERT_EQ(expectedImageTime.epochs, options.imageTime.epochs);


Make a helper macro that will assert over an entire Date time so we can do something like ASSET_TIME(expected, actual)

nguy8tri · 2026-03-28T01:19:10Z

    ASSERT_GE(num, lo) << "Value " << num << " is less than lower bound " << lo; \
    ASSERT_LE(num, hi) << "Value " << num << " is greater than upper bound " << hi;

+#define ASSERT_DATETIME_EQ(expected, actual) \


Ah here it is

nguy8tri · 2026-03-28T01:20:13Z

    --camera-focal-length 85e-3                    \
    --camera-pixel-size 20e-6                      \
-    --reference-orientation 110,0,0
+    --reference-orientation 110,0,0                \


no, there should not have a line continuation at the last one.

Your Name and others added 2 commits November 12, 2025 19:23

initial implementation of adding time as a flag - still needs to past…

0984325

… some tests

change data types to pass tests

a1b73cc

Eesha-Jain linked an issue Nov 15, 2025 that may be closed by this pull request

Enhancement/Optimization: Time Flag #44

Open

Eesha-Jain added 5 commits November 17, 2025 06:06

parser: reset optind per-parse and free image on parse exception to a…

49affdf

…void leaks

reverting some changes to debug

575cd8d

including import statement

af11966

debugging tests

5449426

updating argc values

a966fa2

j4lando reviewed Nov 19, 2025

View reviewed changes

Comment thread test/integration/integration-test.cpp

j4lando reviewed Nov 19, 2025

View reviewed changes

Comment thread README.md Outdated

Your Name and others added 2 commits December 14, 2025 23:38

compatibility with nanoseconds

399c18f

Update image-time format in README

84d6b01

Copilot AI review requested due to automatic review settings December 14, 2025 23:41

Copilot started reviewing on behalf of Eesha-Jain December 14, 2025 23:41 View session

resolving merge conflicts

0f5b974

Copilot AI reviewed Dec 14, 2025

View reviewed changes

Comment thread README.md

Comment thread README.md

Comment thread README.md

Comment thread src/providers/converters.hpp

Comment thread README.md Outdated

Eesha-Jain added 4 commits December 20, 2025 12:11

representing position in ecef

b03d046

improving celestial to ECEF conversion and developing tests

51e1245

updating from UTC to UT1

4a0b6fb

changing floats into decimals to pass float test

3350703

nguy8tri requested changes Jan 9, 2026

View reviewed changes

implementing feedback from tri

15dac5b

nguy8tri requested changes Feb 2, 2026

View reviewed changes

Eesha-Jain and others added 2 commits March 24, 2026 13:08

implementing tri's comments

737ed40

attempting to trigger build

1d8013f

Eesha-Jain force-pushed the optimization-with-time-flag branch from 385ce43 to 1d8013f Compare March 24, 2026 20:44

Eesha-Jain added 2 commits March 24, 2026 20:52

fix minor float issue

6b04f7b

modifications to pass test in float

813e10b

changing tolerance for julian dates

92d8982

Eesha-Jain requested a review from nguy8tri March 26, 2026 18:34

nguy8tri requested changes Mar 28, 2026

View reviewed changes

Conversation

Eesha-Jain commented Nov 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Artifacts for PR #60 (DO NOT CHANGE)

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nguy8tri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nguy8tri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

j4lando commented Mar 3, 2026

Uh oh!

Eesha-Jain commented Mar 24, 2026

Uh oh!

nguy8tri left a comment

Choose a reason for hiding this comment

Uh oh!

nguy8tri Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

nguy8tri Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

nguy8tri Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

nguy8tri Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

nguy8tri Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

nguy8tri Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

nguy8tri Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Eesha-Jain commented Nov 15, 2025 •

edited

Loading