[mdrum2304]

• Tickets addressed: #000
• Review: By commit
• Merge strategy: Merge (no squash)

Description

It is desired to enable state effector branching off of a prescribed motion component. This issue will configure the linearSpringMassDamper module for this capability.

Verification

The prescribed branching is tested in the test_PrescribedMotionStateEffectorBranching.py file within the prescribedMotion/UnitTest folder. The test verifies that energy is conserved within tolerance and the motion behaves as expected.

Documentation

N/A

Future work

N/A

[leahkiner]

Nice PR, there are just a few small changes you should make before we merge.

[leahkiner]

Looks good, thanks for fixing those small things. I requested one more change and also a bunch of your commits should be squashed before we merge. One commit should contain everything added to the effector files to enable branching, and another can be all the unit test changes. So commit 3 can be split and the test part can be squashed with commit 4 for the unit test changes. The other part of commit 3 can be squashed with commits 1, 2, 5, 6, and 8 for the changes to the effector. Commit 7 can be kept separate. You can delete the last commit (see my previous comment). Also please add a release note snippet as the final commit.

[schaubh]

Reviewed b46e275. Requesting changes for the following five findings; inline comments cover findings 1, 2, 4, and 5.

1. [P1] Configurable hub state names cause a segmentation fault. linkInStates() hard-codes hubOmega. Setting sc_object.hub.nameOfHubOmega = "customOmega" before initialization produces a missing-state warning and then a null-pointer crash in the coupling callback. Use the configured state name or angular velocity supplied through the dynamics callbacks.

2. [P2] The new test passes without the spring attachment. Removing prescribed_platform.addStateEffector(smd) still passes. The test never verifies rho or rhoDot, its collinear geometry leaves rotational coupling terms unexercised, and conservation checks only compare endpoints. The baseline transient relative angular-momentum error reaches approximately 4.85e-5 despite passing the endpoint tolerance of 1e-6. Add independent particle-response checks, non-collinear configurations, and full-history conservation checks with justified tolerances.

3. [P2] User documentation contradicts the new capability. src/simulation/dynamics/prescribedMotion/prescribedMotionStateEffector.rst:90-96 still lists only three supported effectors, and its testing section describes three tests. Update that guide and add a spring-module attachment example explaining the prescribed-parent-frame interpretation of pHat_B and r_PB_B. Retain the restriction against nested variable-mass operation. A release-note snippet alone does not explain how to use this feature.

4. [P3] New API methods lack parameter documentation. linkInPrescribedMotionProperties() and addPrescribedMotionCouplingContributions() have brief comments but no @param sections. Generated Doxygen XML confirms these sections are absent. Document the arguments, contribution frame, and initialization/call prerequisites.

5. [P3] New test literals need correct unit comments. Earth's gravitational parameter is labeled meters instead of m^3/s^2; the new timestep and profiler duration literals also need unit comments under AGENTS.md.

Validation: rebuilt the affected targets, compiled the header independently, and ran 276 focused tests successfully. An independent Newton/Euler check over 20 arbitrary geometries found force, torque, and spring-equation residuals below 2e-13, supporting the coupling algebra. Sphinx/Doxygen previews rendered with expected single-page cross-reference warnings. The full repository suite and full documentation build were not run.

[schaubh]

1. [P1] Avoid hard-coding the hub angular-velocity state. Renaming the hub state to customOmega before initialization produces a missing-state warning and a segmentation fault at the later hubOmega->stateView() call. Propagate the configured state name or obtain angular velocity through the dynamics callback, and add a renamed-state regression test.

[schaubh]

pushed a fix, @mdrum2304 , please review.

[schaubh]

2. [P2] Assert the attached particle's actual response. This test still passes when prescribed_platform.addStateEffector(smd) is removed. Record rho and rhoDot and compare against an independent reference; include non-collinear geometry and check conservation across the time history with justified tolerances.

[schaubh]

pushed a fix, @mdrum2304 , please review.

[schaubh]

4. [P3] Document the new callback parameters. Neither new callback produces a parameter section in Doxygen XML. Add @param documentation for backSubContr and for states in linkInPrescribedMotionProperties(), describing frames and required initialization/call order.

[schaubh]

pushed a fix, @mdrum2304 , please review.

[schaubh]

5. [P3] Correct and complete physical unit comments. The gravitational parameter has units m^3/s^2, not meters. Also annotate the newly added timestep and profiler duration literals with their units, as required by AGENTS.md.

[schaubh]

pushed a fix, @mdrum2304 , please review.

[schaubh]

@mdrum2304, thanks for the contribution. I pushed a few review comments and pushed commits to address each one of them. Please review and let me know if you agree or see issues. If you are ok with these changes, I'll go ahead and approve.