embeddedmz / ftpclient-cpp

C++ client for making FTP requests
MIT License
204 stars 65 forks source link

make this project Qt - compatible. #26

Open quarcko opened 3 years ago

quarcko commented 3 years ago

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 commented 3 years ago

<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 commented 3 years ago

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 QOS** macros, and then checks for them and defines WINDOWS or LINUX. without <QtGlobal> i cannot check for QOS****

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 commented 3 years ago

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 commented 3 years ago

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 commented 2 years ago

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

embeddedmz commented 2 years ago

@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 :laughing:

nicmorais commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

And be careful when using this library with Qt (thread safety concerns) ! I have already made a discussion here : https://github.com/embeddedmz/ftpclient-cpp/issues/27

nicmorais commented 2 years ago

@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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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