Skip to content
This repository has been archived by the owner on Apr 23, 2020. It is now read-only.

Remove warnings and errors that occur when using GCC >= 7.0. #17

Merged
merged 52 commits into from
Aug 2, 2017

Conversation

allenh1
Copy link

@allenh1 allenh1 commented Jul 31, 2017

This is another small GCC >= 7 change.

connects to ros2/rclcpp#343

azaganidis and others added 18 commits December 20, 2016 14:57
fix cmake use of yaml-cpp and sdl / sdl-image in map_server
* [kinetic] amcl: Add std_srvs to find_package and catkin_package
* [kinetic] costmap_2d: Add visualization_msgs to find_package and catkin_package
* [kinetic] amcl: std_srvs only needed in find_package
* [kinetic] move_base: Add std_srvs to find_package
Initialization of filter with GPS and odometry.
When building amcl with recent enough gcc v7 compilation fails with
the error

src/amcl/map/map_cspace.cpp: In function 'void enqueue(map_t*, unsigned int, unsigned int, unsigned int, unsigned int, std::priority_queue<CellData>&, CachedDistanceMap*, unsigned char*)':
src/amcl/map/map_cspace.cpp:98:34: error: call of overloaded 'abs(unsigned int)' is ambiguous
   unsigned int di = abs(i - src_i);

Use `int abs(int)` flavour of the abs() function.
@allenh1 allenh1 added in progress Actively being worked on (Kanban column) in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Jul 31, 2017
@allenh1
Copy link
Author

allenh1 commented Jul 31, 2017

@mikaelarguedas Do I run a packaging job for this one or the normal CI?

@mikaelarguedas
Copy link
Member

Can this be submitted upstream instead? looks like this overlap fixes already merged upstream (ros-planning#587)

@mikaelarguedas
Copy link
Member

The tf2 stuff can be submitted in this repo given that ROS1 navigation port to tf2 is still pending.

@mikaelarguedas Do I run a packaging job for this one or the normal CI?

Neither the packaging job or normal CI test this repository. Please launch a ci_turtlebot-demo_linux and ci_turtlebot-demo_linux-aarch64

@allenh1
Copy link
Author

allenh1 commented Jul 31, 2017

Can this be submitted upstream instead? looks like this overlap fixes already merged upstream

Yes, that appears to be a similar patch.

Should I bother with a submission upstream since that fix was already merged? I could rebase on top of ros-planning/navigation, if that's the best solution (might be good to catch up anyway).

Neither the packaging job or normal CI test this repository. Please launch a ci_turtlebot-demo_linux and ci_turtlebot-demo_linux-aarch64

Thanks.

@mikaelarguedas
Copy link
Member

Should I bother with a submission upstream since that fix was already merged?

If the fix is similar no need to submit anything.

I could rebase on top of ros-planning/navigation, if that's the best solution (might be good to catch up anyway).

Yeah it would be great to catch up, that may be a mess to resolve conflicts. maybe it's worth waiting after ros-planning#590 and ros-planning#591 are merged

…pendency

Cleanup PCL imports: fixes compilation on zesty and stretch
@allenh1
Copy link
Author

allenh1 commented Jul 31, 2017

Yeah it would be great to catch up, that may be a mess to resolve conflicts. maybe it's worth waiting after ros-planning#590 and ros-planning#591 are merged

Definitely. I'll keep an eye on ros-planning#591.

mintar and others added 2 commits July 31, 2017 18:04
* Fix CMakeLists + package.xmls

This fixes compilation errors when rosjava is installed on the system.
It also removes a lot of errors reported by catkin_lint.

Fixes ros-planning#537 .

* Fix more CMakeLists.txt + package.xmls

* amcl: disambiguate include path

This fixes the following catkin_lint warning:

    amcl: warning: include paths 'include/amcl/map' and 'include' are ambiguous
         * You have used two include paths where one is a parent of the
         * other. Thus the same headers can be included with two different
         * include paths which may confuse users. It is recommended that
         * you keep your include paths consistent.
dhood and others added 7 commits August 1, 2017 11:39
* Remove dynamic reconfigure

* Remove rosbag functionality

* Remove message filter use

* tf -> tf2

* Strip leading / from frame IDs

* Re-enable message filter

* Don't use deperecated quaternion constructor

* Revert "Re-enable message filter"

This reverts commit c4160c2.

* Fix test failures by increasing transform timeouts

* WIP ros1->ros2

* Namespace of ::msg or ::srv

* ConstPtr -> ::ConstPtr

* ros::Time -> tf2::TimePoint

* ros::Duration -> tf2::Duration

* ros -> rclcpp

* ros2 timer

* Disable sigint handler

* Not using boost::shared_ptr

* Appropriate boost header for recursive_mutex

* Shared ptrs everywhere

* Use new durationFromSec tf2 function

* Subscribers using qos settings

* Leftover missing ::msg

* Use new tf2_ros::fromMsg

* Reinstate node ptr

* Disable most logging

* chrono fix

* 'msg.' -> 'msg->'

* Store node

* Create services properly

* Set parameters

* Comment more logging

* tf2 fixes

* fix service callback signatures

* tf2 changes

* misc

* only callbacks use shared_ptr

* map request service

* Fix boost link error

* Comment these for now just to move forward

* Use ament_auto_package

* tf2::Duration -> tf2::durationFromSec

* Re-enable "logging"

* Prevent node already added to executor crash

* (hack) use 'latest' TF info

* Debug

* Fixup (whitespace etc)

* Use rate for map request

* Don't use tf2 time for checkLaserReceived timer

* Clarify which TODOs are for before merge

* Add wait_for_service

* Restore initial qos choices

* Restore tolerance

* Demo ROS 1 launch file to be used with bridge

* [hack] Use map from topic instead of service

* Remove debug prints

* It probably won't be the map server publishing the map topic in that case (or we'd use the service)

* Expose use_map_topic as command line option

* Put initial pose back to 0 (can be set on 'initialpose' topic)

* Use tf2 time because ROS times can't be subtracted a.t.m.

* Revert test tolerance increase (needed for use without message filters)

* Remove commented delete that will never be re-enabled

* Tidy up workaround for 'latest' TF data

If the timestamp of the requested transform is 0 it will get latest

* Remove most workarounds for 'latest' TF data

* Prevent unused variable warnings

* Remove boost dependency

* Use CMAKE_CXX_STANDARD

* Alphabetical ordering

* Whitespace fixup

* Leftover debug prints

* Restore original default d_thresh

* Revert "Prevent unused variable warnings"

This reverts commit e624b1f.

* Prevent unused variable warnings

* Let ament_cmake_ros have control over the BUILD_SHARED_LIBS cmake flag

* Pedantic is a bit much

* simplify ament_target_dependencies call

* Correct usage of ament_target_dependencies

* Remove custom leading forward slash removal

Not necessary in ROS 2

* Remove ROS 1 testing launch file

* Avoid race condition with map_server topic usage

Issue with large services using fastrtps
* Make laser subscription best effort

Otherwise it can't subscribe to best effort publishers (and is generally more appropriate)

* Use rcutils logging macros

* Start a parameter server for external nodes to interact with

* Reduce diff with upstream
@allenh1
Copy link
Author

allenh1 commented Aug 1, 2017

@mikaelarguedas Ok, rebased on top of the lunar branch. I'll launch a CI job after I finish rebuilding everything locally (started from a clean build just to make sure).

@@ -55,6 +55,7 @@ target_link_libraries(map_server
)

add_executable(map_server-map_saver src/map_saver.cpp)
add_dependencies(map_server-map_saver ${${PROJECT_NAME}_EXPORTED_TARGETS} ${catkin_EXPORTED_TARGETS})
Copy link
Member

Choose a reason for hiding this comment

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

This is catkin specific. catkin_EXPORTED_TARGETS will not exist in an ament build

Copy link
Author

Choose a reason for hiding this comment

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

Yup -- caught this one while building. My bad.

@@ -42,6 +42,9 @@
// We use SDL_image to load the image from disk
#include <SDL/SDL_image.h>

// Use Bullet's Quaternion object to create one from Euler angles
#include <LinearMath/btQuaternion.h>
Copy link
Member

Choose a reason for hiding this comment

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

bullet is not installed on the build machines AFAIK so this will likely fail to compile. We use tf2 quaternions in ROS 2 for now I think (see line 49)

Copy link
Author

Choose a reason for hiding this comment

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

Ya, that's right. I'll fix it.

@allenh1
Copy link
Author

allenh1 commented Aug 1, 2017

@mikaelarguedas Ok, all should be ready for CI now.

@@ -91,25 +91,25 @@ class MapServer
#endif
try {
doc["resolution"] >> res;
} catch (YAML::InvalidScalar) {
} catch (YAML::InvalidScalar &) {
Copy link
Member

Choose a reason for hiding this comment

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

These should be submitted upstream rather than here isn't it ? Or is it ROS 2 specific code?

Copy link
Author

Choose a reason for hiding this comment

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

I suppose that should be upstreamed, yes. I'll do that right now.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, filed upstream.

Copy link
Member

Choose a reason for hiding this comment

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

can you put them in a separate commit in this PR ? That'll be cleaner for history and rebasing

Copy link
Author

Choose a reason for hiding this comment

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

yes

@allenh1
Copy link
Author

allenh1 commented Aug 1, 2017

@mikaelarguedas done

@mikaelarguedas
Copy link
Member

thanks ! looks good to me, feel free to launch CI on this. (note we wont "squash and merge" this one to keep history close to the one of the upstream repo)

@allenh1
Copy link
Author

allenh1 commented Aug 1, 2017

thanks ! looks good to me, feel free to launch CI on this

will do

(note we wont "squash and merge" this one to keep history close to the one of the upstream repo)

I was just about to ask... This is a "Rebase and Merge" then, right (assuming CI passes).

@mikaelarguedas
Copy link
Member

Not it will be a simple merge because we want the ros2 commits on top of the ros1 commits.
If we do "rebase and merge" we will rebase the ros1 commits on top of the ros2 commits.
(which is overall good news because rebase and merge seems to conflict :S)

@allenh1
Copy link
Author

allenh1 commented Aug 1, 2017

  • Linux Build Status
  • Linux aarch64 Build Status

@allenh1
Copy link
Author

allenh1 commented Aug 1, 2017

Not it will be a simple merge because we want the ros2 commits on top of the ros1 commits.

Ok. Got it.

@allenh1
Copy link
Author

allenh1 commented Aug 1, 2017

Building and testing on a physical turtlebot 2, just to put this in the record.

@allenh1
Copy link
Author

allenh1 commented Aug 2, 2017

Ok, it works. Merging.

@allenh1 allenh1 merged commit ee42da3 into ros2 Aug 2, 2017
@allenh1 allenh1 removed the in review Waiting for review (Kanban column) label Aug 2, 2017
@allenh1 allenh1 deleted the compile-for-gcc-7 branch August 2, 2017 02:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet