search

openambitproject / openambit

openambit
280 stars 82 forks source link

Qt5 OpenAmbit client #37

Closed mariusor closed 7 years ago

mariusor commented 10 years ago

Hi, I have created a branch[1] for the OpenAmbit client to compile under Qt5.

I would like to know if there is any interest in this, and if you guys would consider a merge, which way should we do it - parallel builds for Qt4 and Qt5 or moving just to Qt5.

Thank you.

[1] https://github.com/habarnam/openambit/tree/Qt5_Client

svenstorp commented 10 years ago

Hi habarnam

Looks good! I would prefer a merge. Since I for myself is a KDE user I would like to keep Qt4 compatibility, at least until there is a stable build of KDE which uses Qt5 (KDE Frameworks 5).

mariusor commented 10 years ago

Ok, great. I'll make some changes to allow for both Qt4/Qt5 builds for now, and hide them behind a cmake switch. Does that sound better?

mbernasocchi commented 10 years ago

On 22.04.2014 10:57, Habar Nam wrote:

Ok, great. I'll make some changes to allow for both Qt4/Qt5 builds for now, and hide them behind a cmake switch. Does that sound better? yes, or even better auto detect the qt version if possible.

thanks a lot Marco

Reply to this email directly or view it on GitHub: https://github.com/openambitproject/openambit/issues/37#issuecomment-41017608

Marco Bernasocchi http://opengis.ch

mariusor commented 10 years ago

https://github.com/habarnam/openambit/tree/Qt5_Client

Hi Marco, I have pushed some changes to have the Qt client be build either with Qt4 or Qt5. For now it defaults on Qt5, so there could be some minor issues when compiling with Qt4 (it builds and it runs, but I haven't tested more than that). If you can, please try to build it, run it and tell me what you think and if you need more changes.

mbernasocchi commented 10 years ago

Building on 14.04 using Qt 5.2.1 I get theese warnigs.

/home/marco/dev/openambit/src/openambit/movescount/movescountjson.cpp: In member function ‘int MovesCountJSON::parseFirmwareVersionReply(QByteArray&, u_int8_t*)’:
/home/marco/dev/openambit/src/openambit/movescount/movescountjson.cpp:63:12: warning: ‘ok’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (ok && result["LatestFirmwareVersion"].toString().length() > 0) {
            ^
/home/marco/dev/openambit/src/openambit/movescount/movescountjson.cpp: In member function ‘int MovesCountJSON::parseLogReply(QByteArray&, QString&)’:
/home/marco/dev/openambit/src/openambit/movescount/movescountjson.cpp:93:56: warning: ‘ok’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (ok && result["MoveID"].toString().length() > 0 && result["MoveID"].toString() != "0") {
                                                        ^
/home/marco/dev/openambit/src/openambit/movescount/movescountjson.cpp: In member function ‘int MovesCountJSON::generateLogData(LogEntry*, QByteArray&)’:
/home/marco/dev/openambit/src/openambit/movescount/movescountjson.cpp:344:24: warning: ‘ok’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     return (ok ? 0 : -1);
                        ^
/home/marco/dev/openambit/src/openambit/movescount/movescountjson.cpp: In member function ‘int MovesCountJSON::parseLogDirReply(QByteArray&, QList<MovesCountLogDirEntry>&)’:
/home/marco/dev/openambit/src/openambit/movescount/movescountjson.cpp:122:5: warning: ‘ok’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     if (ok) {
mbernasocchi commented 10 years ago

also, the try icon is not available

mariusor commented 10 years ago

OK, I will clean it up a bit. For me the Icon was showing, but maybe it was from the Qt4 install...

Thanx for the feedback.

mbernasocchi commented 10 years ago

Also it would be great if you allow forcing qt4: some thing like

if(NOT FORCE_QT4)
    find_package(Qt5 COMPONENTS Core Widgets Network)
endif(NOT FORCE_QT4)

and if you would not build the outputs directly in /src

beside that it looks good to me

mariusor commented 10 years ago

Ok... I think I've got everything you mentioned: -DFORCE_QT4=1 as a switch fixed the warnings fixed the icon issue fixed the in tree build issue

The only major modification was to remove searching for an external libambit in the openambit CMake file, as I couldn't find an elegant method to allow using the system's libambit vs. the local one. I'm not sure this is a major issue

manisandro commented 7 years ago

Qt5 support added in 566939bf3b34788161f314cb6367c71b8162b926