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

Solve #600 (ExtrapolationException & its error_code) #601

Merged
merged 2 commits into from
May 8, 2023

Conversation

roncapat
Copy link
Contributor

@roncapat roncapat commented May 6, 2023

This solves #600. There was an oversight in how the code, before introduction of error_code, handled the error_string. In particular, one code path saves the error in a "side" (so not directly in error_string but in extrapolation_error_string) and this HAS to be done also with the new error_code stuff.

@SteveMacenski
Copy link
Contributor

This patch would be important to include in Iron's release

@clalancette
Copy link
Contributor

clalancette commented May 7, 2023

Thank you both for this! I'll fire up CI on this, and assuming that it passes I'll merge it in and then backport to Iron.

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@clalancette clalancette linked an issue May 7, 2023 that may be closed by this pull request
@ruffsl
Copy link
Member

ruffsl commented May 8, 2023

@clalancette CI looks green! 🐢

@clalancette clalancette merged commit ac2479e into ros2:rolling May 8, 2023
@clalancette
Copy link
Contributor

@Mergifyio backport iron

@mergify
Copy link
Contributor

mergify bot commented May 8, 2023

backport iron

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request May 8, 2023
* error_code shall follow same path of its error string

* Lint

(cherry picked from commit ac2479e)
@clalancette
Copy link
Contributor

@roncapat Thanks again for the fix!

Since this was a pretty urgent fix, I went ahead and merged it as-is, and also backported it to iron in #602. However, I think it would be best if we had a test for this issue to make sure we don't run into it again. If you have any time to look at adding a test, I would be most appreciative.

@roncapat
Copy link
Contributor Author

roncapat commented May 8, 2023

@clalancette it was a pleasure for me (and also an headache, I'm very sorry for having introduced this nasty bug). I was thinking exactly the same... we need a test for it. I lack time now for work reason, but if you want assign to me an issue as a reminder, I will make sure to think about it as soon as I can.

clalancette pushed a commit that referenced this pull request May 8, 2023
* error_code shall follow same path of its error string

* Lint

(cherry picked from commit ac2479e)

Co-authored-by: Patrick Roncagliolo <[email protected]>
ruffsl added a commit to ros-navigation/navigation2 that referenced this pull request May 8, 2023
in interim until next rolling sync
- ros2/geometry2#601
@clalancette
Copy link
Contributor

@clalancette it was a pleasure for me (and also an headache, I'm very sorry for having introduced this nasty bug). I was thinking exactly the same... we need a test for it. I lack time now for work reason, but if you want assign to me an issue as a reminder, I will make sure to think about it as soon as I can.

Thank you, that is appreciated. I've now opened up #603 and assigned it to you.

ruffsl added a commit to ros-navigation/navigation2 that referenced this pull request May 9, 2023
in interim until next rolling sync
- ros2/geometry2#601
jwallace42 pushed a commit to jwallace42/navigation2 that referenced this pull request May 9, 2023
enricosutera pushed a commit to enricosutera/navigation2 that referenced this pull request May 19, 2024
in interim until next rolling sync
- ros2/geometry2#601

Signed-off-by: enricosutera <[email protected]>
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.

#586 done broke me
4 participants