-
Notifications
You must be signed in to change notification settings - Fork 110
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
Allow to set a robot's base (RCF) #762
Conversation
self._origin = value | ||
# The following is a world-relative frame representing the origin, which change with | ||
# the joint state, while `origin` is parent-relative and static. | ||
self.current_origin = value.copy() |
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.
maybe i haven't properly thought this through, but should this be
self.current_origin = self.current_origin @ self.origin.inverse() @ value
where i'm ignoring the distinction between a frame and its transformation
self._axis = value | ||
# The following is a world-relative axis which change with | ||
# the joint state, while `axis` is parent-relative and static. | ||
self.current_axis = value.copy() |
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.
And similarly here, should we not 'remember' the transformations that have already been applied to the current_axis
?
src/compas/robots/model/robot.py
Outdated
base_link = self.get_link_by_name(self.get_base_link_name()) | ||
|
||
# if there's no fixed joint and there is an RCF is explicitly set, return it as a transform | ||
ref_frame = Frame.worldXY() |
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.
I don't understand. Say you've got a robot with a proper base link. Then you set robot.rfc = <some frame>
. This will change the origin of the base link. But if then you access robot.rcf
you'll get the identity transformation in return. Does that make sense? Is that supposed to make sense?
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.
nope, the comment might be somewhat misleading if it's right above this line, but Frame.worldXY()
is the default if the if
right after is not true. In the case where you set the rcf
, then the if
goes in, and the ref_frame
is that one instead.
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.
i think i didn't express myself well. consider the following code:
import math
import compas_fab
from compas.robots import LocalPackageMeshLoader, RobotModel, Frame
urdf_filepath = compas_fab.get('universal_robot/ur_description/urdf/ur5.urdf')
loader = LocalPackageMeshLoader(compas_fab.get('universal_robot'), 'ur_description')
model = RobotModel.from_urdf_file(urdf_filepath)
assert model.rcf == Frame.worldXY()
model.rcf = Frame.from_euler_angles([math.pi, 0, 0], point=[1,0,0])
assert model.rcf == Frame.worldXY()
Without knowing the internals of rcf
, that doesn't make any sense to the user.
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.
You're totally right. I'll fix.
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 should be now fixed (a4a3e8f) along with simpler code path to determine what happens where (and a bit less duplication)
It will also need to be propagated to the |
Co-authored-by: beverlylytle <[email protected]>
Co-authored-by: beverlylytle <[email protected]>
src/compas/utilities/decorators.py
Outdated
if event_observers: | ||
event_observers.remove(callback) | ||
if not event_observers: | ||
del event_source.__event_observers__[event_key] |
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.
I see a lot of potential for exceptions being raised with this function. If there is a typo in event_name
, there will be a KeyError
. If the wrong callback is passed there could be a ValueError
. Does it make sense to raise custom exceptions for this, rather than the generic and perhaps mysterious ones?
src/compas/utilities/decorators.py
Outdated
Examples | ||
-------- | ||
>>> class Test(object): | ||
... @observable(event_name='init') |
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.
I trust myself to type 'init' correctly, but I do not trust myself to type 'updated_transformations' correctly. I know this is what tests are for, but somethings are difficult to test (say, did the image of the robot in blender update correctly when the rcf changed?). Is there a nice way of throwing an exception somewhere if the observable event names for a class don't match up with the observed event names or vice versa? or is that something that shouldn't be protected against?
I just noticed that |
@gonzalocasas should this PR move to |
@gonzalocasas I guess we have decided to shelf this idea for the time being. How about we move it to compas_robots first and manage it from there? |
I know we said that we will not work on this issue soon, but just to collect one more argument for developing this function, is that the ability to set the robot base (to be different from World frame), can easily fix the orientation of a UR base. |
This adds a property RCF to the robot model (
model.rcf
) to allow changing the base frame of the robot. Since operation is potentially quite involved once the model is used inCOMPAS FAB
, I will need to add an event emitter or some way in which this model can be observed from an external class and hook into changes (like rcf change) so that those are propagated to backends. I'm still not sure how to do this best so, I'm opening this a draft for comments already now and I will update soon.What type of change is this?
Checklist
Put an
x
in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.CHANGELOG.md
file in theUnreleased
section under the most fitting heading (e.g.Added
,Changed
,Removed
).invoke test
).invoke lint
).compas.datastructures.Mesh
.