osrf / vrx

Virtual RobotX (VRX) resources.
Apache License 2.0
437 stars 198 forks source link

Additional VRX 2023 Tutorials #645

Closed M1chaelM closed 1 year ago

M1chaelM commented 1 year ago
M1chaelM commented 1 year ago

@j-herman Can you look at these two tutorials:

j-herman commented 1 year ago

@M1chaelM My plan for these and future tutorials is to assume the code is correct, and write the tutorial to match the code. I'll leave notes here when there's a significant change, in case it was not intended. Example: we used to have tags to edit in the xacro to enable/ disable thruster articulation, change the max angle, change topics... none of those parameters still exist, so I am removing that section from the 2023 tutorial. Also, we state that thruster angle control is via a "pre-tuned PID controller" - we are actually just using the default PID values for a position controller in Gazebo. Should we do some tuning? Do we want to allow the teams to mess with that? It would be set in wamv_gazebo_thruster_config.xacro. For now I will comment out the note on PID altogether.

M1chaelM commented 1 year ago

Example: we used to have tags to edit in the xacro to enable/ disable thruster articulation, change the max angle, change topics... none of those parameters still exist, so I am removing that section from the 2023 tutorial.

Seems ok, but let's check with @caguero about what is still supported. I think he worked on this.

Also, we state that thruster angle control is via a "pre-tuned PID controller" - we are actually just using the default PID values for a position controller in Gazebo. Should we do some tuning? Do we want to allow the teams to mess with that? It would be set in wamv_gazebo_thruster_config.xacro. For now I will comment out the note on PID altogether.

Do the default values give reasonable behavior? I don't think the teams need to adjust this but again I will defer to @caguero

j-herman commented 1 year ago

Also, we state that thruster angle control is via a "pre-tuned PID controller" - we are actually just using the default PID values for a position controller in Gazebo. Should we do some tuning? Do we want to allow the teams to mess with that? It would be set in wamv_gazebo_thruster_config.xacro. For now I will comment out the note on PID altogether.

Do the default values give reasonable behavior? I don't think the teams need to adjust this but again I will defer to @caguero

Actually, after further research it looks like we aren't using the PID at all. There is an option in the JointPositionController that we have enabled:

<use_velocity_commands> Bypasses the PID and creates a perfect /// position. The maximum speed on the joint can be set using the <cmd_max> /// tag.

This seems fine as long as this isn't an area where we are trying to maximize realism for the teams. I think we are focusing more on the perception and algorithms, rather than on the control details, and so a very fast and accurate repositioning of the thruster might be ok. If we want to use the PID, I'd recommend against the defaults, as they produce undesirable behavior. Good starter values might be:

<p_gain>10</p_gain>
<i_gain>1</i_gain>
<d_gain>2</d_gain> 
M1chaelM commented 1 year ago

This seems fine as long as this isn't an area where we are trying to maximize realism for the teams. I think we are focusing more on the perception and algorithms, rather than on the control details, and so a very fast and accurate repositioning of the thruster might be ok.

Yes, this is right--I think bypassing the PID is our way of simulating a PID that has been pre-tuned as well as possible. I think we can keep this as-is.

caguero commented 1 year ago

I think we're good removing the parameters that we don't have anymore. And about the angular movement of the thruster, as Jessica said we bypass the internal PID to avoid gain tunning and simplify configuration.

j-herman commented 1 year ago

Will need to merge PR 664 for the RViz tutorial. It's in a working state but not yet complete.

j-herman commented 1 year ago

This is tutorial-related, but too much of a nit to open a new issues, so I'll pose the question here: I've been doing many fresh installs of Gazebo/ROS2/VRX and keep forgetting to write down all of the additional packages that need to be installed beyond the humble desktop full install (xacro, for instance, is the first one that pops up). There was a question on the discord page that looked like the same issue. So, I think we need to add something to the installation tutorial to account for these extra packages for those who are installing directly on their systems rather than using docker. rosdep is an option; a list of other packages is another option. Which would we prefer? rosdep has been working for me to fix all the errors at once, but I know there are some reasons we might not want to recommend that.

M1chaelM commented 1 year ago

@j-herman I'm slightly worried by the phrase "all of the additional packages." The only one I've seen so far is xacro. We can definitely add that to the tutorial. Can you let me know if you encounter anything else?

j-herman commented 1 year ago

I can't remember - just know I had a sequence of two or three the first time, which wasn't a 100% fresh install. Agreed, let's just add xacro and I will recheck next time I reset a system.

youxu2017 commented 1 year ago

The tutorial of the Rviz is not working right now ( July 12), right?

j-herman commented 1 year ago

The tutorial of the Rviz is not working right now ( July 12), right?

That's correct. There are some code changes required to support this and they're still in review.

M1chaelM commented 1 year ago

Compliance tutorials todo list:

j-herman commented 1 year ago

The tutorial of the Rviz is not working right now ( July 12), right?

@youxu2017 The files are now incorporated so this tutorial should be working.

caguero commented 1 year ago

Done