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.

Sign up for GitHub

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

Jump to bottom

Clean up of mdr_find_people #172

Closed
argenos opened this issue May 25, 2019 · 5 comments
Closed

Clean up of mdr_find_people #172

argenos opened this issue May 25, 2019 · 5 comments
Assignees
@henrikschnor
Labels
Type: Cleanup Changes only making the code cleaner and that don't change how the code works.

Comments

@argenos
Copy link
Contributor

argenos commented May 25, 2019

The current version of the package contains comments in the package.xml and CMakeLists.txt files. These files also need to be updated to handle the dependencies correctly.
@argenos argenos added the Type: Cleanup Changes only making the code cleaner and that don't change how the code works. label May 25, 2019
@argenos
Copy link
Contributor Author

argenos commented May 25, 2019

This topic should be passed as an argument

mas_domestic_robotics/mdr_planning/mdr_actions/mdr_perception_actions/mdr_find_people/ros/src/mdr_find_people/action_states.py

Line 18 in fb37f55

POINTCLOUD_TOPIC = '/hsrb/head_rgbd_sensor/depth_registered/rectified_points'

And we need an updated launcher for mas_hsr which then modifies that topic to the HSR-specific one.

This was referenced May 25, 2019
Merged
Merged
@alex-mitrevski
Copy link
Member

One other thing that needs to be done is reimplementing the action using FTSM.

@minhnh
Copy link
Member

minhnh commented May 25, 2019

duplicate #152?

@alex-mitrevski
Copy link
Member

I don't think this is a duplicate. This issue is about small refactoring of the existing package; your issue is about the logic behind the action itself, which I see as considerably more involved.

@alex-mitrevski
Copy link
Member

The cleanup has already been done (commits 52e0c8e, f4b9b8b, and 9ea3c8e), so I'll close the issue. The migration to FTSM can/should be a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@henrikschnor henrikschnor

Labels
Type: Cleanup Changes only making the code cleaner and that don't change how the code works.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@minhnh @alex-mitrevski @argenos @henrikschnor