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

Refactor/856 refactor facet srp dynamic effector #865

Merged
merged 28 commits into from
Dec 9, 2024

Conversation

leahkiner
Copy link
Contributor

@leahkiner leahkiner commented Dec 3, 2024

Description

The facetSRPDynamicEffector module currently has a bug when mapping the facet normal vectors to the spacecraft body frame. The computed DCM from the facet frame to the body frame must be transposed to obtain the correct transformation. Along with fixing this issue, this PR cleans up the module code, unit test, and documentation.

A significant change this PR introduces to the module code is the requirement for the user to specify the initial facet attitudes relative to the spacecraft body frame. These DCMs are added to the FacetedSRPSpacecraftGeometryData structure and the vector is called facetDcm_F0BList. The second significant change this PR introduces is frame specification for the facet normal vectors and rotation axes. These vectors contained in the FacetedSRPSpacecraftGeometryData are renamed to facetNHat_FList and facetRotHat_FList. As these variables now indicate, these vectors are required to be specified in the facet F frames instead of the spacecraft body frame B.

Verification

The unit test for this module is updated to reflect these changes. An additional test is added to verify the SRP torque is correctly calculated.

Documentation

The documentation is updated to reflect these changes.

Future work

N/A

@leahkiner leahkiner added bug Something isn't working refactor Clean up with no new functionality labels Dec 3, 2024
@leahkiner leahkiner requested review from rcalaon and schaubh December 3, 2024 20:18
@leahkiner leahkiner self-assigned this Dec 3, 2024
@leahkiner leahkiner requested a review from a team as a code owner December 3, 2024 20:18
@leahkiner leahkiner linked an issue Dec 3, 2024 that may be closed by this pull request
@leahkiner leahkiner linked an issue Dec 3, 2024 that may be closed by this pull request
@leahkiner leahkiner force-pushed the refactor/856-refactor-facetSRPDynamicEffector branch 3 times, most recently from 96cb6b5 to efbf0b8 Compare December 4, 2024 19:50
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.

just small changes. Nice work with the refactor and bug fix.

To give users 1 year to update to using setters and getters, let's leave the two variables public for now, but warn the user about directly setting them being deprecated and will vanish Dec. 2025.

@leahkiner leahkiner force-pushed the refactor/856-refactor-facetSRPDynamicEffector branch 2 times, most recently from 999c710 to cd82174 Compare December 6, 2024 19:53
@leahkiner leahkiner requested a review from schaubh December 6, 2024 20:02
@leahkiner
Copy link
Contributor Author

@schaubh This branch is ready for a re-review. I have fixed all of your requested changes.

@leahkiner leahkiner force-pushed the refactor/856-refactor-facetSRPDynamicEffector branch from cd82174 to 82100a0 Compare December 6, 2024 20:05
examples/scenarioSepMomentumManagement.py Outdated Show resolved Hide resolved
examples/scenarioSepMomentumManagement.py Outdated Show resolved Hide resolved
@schaubh schaubh force-pushed the refactor/856-refactor-facetSRPDynamicEffector branch from 7a3845e to 45d1fec Compare December 8, 2024 16:30
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.

need to rebase on latest develop. This will make the Window CI build work again. Otherwise good to go.

@leahkiner leahkiner force-pushed the refactor/856-refactor-facetSRPDynamicEffector branch from 45d1fec to 7143d6c Compare December 9, 2024 17:50
@leahkiner leahkiner merged commit a22790b into develop Dec 9, 2024
9 checks passed
@leahkiner leahkiner deleted the refactor/856-refactor-facetSRPDynamicEffector branch December 9, 2024 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working refactor Clean up with no new functionality
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Refactor facetSRPDynamicEffector module
3 participants