gz-math icon indicating copy to clipboard operation
gz-math copied to clipboard

Added Conditions for Slerp vs Squad interpolation

Open sdhar04 opened this issue 2 years ago β€’ 12 comments

πŸŽ‰ New feature

Closes #568

Summary

I have added conditionals for using the Slerp algorithm when the difference in final and initial orientations falls within a threshold. Otherwise Squad is invoked.

Test it

You can open Gazebo and run the bash script I have added in gz-math/test which publishes a new position and orientation to the camera every 2 seconds.

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

πŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”ΈπŸ”Έ

sdhar04 avatar Feb 07 '24 23:02 sdhar04

@ahcorde I have removed the bash script

sdhar04 avatar Mar 10 '24 17:03 sdhar04

@sdhar04 Can you address these lint issues?

  /github/workspace/src/RotationSpline.cc:95:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
  /github/workspace/src/RotationSpline.cc:98:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
  /github/workspace/src/RotationSpline.cc:102:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]
  /github/workspace/src/RotationSpline.cc:104:  Missing spaces around <  [whitespace/operators] [3]
  /github/workspace/src/RotationSpline.cc:108:  Line ends in whitespace.  Consider deleting these extra spaces.  [whitespace/end_of_line] [4]

mjcarroll avatar Mar 28 '24 19:03 mjcarroll

Codecov Report

Attention: Patch coverage is 87.50000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 94.11%. Comparing base (91c73b9) to head (7dfe977).

:exclamation: Current head 7dfe977 differs from pull request most recent head cf8b74c. Consider uploading reports for the commit cf8b74c to get more accurate results

Files Patch % Lines
src/RotationSpline.cc 87.50% 1 Missing :warning:
Additional details and impacted files
@@             Coverage Diff              @@
##           gz-math7     #577      +/-   ##
============================================
- Coverage     94.11%   94.11%   -0.01%     
============================================
  Files           146      146              
  Lines          9809     9816       +7     
============================================
+ Hits           9232     9238       +6     
- Misses          577      578       +1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 28 '24 20:03 codecov[bot]

@mjcarroll @azeey I have addressed the lint issues and reduced the scope of 2 variables only required for Squad interpolation. However, the tests don't seem to invoke the cases where Slerp is used, hence the codecov warnings. I have a bash script I used during testing to supply target orientations and both algorithms are used in my test cases (I checked by printing which one was invoked), with the interpolation being desirably smooth. I have seen the tests and they seem to check for the midpoint of an interpolation. I don't know what the midpoint would be for a given Slerp interpolation, although I could try to figure things out by printing values while interpolating. Please guide me regarding what to do now. Do I need to change the RotationSpline_TEST.cc file only?

sdhar04 avatar Mar 28 '24 22:03 sdhar04

Hi @sdhar04 , was wondering what the status on this was. Do we intend shipping this change with ionic (code freeze 26/8)? If we can't come to a consensus by then I will remove the beta label (I personally would prefer seeing it in this release).

arjo129 avatar Aug 12 '24 01:08 arjo129

Hi @sdhar04 as we haven't heard back from you, I'm removing the beta label. @ahcorde's suggestions still apply.

arjo129 avatar Aug 19 '24 09:08 arjo129