Add planar arm safety factor #267
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
abhaybd
requested changes
Oct 1, 2023
There was a problem hiding this comment.
Mostly looks good, but one thing I think we missed in our discussion. The set point should always be within the allowed region, so we should perform the same check (check if outside the allowed radius and if so then correct it) in set_setpoint() as well. Since we'll be doing the same check twice we should factor out that logic into a method called adjust_setpoint() or something. Lmk if you have any questions!
abhaybd
requested changes
Oct 3, 2023
abhaybd
requested changes
Oct 3, 2023
abhaybd
requested changes
Oct 10, 2023
abhaybd
requested changes
Oct 19, 2023
AyushKulk
pushed a commit
that referenced
this pull request
Oct 31, 2023
* Added safetyFactor check * Move safety factor constant into Constants * Add safetyFactor range limit assertion * Update planar arm tests to use safety factor * Set setpoint in safety factor test * Test overreaching arm is normalized * Bound resolved joint pos within safety factor * Added within factor test * Try shorter extension * Improved formatting * Remove now-obsolete todo * More formatting fixes * Use std abs over fabs in test * Move vector normalization code to own func * Add more logic to normalizeVectorWithinRadius * Fix return type * Add normalize functionality to set_setpoint * Actually compute unnormalized setpoint instead of magic num * Make normalizeVector private * Rename input vector to better reflect end effector * Make normalize func name more descriptive * Add actual vector comparison * Refactored redundant code * Remove unnecessary checks * Minor cleanup * Fixed tests * More cleanup * Improve formatting * More formatting improvements * Rm unnecessary INFO wrapping * Remove unused constant * Improve REQUIRE logging * Add safety factor logic in PlanarArmController constructor
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add overall length percentage bound to planar arm joints to prevent singularity lockup.