Skip to content

Fetch NLopt when not available (and fix compiler warnings) - #74

Merged
Butakus merged 7 commits into
EasyNavigation:rollingfrom
Butakus:fix_warnings
Aug 11, 2026
Merged

Butakus merged 7 commits into
EasyNavigation:rollingfrom
Butakus:fix_warnings

Conversation

@Butakus

@Butakus Butakus commented Jul 27, 2026

Copy link
Copy Markdown
Member

Hi,

This PR tries to solve a major issue with the package releases: the nlopt library is not available for RHEL, so those packages are not being built (and we are reminded of it every day by the buildfarm).

When the package is not available in the system (when find_package fails), it is downloaded at build time and compiled via CMake's FetchContent. The prefered option is still getting the package via find_package, but this allows using the library when not available.

Additionally, I fixed some compiler warnings about using uninitialized variables (parameters) and a couple of unused variables.

@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.59459% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...src/easynav_serest_controller/SerestController.cpp 0.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@Butakus

Butakus commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

I see some of the warnings have been already fixed in #76 🚀

I will merge and resolve conflicts so we can add the NLopt stuff.

@fmrico

fmrico commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Why the changes in the SerestController?

@Butakus

Butakus commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

Why the changes in the SerestController?

I got them before your fix in #76. It goes further by avoiding unnecesary local variables goal_pos_tol and goal_yaw_tol.

Same with the param changes in AMCL localizers, I just made the param declaration more homogeneous.

In both cases, the behavior remains the same.

@fmrico

fmrico commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Ok. LGTM then. Let's wait for CI in green and then merge

@fmrico

fmrico commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

One more question, @Butakus : Why reverting the get_safe call?

@Butakus

Butakus commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

One more question, @Butakus : Why reverting the get_safe call?

Oooooops! That was a typo when merging, thanks for the catch!!

@Butakus

Butakus commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

All green now. Merging! 🚀

@Butakus
Butakus merged commit 0b90027 into EasyNavigation:rolling Aug 11, 2026
2 checks passed
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