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

navigation2 does not build #286

Open
muellerbernd opened this issue Aug 8, 2023 · 13 comments
Open

navigation2 does not build #286

muellerbernd opened this issue Aug 8, 2023 · 13 comments

Comments

@muellerbernd
Copy link
Contributor

The navigation2 package does not build because nav2_mppi_controller is missing. In the last rosdistro sync there is a dependency (xtensor) of nav2_mppi_controller listed as missing. But xtensor is available in nixpkg (see search).

Because of this rtabmap-ros does also not compile.

I want to contribute more to this repo is there a guideline on how to do so?

@hacker1024
Copy link
Contributor

You'll need to make a PR to rosdistro, like this recent one that added a dependency of RViz.

@muellerbernd
Copy link
Contributor Author

muellerbernd commented Aug 8, 2023

@hacker1024 thanks for this information. I made a PR to rosdistro here

@lopsided98
Copy link
Owner

It looks like nav2_mppi_controller needs xsimd as well.

@muellerbernd
Copy link
Contributor Author

I created a PR for rosdep to include xsimd

@lopsided98
Copy link
Owner

nav2-mppi-controller is now generated but fails to build due to some non-obvious issue with the xsimd/xtensor headers. Do you need that specific package or were you just trying to use the navigation2 meta package which happens to include that package?

@muellerbernd
Copy link
Contributor Author

muellerbernd commented Sep 12, 2023

I was trying to build rtabmap-ros and it seemed like navigation2 was a dependency.

@hacker1024
Copy link
Contributor

hacker1024 commented Sep 24, 2023

nav2-mppi-controller is now generated but fails to build due to some non-obvious issue with the xsimd/xtensor headers. Do you need that specific package or were you just trying to use the navigation2 meta package which happens to include that package?

Now I need it! I'll look into it. A bunch of nav2 packages also need -Wno-error=maybe-uninitialized. I'll submit a PR with overrides sometime this week.

@hacker1024
Copy link
Contributor

hacker1024 commented Sep 24, 2023

Alright, got it all working:

Issues

a) The following packages need to be compiled with -Wno-error=maybe-uninitialized: nav2-behaviors, nav2-constrained-smoother, nav2-planner, nav2-smoother, nav2-waypoint-follower, dwb-critics, dwb-plugins.
b) OMPL requires a fix upstream: ompl/ompl#1101
c) xtensor needs upgrading, but xsimd needs downgrading (from Nixpkgs master... it's actually an upgrade on nix-ros). This handles the former so far: NixOS/nixpkgs#225904

Solutions

a) can be fixed here in an overlay, though it's probably best fixed upstream. That seems like a fair bit of effort, though.
b) can be applied in an overlay as well, though it's probably best to just wait for it to get merged.
c) is trickier. There's no easy solution yet. Apparently waiting on xtensor-stack/xtensor#2721 to make a decision. For now, I just downgraded it globally.

@doronbehar
Copy link

Hey all, feel free to suggest a solution to the xtensor update going on at #2721 that will add xsimd support to xtensor. An option I was also thinking about is creating a dedicated xsimd10 attribute in all-packages.nix.

@thearthur
Copy link

thearthur commented Apr 29, 2024

Has anyone gotten navigation2 to build?
just checking if this is the same issue:

error: builder for '/nix/store/j161k57jy8mlgfrxybhqj6498n0ga61h-ros-iron-nav2-lifecycle-manager-1.2.7-r1.drv' failed with exit code 2;
       last 10 log lines:
       >    92 |     level = lvl;
       >       |     ~~~~~~^~~~~
       > /build/navigation2-release-release-iron-nav2_lifecycle_manager-1.2.7-1/src/lifecycle_manager.cpp: In member function 'void nav2_lifecycle_manager::LifecycleManager::CreateDiagnostic(diagnostic_updater::DiagnosticStatusWrapper&)':
       > /build/navigation2-release-release-iron-nav2_lifecycle_manager-1.2.7-1/src/lifecycle_manager.cpp:161:17: note: 'error_level' was declared here
       >   161 |   unsigned char error_level;
       >       |                 ^~~~~~~~~~~
       > cc1plus: all warnings being treated as errors

This sounds like issue a mentioned above:
a) The following packages need to be compiled with -Wno-error=maybe-uninitialized: nav2-behaviors, nav2-constrained-smoother, nav2-planner, nav2-smoother, nav2-waypoint-follower, dwb-critics, dwb-plugins.

Maybe we need to add nav2-lifecycle-manager to this list.

@wentasah
Copy link
Contributor

Some (if not all) these errors have already been fixed upstream, but probably are not yet released. The problem you mention can be addressed by this commit (in my branch).

If you apply it, you will see the other problems, which are addressed by ros-navigation/navigation2@fbec0fa. You can apply the changes from that commit similarly as above. The complication is that the commit is a squashed commit and you need to split it to individual packages by adding stripLen and includes arguments to fetchpatch as shown below:

          (self.fetchpatch {
            url = "https://github.com/open-navigation/navigation2/commit/fbec0fa68a23e8257e1420075745aba0735b07c4.patch";
            stripLen = 1;
            includes = [ "*/subdir..../*" ];
            hash = "";
          })

Beware that the value of those arguments influences the hash.

To be honest, I'm not sure whether it is worth the effort. Especially if the package will be released with these fixes sometimes soon, it may be better to wait :-/. However, I don't use these packages and don't know what's their release policy.

@RandomSpaceship
Copy link

RandomSpaceship commented Nov 4, 2024

Hey all! I'm trying to get this working myself, and I'm running into the nav2-mppi-controller build issues now that I've patched all the nav2 packages in an overlay.

@hacker1024 sorry for the year-later ping, but is overriding xtensor/xsimd still the solution, and if so, which versions worked for you? Currently these are what I have overridden, although it's still failing to build:

  nav2-mppi-controller = rosPrev.nav2-mppi-controller.override (
    {
      inherit xtensor;
    }
    // prev.lib.optionals (!(prev.stdenv.isAarch64 && prev.stdenv.isLinux)) { xsimd = xsimd10; }
  );
  xsimd10 = prev.xsimd.overrideAttrs (old: rec {
    version = "10.0.0";
    src = prev.fetchFromGitHub {
      owner = "xtensor-stack";
      repo = "xsimd";
      rev = version;
      hash = "sha256-+ewKbce+rjNWQ0nQzm6O4xSwgzizSPpDPidkQYuoSTU=";
    };
  });
  xtensor = prev.xtensor.overrideAttrs (
    old: with prev; rec {
      version = "0.24.7";

      src = prev.fetchFromGitHub {
        owner = "xtensor-stack";
        repo = "xtensor";
        rev = version;
        hash = "sha256-dVbpcBW+jK9nIl5efk5LdKdBm8CkaJWEZ0ZY7ZuApwk=";
      };

      propagatedBuildInputs =
        [
          nlohmann_json
          xtl
        ]
        ++ lib.optionals (!(stdenv.isAarch64 && stdenv.isLinux)) [
          # xsimd support is broken on aarch64-linux, see:
          # https://github.com/xtensor-stack/xsimd/issues/945
          xsimd10
        ];
    }
  );

Edit: I've managed to make it build (with default xsimd/xtensor) by adding the -Wno-error=array-bounds flag. It feels like a very bad and hacky solution but it makes the build pass.

@RandomSpaceship
Copy link

RandomSpaceship commented Nov 5, 2024

So I did some more investigation - the only difference I could find between building from source on Ubuntu vs with Nix was the gcc version (13.3.0 with Nix vs 11.4.0 on Ubuntu), which made me think the compile errors were specific to GCC 13. After modifying the CMakeLists.txt to link against GCC 13 libstdc++, the project built with no errors using GCC 11 and GCC 12, meaning that the error is compiler version dependent.

I also confirmed that the error is specific to xtensor, not nav2-mppi-controller, since after commenting out all code in the optimizer.cpp file it still failed to build. It looks like ros-navigation/navigation2#4621 fully replaces the xtensor stack with eigen, eliminating the issue anyway, so this whole thing will go away with the release after that's merged. For now I'll just disable the error flag, but it looks like it hasn't become an issue upstream because it's an issue that GCC 13 introduces, and they're compiling with GCC 11. Presumably it's due to the introduction of -Wanalyzer-out-of-bounds in the upgrade (see the changes).

For anybody running into the same issues as me, here's the list of overrides to perform:

  1. Apply the following patch to nav-2d-utils:
