-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat: enrich StateBuilder capabilities #239
Conversation
Codecov Report
@@ Coverage Diff @@
## main #239 +/- ##
==========================================
+ Coverage 83.01% 83.23% +0.22%
==========================================
Files 63 63
Lines 4804 4874 +70
==========================================
+ Hits 3988 4057 +69
- Misses 816 817 +1
|
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.
These are good and I think we should keep them, but I think we will also need some additional functions to operate on the State.
I.e. given a high dimensional State and a low dimensional Builder, produce a low dimensional state
also:
given a low dimensional state, a high dimensional builder, and a default values state, produce a high dimensional state
include/OpenSpaceToolkit/Astrodynamics/Trajectory/StateBuilder.hpp
Outdated
Show resolved
Hide resolved
test/OpenSpaceToolkit/Astrodynamics/Trajectory/StateBuilder.test.cpp
Outdated
Show resolved
Hide resolved
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.
small nit suggestions for error printing (can just implement them on the base MR if it's annoying to rebase)
test/OpenSpaceToolkit/Astrodynamics/Trajectory/StateBuilder.test.cpp
Outdated
Show resolved
Hide resolved
0600f2a
to
00aa429
Compare
000a8cd
to
b969467
Compare
13f5ab5
to
250ed7a
Compare
5abe6fd
to
ff6b1ce
Compare
Rebased and ready for review, see updated description. |
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 will let @kyle-cochran have the final approval but this looks great to me!
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.
2 small comments, otherwise lgtm!
Add
StateBuilder
features:Additional:
buildState
->build
State
== comparator was not assessing number of coordinates