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

Do a complete overhaul of the main follow_waypoints executable #17

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

Conversation

bergercookie
Copy link

@bergercookie bergercookie commented Sep 23, 2020

Hi @danielsnider , thanks for this package.

This is not really a PR, but rather a way to let you know of the changes I did in my fork.
I reimplemented large chunks of the follow_waypoints.py functionality in an attempt to make it simpler to develop on, and also make it more flexible.

Since this is a major rewrite, I don't expect to merge it :)
Feel free to extract any functionality you think is useful for your project.

Changes include:

  • Removal of the smach dependency - State machine is not needed
  • Implement simpler design, where new goals are added to a FIFO and they
    get popped out, and passed to move_base. This allows adding additional
    waypoints during the actual navigation run
  • Removal of the redundant journey_* logic, reading/writing to files
    of the repo
  • Allow for specifying the waypoints as either poses (using the "Pose
    Estimation" button from RViz) or by using the "Publish Point"
    functionality. In case of the latter, a default (0, 0, 0, 1)
    quaternion orientation is assumed. In the future, we could also use
    an orientation based on the vector connecting previous and current
    waypoint.
  • Add ROS Parameters for most of the settings in the algorithm.
  • Reformat code using isort and black.
  • Make it python3-compatible - see print statements.
  • path_ready, path_reset are now one-shot std_srvs::Trigger services.
  • 16/10: patrol_mode dynamic-reconfigurable flag: Traverse the given waypoints over and over until path_reset is given.

@kostaskonkk
Copy link
Contributor

I have taken a quick look to the proposed changes and I am very happy that somebody else is also using this project and pushing merge requests! 😍
I will start using your commit immediately and I will let you know if I find any problems!

@bergercookie
Copy link
Author

Awesome!

Ευχαριστώ! :)

@danielsnider
Copy link
Owner

Awesome! 👍
I have scheduled time to take a look and help out in 2-3 weeks. I'm sorry for the delay!

@danielsnider
Copy link
Owner

I will be watching your fork too 🤩 great stuff!

nikoskoukis-slamcore and others added 5 commits October 27, 2020 12:21
Changes include:

* Removal of the smach dependency
* Imlement simpler design, where new goals are added to a FIFO and they
  get poped out, and passed to move_base. This allows adding additonal
  waypoints during the actual navigation run
* Removal of the redundant `journey_*` logic, reading/writing to files
  of the repo
* Allow for specifying the waypoints as either poses (using the "Pose
  Estimation" button from RViz) or by using the "Publish Point"
  functionality. In case of the latter, a default (0, 0, 0, 1)
  quaternion orientation is assumed. In the future, we could aslo use
  an orientation based on the vector connecting previous and current
  waypoint.
* Add ROS Parameters for most of the settings in the algorithm.
* Reformat code using `isort` and `black`.
* Make it python3-compatible - see `print` statements.
Co-authored-by: Nikos Koukis <[email protected]>
Co-authored-by: Konstantinos Konstantinidis <[email protected]>
@swjtuyang
Copy link

@bergercookie , in these changes:

Removal of the redundant journey_* logic, reading/writing to files
of the repo

why is it redundant? how to save the waypoints so next time it will be loaded ?

@bergercookie
Copy link
Author

@swjtuyang, well, it was redundant for the application I had in mind, but in general, there are more elegant ways of implementing this tbh, without having to explicitly code this behavior, or writing to a file inside the repository, for example:

Subscribe to the poses topic and redirect the output to a file, using a variant of rostopic echo, then when you want to reload these as new navigation goals, one can have a simple bash script that reads that file and sends them again, one by one, using rostopic pub -n 1 to the same topic.

@danielsnider
Copy link
Owner

Hi @bergercookie, I appreciate your efforts to simply this project. Unfortunately, I have realized that I do not have time to test large changes such as this. If you think you'll continue using follow_waypoints frequently, you might consider taking over as project maintainer of the code, the ROS wiki page, and the ROS package builds. I would be happy for you to do so. Take care!

@bergercookie
Copy link
Author

Hi @danielsnider,

Sorry to hear that,
Unfortunately I don't have time to maintain it at this stage either.

Anyway, handle this PR as you see fit :)

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.

5 participants