Skip to content
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

Merged
merged 34 commits into from
May 21, 2019
Merged

Changes to the FlightPlan API #2174

merged 34 commits into from
May 21, 2019

Conversation

pleroy
Copy link
Member

@pleroy pleroy commented May 19, 2019

  • Makes it possible to keep "broken" manœuvres in the flight plan.
    • Tracks the number of anomalous manœuvres in case of problems during integration.
    • The trajectories after the first anomalous one have length 0.
  • Reports errors to the caller using Status.
  • The functions that change manœuvres generally have an effect (which can include making parts of the plan anomalous), except if the input parameters are absurd.
  • This is only the C++ side of the changes, the interface tries to preserve compatibility.
  • This is the first step in addressing [Request] Multiple nodes editing #1936.


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;
Copy link
Member

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.

Copy link
Member Author

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;
Copy link
Member

@eggrobin eggrobin May 21, 2019

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 (anomalous_segments_ + 1) / 2 (0 for 1 or 2 anomalous segments, 1 for 3 because the segment preceding the last burn is anomalous).

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@eggrobin eggrobin added the LGTM label May 21, 2019
@pleroy pleroy merged commit 71cb0eb into mockingbirdnest:master May 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants