-
Notifications
You must be signed in to change notification settings - Fork 173
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Added AllowError option to campt #5437
Conversation
Gonna tag @acpaquette here since I think he is best to review this. |
@amystamile-usgs So far this looks good but I think it needs a little more. While campt will no longer fail it seems like the desired output won't be provided, specifically the ephemeris data. I may be wrong but we will need to expose the ephemeris data somewhere around here in CameraPointInfo.cpp Given the current changes, does the ephemeris data get reported even on error? |
@acpaquette This is the output with these changes:
|
So even if we don't get a target intersection we still know where the spacecraft is, where the sun is, as well as the various look directions. Rather than those being NULL they should be some value |
Gotcha. Okay I will look into adding this. |
@acpaquette Here is the new output with my most recent push. Let me know if this is what you had in mind.
|
Overall looks great! I don't fully understand how you are able to get lats and lons as well as a bodyfixed coordinate if you don't intersect the target. The resolutions also don't seem to make much sense. Could we make sure that those values aren't being populated by some previous |
@acpaquette SetImage failed so it was using a previous SetImage from LoadCache. So I just changed it to only grab SpacecraftPosition, SunPosition, and LookDirection. New Output:
|
(*gp)[i].addValue("NULL"); | ||
(*gp)[i].addValue("NULL"); | ||
(*gp)[i].addValue("NULL"); | ||
if (!allowErrors) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic seems like it will put all Nulls in a normal campt case if allowErrors isn't enabled. It seems like the logic should be !allowErrors && !noErrors
I can pull the branch and test this locally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realize this was nested. Scratch my comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, all campt tests passed locally. This may break other tests so I will fire up a AWS EC2 to run tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests look good!
Description
Related Issue
closes #5393
How Has This Been Validated?
Types of changes
Checklist:
Licensing
This project is mostly composed of free and unencumbered software released into the public domain, and we are unlikely to accept contributions that are not also released into the public domain. Somewhere near the top of each file should have these words: