-
Notifications
You must be signed in to change notification settings - Fork 437
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
consistent notation #116
base: master
Are you sure you want to change the base?
consistent notation #116
Conversation
…f q, C_q to be more consistent
* state.Get<StateLIdx>()(0); // p | ||
|
||
H.block<3, 3>(0, kIdxstartcorr_q) = -C_wv * C_q * pci_sk | ||
H.block<3, 3>(0, kIdxstartcorr_q) = -C_wv.transpose() * C_wi * pci_sk | ||
* state.Get<StateLIdx>()(0); // q | ||
|
||
H.block<3, 1>(0, kIdxstartcorr_L) = | ||
scalefix ? |
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.
can you also fix this horrible ? :
syntax ?
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.
if you do so, please also change below:
if (scalefix) {
H.block<3, 1>(0, kIdxstartcorr_L).setZero();
} else {
H.block<3, 1>(0, kIdxstartcorr_L) = ...;
}
@otim thanks for the nice cleanup. lgtm. |
I found another occurence of |
I just tested with both pose measurements and position measurements and everything still looks good. please take a look at the latest changes and pay extra attention to things related to spherical measurements and pressure sensor, as I am not able to test these. Thanks! |
@@ -170,16 +170,16 @@ class PoseSensorManager : public msf_core::MSF_SensorManagerROS< | |||
P.setZero(); // Error state covariance; if zero, a default initialization in msf_core is used | |||
|
|||
p_vc = pose_handler_->GetPositionMeasurement(); | |||
q_cv = pose_handler_->GetAttitudeMeasurement(); | |||
q_vc = pose_handler_->GetAttitudeMeasurement(); |
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.
Are you sure about this? At least vicon publishes p_vc
and q_cv
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.
looks like we need to measurement_world_sensor parameter here
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.
Sorry about that! For me this was the case, so this notation made it easier to understand for me... Simon, can you please elaborate on your idea?
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.
Depending on the subscribed topic (ptam/vicon) the pose provided can be
either q_cv (ptam) or q_vc/q_wc (vicon). If you look at the implementation
of the pose update you see that there is a fix-parameter (from yaml) that
defines if the pose measurement needs to be inverted or not before being
applied as update. You would need to add the same logic here.
On Tue, Jun 16, 2015, 09:39 Tim Oberhauser [email protected] wrote:
In msf_updates/src/pose_msf/pose_sensormanager.h
#116 (comment):@@ -170,16 +170,16 @@ class PoseSensorManager : public msf_core::MSF_SensorManagerROS<
P.setZero(); // Error state covariance; if zero, a default initialization in msf_core is usedp_vc = pose_handler_->GetPositionMeasurement();
- q_cv = pose_handler_->GetAttitudeMeasurement();
- q_vc = pose_handler_->GetAttitudeMeasurement();
Sorry about that! For me this was the case, so this notation made it
easier to understand for me... Simon, can you please elaborate on your idea?—
Reply to this email directly or view it on GitHub
https://github.com/ethz-asl/ethzasl_msf/pull/116/files#r32497243.
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.
please have a look at my latest commit
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.
If I understand correctly, the first attitude measurement was always added inversed in the case of vicon... Ok, vicon measurements are added at a high rate so the effect was maybe never visible...
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.
Here it says that measurement_world_sensor is false for ptam:
http://wiki.ros.org/ethzasl_sensor_fusion/Tutorials/sfly_framework
…vc or q_cv. added a check during initialization to distinguish between the two cases
Conflicts: msf_updates/src/pose_msf/pose_sensormanager.h msf_updates/src/pose_pressure_msf/pose_pressure_sensormanager.h msf_updates/src/position_msf/position_sensormanager.h msf_updates/src/position_pose_msf/position_pose_sensormanager.h
@burrimi could you take a look at getting this one into master after testing? |
+1 |
What would it take to test this one? |
@marija-p I saw that you recently took on the task of merging PRs, do you think you could also merge this one? Thanks! |
No description provided.