robotpy / robotpy-cscore

Moved to https://github.com/robotpy/mostrobotpy
Other
17 stars 12 forks source link

2020 port #81

Closed prensing closed 4 years ago

prensing commented 4 years ago

I think this is ready, but this is my first PR to Robotpy, so I expect there are tweaks. A few notes:

prensing commented 4 years ago

Oh, also, the file "cscore_src/cscore/src/main/native/cpp/jni/CameraServerJNI.cpp" was part of the compile, which seemed unnecessary. I updated setup.py to exclude it.

auscompgeek commented 4 years ago

I would prefer using AccessFlag = int;.

prensing commented 4 years ago

OK. My C++ is rusty, so that is new to me. Not sure it is relevant, but PyInt_Check uses #define just above that.

auscompgeek commented 4 years ago

OpenCV4 got installed a bit oddly for me

Could you explain?

prensing commented 4 years ago

OpenCV4 got installed a bit oddly for me

Could you explain?

I did a fairly simple install with the plain OpenCV 4.2.0 download. It was set to install into /usr/local. The include files installed into directory /usr/local/include/opencv4/opencv2. When the code tried to include "opencv2/xxx.h" it could not find it. I worked around it by linking /usr/local/include/opencv2 -> opencv4/opencv2.

I thought about adding code to setup.py to search for the OpenCV install, but a clean solution was not obvious.

prensing commented 4 years ago

The auto format checks appear to be flagging only code in cscore_src, so presumably should not be fixed?

auscompgeek commented 4 years ago

I would recommend actually reading my notes in #39. Plenty of notes about that there.

auscompgeek commented 4 years ago

The auto format checks appear to be flagging only code in cscore_src, so presumably should not be fixed?

Ha. Could you add cscore_src to the black exclude in pyproject.toml?

prensing commented 4 years ago

I will look through #39, but have to go do other stuff for now. Everything seems to be working with OpenCV4, but my testing was limited to trying our 2019 vision code (and not for long).

Do we want to maintain compatibility with OpenCV3? If so, the include path should be flexible...

auscompgeek commented 4 years ago

Do we want to maintain compatibility with OpenCV3?

Yes. We will probably still be using OpenCV 3 for the roboRIO for 2020, matching WPILib. I would also like to be able to continue building against distro packages.

prensing commented 4 years ago

I updated setup.py to use the pkgconfig package. It worked correctly for OpenCV4 (compiled localled) and the Ubuntu-provided OpenCV 3.2 packages. pkg-config is available for non-Linux (incl. Windows) so I coded it to use "pkgconfig" (the Py module) on any platform if it loads.

Remaining possible issues that I can think of:

prensing commented 4 years ago

So I think the JNI filtering is done. What is the etiquette for Resolving conversations (assuming you want that)? May I, or is that left to the person who started it?

virtuald commented 4 years ago

I suppose whoever thinks it's resolved?

auscompgeek commented 4 years ago

Anything else we need to do here?

prensing commented 4 years ago

From my POV, I believe this is done.

virtuald commented 4 years ago

I haven't built it yet, but it looks fine and the tests pass (which includes a build) so I'm going to merge this.