Skip to content

Conversation

@efernandez
Copy link
Member

This PR is on top of #22

This PR adds:

  1. Computes the twist at the odometry publish rate. This is very important because any software that uses the odometry can only retrieve the dt used to compute the twist from the difference between the msgs timestamps.
  2. Add a unit test to verify that the current pose P_1 can be computed from the previous pose P_0, the current twist T_1 and the dt obtained as the difference between the msgs timestamps, as P_1 = P_0 * T_1 / dt (which is the rigid motion equation in SE(2), in matrix form).
  3. Extend the rostests with the unittest checks.
  4. All the test checks include the covariance, i.e. the pose covariance of P_1 as reported in the odometry msg, can be retrieved using the pose covariance of P_0 and the twist covariance of T_1, using the (analytical) Jacobian of the rigid motion equation shown before (implemented in element-wise form).

@ayrton04 @servos @afakihcpr

Copy link
Member Author

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Awesome! Hopefully I'll get around to adding a 2D version of the EKF anyway.

@efernandez efernandez force-pushed the compute_twist_on_publish branch 2 times, most recently from 228b683 to 3f844af Compare January 29, 2016 05:35

Choose a reason for hiding this comment

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

These look good.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for confirming it.

@ayrton04
Copy link

I'm not an expert on the ros_controller package, but what I see looks correct! Nice work on the very thorough tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ayrton04 This one is actually wrong. I don't want to bias you... can you review it again?

Choose a reason for hiding this comment

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

My unbiased correction:

 J_twist << -cos_yaw * dt, sin_yaw * dt,  0.0,
            -sin_yaw * dt,  -cos_yaw * dt, 0.0,
            0.0,           0.0,            1.0;

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you sure?, I don't see why the sign changes.

The error I found is on the derivative wrt v_yaw, which should be dt instead of 1.0.
I've just updated the PR with that fix. I think the rest it's fine, indeed now the relevant tests are passing 😃

@efernandez efernandez force-pushed the compute_twist_on_publish branch 3 times, most recently from f7a0703 to 6d190f0 Compare February 1, 2016 02:02
@efernandez efernandez force-pushed the add_limited_state branch 2 times, most recently from dc1b8a6 to 4c30982 Compare February 1, 2016 18:24
@efernandez efernandez force-pushed the compute_twist_on_publish branch from 6d190f0 to 088839f Compare February 1, 2016 18:58
@efernandez
Copy link
Member Author

#22 was closed and merged in #31
Closing this PR to create a new one on top of indigo-devel

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.

2 participants