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

Ign pkg_create #1582

Closed

Conversation

harshmahesheka
Copy link
Contributor

@harshmahesheka harshmahesheka commented Jul 10, 2022

🎉 New feature

Depends on gazebosim/gz-cmake#262

Summary

This pull request is regarding my GSoC project which you can visit here . The idea is to create an ignition subcommand that can create a template ignition package based on the user's demands. You can see currently supported options here. Currently in the advanced package option a sample world, model, executable and plugin is made with code to install it.

Would love everyone's review on how to shape it better for users!

Test it

Build this repo with its dependencies and run ign pkg_create --help to see various options.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

src/cmd/cmdpkg_create.rb.in Outdated Show resolved Hide resolved
src/cmd/cmdpkg_create.rb.in Outdated Show resolved Hide resolved
@@ -0,0 +1,17 @@
//This is a sample code showing how an ignition executable works
Copy link
Contributor

Choose a reason for hiding this comment

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

Add license?

Suggested change
//This is a sample code showing how an ignition executable works
// This is a sample code showing how an Gazebo simulator executable works

Copy link
Contributor Author

@harshmahesheka harshmahesheka Jul 11, 2022

Choose a reason for hiding this comment

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

Should I add the license to CMakeLists.txt and the package.xml file I am creating as well? But that would mean anyone using this command will have licensed code to begin with

src/cmd/resources/plugins/HelloWorld.cc Outdated Show resolved Hide resolved
src/cmd/resources/plugins/HelloWorld.hh Outdated Show resolved Hide resolved
src/cmd/resources/worlds/empty_platform_with_elevator.sdf Outdated Show resolved Hide resolved
src/cmd/resources/worlds/empty_platform_with_elevator.sdf Outdated Show resolved Hide resolved
test/integration/log_system.cc Outdated Show resolved Hide resolved
@chapulina chapulina added the 🏯 fortress Ignition Fortress label Jul 11, 2022
@chapulina chapulina requested a review from arjo129 July 11, 2022 16:40
Signed-off-by: Harsh Mahesheka <[email protected]>
Copy link
Contributor

@arjo129 arjo129 left a comment

Choose a reason for hiding this comment

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

Done a first pass. There's some front end work that could be done to make things neater. Also have a few knits.

opts.on('-e [arg]', '--export_type [arg]', String, 'Export type between colcon or plain-camke') do |e|
options['export_type'] = e
end
opts.on('-d','--standard_dependecies', String, 'Package depends on standard ign packages') do
Copy link
Contributor

Choose a reason for hiding this comment

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

What are standard dependencies? Perhaps it'd be better to name dependencies individually.
You could have a --all-gz-libs to have it depend on all dependencies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I will change the name to all-ign-libs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have changed it to --ign_dependencies because I am not including all ign packages as some of them are unnecessary

src/cmd/cmdpkg_create.rb.in Outdated Show resolved Hide resolved
puts usage
exit
end
opts.on('-n [arg]', '--name [arg]', String, 'Name of new package') do |n|
Copy link
Contributor

Choose a reason for hiding this comment

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

Include author and license as options as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, that information will be used when using ament cmake. Will that be ok?

@@ -1642,4 +1642,4 @@ TEST_F(LogSystemTest, IGN_UTILS_TEST_DISABLED_ON_WIN32(LogTopics))
this->RemoveLogsDir();
this->CreateLogsDir();
#endif
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to add a newline to make git happy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the exact same file,I don't know why git is showing changes

point2.set_z(std::rand());
std::cout << "Random_point1:\n" << point1.DebugString() << std::endl;
std::cout << "Random_point2:\n" << point2.DebugString() << std::endl;
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Knit: New line at end of file. Might be more worthwhile to spawn a simulator like we do in unit tests to make the example more interesting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea was to keep the example simple and easy to replicate.What do you think?

src/cmd/resources/plugins/HelloWorld.hh Outdated Show resolved Hide resolved
src/cmd/resources/package_xml.erb Outdated Show resolved Hide resolved
@chapulina chapulina added the enhancement New feature or request label Jul 23, 2022
Signed-off-by: Harsh Mahesheka <[email protected]>
Signed-off-by: Harsh Mahesheka <[email protected]>
@harshmahesheka
Copy link
Contributor Author

Also, https://raw.githubusercontent.com/ignition-tooling/gazebodistro/master/collection-fortress.yaml has gz packages names instead of ign.I am confused, shouldn't fortress have ign name?

@chapulina
Copy link
Contributor

@chapulina chapulina closed this Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request 🏯 fortress Ignition Fortress
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants