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 this project Qt - compatible. #26

Open
quarcko opened this issue Apr 30, 2021 · 15 comments
Open

make this project Qt - compatible. #26

quarcko opened this issue Apr 30, 2021 · 15 comments

Comments

@quarcko
Copy link
Contributor

quarcko commented Apr 30, 2021

You can add on top of FTPClient.h, right after FTPCLIENT_VERSION define:

#ifdef QT_CORE_LIB #include <QtGlobal> #ifdef Q_OS_LINUX #define LINUX #endif #ifdef Q_OS_WINDOWS #define WINDOWS #endif #endif

@embeddedmz
Copy link
Owner

embeddedmz commented Apr 30, 2021

<QtGlobal> is not needed as the class doesn't depend on Qt.

I guess you are using QMake and not CMake, so for the LINUX or WINDOWS macros, you can define them in the .pro file (google for the win32/unix qmake directives).

@quarcko
Copy link
Contributor Author

quarcko commented Apr 30, 2021

So this code basically checks if QT_CORE_LIB is defined (it is defined by qmake),
if so, it includes <QtGlobal>
as in turn it contains Q_OS_****** macros, and then checks for them and defines WINDOWS or LINUX.
without <QtGlobal> i cannot check for Q_OS_****

Yes i know i can do it in *.pro file, but as i added your files directly to my project source tree
and not building separately (no need) i just don't want that my whole project sees WINDOWS and LINUX macros defined.

So it is like more local solution, applicable to this file only.

If you feel it is not needed, just close the issue :)

@embeddedmz
Copy link
Owner

A better solution is to get rid off the macros WINDOWS and LINUX and use Visual Studio macro for Windows (and MinGW) and a GCC macro for Linux. There's plenty of project on github (e.g. libmodbus) that uses compilers macros to detect the platform.

@quarcko
Copy link
Contributor Author

quarcko commented Apr 30, 2021

Don't know if it's correct list, but here what i found:

#ifdef __clang__
/*code specific to clang compiler*/
#elif __GNUC__
/*code for GNU C compiler */
#elif _MSC_VER
/*usually has the version number in _MSC_VER*/
/*code specific to MSVC compiler*/
#elif __BORLANDC__
/*code specific to borland compilers*/
#elif __MINGW32__
/*code specific to mingw compilers*/
#endif

@nicmorais
Copy link

I'm also interested in Qt support, don't think I could help though.

@embeddedmz
Copy link
Owner

@nicmorais What build system do you use ? CMake or qmake ? Qt is now fully supporting CMake and will not use qmake in the future (CMake will be the default build system for Qt).

You can use CMake's add_subdirectory to include ftpclient to the build and then use target_link_libraries to link the library to your output program.

The problem with learning CMake is that there is the old one and the modern one (aim for the modern CMake). I think it's better to always use the same build system in your C++ projects.

If you are a C++ developer, knowing CMake is a very valuable asset. I don't really like it (it's complex and the script language is ugly, it sucks like the other Kitware products I have already worked with such as VTK and ParaView), but it's becoming the de-facto build system for C++ projects.

Like git or the C++ language, if you stick to a small subset, everything will be fine 😆

@nicmorais
Copy link

Hello @embeddedmz
So, I'm trying to include the library into a project with CMake, on Linux. I tried including with add_subdirectory(ftpclient-cpp), but there was an error with direct.h file. I commented out that line, and more errors started to show:
‘GetProgressFnCallback’ function uses ‘auto’ type specifier without trailing return type

Am I doing something wrong? Thanks in advance for your effort.

@embeddedmz
Copy link
Owner

embeddedmz commented Dec 25, 2021

@nicmorais if you search the web for this error, you will see that it's because GetProgressFnCallback definition works only with C++14, what you can do is to change auto to "ProgressFnCallback" (which is an alias of std::function<int(void *, double, double, double, double)>).

In fact CMake is a script build generator (it can generate GNU/Linux Makefiles, solutions for Visual Studio etc...).

@nicmorais
Copy link

@embeddedmz Thanks a lot, I'm almost there. Now it says:
"no viable conversion from returned value of type 'int (*const *)(void *, double, double, double, double)' to function return type 'embeddedmz::CFTPClient::ProgressFnCallback' (aka 'function<int (void *, double, double, double, double)>')"

I tried changing the return type to void as an workaround, but there are more errors
Captura de tela_2021-12-25_15-09-58-4414318

@embeddedmz
Copy link
Owner

@nicmorais You are trying to compile some parts that should be hidden with "#ifdef WINDOWS" WINDOWS macro should not be defined ! I added some helpers for Windows users to be able to handle file names correctly because under Windows, file names are not encoded in UTF-8 unlike on GNU/Linux systems.

If you read the CMakeLists.txt, you will see that this macro is only enabled when using CMake to generate a Visual Studio solution.

I hope it's clear, what you can do is discard all the code that requires that "WINDOWS" preprocessor macro is defined. Please ensure that this macro is not defined.

@embeddedmz
Copy link
Owner

And be careful when using this library with Qt (thread safety concerns) ! I have already made a discussion here : #27

@nicmorais
Copy link

@embeddedmz So, that's what I've tried here, now I'm able to build it separately under Qt Creator. I'm trying to figure out how to add the library to a project and build them. Once again, thanks a lot for your help!

@yuusakuri
Copy link

yuusakuri commented Nov 7, 2022

I change auto to ProgressFnCallback.
After that, the following error was output.

GCC
C++11
Qt 5.7

could not convert ‘((const embeddedmz::CFTPClient*)this)->embeddedmz::CFTPClient::m_fnProgressCallback.std::function<_Res(_ArgTypes ...)>::target<int (*)(void*, double, double, double, double)>()’ from ‘int (* const*)(void*, double, double, double, double)’ to ‘embeddedmz::CFTPClient::ProgressFnCallback {aka std::function<int(void*, double, double, double, double)>}’
     inline ProgressFnCallback GetProgressFnCallback() const { return m_fnProgressCallback.target<int (*)(void*, double, double, double, double)>(); }

@embeddedmz
Copy link
Owner

embeddedmz commented Nov 7, 2022

@yuusakuri

use this typedef : using ProgressFnPtr = int (*const *)(void *,double,double,double,double);
or use this syntax : typedef int (*const *ProgressFnPtr)(void *, double, double, double, double);

You can get the meaning of this last using https://cdecl.org/

declare ProgressFnPtr as pointer to const pointer to function (pointer to void, double, double, double, double) returning int

it was not easy to find the type definition, I used this code to find the answer :

using namespace embeddedmz;
CFTPClient cli(PRINT_LOG);
std::cout << typeid(cli.GetProgressFnCallback()).name() << '\n';

C++ is a pain in this ass. If you want, in your fork you can get rid of std::function and use a simple function pointer. std::function offers more security at compile time, that's why I used it but it's better to keep things stupid simple.

@yuusakuri
Copy link

yuusakuri commented Nov 7, 2022

@embeddedmz
I also wanted to use std::function, but My project is old. Thank you very much for your help.

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

No branches or pull requests

4 participants