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 MTC to concepts #809

Merged
merged 43 commits into from
Jan 15, 2024
Merged

Add MTC to concepts #809

merged 43 commits into from
Jan 15, 2024

Conversation

Abishalini
Copy link
Contributor

@Abishalini Abishalini commented Nov 1, 2023

MTC does not have a lot of docs and here is an attempt to make MTC easier to get started with.

This doc will go over what is an MTC task and the different kinds of stages and containers that can be added to an MTC task (with code examples).
I also want to add a detailed section on how to debug MTC tasks and MTC stage solutions.

Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

This is awesome! Found a few typos and inconsistencies, but it looks great.

One suggestion, if you want, is that because there's so many class names thrown around, to highlight them using CodeFont or Bold or something in the text.

doc/concepts/moveit_task_constructor/connecting_stages.rst Outdated Show resolved Hide resolved
doc/concepts/moveit_task_constructor/connecting_stages.rst Outdated Show resolved Hide resolved
doc/concepts/moveit_task_constructor/connecting_stages.rst Outdated Show resolved Hide resolved
doc/concepts/moveit_task_constructor/wrappers.rst Outdated Show resolved Hide resolved
@rhaschke
Copy link
Contributor

rhaschke commented Nov 1, 2023

I would prefer if you would augment the existing MTC documentation here instead of starting a new one.

Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

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

Again, I propose to update the original MTC docs instead of starting/maintaining a second set of MTC docs here!

@Abishalini
Copy link
Contributor Author

Again, I propose to update the original MTC docs instead of starting/maintaining a second set of MTC docs here!

Yep, I will do that today. Just organizing my thoughts here and addressing some reviews before I open a PR to MTC.

@Abishalini Abishalini marked this pull request as ready for review January 11, 2024 15:50
@Abishalini Abishalini requested a review from sea-bass January 11, 2024 15:50
@Abishalini Abishalini changed the title Draft - Add MTC to concepts Add MTC to concepts Jan 11, 2024
@Abishalini Abishalini requested a review from adlarkin January 11, 2024 18:47
@Abishalini Abishalini requested a review from sea-bass January 11, 2024 21:29
Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

Thank you @Abishalini!

Copy link
Contributor

@adlarkin adlarkin left a comment

Choose a reason for hiding this comment

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

This is awesome, thanks Abi! Left a few grammer/style comments, and a few questions from the perspective of someone who doesn't have much experience with MTC.

state.setToDefaultValues(state.getJointModelGroup("left_arm"), "home");
state.setToDefaultValues(state.getJointModelGroup("right_arm"), "home");
state.update();
spawnObject(scene); // Add a CollisionObject to planning scene
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a user-defined function? If so, it'd be worth clarifying this so that readers don't try to call spawnObject without implementing it and then wonder why they're getting errors 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the comment

Copy link
Contributor

@adlarkin adlarkin left a comment

Choose a reason for hiding this comment

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

Thanks!

@Abishalini Abishalini merged commit 5757d9f into main Jan 15, 2024
9 checks passed
@Abishalini Abishalini deleted the pr-add-docs-on-mtc branch January 15, 2024 13:56
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.

4 participants