Closed GijsGroote closed 2 years ago
Great thinking!
I agree that the variables are not very telling at the moment, so thanks for the suggestions.
I agree with the naming for obstacle positions and orientations, pose and twist seem very reasonable. For the base state, i have some comments:
min
? What does it stand for?You are right that the information about twist_min
and output
is redundant in the sense that the angular velocity is the same. The forward velocity is not the same as it is once in the robot frame and once in the world frame.
In summary, I agree with the need for renaming. However, I strongly disagree with making an explicit separation between base joint position and arm joint position. This results in the same paradigm that is now present in all simulation environments for mobile manipulators.
Why do add the suffix min? What does it stand for? I want to make a distinction between Pose which contains position (x, y, z) and orientation (x, y, z, w) and with pose_min (x, y, orientation_around_z_axis). As Pose contains every positional/rotational and pose_min only the important once. The min suffix stands for "minified". Inspired by bootstrap.css and bootstrap.min.css. Which I think is not a proper name since pose and pose_min do not contain the same information (which _min does suggest). I though of pose_important, pose_imp, as name instead of pose_min, but that one does not seem logical. Hmm, this should have some more attention.
I disagree with the splitting of joint state for arm and joint states for the base. What is the rational behind splitting the joint states into two parts? The base is expressed in the world frame (correct me if I am wrong). With the exception of output/base_output which is in Robot frame. This is different than the joints which are expressed in a joint frame (different frame for every joint). Again I could be wrong here about the frames.
What I think is important.
Why do add the suffix min? What does it stand for? I want to make a distinction between Pose which contains position (x, y, z) and orientation (x, y, z, w) and with pose_min (x, y, orientation_around_z_axis). As Pose contains every positional/rotational and pose_min only the important once. The min suffix stands for "minified". Inspired by bootstrap.css and bootstrap.min.css. Which I think is not a proper name since pose and pose_min do not contain the same information (which _min does suggest). I though of pose_important, pose_imp, as name instead of pose_min, but that one does not seem logical. Hmm, this should have some more attention.
In ros, they use Pose2D. That might be a more meaningful name to it.
I disagree with the splitting of joint state for arm and joint states for the base. What is the rational behind splitting the joint states into two parts? The base is expressed in the world frame (correct me if I am wrong). With the exception of output/base_output which is in Robot frame. This is different than the joints which are expressed in a joint frame (different frame for every joint). Again I could be wrong here about the frames.
You have to be very careful about frames here. First, we make the distinction between workspace and configuration space. Frames only refer to Cartesian coordinates, whereas joint configurations do not have a reference frame. It happens to be that the workspace and configuration space for mobile robots are very similar. Then, what you refer to as robot frame and world frame is configuration velocity and workspace velocity, rather than frames.
What I think is important.
* Having the robot's location is world frame x, y, orientation and the first derivatives xdot, ydot, angular velocity. * Having every joint pos and vel.
These two first points are the same, the location and the world frame can somehow be seen as the 'joint_state' of the base joints.
* output or base_output is debatable. What should this be used for?
This is used for some motion planners, such as MPC or Fabrics. You might argue that it can be computed by the planner, but it is easier to have inside the environment, as the environment already computes the forward/angular velocities to actually compute xdot and ydot.
* Seperate everything which is not in the same frame by name and the dictionary which it is in.
Then the only logical separation is between velocities expressed in the configuration space and workspace. This is precisely why I added the 'vel'-key (which is badly named, I agree).
Currently the naming of the observation not self explaining. Additionally observations with equal names have inconsistent structure.
x and obstaclesensor.obstacle_1.x are having a different structure [x_pos, y_pos, theta_orientation] and [x_pos, y_pos ,z_pos]
These should have self explaining names and if the names are equal, equal structure.
EDIT I propose the following structure:
{"pose": {position, orientation}}
Position in Cartesian coordinates with shape (3, ) and orientation in quaternions with shape (4, ){"twist:{linear, angular}}
Linear in Cartesian coordinates with shape (3, ) and angular in Cartesian coordinates with shape (3, ){"base_state":{"pose_min": {position_min, orientation_min}, "twist_min": {linear_min, angular_min}, "base_output": {forward_velocity, angular_velocity}}
position_min contains x and y positions, Cartesian coordinates, shape is (2, ) orientation_min contains the orientation around the vertical z-axis, shape is (1, ), the value will be between -pi and pi. linear_min contains x and y velocities (that's Cartesian), shape is (2, ) angular_min contains the angular velocity around the vertical z-axis, shape is (1, ) output is the output of the base. For robot pointRobotUrdf-vel-v0 this would be array [forward_velocity, angular_velocity], for pointRobotUrdf-ang-v0 this would be array [forward_acceleration, angular_acceleration], shape is (2, ){"joint_state": {position, velocity}} position contains the joint positions with the exception of the base velocity contains the joint velocities with the exception of the base The following piece will handle the joint_state:
Questions: