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

Make the repo compile under ROS Lunar (OpenCV 3.2.0) #72

Merged
merged 9 commits into from
Jun 21, 2017

Conversation

pierrekilly
Copy link
Contributor

@pierrekilly pierrekilly commented Jun 13, 2017

This Github issue helped a lot:
wg-perception/people#44

I haven't tested it, it "just" compiles.

This is an open discussion, so if improvements are needed, please tell :)

@pierrekilly
Copy link
Contributor Author

The build has failed, which is what I expected, as some names in OpenCV 3 have changed.

I think I can try to make those changes depending on the OpenCV version. I'll try to make something compatible in the next few days.

@ipa-rmb
Copy link
Contributor

ipa-rmb commented Jun 14, 2017

Many thanks! Well done. I guess you can easily make it work with both OpenCV versions using this macro

	#if CV_MAJOR_VERSION == 2
		cv::SVM ...
	#else
		// OpenCV 3
		cv::ml::SVM
	#endif

@pierrekilly
Copy link
Contributor Author

The build failed trying to execute code for OpenCV 3.
I can't look further more right now, but obviously OPENCV_MAJOR_VERSION == 2 does not evaluate to true for whatever reason.

@ipa-rmb
Copy link
Contributor

ipa-rmb commented Jun 14, 2017

No problem. Thanks for your assistance. I will fix the errors for OpenCV 2 and complement your pull request so that Travis is happy. Again, many thanks for your contribution.

By the way, a new colleague of mine will extend the functionality of this package significantly, soon, especially with major improvements on the person recognition quality. So stay tuned.

@ipa-rmb
Copy link
Contributor

ipa-rmb commented Jun 14, 2017

OPENCV_MAJOR_VERSION was wrong, CV_MAJOR_VERSION seem to be correct, sorry for the wrong hint.

@ipa-rmb
Copy link
Contributor

ipa-rmb commented Jun 14, 2017

Now you received a pull request from me. Can you please merge them in order to update this pull request?

This was referenced Jun 14, 2017
Fixes for opencv 2 in indigo
@ipa-rmb ipa-rmb merged commit f3d01f7 into ipa320:indigo_dev Jun 21, 2017
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.

2 participants