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

Draft: Galactic-Devel (Help wanted) #96

Draft
wants to merge 47 commits into
base: galactic
Choose a base branch
from

Conversation

relffok
Copy link

@relffok relffok commented Nov 18, 2021

Hi there,
I have started porting this package to ROS2 galactic a while ago. It was a lot of effort so far and there is still heaps to do. I wanted to get further in the development before opening a PR but because of the lack of time I am guessing it would be easiest to all work together (hopefully) on this one to move along. And finally be able to use ROS2 for all the MiR platforms.

It is still pretty rudimentary, but the simulation and driver work together with our MiR100 platform and it is great to be able to use ROS2 package once the driver connection is up and running.

Overview:

  • mir description: ported
  • mir_driver: supports the laser_scans, cmd_vel, odom and tf so far. Since I didn't need the other topics, I have commented them out. (ROS1 to ROS2 msg require some dict filter, but that shouldn't be a big effort to implement for most topics) This was the biggest chunk to be solved, I ended up porting rospy_msg_converter to ROS2 (rclpy_msg_converter) as well. Will link the PR here shortly.
  • mir_gazebo: simulation is up and running
  • mir_navigation: mapping + navigation ported using slam_toolbox and nav2 (which is great) but there is still a lot of parameter tuning to be done (mostly waiting on 1.0.8 release right now)

Some issues to point out:

  • Separate laserscans: The laserscans are the most important part in mapping, localization and mapping and we have two separate scans (front and back). In comparison to ROS1 the slam and nav nodes require the scan to be merged and not only be passed alternately to the same topic. So I needed to merge those (using another package I ported) from laserscan to pointcloud then merge the clouds and then convert it back to a single laserscan. At that time I could not find a better solution, if somebody else can point out anything, I'd be happy to review that.
  • Known Bug: Gazebo simulated Laserscans drift away from time to time (before merging in a pointcloud). This seems to be an issue in the urdf configuration, but I haven't had the time to track it down. [Gazebo] laserscans drift away without movement relffok/mir_robot#1
  • Missing namespace support: While porting I missed to drag the namespace everywhere (I also felt like there is an ongoing discussion about namespaces in lots of ros2 pkg), but this will also be added some time soon Namespace support for all existing mir_pkgs relffok/mir_robot#2

To sum up, it is useable for a few tasks, but there is still lots of things to do and bugs to remove. Help wanted and appreciated!

Added headless launch file to mir_driver in galactic-devel
    * using NavFn planner + DWB local planner
    * added custom behavior tree for MiR
    * added robot footprint
    * adapted a few parameters
    * visualization updates in rviz
@mintar
Copy link
Member

mintar commented Nov 22, 2021

Thanks a lot for all this work! I'll review it as soon as possible (I'm pretty swamped with work right now). Just a quick comment on one of your points:

Separate laserscans: The laserscans are the most important part in mapping, localization and mapping and we have two separate scans (front and back). In comparison to ROS1 the slam and nav nodes require the scan to be merged and not only be passed alternately to the same topic. So I needed to merge those (using another package I ported) from laserscan to pointcloud then merge the clouds and then convert it back to a single laserscan. At that time I could not find a better solution, if somebody else can point out anything, I'd be happy to review that.

This is really unfortunate. The problem with merging the laserscans like that is that you lose the information which frame a particular point was recorded from. At least in mapping and navigation there's some ray tracing going on between the sensor frame and the point (to determine free space), so that'll probably be a problem. Perhaps for pure localization it's going to work.

I remember having to do the same hack because gmapping in ROS1 also doesn't support multiple laser scanners, but it didn't produce good results, so I switched to hector_mapping instead (which supports multiple laser scanners).

@relffok
Copy link
Author

relffok commented Nov 22, 2021

Thanks a lot for all this work! I'll review it as soon as possible (I'm pretty swamped with work right now).

Thank you. I'm looking forward to get this one fully ported!

This is really unfortunate. The problem with merging the laserscans like that is that you lose the information which frame a particular point was recorded from. At least in mapping and navigation there's some ray tracing going on between the sensor frame and the point (to determine free space), so that'll probably be a problem. Perhaps for pure localization it's going to work.

I used it on MiR for mapping and navigation and also SLAM with a what I called virtual_laser_link (which I placed in the middle of the robot platform) and I didn't run into any issues. But maybe I am overlooking something here and you'll find something explicit to show me once you tested it.

niemsoen and others added 8 commits November 26, 2021 08:44
* Added mir_ready service as a workaround for lifecycle nodes (pythoN)
* add namespace for simulation and teleop

* add namespace to gazebo entity

* change to prefix only in urdfs

* enable ns for joint_state_publisher

* add namespace support to mir_navigation + ns instructions to readme

* switched namespaces to remappings in urdfs

* resolved gazeboplugin namespace errors

* fixes for namespace support driver

* added navigation config namespace examples

* minor fixes for namespace launch and adaptions in rviz configs

* fixed tf namespace bug in driver

* split up of driver launch files

* fixed params filename + added namespace args in nav launch

* enabled qos settings for ros2 topics in driver

* removed unused rviz parameter
* initial commit: added restapi state of work

* added mir_ready publisher

* added reliable QOS for mir_bridge_ready topic

* added mir_bridge_ready=false on crash/shutdown

* attribute bugfix

* added readiness polling and simplified ready publishing

* uneccesary main->publish_mir_ready_state: button checks bridge alive

* uneccesary main->publish_mir_ready_state: button checks bridge alive

* changed mir_ready pub/sub to Trigger-service

* possible bugfix

* changed response to success=True always and message=True/False

* Revert "changed response to success=True always and message=True/False"

This reverts commit f31efb8.

* added custom CheckReady srv-definition

* find bug in service

* renamed interface to match usb_pushbutton

* fixed dependency

* fixed another dependency

* Revert "Merge pull request #2 from niemsoen/mir-bridge-as-service-server"

This reverts commit cf0f5c1, reversing
changes made to 39e025e.

* Use std_srvs/Trigger as mir-ready-service Type

* changed structure to service-client communication

* fix for build error?

* wrong directory name

* added logging output

* bugfix

* better logging

* added message to response of service call

* bugfix

* added auth as parameter

* catch token not set

* bugfix response

* added check for error in setting as response to service call

* better log

* introduced parameters for hostname and auth-token

* make sure api handle is setup at launch

* better function names

* file rename

* launch of mir driver triggers launch of restapi_server

* mir_bridge creates restapi_client node

* import error in mir_bridge for MirRestAPIClient

* include test client for rest api

* add sleep for test call

* working, launch file not needed

* add client in mir_bridge.py

* trying to fix import

* Revert "trying to fix import"

This reverts commit 1a97c70.

* fix for import bug

* fix message type error

* try to fix connection refusal of rosbridge after restapi

* try to replace sleep after setTime waiting for reboot

* catch CannotSendRequest

* replace print with logger

* disable status call

* try output of get status call

* wait for restapi restart working

* make waiting for availability more elegant

* clean-up

* removed unused imports

* add potential fix

* Call mir_restapi_server in tendobot_driver

* Added api-call abstarction layer & tested api call while bridge connected (#5)

* init (#6)

Co-authored-by: Sönke Niemann <[email protected]>

* readme section: time sync in ros2 /w restapi

* remove automatic time sync call at driver start

* improved naming style in mir_restapi

* catch case of no api token set, warn user

* added more untested calls

* added waiting condition for honking

* failed try to find system time in restapi

* added client for time sync

* wait for service available

* added requested changes

Co-authored-by: relffok <[email protected]>
Co-authored-by: soenke.niemann <[email protected]>
Co-authored-by: Niemann <[email protected]>
Co-authored-by: Sönke Niemann <[email protected]>
* fix python setuptools install error

* created twist_stamper fork with fixed setuptools

Co-authored-by: Sönke Niemann <[email protected]>
@mintar mintar changed the base branch from noetic to galactic September 14, 2022 08:50
@mintar
Copy link
Member

mintar commented Sep 14, 2022

@ros-pull-request-builder retest this please

@mintar
Copy link
Member

mintar commented Sep 14, 2022

That doesn't seem to have worked. Closing and reopening to trigger retesting.

@mintar mintar closed this Sep 14, 2022
@mintar mintar reopened this Sep 14, 2022
relffok and others added 5 commits September 20, 2022 09:50
* separated wheel parts to simplify urdf build

* Changed MiR100 mass to 78.2kg

* allow urdf to parse collision parameters for gazbeo

* cherry-picked intertia from 91952d8

* applied torsional force, updated contact collisons

* tuned wheel frictions and contacs more
* added service to execute mission

* added exec mission function

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

Successfully merging this pull request may close these issues.

3 participants