osrf / vrx

Virtual RobotX (VRX) resources.
Apache License 2.0
390 stars 180 forks source link

Jessica/wind plugin port #637

Closed j-herman closed 1 year ago

j-herman commented 1 year ago

To test:

ros2 launch vrx_gz competition.launch.py world:=practice_2022_stationkeepingX_task

You should see no movement due to wind in stationkeeping0, a slow drift towards the docks in stationkeeping1, and a faster drift towards the shore in stationkeeping2.

Wind speed and direction are published on /vrx/debug/wind/speed and /vrx/debug/wind/direction. As in gazebo classic, they're referenced to the Gazebo world frame.

EDIT: Removed old description for the draft PR and copied testing instructions from below.

M1chaelM commented 1 year ago

@j-herman Thanks! When it is ready for testing, could we swap out test_wind_world with the 3 versions of practice_2023_stationkeeping#_task.sdf configured to restore the original wind settings for those worlds (found here)? I think this would help us get an intuitive sense of whether things are working as expected. Once it's set, we'll eventually update all the tasks with these settings.

j-herman commented 1 year ago

@j-herman Thanks! When it is ready for testing, could we swap out test_wind_world with the 3 versions of practice_2023_stationkeeping#_task.sdf configured to restore the original wind settings for those worlds (found here)? I think this would help us get an intuitive sense of whether things are working as expected. Once it's set, we'll eventually update all the tasks with these settings.

@M1chaelM I don't see any 2023 practice worlds - do I need to compile them locally from the xacros? The link points to the gazebo_classic branch. Once I have the right world files I'll add versions with the new plugin. I do want to keep a more aggressive world for testing since it makes the effects easier to identify, especially for changing things that we don't usually mess with for the competition worlds, but we can delete it before merging.

M1chaelM commented 1 year ago

@M1chaelM I don't see any 2023 practice worlds

Sorry, typo. I should have said 2022 practice worlds. We ported the 2022 practice worlds to VRX 2.0, but the ported versions are all using the gazebo wind plugin. The versions from Gazebo classic still have the settings for our custom wind plugin, so I was trying to suggest that you could copy those settings to the ported 2022 practice worlds for stationkeeping (as a simple example) and it would help us check that the wind is functioning the way it used to.

Edit: One thing that's making this more confusing is that we created new practice worlds for tasks that didn't exist in 2022, but they have also been named with the prefix practice_2022_ and put in the 2022_practice folder. (Maybe we should just eventually change these names to 2023.)

j-herman commented 1 year ago

[like] Herman, Jessica (CIV) reacted to your message:


From: M1chaelM @.> Sent: Tuesday, May 9, 2023 1:26:07 PM To: osrf/vrx @.> Cc: Herman, Jessica (CIV) @.>; Mention @.> Subject: Re: [osrf/vrx] Jessica/wind plugin port (PR #637)

NPS WARNING: external sender verify before acting.

@M1chaelMhttps://github.com/M1chaelM I don't see any 2023 practice worlds

Sorry, typo. I should have said 2022 practice worlds. We ported the 2022 practice worlds to VRX 2.0, but the ported versions are all using the gazebo wind plugin. The versions from Gazebo classic still have the settings for our custom wind plugin, so I was trying to suggest that you could copy those settings to the ported 2022 practice worlds for stationkeeping (as a simple example) and it would help us check that the wind is functioning the way it used to.

— Reply to this email directly, view it on GitHubhttps://github.com/osrf/vrx/pull/637#issuecomment-1540123171, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AORRLBT6ASGXGDWAYMYMMZTXFJAW7ANCNFSM6AAAAAAXXUYTIA. You are receiving this because you were mentioned.Message ID: @.***>

j-herman commented 1 year ago

@j-herman Thanks! When it is ready for testing, could we swap out test_wind_world with the 3 versions of practice_2023_stationkeeping#_task.sdf configured to restore the original wind settings for those worlds (found here)? I think this would help us get an intuitive sense of whether things are working as expected. Once it's set, we'll eventually update all the tasks with these settings.

Three stationkeeping 2022 practice worlds updated as requested. They all run but I haven't verified the behavior matches expectations yet.

j-herman commented 1 year ago

@j-herman Thanks! When it is ready for testing, could we swap out test_wind_world with the 3 versions of practice_2023_stationkeeping#_task.sdf configured to restore the original wind settings for those worlds (found here)? I think this would help us get an intuitive sense of whether things are working as expected. Once it's set, we'll eventually update all the tasks with these settings.

Three stationkeeping 2022 practice worlds updated as requested. They all run but I haven't verified the behavior matches expectations yet.

j-herman commented 1 year ago

There is a potential discrepancy between the legacy code implementation and the desired (published) behavior, unless I'm misunderstanding something. In lines 254-260 of usv_gazebo_wind_plugin.cc an apparent wind value is calculated (subtracting the vehicle's velocity from the wind vector), but the uncorrected wind is used in the force calculations. I have included that correction in the new version.

Overall the ported plugin seems to be working as desired. In comparison to the gazebo classic version, an equivalent force seems to produce less drift in the WAM-V. I don't know whether other parameters may have changed in the model or in the port of the physics engines, or if there is something going on with how I am applying the force to the link. There are a few more things I can troubleshoot in the implementation, but I do think this may be close enough to include in the next release even if we need to make tweaks later. Qualitatively the behavior looks good and the interfaces are set.

Wind speed trace for stationkeeping1 world in gazebo classic: image Wind speed trace for stationkeeping1 world in gazebosim (note that the timescale is not in seconds - I haven't figured out how to fix that, so this graph represents more than 30 seconds): image Wind speed trace for stationkeeping2 world in gazebo classic: image Wind speed trace for stationkeeping2 world in gazebosim (same timescale issue): image

In both worlds, the wind direction and the general behavior of the WAM-V were consistent - that is, the vehicle runs into the docks in stationkeeping1, and into the shore by the tent in stationkeeping2.

j-herman commented 1 year ago

To test: ros2 launch vrx_gz competition.launch.py world:=practice_2022_stationkeepingX_task You should see no movement due to wind in stationkeeping0, a slow drift towards the docks in stationkeeping1, and a faster drift towards the shore in stationkeeping2.
Wind speed and direction are published on /vrx/debug/wind/speed and /vrx/debug/wind/direction. As in gazebo classic, they're referenced to the Gazebo world frame. A potential enhancement could be to also publish this information in the format that would be received from a weather station (ie, in knots and with the direction specified as the compass point from which the wind is blowing).

M1chaelM commented 1 year ago

@j-herman Thanks! Should we mark this ready for review? If so I can test it.

j-herman commented 1 year ago

Ah, I forgot about the ROS bridge! I'll knock that out today. The blank output is odd and I'll see what's causing that.

Edit: Now publishing ros2 topics correctly... including zero values. The message is published, and I can extract the value several different ways. The same thing happens if I set the wind direction to zero.
Confirmed this is a gazebo issue and not specific to our code. Simple test case with pub/sub from the command line: image Publishing 0 does not result in printed output. Known issue with protobuf: see this closed issue