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

Add argument to select launching teleop #40

Merged
merged 2 commits into from
Apr 23, 2019

Conversation

708yamaguchi
Copy link
Contributor

@708yamaguchi 708yamaguchi commented Apr 19, 2019

In this PR, I add argument to select launching teleop in fetch.launch.

I need this argument because I want to launch our original teleop program in other launch file.

Cc @knorth55

@knorth55
Copy link
Contributor

We need this PR to customize our fetch teleoperation PS3joystick command.
We want to set similar joystick command as PR2.

Copy link
Contributor

@moriarty moriarty left a comment

Choose a reason for hiding this comment

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

Seems reasonable.

@erelson @cjds any thoughts before merging this?

Copy link
Contributor

@cjds cjds left a comment

Choose a reason for hiding this comment

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

LGTM good idea

@erelson
Copy link
Collaborator

erelson commented Apr 22, 2019

Seems fine, though my own typical approach would just be to comment out the teleop portion of /etc/ros/$distro/robot.launch

Note that in order for this change to take effect on your current robot(s), you must manually update your robot.launch file! The file you've modified is only used at install time when the fetch-melodic-config package creates /etc/ros/melodic/robot.launch by copying it.

@knorth55
Copy link
Contributor

Seems fine, though my own typical approach would just be to comment out the teleop portion of /etc/ros/$distro/robot.launch

Note that in order for this change to take effect on your current robot(s), you must manually update your robot.launch file! The file you've modified is only used at install time when the fetch-melodic-config package creates /etc/ros/melodic/robot.launch by copying it.

Thank you for your information.
We already change /etc/ros/indigo/robot.launch for calibration data.

@moriarty
Copy link
Contributor

Seems fine, though my own typical approach would just be to comment out the teleop portion of /etc/ros/$distro/robot.launch

Note that in order for this change to take effect on your current robot(s), you must manually update your robot.launch file! The file you've modified is only used at install time when the fetch-melodic-config package creates /etc/ros/melodic/robot.launch by copying it.

I think commenting out code in /etc should be avoided. I think we should have this PR, and open a related issue for looking into fetch-melodic-config to use a symlink or something similar so that this change could become effective.

If you don’t want to add the frieght change, we can open a new issue for that and merge this PR as is.

@708yamaguchi
Copy link
Contributor Author

Thank you for your review.
I pushed an additional commit and added freight change.

I also created a PR (#41 ) for indigo.
Could you merge this if there is no problem?

@moriarty moriarty merged commit 91093fb into ZebraDevs:melodic-devel Apr 23, 2019
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.

None yet

5 participants