Resolve: #166 Unintialized Variable (#167)

* first fix of uninitialized variables * install valgrind * fix all memory/ uninitialized variables issues * match code style * add ruff python linting * remove import via sys.path * update pybind11 * fix ci * fix readme link to docs * rename source files to underscore notation * split cloud client to own source file * turn cloud client on by default * patch cloud api for c++11 --------- Co-authored-by: lars <[email protected]> Co-authored-by: pantor <[email protected]> Co-authored-by: Berscheid <[email protected]>
pantor · Sep 3, 2023 · 996b169 · 996b169
1 parent 5e0b504
commit 996b169
Show file tree

Hide file tree

Showing 8 changed files with 48 additions and 17 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -47,6 +47,10 @@ jobs:
         echo "CXX=g++-${{ matrix.version }}" >> $GITHUB_ENV
         bash scripts/patch-c++11.sh inplace
 
+    - name: Install valgrind
+      if: matrix.os == 'ubuntu-latest'
+      run: sudo apt-get install -y valgrind
+
     - name: Configure and make
       run: |
         cmake -B build -DCMAKE_BUILD_TYPE=Release -DBUILD_PYTHON_MODULE=ON -DBUILD_EXAMPLES=ON -DBUILD_BENCHMARK=ON ${{ matrix.cmake_flags }} -DCMAKE_RUNTIME_OUTPUT_DIRECTORY_RELEASE=. -DCMAKE_LIBRARY_OUTPUT_DIRECTORY_RELEASE=.
@@ -80,6 +84,11 @@ jobs:
       env:
         CTEST_OUTPUT_ON_FAILURE: 1
 
+    - name: Memory Test
+      if: matrix.os == 'ubuntu-latest'
+      run: |
+        ctest --test-dir ./build -T memcheck
+
 
   lint-python:
     runs-on: ubuntu-latest

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -155,6 +155,15 @@ if(BUILD_TESTS)
     target_compile_options(test-target PRIVATE /W4)
   else()
     target_compile_options(test-target PRIVATE -Wall -Wextra)
+    find_program(MEMORYCHECK_COMMAND valgrind)
+    set(MEMORYCHECK_COMMAND_OPTIONS
+       "--tool=memcheck \
+       --leak-check=full \
+       --trace-children=yes \
+       --track-origins=yes \
+       --keep-debuginfo=yes \
+       --error-exitcode=100")
+    include(CTest)
   endif()
 
   add_test(NAME test-target COMMAND test-target)

diff --git a/include/ruckig/brake.hpp b/include/ruckig/brake.hpp
@@ -37,7 +37,7 @@ class BrakeProfile {
     void get_velocity_brake_trajectory(double a0, double aMax, double aMin, double jMax);
 
     //! Calculate brake trajectory for second-order velocity interface
-    void get_second_order_velocity_brake_trajectory() { }
+    void get_second_order_velocity_brake_trajectory();
 
     //! Finalize third-order braking by integrating along kinematic state
     void finalize(double& ps, double& vs, double& as) {

diff --git a/include/ruckig/calculator_target.hpp b/include/ruckig/calculator_target.hpp
@@ -232,6 +232,11 @@ class TargetCalculator {
         for (size_t dof = 0; dof < degrees_of_freedom; ++dof) {
             auto& p = traj.profiles[0][dof];
 
+            inp_min_velocity[dof] = inp.min_velocity ? inp.min_velocity.value()[dof] : -inp.max_velocity[dof];
+            inp_min_acceleration[dof] = inp.min_acceleration ? inp.min_acceleration.value()[dof] : -inp.max_acceleration[dof];
+            inp_per_dof_control_interface[dof] = inp.per_dof_control_interface ? inp.per_dof_control_interface.value()[dof] : inp.control_interface;
+            inp_per_dof_synchronization[dof] = inp.per_dof_synchronization ? inp.per_dof_synchronization.value()[dof] : inp.synchronization;
+
             if (!inp.enabled[dof]) {
                 p.p.back() = inp.current_position[dof];
                 p.v.back() = inp.current_velocity[dof];
@@ -243,11 +248,6 @@ class TargetCalculator {
                 continue;
             }
 
-            inp_min_velocity[dof] = inp.min_velocity ? inp.min_velocity.value()[dof] : -inp.max_velocity[dof];
-            inp_min_acceleration[dof] = inp.min_acceleration ? inp.min_acceleration.value()[dof] : -inp.max_acceleration[dof];
-            inp_per_dof_control_interface[dof] = inp.per_dof_control_interface ? inp.per_dof_control_interface.value()[dof] : inp.control_interface;
-            inp_per_dof_synchronization[dof] = inp.per_dof_synchronization ? inp.per_dof_synchronization.value()[dof] : inp.synchronization;
-
             // Calculate brake (if input exceeds or will exceed limits)
             switch (inp_per_dof_control_interface[dof]) {
                 case ControlInterface::Position: {

diff --git a/include/ruckig/ruckig.hpp b/include/ruckig/ruckig.hpp
@@ -191,7 +191,7 @@ class Ruckig {
         output.new_calculation = false;
 
         Result result {Result::Working};
-        if (input != current_input || !current_input_initialized) {
+        if (!current_input_initialized || input != current_input) {
             result = calculate(input, output.trajectory, output.was_calculation_interrupted);
             if (result != Result::Working && result != Result::ErrorPositionalLimits) {
                 return result;

diff --git a/src/ruckig/brake.cpp b/src/ruckig/brake.cpp
@@ -121,4 +121,11 @@ void BrakeProfile::get_velocity_brake_trajectory(double a0, double aMax, double
     }
 }
 
+void BrakeProfile::get_second_order_velocity_brake_trajectory() {
+    t[0] = 0.0;
+    t[1] = 0.0;
+    j[0] = 0.0;
+    j[1] = 0.0;
+}
+
 } // namespace ruckig
diff --git a/src/ruckig/position_second_step1.cpp b/src/ruckig/position_second_step1.cpp
@@ -27,16 +27,15 @@ void PositionSecondOrderStep1::time_none(ProfileIter& profile, double vMax, doub
     if (h1 >= 0.0) {
         h1 = std::sqrt(h1);
 
-        profile->t[3] = 0;
-        profile->t[4] = 0;
-        profile->t[5] = 0;
-        profile->t[6] = 0;
-
         // Solution 1
         {
             profile->t[0] = -(v0 + h1)/aMax;
             profile->t[1] = 0;
             profile->t[2] = (vf + h1)/aMin;
+            profile->t[3] = 0;
+            profile->t[4] = 0;
+            profile->t[5] = 0;
+            profile->t[6] = 0;
 
             if (profile->check_for_second_order<ControlSigns::UDDU, ReachedLimits::NONE>(aMax, aMin, vMax, vMin)) {
                 add_profile(profile);
@@ -51,6 +50,10 @@ void PositionSecondOrderStep1::time_none(ProfileIter& profile, double vMax, doub
             profile->t[0] = (-v0 + h1)/aMax;
             profile->t[1] = 0;
             profile->t[2] = (vf - h1)/aMin;
+            profile->t[3] = 0;
+            profile->t[4] = 0;
+            profile->t[5] = 0;
+            profile->t[6] = 0;
 
             if (profile->check_for_second_order<ControlSigns::UDDU, ReachedLimits::NONE>(aMax, aMin, vMax, vMin)) {
                 add_profile(profile);

diff --git a/src/ruckig/velocity_third_step1.cpp b/src/ruckig/velocity_third_step1.cpp
@@ -27,16 +27,15 @@ void VelocityThirdOrderStep1::time_none(ProfileIter& profile, double aMax, doubl
     if (h1 >= 0.0) {
         h1 = std::sqrt(h1);
 
-        profile->t[3] = 0;
-        profile->t[4] = 0;
-        profile->t[5] = 0;
-        profile->t[6] = 0;
-
         // Solution 1
         {
             profile->t[0] = -(a0 + h1)/jMax;
             profile->t[1] = 0;
             profile->t[2] = -(af + h1)/jMax;
+            profile->t[3] = 0;
+            profile->t[4] = 0;
+            profile->t[5] = 0;
+            profile->t[6] = 0;
 
             if (profile->check_for_velocity<ControlSigns::UDDU, ReachedLimits::NONE>(jMax, aMax, aMin)) {
                 add_profile(profile);
@@ -51,6 +50,10 @@ void VelocityThirdOrderStep1::time_none(ProfileIter& profile, double aMax, doubl
             profile->t[0] = (-a0 + h1)/jMax;
             profile->t[1] = 0;
             profile->t[2] = (-af + h1)/jMax;
+            profile->t[3] = 0;
+            profile->t[4] = 0;
+            profile->t[5] = 0;
+            profile->t[6] = 0;
 
             if (profile->check_for_velocity<ControlSigns::UDDU, ReachedLimits::NONE>(jMax, aMax, aMin)) {
                 add_profile(profile);