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

DOCS-2889: Turn access arm tutorial into how-to #3650

Merged

Conversation

sguequierre
Copy link
Collaborator

@sguequierre sguequierre commented Nov 1, 2024

@viambot viambot added the safe to build This pull request is marked safe to build from a trusted zone label Nov 1, 2024
@sguequierre sguequierre changed the title DOCS-2889: Turn access arm guide into how-to DOCS-2889: Turn access arm tutorial into how-to Nov 1, 2024
docs/how-tos/move-robot-arm.md Show resolved Hide resolved
docs/how-tos/move-robot-arm.md Outdated Show resolved Hide resolved
docs/how-tos/move-robot-arm.md Outdated Show resolved Hide resolved
docs/how-tos/move-robot-arm.md Outdated Show resolved Hide resolved
docs/how-tos/move-robot-arm.md Outdated Show resolved Hide resolved
## Move the arm

The two main options for specifying arm movement are through **joint position commands** and through **pose commands**.

Copy link
Collaborator

Choose a reason for hiding this comment

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

can you add some info on what these are and why you might choose one or the other?

Copy link
Collaborator

@JessamyT JessamyT Nov 4, 2024

Choose a reason for hiding this comment

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

I'm going to ask Esha and Ray whether we should just take out the pose command stuff from this tutorial anyway since they are deprecated and the motion service does the same thing.
Edit: Per slack convo (cc'ed Sierra), sounds like we can leave in for now (or not, but probably easier to leave in). TBD what happens with this API anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@JessamyT can you check what I wrote on line 114? not entirely sure if that's the best description

{{% tablestep %}}
**1. Initiate motion with a joint position command**

Add the following code to your script:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think put that inside the tabs under the import and above the code? That's less awkward

docs/how-tos/move-robot-arm.md Outdated Show resolved Hide resolved
docs/how-tos/move-robot-arm.md Outdated Show resolved Hide resolved
docs/how-tos/move-robot-arm.md Outdated Show resolved Hide resolved
@npentrel npentrel requested a review from JessamyT November 4, 2024 18:01
@npentrel
Copy link
Collaborator

npentrel commented Nov 4, 2024

Love the GIFs and how this is looking. I think @JessamyT might actually be a better reviewer so tagging her here as well

docs/how-tos/move-robot-arm.md Outdated Show resolved Hide resolved
docs/how-tos/move-robot-arm.md Outdated Show resolved Hide resolved
docs/how-tos/move-robot-arm.md Outdated Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

This image is oddly narrow--I know we don't like to add too much whitespace but I imagine most people will use at least a slightly wider browser window. Most of our config screenshots are a bit wider than this which I'd recommend so that it looks more like what people will see, and so that it fits into the page content more nicely.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Deleting the image anyways

docs/how-tos/move-robot-arm.md Outdated Show resolved Hide resolved
@JessamyT
Copy link
Collaborator

JessamyT commented Nov 4, 2024

Big fan of the GIFs!

@JessamyT
Copy link
Collaborator

JessamyT commented Nov 4, 2024

I think it'd be good to show the full code at the end.
Also [nit] consider, in the code snippets, either removing the line numbers or adding like data-start="94" so they better reflect what the user will do.

@sguequierre sguequierre requested a review from npentrel November 6, 2024 20:47
Comment on lines 23 to 24
{{<imgproc src="/tutorials/motion/access_01_xarm6.png" resize="500x" declaredimensions=true alt="A picture of the UFACTORY xArm 6." style="width: 150px" class="fill alignleft" >}}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this actually looks weird here I think let's remove

Suggested change
{{<imgproc src="/tutorials/motion/access_01_xarm6.png" resize="500x" declaredimensions=true alt="A picture of the UFACTORY xArm 6." style="width: 150px" class="fill alignleft" >}}


{{< /alert >}}

{{< alert title="Caution" color="caution" >}}
Copy link
Collaborator

Choose a reason for hiding this comment

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

To avoid having two alerts here, let's put this with the Move the arm section

weight: 70
type: "docs"
tags: ["arm", "components"]
images: ["/tutorials/motion/preview.jpg"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you add your gif as a preview here?

@sguequierre
Copy link
Collaborator Author

I think it'd be good to show the full code at the end. Also [nit] consider, in the code snippets, either removing the line numbers or adding like data-start="94" so they better reflect what the user will do.

removed line numbers

Copy link
Collaborator

@npentrel npentrel left a comment

Choose a reason for hiding this comment

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

LGTM once Jessamy approves

@JessamyT
Copy link
Collaborator

Thoughts on showing the full script?

Some sections still have line numbers so the mix looks odd. Either remove from all or add to all with somewhat correct-ish numbers.

@sguequierre
Copy link
Collaborator Author

sguequierre commented Nov 14, 2024

Thoughts on showing the full script?

Some sections still have line numbers so the mix looks odd. Either remove from all or add to all with somewhat correct-ish numbers.

@JessamyT I don't think the full script is necessary for a how-to. unless you feel strongly otherwise I'm fine with it as-is
Removed remaining line numbers

@viambot
Copy link
Member

viambot commented Nov 14, 2024

You can view a rendered version of the docs from this PR at https://docs-test.viam.dev/3650

Copy link
Collaborator

@JessamyT JessamyT left a comment

Choose a reason for hiding this comment

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

LGTM

@sguequierre sguequierre merged commit 6927777 into viamrobotics:main Nov 14, 2024
9 checks passed
@sguequierre sguequierre deleted the DOCS-2889/turn-access-arm-into-how-to branch November 14, 2024 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to build This pull request is marked safe to build from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants