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

deprecate this project / robot_body_filter #23

Open
v4hn opened this issue Aug 13, 2020 · 4 comments
Open

deprecate this project / robot_body_filter #23

v4hn opened this issue Aug 13, 2020 · 4 comments

Comments

@v4hn
Copy link
Member

v4hn commented Aug 13, 2020

@peci1 @k-okada @ipa-mjp @Martin-Oehler

The fork tree of this project reached depth four:

forks

Can we/should we do anything about that?

Now, the situation is that @k-okada and I do have admin permissions to this repo as part of the ROS-orphaned initiative but are not involved with the code.
From my current perspective, we could simply deprecate this repository altogether if @peci1 actually maintains the - much improved - robot_body_filter repository.
Alternatively, we could just as well transfer ownership of this repository if that makes any sense.

Is the robot_body_filter compatible with the old package or is there a reasonable transition guideline?

@k-okada apparently already released the robot_self_filter into noetic a few days ago, but we could probably pull it out again.

The most prominent thing, aside from transferring the repository, would be to add a new default branch for melodic/noetic-devel and keep only a README.md in there that forwards users to the robot_body_filter package.

@k-okada
Copy link
Contributor

k-okada commented Aug 14, 2020

@v4hn,
I did know know about robot_body_filter. Thanks for let me know.
We'd like to know transition guideline that covers two use cases.

  1. use robot_self_filter as a stand alnoe node: https://github.com/jsk-ros-pkg/jsk_robot/blob/master/jsk_pr2_robot/jsk_pr2_startup/jsk_pr2_sensors/lasers_and_filters.xml#L29
  2. use robot_self_filter functino within other node: https://github.com/jsk-ros-pkg/jsk_recognition/blob/master/jsk_pcl_ros/src/collision_detector_nodelet.cpp

@peci1
Copy link

peci1 commented Aug 14, 2020

robot_body_filter is not compatible with robot_self_filter in almost no way. I highlighted the changes in https://github.com/peci1/robot_body_filter#changes-vs-pr2robot_self_filter . Except for not being compatible, it is also more complex to correctly configure.

So abandoning robot_self_filter doesn't look like a good idea to me. There probably are (lots of) users who would get really sad if we broke their pipelines.

I see a few possible things that could be done:

  • move robot_body_filter under ros-perception to give it more trustworthiness and attention
  • edit readme and wiki of robot_self_filter and add references to robot_body_filter as a more versatile alternative (but also more complex and more difficult to set up)... also a feature comparison could be added to the pages so that users can easily decide what they need
  • I could write some robot_body_filter tutorials e.g. for some typical use-cases (manipulators, mobile robots, etc).
  • somebody could refactor robot_self_filter to utilize more of geometric_shapes which should fix a few issues (shapes., bodies., load_mesh.cpp)
  • we could try to even further and find synergies between the two packages, e.g. extract the common parts to a third package off which body and self filter would be based (if that's possible, I don't know)

To answer to @k-okada's questions:

  1. a compatibility executable could be added that would just load the filter into a cloud_to_cloud chain
  2. robot_body_filter is built to be extensible and reusable. So it shouldn't be a problem to use the mask from other programs directly (though nobody tried that yet).

There are still a few hacks in the code (like https://github.com/peci1/robot_body_filter/blob/7b69ec6d5a9231adc7eb0b52c9c270903522a21c/src/utils/bodies.cpp#L1). I'm not sure if a package with such hacks is "eligible" to be promoted as the canonical solution. I've sent PRs around the eventually get rid of the hacks, but it's gonna take some time.

@peci1
Copy link

peci1 commented Aug 14, 2020

It's also good to know that it's impossible to get robot_body_filter to work in kinetic, and I haven't tested it in noetic (which could, however, be easily done).

@v4hn
Copy link
Member Author

v4hn commented Aug 14, 2020

robot_body_filter is not compatible with robot_self_filter in almost no way

Too bad. I guess there's also no chance you would spend time on a compatible simple interface for robot_body_filter? :puppy:
I did not work with robot_body_filter myself yet, so I don't know if it's actually feasible in any way, but you are right in that we should not light-heartedly break pipelines. On the other hand it looks like you also fixed some bugs that were still present in this repository.

  • move robot_body_filter under ros-perception to give it more trustworthiness and attention
  • edit readme and wiki of robot_self_filter and add references to robot_body_filter as a more versatile alternative (but also more complex and more difficult to set up)...
  • also a feature comparison could be added to the pages so that users can easily decide what they need
  • I could write some robot_body_filter tutorials e.g. for some typical use-cases (manipulators, mobile robots, etc).

All of these seem like wonderful improvements you (as the author/maintainer of robot_body_filter) could pursue to get your package advertised! As a current (orphanage) maintainer of robot_self_filter I can offer you support with access where needed.
Of course you could also take over maintenance of robot_self_filter altogether. You clearly know more about this package than @k-okada and me. 😇

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

No branches or pull requests

3 participants