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.

Sign up for GitHub

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

Jump to bottom

Fix linker error caused by templating in the conversions.cpp file #718

Merged
merged 1 commit into from
Apr 26, 2022
Merged

Fix linker error caused by templating in the conversions.cpp file #718

merged 1 commit into from
Apr 26, 2022

Conversation

flynneva
Copy link
Contributor

@flynneva flynneva commented Jan 4, 2022

Fixes linker error found by @simwijs on this thread here

Essentially I made the mistake of including template implementations in the .cpp file when they all need to be in the header.

Signed-off-by: Evan Flynn evan.flynn@apex.ai

@flynneva
Copy link
Contributor Author

flynneva commented Jan 4, 2022

@simwijs let me know if this fix solves your issue - I tested it locally using the cmake flags you mentioned and it seemed to work I think.

@simwijs
Copy link

simwijs commented Jan 4, 2022

You're fast lol 👍
I'll test it right away!

flynneva referenced this pull request Jan 4, 2022
@flynneva
move to hpp/cpp structure, create conversions file
635bbb9
@simwijs
Copy link

simwijs commented Jan 4, 2022 •
edited
Loading

Works beautifully. You have my 🥇
Nice to see an engineer in action :) Keep moving forward! Also glad to have contributed something, even though you made the fix. I learned from it! (I spent way too much time finding this error and trying to fix it when you fixed it in minutes haha)

@flynneva
Copy link
Contributor Author

flynneva commented Jan 4, 2022

@simwijs you give me too much credit 😅 but glad to help out!

This is what open source is all about - finding bugs you otherwise would miss and fixing them together.

@flynneva
Copy link
Contributor Author

flynneva commented Jan 8, 2022

@JWhitleyWork friendly ping, fixes a hidden bug in the ros2 depth_image_proc package (template implementation in cpp file instead of all in the header)

@clalancette clalancette changed the base branch from ros2 to rolling April 26, 2022 18:06
@evan-flynn-apexai @clalancette
Fix linker error caused by templating in the conversions.cpp file
4bf4f4f 
Signed-off-by: Evan Flynn <evan.flynn@apex.ai>
@clalancette clalancette force-pushed the ros2-fix-conversions-linker-error branch from 1afe9e6 to 4bf4f4f Compare April 26, 2022 18:07
@clalancette
Copy link
Contributor

CI is failing, but that has nothing to do with this PR. We'll fix that up separately. This looks like a good fix to me, so merging it in.

@clalancette clalancette merged commit 0fa817a into ros-perception:rolling Apr 26, 2022
@quentingllmt quentingllmt mentioned this pull request May 22, 2022
Closed
@jplapp jplapp mentioned this pull request Jun 21, 2022
Merged
omar-abugharbiyeh added a commit to logivations/image_pipeline that referenced this pull request Jun 21, 2022
@omar-abugharbiyeh
Fix linker error caused by templating in the conversions.cpp file (ro…
c422a42 
…s-perception#718)

Signed-off-by: Evan Flynn <evan.flynn@apex.ai>
(cherry picked from commit 0fa817a)
@JWhitleyWork
Copy link
Collaborator

@flynneva Would you mind back-porting this to Foxy and Galactic? It appears to be present there too based on #696.

@flynneva
Copy link
Contributor Author

flynneva commented Jul 7, 2022

@JWhitleyWork id be happy to! Sorry for causing such a stink with this.

flynneva added a commit to flynneva/image_pipeline that referenced this pull request Jul 7, 2022
@flynneva @evan-flynn-apexai
Fix linker error caused by templating in the conversions.cpp file (ro…
c5a56a4 
…s-perception#718)

Signed-off-by: Evan Flynn <evan.flynn@apex.ai>
@flynneva flynneva mentioned this pull request Jul 7, 2022
Merged
@JWhitleyWork
Copy link
Collaborator

@flynneva Don't worry about it! As long as the outcome is improvement then the journey is always worth it!

JWhitleyWork pushed a commit that referenced this pull request Jul 8, 2022
@flynneva @JWhitleyWork
Fix linker error caused by templating in the conversions.cpp file (#718)
07b2cde 
Signed-off-by: Evan Flynn <evan.flynn@apex.ai>
@ijnek ijnek mentioned this pull request Jul 19, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@clalancette clalancette clalancette approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@flynneva @simwijs @clalancette @JWhitleyWork @evan-flynn-apexai