Skip to content

Feature/se3vel #94

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Feature/se3vel #94

wants to merge 8 commits into from

Conversation

nosmokingsurfer
Copy link
Collaborator

Extended SE3 group added according to the Brossard paper "Associating Uncertainty to Extended Poses for on
Lie Group IMU Preintegration with Rotating Earth"

@nosmokingsurfer nosmokingsurfer added the enhancement New feature or request label Aug 3, 2022
@nosmokingsurfer nosmokingsurfer self-assigned this Aug 3, 2022
@@ -68,6 +70,7 @@ typedef Eigen::Matrix<matData_t, 1,3> Mat13;
typedef Eigen::Matrix<matData_t, 1,4> Mat14;
typedef Eigen::Matrix<matData_t, 1,5> Mat15;
typedef Eigen::Matrix<matData_t, 1,6> Mat16;
typedef Eigen::Matrix<matData_t, 1,9> Mat19;
Copy link
Member

Choose a reason for hiding this comment

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

actually, you can also use the form Mat<1,9> or Vec<9> but i guess this was more convenient... I

#include "mrob/SO3.hpp"

namespace mrob{

Copy link
Member

Choose a reason for hiding this comment

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

here requires a brief comment on the class, what kind of 5x5 structure is in here

Copy link
Member

@g-ferrer g-ferrer left a comment

Choose a reason for hiding this comment

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

I think you can also add a simple example (in cpp) of composition of do the python bindings, in addition to some comments inside

Also, it does not compile: https://github.com/MobileRoboticsSkoltech/mrob/runs/7648022437?check_suite_focus=true#step:4:51

}


Mat3 SE3vel::left_jacobian(const Mat31& phi)
Copy link
Member

Choose a reason for hiding this comment

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

is this not inside SO3? If that is the case, should it be added directly there? I am ok adding an issue and changing SO3

}


Mat3 SE3vel::inv_left_jacobian(const Mat31 &phi)
Copy link
Member

Choose a reason for hiding this comment

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

Same case here, maybe this is better to be moved?

#include "mrob/SE3vel.hpp"

namespace mrob
{
Copy link
Member

Choose a reason for hiding this comment

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

Brief explnation

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants