Implement hardstops for spinningBodiesOneDOF #646
Conversation
|
@joaogvcarneiro , could you please take a look at this PR to your module? Thanks. |
@joaogvcarneiro , final finals over now, have you had a chance to look at this PR? Thanks for your review. |
There was a problem hiding this comment.
Finally got to look at this full branch. Sorry for the delay, this PR definitely got lost in the cracks this summer.
I like your contribution and added some comments. Overall clean work, but one unit test doesn't appear to be working yet. Also, if something is off I'd like to use the BSK_ERROR message that the BSK software can control. If you have feedback on this last request, glad to chat more :-)
Please rebase your branch on the latest develop as well. I just rebased my copy and didn't have any issues.
Thanks again for supporting the BSK framework. I appreciate this contribution and think these limits will be of interest to the community.
| spinningBody.theta_min = np.deg2rad(min_lim) | ||
|
|
||
| with pytest.raises(ValueError): | ||
| spinningBody.Reset(0) |
There was a problem hiding this comment.
This test doesn't have an assert statement? It passes regardless of what values I put in it?
There was a problem hiding this comment.
Squash this with the prior commits to clean up the commit history.
| torque = torque_log.motorTorque[:, 0] | ||
| thetaDot = spinning_body_log.thetaDot | ||
| omega = sc_log.omega_BN_B | ||
|
|
There was a problem hiding this comment.
please close all prior plots before making new plots. This is helpful when running lots of tests at once.
| @@ -310,8 +310,8 @@ def test_spinning_body_enforces_limits(max_lim, min_lim): | |||
| spinningBody.theta_max = np.deg2rad(max_lim) | |||
| spinningBody.theta_min = np.deg2rad(min_lim) | |||
|
|
|||
| with pytest.raises(ValueError): | |||
| spinningBody.Reset(0) | |||
| # with pytest.raises(ValueError) as exc: | |||
There was a problem hiding this comment.
please split and squash these commits to the final version to clean up the commit history.
| @@ -70,13 +70,14 @@ void SpinningBodyOneDOFStateEffector::Reset(uint64_t CurrentClock) | |||
| } | |||
| // Ensure user specified valid angular limits | |||
| if (this->theta_max < this->theta_min) { | |||
| bskLogger.bskLog( | |||
| BSK_ERROR, "theta_max (%f) must be greater than theta_min (%f).", this->theta_max, this->theta_min); | |||
| throw std::invalid_argument("theta_max (" + std::to_string(this->theta_max) | |||
There was a problem hiding this comment.
I would like to keep the BSK_ERROR message as well, as this is expected behavior of a BSK module across the framework. I don't mind if you also include the throw argument as well.
|
|
||
| #include <stdexcept> | ||
|
|
||
| %exception { |
There was a problem hiding this comment.
I'm not following what this swig code is doing? Could you please add some comments on what is being done here?
|
Howdy @ascended121 , just checking that you are still actively pursuing this PR? Thanks for the quick feedback. The requested edits are fairly minor. |
|
Howdy @ascended121 , just a quick ping to see if you are still active on this branch? If not, I can try to pull this branch and make the required fixes before pushing? Thanks for the quick feedback. |
Description
It is not uncommon for realistic actuators have a non-finite range of motion (ie, if they don't use slip-rings and have cable bundles running through them which cannot rotate, or if there would be mechanical interferences with other parts of the structure at certain angles). Modeling these limits can be important to the practical use of these actuators and associated bodies.
To increase the fidelity of the
spinningBodyOneDOF, this PR adds (optional) angular limits to the model. Oncethetareaches these locations, thespinningBodyOneDOFwill not move further than the boundary.These limits are set to +/-Inf by default, so that they have no effect unless the caller explicitly sets them to a smaller range.
Verification
Added unit tests verifying that the model respects various configurations of the limits. Existing tests verify that not specifying the limits does not impact default behavior of model.
Documentation
Updated rst file associated with model to document the existence/example usage of these limits.
Future work
Its not of interest for my work, but for high fidelity dynamics simulation it would be possible to model the interaction of the spinningBody with the hardstop (by specifying the mechanical stiffness/damping of the angular limit).
Also, it seems that raising an exception (to stop the program) is the proper response for an invalid configuration (rather than just logging an exception, which the user might ignore (either intentionally or accidentally) and get invalid results). I implemented the ability to propagate a C++ exception to python in the SWIG wrapper for this function. Perhaps it would make sense to generalize this exception handling to make it more widely available to other basilisk modules.