Unclear description of AttitudeProvider behavior for ImpulsiveManeuver
Working with the ImpulsiveManeuver
class, I found that the docs do not make it entirely clear how deltaVSat
is interpreted when constructing the ImpulsiveManeuver
without an AttitudeProvider
, i.e. when using the 3-parameter constructor for an ImpulsiveManeuver
.
Say I am trying to construct an ImpulsiveManeuver
where I know the deltaVSat
is defined in VNC coordinates.
I can accomplish the desired behavior for the maneuver by using the 4-parameter constructor for an ImpulsiveManeuver
, providing a 'LofOffset' (constructed using satellite inertial frame and LOFType.VNC
) for the AttitudeProvider
and, and providing the deltaVSat
in VNC coordinates (also assuming I have correctly configured the trigger and specific impulse).
I can also accomplish the desired behavior for the maneuver using the 3-parameter constructor for an ImpulsiveManeuver
, but only if I provide deltaVSat
defined in the same inertial frame as the 'SpacecraftState' used to construct the propagator to which I add the maneuver.
After tooling with it as I have it makes sense to me that this would be the behavior, but I think it should be clearer from the get-go, i.e. in the docs, what happens if you don't provide an 'AttitudeProvider' for 'attitudeOverride'. As of now the docs state:
The maneuver is defined by a single velocity increment in satellite frame. The current attitude of the spacecraft, defined by the current spacecraft state, will be used to compute the velocity direction in inertial frame. A typical case for tangential maneuvers is to use a LOF aligned attitude provider for state propagation and a velocity increment along the +X satellite axis.
I think "in satellite frame" is not very clear. Nor is it clear that "the current attitude of the spacecraft..." is used only when no override is provided. Also, saying the sc's attitude "will be used to compute the velocity direction in inertial frame" seems inaccurate.
I propose that the docs be updated to read something like the following (formatting should be edited as appropriate):
The maneuver is defined by a single velocity increment. If no AttitudeProvider is given, the current attitude of the spacecraft, defined by the current spacecraft state, will be used as the AttitudeProvider, so the velocity increment should be given in the same pseudoinertial frame as the SpacecraftState used to construct the propagator that will handle the maneuver. If an AttitudeProvider is given, the velocity increment given should be defined appropriately in consideration of that provider. So, a typical case for tangential maneuvers is to provide a LOF aligned attitude provider along with a velocity increment defined in accordance with that LOF aligned attitude provider; e.g. if the LOF aligned attitude provider was constructed using LOFType.VNC, the velocity increment should be provided in VNC coordinates.