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

Added IKFast factory boilerplate #916

Merged
merged 7 commits into from Jul 30, 2023

Conversation

rjoomen
Copy link
Contributor

@rjoomen rjoomen commented Jul 21, 2023

This adds IKFast boilerplate code as suggested by @marip8.

@Levi-Armstrong
Copy link
Contributor

Just so this code gets tested could we updated the ikfast test plugins to utilize this?

@rjoomen
Copy link
Contributor Author

rjoomen commented Jul 23, 2023

Yes, that's a good idea. And the docs could then be simplified a lot (also, there's a number of errors/typos in the IKFast solver/plugin section, but those would be gone then).

@rjoomen
Copy link
Contributor Author

rjoomen commented Jul 23, 2023

BTW, I also had the idea to enhance the IKFast plugin boilerplate to use a discretization 'resolution' parameter for each free joint (making use of the joint limits from the URDF) instead of the list of exact joint values that is required currently. Because if you have for example a free joint with a 210 degree range and want to sample it in one degree steps, it requires a long and impractical list of values (I tested this).

@Levi-Armstrong
Copy link
Contributor

Sounds good to me.

@rjoomen
Copy link
Contributor Author

rjoomen commented Jul 24, 2023

Would you mind to already merge, and I'll add the free joint enhancements later?

@Levi-Armstrong
Copy link
Contributor

Waiting on the joint enhancement is good, but I would like to update the unit tests to leverage this so this code is unit tested before merging.

@rjoomen
Copy link
Contributor Author

rjoomen commented Jul 24, 2023

I updated the unit tests.

@Levi-Armstrong
Copy link
Contributor

It looks like clang-tidy is not happy

Co-authored-by: Tyler Marr <41449746+marrts@users.noreply.github.com>
@Levi-Armstrong
Copy link
Contributor

@rjoomen I think this code can be removed and should solve the clang-tidy issue.

@Levi-Armstrong
Copy link
Contributor

@rjoomen I think this code can be removed and should solve the clang-tidy issue.

Actually you may be able to just remove the -DIKFAST_HAS_LIBRARY from here.

@rjoomen
Copy link
Contributor Author

rjoomen commented Jul 28, 2023

Now all that's left are clang-tidy issues in the solvers and external/ikfast.h. Shouldn't these have been suppressed by TESSERACT_COMMON_IGNORE_WARNINGS_PUSH?

@Levi-Armstrong
Copy link
Contributor

I just pushed a commit which should fix the clang-tidy issue which was tested locally.

@Levi-Armstrong Levi-Armstrong merged commit 6601a7a into tesseract-robotics:master Jul 30, 2023
8 checks passed
@rjoomen
Copy link
Contributor Author

rjoomen commented Jul 31, 2023

Great, thanks!

@rjoomen rjoomen deleted the ikfast branch July 31, 2023 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants