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

Update thruster documentation #524

Merged
merged 1 commit into from
Dec 7, 2023

Conversation

joaogvcarneiro
Copy link
Contributor

Description

Added a paragraph that explains how some instances of _B in message variables actually denote _F.

Verification

N/A.

Documentation

See description.

Future work

N/A.

@joaogvcarneiro joaogvcarneiro added the documentation Improvements or additions to documentation label Dec 5, 2023
@joaogvcarneiro joaogvcarneiro self-assigned this Dec 5, 2023
Copy link
Contributor

@schaubh schaubh left a comment

Choose a reason for hiding this comment

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

I pushed some suggested edits to the documentations. I feel we should be more explicit here in what is happening under the hood, and explain better that _B is technically correct, as both output solutions have the 3 variables in the body frame. We just have two body-fixed frames to consider.

Further, I also state that this is only applicable currently to prescribed motion effectors. Please look over, edits if needed, and merge with your commit to do a revision review of this PR.

Also, this PR should be rebased before being put up for review :-)

@joaogvcarneiro
Copy link
Contributor Author

Thanks for taking a look @schaubh! I missed that this wasn't on top of the latest develop branch.

I'm wondering if it's a good idea or not to make it specific in the documentation that this only applies to the prescribedMotionStateEffector. I know that from a dynamics standpoint, this approach is only true for prescribed bodies, but you can technically attach the thruster to any type of effector. What do you think about making this point a little more explicit?

@schaubh
Copy link
Contributor

schaubh commented Dec 5, 2023 via email

@rcalaon rcalaon force-pushed the refactor/update-thruster-documentation branch from 79e5737 to fea0769 Compare December 7, 2023 17:06
Copy link
Contributor

@schaubh schaubh left a comment

Choose a reason for hiding this comment

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

Good to go.

@joaogvcarneiro joaogvcarneiro merged commit 8d12915 into develop Dec 7, 2023
2 checks passed
@joaogvcarneiro joaogvcarneiro deleted the refactor/update-thruster-documentation branch December 7, 2023 17:39
@joaogvcarneiro joaogvcarneiro linked an issue Dec 7, 2023 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Update thruster documentation
3 participants