Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Improve node launching for rosrun #43

Merged
merged 2 commits into from
Jul 18, 2019
Merged

Conversation

gautamjain
Copy link
Contributor

Made the changes that @jubeira mentioned in #33.

I used theinstallDist task instead of installApp.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@gautamjain gautamjain changed the title Improve node launching for rosrun - fixes #33 Improve node launching for rosrun Jul 2, 2019
@gautamjain
Copy link
Contributor Author

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels Jul 2, 2019
@jubeira
Copy link

jubeira commented Jul 2, 2019

Thanks @gautamjain!
I've gave it a quick try and it didn't work for me; I still see the scripts directory after installing. Maybe something changed along the Gradle version since the time I posted that suggested fix?
Have you tried this on your side with a sample package?

@gautamjain
Copy link
Contributor Author

gautamjain commented Jul 2, 2019

Yes, it worked when I tried it. The scripts directory in the subproject's build directory was no longer there.

This is how I tested. Let me know if I missed something:

  • Cloned the modified rosjava_build_tools into my catkin workspace
  • ran catkin_make and sourced devel/setup.bash
  • confirmed that which catkin_create_rosjava_project and which catkin_create_rosjava_pkg was pointing to the scripts under devel/bin
  • ran the scripts and generated new a package and a project
  • Tested by running ./gradlew installDist. When I commented out the new tasks, the scripts directory would reappear.

Using the default gradle wrapper (4.10.2).

@gautamjain
Copy link
Contributor Author

I also removed the << operator which is deprecated (and not available in Gradle 5.0).

@gautamjain
Copy link
Contributor Author

Hey @jubeira,

If you have any ideas on why it might not be working on your end, let me know. I can try to replicate on my machine.

@jubeira
Copy link

jubeira commented Jul 12, 2019

Thanks @gautamjain; I've been a bit packed lately but I have this on my radar. I'll give it another shot and let you know!

Copy link

@jubeira jubeira left a comment

Choose a reason for hiding this comment

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

Re-tested; this works.

The only detail is that this change does not affect rosjava modules (e.g. pubsub tutorial) don't have this task set, so rosrun still can't be used for tutorials inside rosjava_core.

@jubeira jubeira merged commit 9583bf2 into rosjava:kinetic Jul 18, 2019
@jubeira
Copy link

jubeira commented Jul 18, 2019

Thanks @gautamjain!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants