-
Notifications
You must be signed in to change notification settings - Fork 69
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
Changes to the FlightPlan API #2174
Conversation
|
||
static constexpr Error bad_desired_final_time = Error::INVALID_ARGUMENT; | ||
static constexpr Error does_not_fit = Error::INVALID_ARGUMENT; | ||
static constexpr Error singular = Error::INVALID_ARGUMENT; |
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.
The fact that these three are equal is going to lead to a lot of confusion, especially since we test for specific statuses in the interface.
does_not_fit
and bad_desired_final_time
are both FAILED_PRECONDITION
-like (they are about the compatibility of the argument with the flight plan, rather than the argument being intrinsically bad like manœuvre.IsSingular()
).
Make one of them OUT_OF_RANGE
and we will have distinct statuses.
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.
Changed the first two to be OUT_OF_RANGE
.
// that this returns 1 for a flight plan without manœuvres if the first coast | ||
// fails to integrate. The functions that change manœuvres may change the | ||
// number of anomalous manœuvres. | ||
virtual int number_of_anomalous_manœuvres() const; |
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.
Ifnumber_of_anomalous_manœuvres
can be less than number_of_manœuvres
, there is a naming problem.
Let us define an anomalous manœuvre as one whose burn integration cannot be started (because it starts after an anomalous segment). This becomes
(anomalous_segments_ - 1) / 2
, rather than (0 for 1 or 2 anomalous segments, 1 for 3 because the segment preceding the last burn is anomalous).(anomalous_segments_ + 1) / 2
This is only used outside of tests as number_of_anomalous_manœuvres() <= 1
, which should be changed to number_of_anomalous_manœuvres() == 0
: the existing semantics are that there are no anomalous manœuvres, that is, the integration of their burn can at least start, and they are editable in the UI.
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.
Done.
Status
.