Skip to content
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

Fix coordinate frames on state message #81

Closed
wants to merge 19 commits into from
Closed

Conversation

chapulina
Copy link
Contributor

@chapulina chapulina commented Nov 16, 2021

@chapulina chapulina self-assigned this Nov 17, 2021
@chapulina chapulina mentioned this pull request Nov 20, 2021
33 tasks
Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina changed the base branch from main to chapulina/thruster_again November 23, 2021 22:23
Signed-off-by: Louise Poubel <[email protected]>
Base automatically changed from chapulina/thruster_again to main November 24, 2021 01:16
Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina marked this pull request as ready for review December 2, 2021 00:42
@chapulina chapulina changed the title Documenting and cleaning up last message fields Complete message fields: fix coordinate frames Dec 2, 2021
@arjo129 arjo129 self-requested a review December 3, 2021 06:53
@arjo129
Copy link
Member

arjo129 commented Dec 8, 2021

Hey, @chapulina I've not had time to debug the failures in this PR. But on my PC when I run the tests I'm also seeing segfaulting and regression in a lot of the integration test cases. It seems to cause the depth_pitchmass_vbs test to segfault consistently...

Signed-off-by: Louise Poubel <[email protected]>
@chapulina
Copy link
Contributor Author

chapulina commented Dec 9, 2021

I narrowed down the controller failures to the RPH orientation. I'm still trying to understand the expectations around those values, see comments on 5903584.

//
// [VerticalControl](CRITICAL): Excessive depth excursion=10.573917 m, failToGoUpDepth_=17.503380 m, depthRate=0.211946 m/s, pitch =0.114867 deg.
//
// The difference between poseOffsetNED.Rot() and rph is that roll and pitch are swapped.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a simple example, I manually pitched the vehicle's nose down. The frames shown are ENU (red: East, green: North, blue: Up).

image

This corresponds to a negative pitch about North (green). That's the first element of NED, and the second of END. The print outs of the line below are consistent with that:

NED: -0.505718 0.000121 0 -- END: 0.000138 -0.505718 6.8e-05

I think that what could be happening is that the controller expects the vehicle to start facing North instead of West. A nosedown pitch with the robot facing North would be negative on East, and NED would give us a negative angle on it's Y coordinate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just verified that if the vehicle starts facing North, the NED orientation works for the controllers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diff --git a/lrauv_ignition_plugins/worlds/buoyant_tethys.sdf b/lrauv_ignition_plugins/worlds/buoyant_tethys.sdf
index 1618c1e..412ffbb 100644
--- a/lrauv_ignition_plugins/worlds/buoyant_tethys.sdf
+++ b/lrauv_ignition_plugins/worlds/buoyant_tethys.sdf
@@ -315,7 +315,7 @@
     </include-->
 
     <include>
-      <pose>0 0 0 0 0 0</pose>
+      <pose degrees="true">0 0 0  0 0 -90</pose>
       <uri>tethys_equipped</uri>
     </include>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed in 23e7631, checking with @braanan before updating all test expectations.

@chapulina chapulina marked this pull request as draft December 15, 2021 17:58
@chapulina
Copy link
Contributor Author

Moving this PR to draft. I'll be breaking some of the smaller changes into new PRs, and update this PR to reorient the model in a way that it faces North at zero orientation. While at it, I'll also tackle #80 .

Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina changed the title Complete message fields: fix coordinate frames Fix coordinate frames on state message Dec 16, 2021
@caguero caguero mentioned this pull request Jan 19, 2022
39 tasks
@caguero caguero added this to the 2022 M1 milestone Jan 19, 2022
@chapulina
Copy link
Contributor Author

Closing this PR, it's being broken into smaller ones, starting with:

@chapulina chapulina closed this Jan 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LRAUVState fields population status
3 participants