diff --git a/include/nav_2d_utils/parameters.hpp b/include/nav_2d_utils/parameters.hpp
index c3c30428f..c199fe89b 100644
--- a/include/nav_2d_utils/parameters.hpp
+++ b/include/nav_2d_utils/parameters.hpp
@@ -64,7 +64,7 @@ param_t searchAndGetParam(
   const nav2_util::LifecycleNode::SharedPtr & nh, const std::string & param_name,
   const param_t & default_value)
 {
-  param_t value;
+  param_t value = default_value;
   nav2_util::declare_parameter_if_not_declared(
     nh, param_name,
     rclcpp::ParameterValue(default_value));

  1. Apply the following patch to nav2-constrained-smoother:
diff --git a/include/nav2_constrained_smoother/smoother.hpp b/include/nav2_constrained_smoother/smoother.hpp
index b5432bf65..ec7e2a38b 100644
--- a/include/nav2_constrained_smoother/smoother.hpp
+++ b/include/nav2_constrained_smoother/smoother.hpp
@@ -292,7 +292,7 @@ private:
     }
     int last_i = 0;
     int prelast_i = -1;
-    Eigen::Vector2d prelast_dir;
+    Eigen::Vector2d prelast_dir(0,0);
     for (int i = 1; i <= static_cast<int>(path_optim.size()); i++) {
       if (i == static_cast<int>(path_optim.size()) || optimized[i]) {
         if (prelast_i != -1) {

  1. Override nav2-behaviors, nav2-planner, nav2-smoother, and nav2-waypoint-follower by adding " -Wno-error=maybe-uninitialized" to the CXXFLAGS with overrideAttrs
  2. Override the version of nav-2d-utils passed to nav2-smoother with override on the derivation. I'm not sure why but for some reason this doesn't propagate with overrideScope, there's a collision if you try to build the whole thing.
  3. Override nav2-mppi-controller by adding " -Wno-error=array-bounds" to the CXXFLAGS
  4. Apply the patched packages using overrideScope.

Since nav-2d-utils and nav2-constrained-smoother both cause potentially uninitialized errors in their header files, the header files themselves need to be patched to prevent the need to propagate the -Wno-error=maybe-uninitialized flag to dwb-core, dwb-critics, and the like. All the others have the error in their .cpp implementation files so they're fine and can be fixed with just the flag change.

The full list of overrides is as below, update the patch file locations as appropriate:

  # since this gets propagated and the error is in a header file, the error also gets propagated,
  # so we have to patch it properly
  nav-2d-utils = rosPrev.nav-2d-utils.overrideAttrs (
    {
      patches ? [ ],
      ...
    }:
    {
      patches = patches ++ [ ./patches/fix-nav-2d-utils.patch ];
    }
  );
  nav2-behaviors = rosPrev.nav2-behaviors.overrideAttrs (
    {
      CXXFLAGS ? "",
      ...
    }:
    {
      CXXFLAGS = CXXFLAGS + " -Wno-error=maybe-uninitialized";
    }
  );
  # same as with nav-2d-utils - it doesn't have any direct dependencies using its header file,
  # but it's good to patch it anyway
  nav2-constrained-smoother = rosPrev.nav2-constrained-smoother.overrideAttrs (
    {
      patches ? [ ],
      ...
    }:
    {
      patches = patches ++ [ ./patches/fix-nav2-constrained-smoother.patch ];
    }
  );
  nav2-mppi-controller = rosPrev.nav2-mppi-controller.overrideAttrs (
    {
      CXXFLAGS ? "",
      ...
    }:
    {
      CXXFLAGS = CXXFLAGS + " -Wno-error=array-bounds";
    }
  );
  nav2-planner = rosPrev.nav2-planner.overrideAttrs (
    {
      CXXFLAGS ? "",
      ...
    }:
    {
      CXXFLAGS = CXXFLAGS + " -Wno-error=maybe-uninitialized";
    }
  );
  # for some reason overrideScope is not properly replacing nav-2d-utils as an input to this package,
  # so we have to do it manually
  nav2-smoother =
    (rosPrev.nav2-smoother.overrideAttrs (
      {
        CXXFLAGS ? "",
        ...
      }:
      {
        CXXFLAGS = CXXFLAGS + " -Wno-error=maybe-uninitialized";
      }
    )).override
      { inherit nav-2d-utils; };
  nav2-waypoint-follower = rosPrev.nav2-waypoint-follower.overrideAttrs (
    {
      CXXFLAGS ? "",
      ...
    }:
    {
      CXXFLAGS = CXXFLAGS + " -Wno-error=maybe-uninitialized";
    }
  );

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

No branches or pull requests

7 participants