-
Notifications
You must be signed in to change notification settings - Fork 15
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
Addresses #99. Adds the pitchmass + vbs depth. #105
Conversation
Signed-off-by: Arjo Chakravarty <[email protected]>
Signed-off-by: Arjo Chakravarty <[email protected]>
Signed-off-by: Arjo Chakravarty <[email protected]>
Signed-off-by: Arjo Chakravarty <[email protected]>
… narrowed down. Signed-off-by: Arjo Chakravarty <[email protected]>
Signed-off-by: Arjo Chakravarty <[email protected]>
Signed-off-by: Arjo Chakravarty <[email protected]>
Signed-off-by: Arjo Chakravarty <[email protected]>
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.
LGTM, nice idea with the oscillation check
{ | ||
// Vehicle should dive down. | ||
EXPECT_LT(pose.Pos().Z(), 0.1); | ||
// FIXME(arjo): This should dive to a max of 10m I think |
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.
That's my understanding too reading the mission file:
<DefineArg Name="DepthCmd"><Units:meter/><Value>10.0</Value></DefineArg>
So after after a short discussion with @braanan it seems that the normal mode of using VBS to sink does lead to certain behaviors such as spinning and reverse motion. Therefore, MBARI normally uses both the mass shifter and the buoyancy engine to get the vehicle to sink. MBARI has a mission which does this already, however we aren't actually running this in CI. This PR runs this in CI.
Note: As the model in the main repo is not actually up to the mark, it does not perform properly on this test. Hence the expectations have been kept large. However when combined with some of the changes ion #89 and #59 it is possible to narrow the expectations down.