Relative Motion Outline#214
Conversation
56221bc to
74cf1de
Compare
e9937e2 to
841d9fb
Compare
841d9fb to
f686acf
Compare
31aecce to
e1ca657
Compare
There was a problem hiding this comment.
What is this random unit vector used for?
There was a problem hiding this comment.
What does the hardcoded '5700.0' specify here?
There was a problem hiding this comment.
Opened a new issue to track this, since we use hardcoded 5700 as a normalization factor (approx. 1 orbit) in various places. #225
| """ | ||
| if np.linalg.norm(dv_N) > self.dv_available: | ||
| self.satellite.logger.warning( | ||
| f"Maneuver exceeds available Delta V ({np.linalg.norm(dv_N)}/{self.fuel_remaining} m/s)." |
There was a problem hiding this comment.
Doesn't this give you the percentage (above 100%) that the current requested dv maneuver exceeds the available amount by? This division shouldn't give m/s unless I am missing something.
There was a problem hiding this comment.
The print statement results in something like Maneuver exceeds available Delta V (3.5/1.2 m/s).
| f"Maneuver exceeds available Delta V ({np.linalg.norm(dv_N)}/{self.fuel_remaining} m/s)." | ||
| ) | ||
|
|
||
| self._dv_available -= np.linalg.norm(dv_N) |
There was a problem hiding this comment.
if np.linalg.norm(dv_N) > self.dv_available then it would not be subtracted from the _dv_available since it is not a valid/possible maneuver. I think if np.linalg.norm(dv_N) > self.dv_available then we need to pass and step out of the action_magic_thrust function right?
There was a problem hiding this comment.
Yeah, definitely should handle this more gracefully. Could just perform a dv_available sized thrust.
dhutererprats
left a comment
dhutererprats
left a comment
There was a problem hiding this comment.
I have left comments on some of the commits just to clarify... I don't thing they are necessarily things you need to change except in the location where it is checked if np.linalg.norm(dv_N) > self.dv_available and even if that is the case we reduce the amount of _dv_available. We should fire the maneuver if it exceeds are dv capability so we would not reduce future dv_availability.
Also I saw that the build gives some issues (like the python 3.11 or 3.10) but they dont seem to be relevant issues right.
There was a problem hiding this comment.
Not useful so not merging
|
Final PR commit plan:
|
|
Requires bsk commit first |
|
Actually implemented in #274 |
2 participants
This PR isn't going to be merged, breaking it into others. Will delete once all content is merged.