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

Porting GUI to Qt-5 #13

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Porting GUI to Qt-5 #13

wants to merge 8 commits into from

Conversation

bowang
Copy link

@bowang bowang commented Aug 27, 2016

These few commits ported gmapping GUI tools (gfs_simplegui, gfs_nogui, gfs_logplayer, gfs2img) to Qt-5 and re-enabled log tools (log_plot, log_test, rdk2carmen, scanstudio2carmen).
It has been tested on Ubuntu 14.04 with Qt-5.2.1

Copy link

@vrabaud vrabaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall very valuable but impossible to review properly on GitHub as you mixed reformatting with code improvements. I'll let you fix my comments.

@@ -0,0 +1,7 @@
include_directories(../include/gmapping)
add_library(configfile configfile.cpp)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this library an INI parser ? Why not used the standard boost property tree? http://www.boost.org/doc/libs/1_55_0/doc/html/boost_propertytree/parsers.html#boost_propertytree.parsers.ini_parser

endif()

include_directories(../include/gmapping ../ ../include/gmapping/log/)
include_directories(.. ../include/gmapping ../include/gmapping/log)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should first check for find_package(Qt5Widgets) and return if not found: if we add this GUI for gmapping, it will be made into it's own package. For now, we allow to build it in here.

else()
target_link_libraries(gfs_simplegui gridfastslam ${QT_LIBRARIES})
find_package(Qt4 REQUIRED)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the whole code also compile with Qt4?

@@ -15,244 +15,249 @@ using namespace std;
using namespace GMapping;
using namespace GMapping::GFSReader;

inline double min(double a, double b){
return (a<b)?a:b;
inline double min(double a, double b)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

never reformat code and modify code. Always have two commits to make it easier to review. Too late ...

}
void computeBoundingBox(double& xmin, double& ymin, double& xmax, double& ymax, const RecordList& rl, double maxrange)
{
xmin = ymin = DBL_MAX;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -1,133 +0,0 @@
std::vector<unsigned int> sistematicResampler<State,Numeric>::resample(const vector<Particle>& particles) const{
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happened to that file ?

Copy link
Contributor

@k-okada k-okada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bowang could you create new PR against melodic-devel branch? we have switched from master to melodic-branch when we changed licence to BSD

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants