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

Add cubic spline interpolation header and example #14

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

kurogane1031
Copy link

Hi,
This is my first time attempting to contribute to an open source project, so my apologies for my lack of experience.
What I attempting to do is to add the cubic spline interpolation modules in this project.
Basically it's the same module as PythonRobotic cubicspline planner.
But i think the algorithm itself is not path planning, but an interpolation polynomial, therefore I've added a new workspace called Polynomial.

On the side note, seems the algorithm relies on linspace function, so I've created one in the common.h. The unit test for the linspace is included. Also, one thing iIve notice is somehow the deg2rad function causing compile error due to multiple definition, so i added inline to it.

Please let me know anything i can help to make the code better..

@giacomo-b
Copy link
Owner

Hi, thank you so much for contributing. It's weird that deg2rad was causing problems, it really shouldn't need to be inlined. I'll have to look into that.

I am approving the CI pipeline and will review the files as soon as I have more time!

@codecov-commenter
Copy link

Codecov Report

Merging #14 (f8bcf33) into master (f5044a3) will increase coverage by 22.50%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #14       +/-   ##
===========================================
+ Coverage   47.05%   69.56%   +22.50%     
===========================================
  Files           2        2               
  Lines          17       23        +6     
===========================================
+ Hits            8       16        +8     
+ Misses          9        7        -2     
Impacted Files Coverage Δ
include/robotics/common.h 100.00% <100.00%> (+100.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f5044a3...f8bcf33. Read the comment docs.

@kurogane1031
Copy link
Author

I forgot to mention that in my case, the multiple definition build error occurs when I'm attempting to build everything at once. It doesn't happen in standalone build.

Please let me know if I should remove the inline from the pull request. Also if I need to modify any other things from the code (other than the @codecov-commenter checks)

@giacomo-b
Copy link
Owner

The code doesn't seem to compile at the moment. Regarding the multiple definitions, can you describe the sequence of commands you run when you get the error? I can't seem to reproduce it. Thank you!

@kurogane1031
Copy link
Author

kurogane1031 commented Oct 5, 2021

The code doesn't seem to compile at the moment. Regarding the multiple definitions, can you describe the sequence of commands you run when you get the error? I can't seem to reproduce it. Thank you!

Sorry for late response. The multiple definition error only appear after I've added the linspace function and its test.

First I ran the following command

❯ cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -S test -B build/test

Then,

❯ cmake --build build/test
[ 20%] Building CXX object CMakeFiles/RoboticsTests.dir/source/common/linspace.cpp.o
[ 40%] Building CXX object CMakeFiles/RoboticsTests.dir/source/linear-control/linear-quadratic-regulator.cpp.o
[ 60%] Building CXX object CMakeFiles/RoboticsTests.dir/source/main.cpp.o
[ 80%] Building CXX object CMakeFiles/RoboticsTests.dir/source/robotics.cpp.o
[100%] Linking CXX executable RoboticsTests
/usr/bin/ld: CMakeFiles/RoboticsTests.dir/source/linear-control/linear-quadratic-regulator.cpp.o: in function `Robotics::deg2rad(double)':
linear-quadratic-regulator.cpp:(.text+0x0): multiple definition of `Robotics::deg2rad(double)'; CMakeFiles/RoboticsTests.dir/source/common/linspace.cpp.o:linspace.cpp:(.text+0x0): first defined here
collect2: error: ld returned 1 exit status
make[2]: *** [CMakeFiles/RoboticsTests.dir/build.make:145: RoboticsTests] Error 1
make[1]: *** [CMakeFiles/Makefile2:155: CMakeFiles/RoboticsTests.dir/all] Error 2
make: *** [Makefile:146: all] Error 2

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.

3 participants