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

Using angles library to normalize angles #739

Merged

Conversation

firemark
Copy link
Contributor

During experimenting with library I stuck on infinity loop with NaN values because:

>>> from math import pi
>>> float("nan") + pi
nan
>>> float("nan") - pi
nan

@firemark firemark force-pushed the protect-wrapping-angles-from-nan branch from 8e146d0 to 1cc497b Compare March 11, 2022 16:21
src/ekf.cpp Outdated
update_indices[i] == StateMemberPitch ||
update_indices[i] == StateMemberYaw)
update_indices[i] == StateMemberYaw) &&
std::isfinite(innovation_subset(i)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

So if the innovation has values that are not finite, we should look into why. Otherwise, this feels like we're trying to just cover up a bigger issue.

Also, oof. I need to update this code to use this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably is not your fault - I modified little a bit this code and BANG I got infinity loop. I think, this function should be protected against wrong values.

I will try to add "angles" library

@ayrton04 ayrton04 changed the title Protect wrapping angles from NaN and inf +/- in ekf and ukf filter Using angles library to normalize angles Apr 25, 2022
@ayrton04 ayrton04 merged commit a0c1904 into cra-ros-pkg:galactic Apr 25, 2022
@@ -28,6 +28,7 @@
<depend>geographiclib</depend>
<depend>message_filters</depend>
<depend>nav_msgs</depend>
<depend>angles</depend>
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you need to also added in the CMakelist?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's weird because project has compiled properly on bare ros:galactic image with rosdep. Probably another dependency has this library.

tonynajjar added a commit to logivations/robot_localization that referenced this pull request Apr 26, 2022
* SHARED linking for Geographiclib (cra-ros-pkg#624) (cra-ros-pkg#713)

* remove GeographicLib specific linking option

Co-authored-by: Achmad Fathoni <fathoni.id@gmail.com>

* Fixing code style divergence for ament_uncrustify (cra-ros-pkg#742)

* Fixed state history reversion (cra-ros-pkg#736)

Co-authored-by: Zygfryd Wieszok <zwieszok@autonomous-systems.pl>

* Read predict_to_current_time from ROS parameters (cra-ros-pkg#737)

Co-authored-by: Zygfryd Wieszok <zwieszok@autonomous-systems.pl>

* Using angles library to normalize angles (cra-ros-pkg#739)

* Using angles library for innovation angle normalization

Co-authored-by: Stephan Sundermann <stephansundermann@gmail.com>
Co-authored-by: Achmad Fathoni <fathoni.id@gmail.com>
Co-authored-by: Anish <anishgdev@gmail.com>
Co-authored-by: Zygfryd Wieszok <zygfryd.wieszok@gmail.com>
Co-authored-by: Zygfryd Wieszok <zwieszok@autonomous-systems.pl>
Co-authored-by: Marek Piechula <marpiechula@gmail.com>
tonynajjar added a commit to logivations/robot_localization that referenced this pull request Apr 26, 2022
* SHARED linking for Geographiclib (cra-ros-pkg#624) (cra-ros-pkg#713)

* remove GeographicLib specific linking option

Co-authored-by: Achmad Fathoni <fathoni.id@gmail.com>

* Fixing code style divergence for ament_uncrustify (cra-ros-pkg#742)

* Fixed state history reversion (cra-ros-pkg#736)

Co-authored-by: Zygfryd Wieszok <zwieszok@autonomous-systems.pl>

* Read predict_to_current_time from ROS parameters (cra-ros-pkg#737)

Co-authored-by: Zygfryd Wieszok <zwieszok@autonomous-systems.pl>

* Using angles library to normalize angles (cra-ros-pkg#739)

* Using angles library for innovation angle normalization

Co-authored-by: Stephan Sundermann <stephansundermann@gmail.com>
Co-authored-by: Achmad Fathoni <fathoni.id@gmail.com>
Co-authored-by: Anish <anishgdev@gmail.com>
Co-authored-by: Zygfryd Wieszok <zygfryd.wieszok@gmail.com>
Co-authored-by: Zygfryd Wieszok <zwieszok@autonomous-systems.pl>
Co-authored-by: Marek Piechula <marpiechula@gmail.com>
@tonynajjar tonynajjar mentioned this pull request Apr 26, 2022
ayrton04 pushed a commit that referenced this pull request May 5, 2022
* Using angles library for innovation angle normalization
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.

3 participants