RaiMan / SikuliX-2014

SikuliX version 1.1.2 (until February 2018)
http://sikulix.com
806 stars 234 forks source link

sikuli-debug.h uses incorrect header-guard #218

Closed UnitedMarsupials-zz closed 8 years ago

UnitedMarsupials-zz commented 8 years ago

Compiling with a good compiler, I get the following warning:

.../SikuliX-2014-1.1.0/Libslux/src/main/resources/srcnativelibs/Vision/sikuli-debug.h:6:9: warning: '_SIKULU_DEBUG_H' is used as a header guard here, followed by #define of a
      different macro [-Wheader-guard]
#ifndef _SIKULU_DEBUG_H
        ^~~~~~~~~~~~~~~
.../SikuliX-2014-1.1.0/Libslux/src/main/resources/srcnativelibs/Vision/sikuli-debug.h:7:9: note: '_SIKULI_DEBUG_H' is defined here; did you mean '_SIKULU_DEBUG_H'?
#define _SIKULI_DEBUG_H
        ^~~~~~~~~~~~~~~
        _SIKULU_DEBUG_H
1 warning generated.

The fix is obvious:

--- Libslux/src/main/resources/srcnativelibs/Vision/sikuli-debug.h  2015-10-06 14:24:22.000000000 -0400
+++ Libslux/src/main/resources/srcnativelibs/Vision/sikuli-debug.h  2016-06-30 00:07:23.455072000 -0400
@@ -4,5 +4,5 @@
  *
  */
-#ifndef _SIKULU_DEBUG_H
+#ifndef _SIKULI_DEBUG_H
 #define _SIKULI_DEBUG_H
RaiMan commented 8 years ago

ok, thanks. ... but I will not change any C++ source code in version 1.1.x any more.

Version 2 will not have any C++ code any more --- Java only.

UnitedMarsupials-zz commented 8 years ago

... but I will not change any C++ source code in version 1.1.x any more.

That's not entirely fair. As long as 1.1.0 is the only available release and 1.1.1 -- the still upcoming next one, the branch should be getting fixes. Although this particular one does not seem too important -- the code obviously compiles despite the bug -- the fix is also quite trivial...

Version 2 will not have any C++ code any more --- Java only.

Could you share the expected timeline and/or "roadmap" for version 2.0? Is it coming this year, for example?

That said, the decision to go "pure Java" seems like a forced one :) While porting SikuliX to FreeBSD and CentOS, I got the distinct impression, that C++ code is sort of a pariah and that SikuliX developers do not consider themselves C++ programmers.

The two "native" libraries used by 1.1.x are JXGrabKey and VisionProxy. Because the former is a separate project, why not simply list it as a requirement (on Unix/X11 systems)? People like myself, who do software-porting and packaging for a living (and/or as a hobby), would find it much easier to provide it separately -- and you will not have to struggle providing it yourself (and gratuitously limiting the number of supported OS as a result)? For example, as part of my work porting SikuliX to FreeBSD, I've already created a separate port for JXGrabKey.

As for VisionProxy, it does not seem useful to anything other than SikuliX, so I simply compile it as part of the SikuliX build -- but not through Maven. I also patch SikuliX code to use System.loadLibrary() instead of System.load() -- this allows the locations of the shared objects to be specified via the standard means (like LD_LIBRARY_PATH on Unix) and removes the guessing names of the library-files (like does it end with .dll or .so or .dylib?).

So, if your decision to go pure Java was driven -- at least partially -- by the problem of porting C++ code, a better solution may be to leave that porting to others. Concentrate on your own code and simply list that of the numerous third-parties (C++ or Java) as a requirement.

RaiMan commented 8 years ago

When Sikuli was started in 2009 by the former developers, they decided to use OpenCV matchTemplate() as base for the image search. At that time the official OpenCV API was only available for C and C++. So they decided to implement the features around image search in C++. But since Sikuli itself from the beginning was implemented in Java, they had to add some bridge and decided to use JNI. A bit later, the approaches were changed and the basic text features (Tesseract) were also added. Altogether this is some chaos, where nearly every day I am thankful, that it still works. ... and believe me: this code is not worth to be "ported" and would lead to "crazy" Java code if one would do so.

When I took over some years ago, I decided, to not invest in C++ coding. So I restricted myself to adopt the stuff to newer versions of OpenCV and Tesseract and get it built for Windows, Mac and Ubuntu, so it is possible to provide packages, that run out of the box after setup.

Since 3 years OpenCV provides a Java wrapper, that offers the OpenCV API at the Java level (Java classes using a mix of JNI and JNA internally, to access the native libraries). For the implementation of the text features there is also a Java wrapper available (Tess4J).

Using these wrappers will make it possible to completely get rid of any proprietary C++ code in version 2 and hopefully get a much cleaner implementation of the SikuliX API, that is more open for contributions.

Nevertheless: I will check your suggestions and repair the mentioned source code problem.