Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update Backends Documentation #438
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
base: main
Are you sure you want to change the base?
Update Backends Documentation #438
Changes from 1 commit
a6cda74
e0d64fb
ec2a8ad
aa0415b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example doesn't work for multiple reasons:
client
is assigned aMyExamplePlanner
instance, which is not a client (did you meanMyExampleClient
?)MyExamplePlanner
is anyway not the name of the example class aboverobot
as first param, that's only if you call the IK feature directly skipping what the robot class does for you. Maybe you meantik_result = robot.inverse_kinematics(frame)
But I think the example is not the best anyway. I would suggest changing it to limit it to a more concrete example of what actually works if the person reading up to here implements (and understand) only this part.
So, as a first applied example -in my opinion- the correct thing is to show only what was coded above and not more, ie. how to instantiate the planner and use the backend feature:
Right after this semi-trivial example, we could a more elaborate one, but as you implied on the conversation on slack, it's kind of tricky to find one because the current client <-> planner <-> robot relationship is kind of convoluted.
One example using the current implementation could be this (notice that it depends on the tiny PR that I just sent):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a typo in line 70 (not in your changes, but if you're updating, it, it makes sense to change. It says "These interfaces as exist to allow", but I guess it should read "These interfaces also exist to allow"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This second example in general has never made much sense. I think we could kill the entire
ClientA
/ClientB
example.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So do we remove the rest starting from "These interface also exist to allow mixing..."?