Lamont endpoints #1729
Merged
Lamont endpoints #1729
+525
−9
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pleroy
reviewed
Feb 23, 2018
serialization/journal.proto
Outdated
| @@ -340,6 +346,57 @@ message EndInitialization { | |||
| optional In in = 1; | |||
| } | |||
|
|
|||
| // These functions form the external API of principia; announce deprecation one | |||
ksp_plugin/interface_external.cpp
Outdated
|
|
||
| // This is called "New" because it will eventually construct and release the | ||
| // message string. For now it just wraps the error code. | ||
| Status NewStatus(base::Status status) { |
ksp_plugin/interface_external.cpp
Outdated
|
|
||
| namespace { | ||
|
|
||
| // This is called "New" because it will eventually construct and release the |
There was a problem hiding this comment.
I would call in MakeStatus for now. Who knows what it will become once we have proper allocation/deallocation.
ksp_plugin/interface_external.cpp
Outdated
| u8" planned manœuvres")); | ||
| } | ||
| // The index of the coast segment following the desired manœuvre. | ||
| int const segment_index = manoeuvre_index * 2 + 3; |
ksp_plugin/interface_external.cpp
Outdated
| immobile_reference.Append(coast.Begin().time(), | ||
| {reference_position, Velocity<Navigation>{}}); | ||
| if (coast.Begin() != | ||
| coast.last()) { |
There was a problem hiding this comment.
Also, Empty would be more efficient.
| reference_position).Norm²() < | ||
| (coast.last().degrees_of_freedom().position() - | ||
| reference_position).Norm²(); | ||
| *world_body_centred_nearest_degrees_of_freedom = |
serialization/journal.proto
Outdated
| // These functions form the external API of principia; announce deprecation one | ||
| // lunation ahead, then make them return UNIMPLEMENTED. | ||
|
|
||
| message ExternalFlowFreefall { |
pleroy
reviewed
Feb 26, 2018
| {-100'000 * Kilo(Metre), 0 * Metre, 0 * Metre})).coordinates() / | ||
| Metre), | ||
| &result); | ||
| EXPECT_THAT(status.error, Eq(static_cast<int>(base::Error::OK))); |
There was a problem hiding this comment.
I would do the reverse:
EXPECT_THAT(static_cast<base::Error>(status.error), IsOk());
| auto const barycentric_result = | ||
| to_world.Inverse()(FromQP<RelativeDegreesOfFreedom<World>>(result)); | ||
| // The reference position is far above the apoapsis, so the result is roughly | ||
| // the apoapsis. |
There was a problem hiding this comment.
Can the apoapsis be deduced from the other parameters, to avoid hard-wiring numbers?
There was a problem hiding this comment.
It's not quite the apoapsis, so the test would get complicated. This is easy to understand (and with IsNear, it shouldn't break because of changes in the numerical methods).
ksp_plugin_test/test_plugin.cpp
Outdated
| Vessel& TestPlugin::AddVesselInEarthOrbit( | ||
| GUID const& vessel_id, | ||
| std::string const& vessel_name, | ||
| PartId part_id, |
ksp_plugin_test/test_plugin.cpp
Outdated
| SolarSystemFactory::Earth, | ||
| /*loaded=*/false, | ||
| inserted); | ||
| CHECK(inserted); |
| using physics::KeplerianElements; | ||
| using physics::SolarSystem; | ||
|
|
||
| class TestPlugin : public Plugin { |
There was a problem hiding this comment.
Maybe a short comment for the constructor and the method.
serialization/journal.proto
Outdated
| optional Return return = 3; | ||
| } | ||
|
|
||
| // Returns first point of the coast phase following the given manoeuvre which is |
|
LG, but let's make sure that Jenkins is still happy. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introducing an external API. 🚕
Feature request: #1659.
Currently untested, and one of the functions is unimplemented, however @lamont-granquist has at least been able to find it by reflection. Let us get this in master so that it does not get broken by other changes.
We should add a way to marshal a string within a returned struct; currently the status has no error message.