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

moving launch files for processing nodelets to a driver independent package #59

Open
130s opened this issue Jan 6, 2018 · 14 comments
Open

Comments

@130s
Copy link
Member

130s commented Jan 6, 2018

Issue by piyushk
Tuesday Jul 30, 2013 at 16:36 GMT
Originally opened as ros-drivers/openni_launch#5


@jonbinney @bit-pirate

I have moved launch files to a separate rgbd_launch package. You can use this rosinstall file to test the new organization.

If things look alright to you, I will first release rgbd_launch, and then submit a pull request for openni_launch and freenect_launch once rgbd_launch makes it to shadow-fixed.

Additionally, there are a number of examples in freenect_launch that you can test out. See the documentation for reference: http://ros.org/wiki/freenect_launch

@130s
Copy link
Member Author

130s commented Jan 6, 2018

Comment by jonbinney
Thursday Aug 01, 2013 at 23:00 GMT


Very cool. I just ran openni_launch and it worked great. We probably want some way for users to find the new launchfiles. For instance, if they used openni_launch/kinect_frames.launch on groovy in one of their own launchfiles, then their launchfiles wouldn't work anymore.

Maybe dummy launchfiles in openni_launch and freenect_launch which just launch a node that says "this launchfile has moved to rgbd_launch"?

@130s
Copy link
Member Author

130s commented Jan 6, 2018

Comment by bit-pirate
Thursday Aug 01, 2013 at 23:14 GMT


Hm, sounds like too much extra work to me. I would just intentionally break anyone's launch files and leave a text file there containing a comment about the new location. Roslaunch's error message (can't find file) gives a good idea about what's going on.

Additionally, I'd like to propose renaming of launch files, which are not used directly, from .launch to *.xml []. In that way those files won't show up in roslaunch's auto-completion. Also, it gives the user an hint that those files are not intended to be used by them directly.

[*] Actually, it would be better to put an underscore (or similar) as suffix to those "include" launch files, but that hasn't been implemented yet (ros/ros_comm#254).

@130s
Copy link
Member Author

130s commented Jan 6, 2018

Comment by jonbinney
Friday Aug 02, 2013 at 00:04 GMT


An underscore sounds like a good idea to let people know the launchfiles are private.

If we just have a text file to let people know that the old launchfiles have moved, won't they have to dig around for it? They'll call roslaunch with their custom launchfile that uses some launchfile in openni_launch, for instance, and it will fail. Then, if it was me, I'd do roscd openni_launch; ls launch. I notice that hey, almost all the old launchfiles are gone. I suppose a very explicitly named textfile could work... something like openni_launch/launch/WHERE_THE_LAUNCHFILES_WENT

@130s
Copy link
Member Author

130s commented Jan 6, 2018

Comment by bit-pirate
Friday Aug 02, 2013 at 00:23 GMT


What about we rename them to: _somelauncher.xml In this way they won't show up in the tab completion and we already introduce the "underscore means include/private" thinking.

Adding an explicit text file should do the trick, I think. Just to be clear, your idea is much nicer - I'm just too lazy to implement it. :-)

@130s
Copy link
Member Author

130s commented Jan 6, 2018

Comment by piyushk
Friday Aug 02, 2013 at 00:27 GMT


All these suggestions sound great. I'll write a small node that displays a
fatal message about the launch file being moved.

I have some other work to do today and tomorrow. I'll work on this sometime
early next week and ping the thread.

Do you think we still have time to release these changes into Hydro?

On Thu, Aug 1, 2013 at 7:23 PM, Marcus Liebhardt
[email protected]:

What about we rename them to: _somelauncher.xml In this way they won't
show up in the tab completion and we already introduce the "underscore
means include/private" thinking.

Adding an explicit text file should do the trick, I think. Just to be
clear, your idea is much nicer - I'm just too lazy to implement it. :-)


Reply to this email directly or view it on GitHubhttps://github.com/ros-drivers/openni_launch/issues/5#issuecomment-21979397
.

@130s
Copy link
Member Author

130s commented Jan 6, 2018

Comment by jonbinney
Friday Aug 02, 2013 at 00:31 GMT


With the node that displays the fatal message, I'm ok with releasing this
into hydro. I think that most people use only the toplevel launchfiles in
openni_launch/freenect.launch, and for the people that use the individual
launchfiles the fix shouldn't be hard.

On Thu, Aug 1, 2013 at 5:27 PM, Piyush Khandelwal
[email protected]:

All these suggestions sound great. I'll write a small node that displays a
fatal message about the launch file being moved.

I have some other work to do today and tomorrow. I'll work on this
sometime
early next week and ping the thread.

Do you think we still have time to release these changes into Hydro?

On Thu, Aug 1, 2013 at 7:23 PM, Marcus Liebhardt
[email protected]:

What about we rename them to: _somelauncher.xml In this way they won't
show up in the tab completion and we already introduce the "underscore
means include/private" thinking.

Adding an explicit text file should do the trick, I think. Just to be
clear, your idea is much nicer - I'm just too lazy to implement it. :-)


Reply to this email directly or view it on GitHub<
https://github.com/ros-drivers/openni_launch/issues/5#issuecomment-21979397>

.


Reply to this email directly or view it on GitHubhttps://github.com/ros-drivers/openni_launch/issues/5#issuecomment-21979531
.

@130s
Copy link
Member Author

130s commented Jan 6, 2018

Comment by piyushk
Monday Aug 12, 2013 at 20:45 GMT


@jonbinney @bit-pirate @vrabaud

I've updated all 3 packages based on the current discussion. Try merging/updating from the original rosinstall file again and let me know. I will start an issue in rgbd_launch regarding launch file cleanup, but I would like to tackle migration before we try that out.

@130s
Copy link
Member Author

130s commented Jan 6, 2018

Comment by piyushk
Thursday Aug 22, 2013 at 05:50 GMT


Hey folks! I have released a new version of freenect_launch and rgbd_launch that solves #5, #6 and #7. If you can test the system release of freenect_launch (v0.3.0) and let me know if these problems have been solved, I will push the same changes to openni_launch.

@130s
Copy link
Member Author

130s commented Jan 6, 2018

Comment by bit-pirate
Thursday Aug 22, 2013 at 06:22 GMT


Here is a small update on the launch includes naming: ros/ros_comm#254
We are applying the same rule to Kobuki and TurtleBot packages. I think we should adapt this here as well.

@130s
Copy link
Member Author

130s commented Jan 6, 2018

Comment by bit-pirate
Thursday Aug 22, 2013 at 06:23 GMT


@piyushk Could you please name the remaining things you like us to test?

@130s
Copy link
Member Author

130s commented Jan 6, 2018

Comment by piyushk
Thursday Aug 22, 2013 at 21:38 GMT


@bit-pirate I have opened up #8

rgbd_launch is now available as a deb package. Can you verify that the pull request works for you, and that it solves #6 and #7?

Thanks!

@130s
Copy link
Member Author

130s commented Jan 6, 2018

Comment by bit-pirate
Monday Aug 26, 2013 at 00:35 GMT


@piyushk I have tested your PR and I can confirm that it fixes #6 and #7.
My test included the quick start instructions.

@130s
Copy link
Member Author

130s commented Jan 6, 2018

Comment by jonbinney
Monday Aug 26, 2013 at 00:43 GMT


I'll go ahead and merge ros-drivers/openni_launch#8 (i don't have an asus right now to test, but sounds like you guys have covered that). I'll do a release shortly.

@130s
Copy link
Member Author

130s commented Jan 6, 2018

Comment by jonbinney
Monday Aug 26, 2013 at 01:13 GMT


Just released openni_launch into hydro: ros/rosdistro#1857

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

1 participant