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

Have effector receive hub state and parameter names automatically #720

Merged
merged 25 commits into from
Aug 19, 2024

Conversation

schaubh
Copy link
Contributor

@schaubh schaubh commented Jun 11, 2024

  • Review: By commit
  • Merge strategy: Merge (no squash)

Description

The state and dynamic effector require the State Machine state and parameter names of the spacecraft hub.
However, currently these are mostly hard coded when the effector subscribes to the particular states
or parameters from the state machine. A small number have strings that are set and then used to
subscribe.

This branch adds state and parameter names to the state and dynamic effector parent classes such that when
the effector is added to the spacecraft object with .addDynamicEffector() and .addStateEffector
the state machine names are set automatically from the spacecraft hub. There are no functional changes
with this branch.

Verification

Did a clean build and ran all tests. Also, the ultimate test here is being able to change a spacecraft
hub state name in python and still be able to run this code. This test completed successfully.

Documentation

Added release notes.

Future work

This branch cleans up the assignment of parent body state and property names to the effectors. Future work
will build on this to add some abilities to add selected state and dynamic effectors to particular other effectors.

@schaubh schaubh added the enhancement New feature or request label Jun 11, 2024
@schaubh schaubh self-assigned this Jun 11, 2024
@schaubh schaubh requested a review from a team as a code owner June 11, 2024 14:40
@schaubh schaubh added the dont merge Waiting on other changes before merging label Jun 11, 2024
@schaubh schaubh marked this pull request as draft June 11, 2024 14:40
@schaubh schaubh force-pushed the feature/auto-connect-effector branch from d91f40b to a72e95a Compare June 11, 2024 15:28
@schaubh schaubh requested a review from andrewmorell June 12, 2024 20:49
@schaubh schaubh removed the dont merge Waiting on other changes before merging label Jun 12, 2024
@schaubh schaubh force-pushed the feature/auto-connect-effector branch from d8c0ce5 to ad13f6a Compare June 13, 2024 15:15
@schaubh schaubh marked this pull request as ready for review June 13, 2024 16:17
@schaubh schaubh force-pushed the feature/auto-connect-effector branch from ad13f6a to 637569e Compare June 25, 2024 17:26
@schaubh schaubh added the dont merge Waiting on other changes before merging label Jun 26, 2024
@schaubh schaubh marked this pull request as draft June 26, 2024 20:15
@schaubh schaubh removed the dont merge Waiting on other changes before merging label Jul 25, 2024
@schaubh schaubh force-pushed the feature/auto-connect-effector branch 3 times, most recently from 39d2102 to 754e6a7 Compare August 1, 2024 19:49
@schaubh schaubh marked this pull request as ready for review August 1, 2024 20:09
@schaubh schaubh force-pushed the feature/auto-connect-effector branch 2 times, most recently from f732143 to 07169b4 Compare August 7, 2024 00:19
Copy link
Contributor

@andrewmorell andrewmorell left a comment

Choose a reason for hiding this comment

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

Just found a couple weird instances of formatting, otherwise looks great. I've built, tested, and generated the documentation successfully on my end.

@andrewmorell
Copy link
Contributor

Ah I went to go make the quick indentation changes myself and it turns out it is github's UI showing tabs different than four spaces. In my IDEs it looks normal, not sure whether this is worth us changing at the moment.

@andrewmorell andrewmorell force-pushed the feature/auto-connect-effector branch from 07169b4 to 7962373 Compare August 19, 2024 19:18
update spacecraft and spacecraftSystem to automatically set the hub state names within the dynamic effector
If the stateName variable is an empty string then this error message does not
make this apparent.  Edited the warning string to make this clearer.
These class variables are set when the addDynamicEffector() method is called.
The one unit test never used addDynamicEffector() to add the thruster to the spacecraft.
The script had to be edited to manually set the class state name variables now
for the parent rigid body.
update state effectors to register to the state named in the class state name variables
schaubh and others added 18 commits August 19, 2024 16:55
ensure that both spacecraft and spacecraftSystem create properties from these variables.
This will enable us next add effectors property names that are set automatically
when effectors are added to a spacecraft.
Pull these now from from the effector variables.
SpacecraftSystem already did this, but spacecraft wasn't updated apparently.
Need to be added to dynamic Effector base class
spacecraft(System) has been updated to add these to state effectors added to the spacecraft
They now pull the auto-assigned property names from the parent rigid body (i.e. spacecraft for now)
This just cause some odd HTML code to be displayed.  There variables don't have
units, so no need to designate it with --
This reduces code duplication and provides one place to add new names
Some methods provide the select parent body kinematic states.  However, the
method was then pulling these same states from the state engine.
In this commit this behavior is cleaned up such that only the provided
kinematic states are being used.
@schaubh schaubh force-pushed the feature/auto-connect-effector branch from 7962373 to 3b62a23 Compare August 19, 2024 22:56
@schaubh schaubh merged commit 35ce7f9 into develop Aug 19, 2024
7 checks passed
@schaubh schaubh deleted the feature/auto-connect-effector branch August 19, 2024 23:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants