Skip to content

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.

target_link_libraries(gui Qt5::Widgets)
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?


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