Skip to content

Test example programs#64

Open
zultron wants to merge 39 commits intoPickNikRobotics:andyz/squashed_improvementsfrom
zultron:zultron/2020-09-21-test_example_programs
Open

Test example programs#64
zultron wants to merge 39 commits intoPickNikRobotics:andyz/squashed_improvementsfrom
zultron:zultron/2020-09-21-test_example_programs

Conversation

@zultron
Copy link
Contributor

@zultron zultron commented Sep 21, 2020

In PR #62, I broke the examples, which luckily @AndyZe found by
manually testing. The specific breakage will be fixed over there, but in general, broken examples should be detected automatically in the tests.

This PR adds a Python test that simply runs the three examples and fails if they exit non-zero.

AndyZe added 30 commits April 22, 2020 11:55
Third joint acceleration definitely needs some work
AndyZe and others added 8 commits August 23, 2020 11:12
* Clang tidy fixes (#53)

* Clean up inline functions (#54)

- Remove unnecessary inline tags
- Move remaining inline function implementations into header

* Update executable names in Readme (#56)

* Run `clang-format`

Command copied from `moveit_ci`:

    find . -name '*.h' -or -name '*.hpp' -or -name '*.cpp' | \
        xargs /usr/bin/clang-format -i -style=file

* Adjust clang-format rules and re-run

Fix hideous string breaks pointed out by @AndyZe

* single_joint_generator:  Silence compiler warning

    [...]/src/single_joint_generator.cpp: In member function ‘void trackjoint::SingleJointGenerator::extendTrajectoryDuration()’:
    [...]/src/single_joint_generator.cpp:123:30: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
	 for (size_t idx = 0; idx < waypoints_.elapsed_times.size(); ++idx)
			      ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    [...]/src/single_joint_generator.cpp:138:19: warning: unused variable ‘error_code’ [-Wunused-variable]
	 ErrorCodeEnum error_code = forwardLimitCompensation(&index_last_successful_);
		       ^~~~~~~~~~

* single_joint_generator:  Silence compiler warning

    [...]/src/single_joint_generator.cpp: In member function ‘void trackjoint::SingleJointGenerator::extendTrajectoryDuration()’:
    [...]/src/single_joint_generator.cpp:123:30: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
	 for (size_t idx = 0; idx < waypoints_.elapsed_times.size(); ++idx)
			      ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

* test/trajectory_generation_test:  Silence compiler warning

    [...]/test/trajectory_generation_test.cpp: In member function ‘virtual void trackjoint::TrajectoryGenerationTest_VelAccelJerkLimit_Test::TestBody()’:
    [...]/test/trajectory_generation_test.cpp:433:16: error: unused variable ‘duration_tolerance’ [-Werror=unused-variable]
       const double duration_tolerance = 5e-3;
		    ^~~~~~~~~~~~~~~~~~

* Remove junk files

* .gitignore generated `.catkin-tools` directory

These files were removed in previous "Remove junk files" commit;
@AndyZe suggested we keep them out permanently by `.gitignore`ing
them.

* Change `int` to `size_t` for vector indices in tests

Remove unneeded `static_cast<int>`; prepare for upcoming changes.

* trajectory_generator.cpp:  Fix `clang-tidy` errors

    [...]/src/trajectory_generator.cpp:32:58: warning: redundant boolean literal in ternary expression result [readability-simplify-boolean-expr]
      bool timestep_was_upsampled = (upsample_rounds_ > 0) ? true : false;
				    ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
				    upsample_rounds_ > 0
    [...]/src/trajectory_generator.cpp:61:58: warning: redundant boolean literal in ternary expression result [readability-simplify-boolean-expr]
      bool timestep_was_upsampled = (upsample_rounds_ > 0) ? true : false;
				    ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
				    upsample_rounds_ > 0

* single_joint_generator.cpp:  Fix clang-tidy errors

    [...]/src/single_joint_generator.cpp:116:14: warning: local copy 'spline' of the variable 'fit' is never modified; consider avoiding the copy [performance-unnecessary-copy-initialization]
	Spline1D spline(fit);
	~~~~~~~~ ^
	const &

* Slence clang-tidy warnings

    [...]/src/single_joint_generator.cpp:279:5: warning: Value stored to 'delta_v' is never read [clang-analyzer-deadcode.DeadStores]
	delta_v = 0;
	^
    [...]/src/single_joint_generator.cpp:279:5: note: Value stored to 'delta_v' is never read
    1 warning generated.

* Silence clang-tidy warnings

    /usr/include/clang/6.0.0/include/emmintrin.h:1593:10: warning: Dereference of null pointer [clang-analyzer-core.NullDereference]
      return *(__m128d*)__dp;
	     ^
    [...]/test/trajectory_generation_test.cpp:1093:33: note: Calling 'TrajectoryGenerationTest::calculatePositionAccuracy'
      const double position_error = calculatePositionAccuracy(goal_joint_states_, output_trajectories_);
				    ^
    [...]/test/trajectory_generation_test.cpp:146:28: note: Assuming the condition is false
	for (size_t joint = 0; joint < trajectory.size(); ++joint)
			       ^
    [...]/test/trajectory_generation_test.cpp:146:5: note: Loop condition is false. Execution continues on line 152
	for (size_t joint = 0; joint < trajectory.size(); ++joint)
	^
    [...]/test/trajectory_generation_test.cpp:152:20: note: Calling 'MatrixBase::norm'
	double error = (final_positions - goal_positions).norm();
		       ^
    /usr/include/eigen3/Eigen/src/Core/Dot.h:107:23: note: Calling 'MatrixBase::squaredNorm'
      return numext::sqrt(squaredNorm());
			  ^
    /usr/include/eigen3/Eigen/src/Core/Dot.h:95:23: note: Calling 'DenseBase::sum'
      return numext::real((*this).cwiseAbs2().sum());
			  ^
    /usr/include/eigen3/Eigen/src/Core/Redux.h:451:6: note: Left side of '||' is false
      if(SizeAtCompileTime==0 || (SizeAtCompileTime==Dynamic && size()==0))
	 ^
    /usr/include/eigen3/Eigen/src/Core/Redux.h:451:31: note: Left side of '&&' is true
      if(SizeAtCompileTime==0 || (SizeAtCompileTime==Dynamic && size()==0))
				  ^
    /usr/include/eigen3/Eigen/src/Core/Redux.h:451:3: note: Taking false branch
      if(SizeAtCompileTime==0 || (SizeAtCompileTime==Dynamic && size()==0))
      ^
    /usr/include/eigen3/Eigen/src/Core/Redux.h:453:10: note: Calling 'DenseBase::redux'
      return derived().redux(Eigen::internal::scalar_sum_op<Scalar,Scalar>());
	     ^
    /usr/include/eigen3/Eigen/src/Core/Redux.h:418:10: note: Calling 'redux_impl::run'
      return internal::redux_impl<Func, ThisEvaluator>::run(thisEval, func);
	     ^
    /usr/include/eigen3/Eigen/src/Core/Redux.h:231:8: note: Assuming 'alignedSize' is not equal to 0
	if(alignedSize)
	   ^
    /usr/include/eigen3/Eigen/src/Core/Redux.h:231:5: note: Taking true branch
	if(alignedSize)
	^
    /usr/include/eigen3/Eigen/src/Core/Redux.h:233:34: note: Calling 'redux_evaluator::packet'
	  PacketScalar packet_res0 = mat.template packet<alignment,PacketScalar>(alignedStart);
				     ^
    /usr/include/eigen3/Eigen/src/Core/Redux.h:377:12: note: Calling 'unary_evaluator::packet'
      { return m_evaluator.template packet<LoadMode,PacketType>(index); }
	       ^
    /usr/include/eigen3/Eigen/src/Core/CoreEvaluators.h:563:31: note: Calling 'binary_evaluator::packet'
	return m_functor.packetOp(m_argImpl.template packet<LoadMode, PacketType>(index));
				  ^
    /usr/include/eigen3/Eigen/src/Core/CoreEvaluators.h:734:31: note: Calling 'evaluator::packet'
	return m_functor.packetOp(m_lhsImpl.template packet<LoadMode,PacketType>(index),
				  ^
    /usr/include/eigen3/Eigen/src/Core/CoreEvaluators.h:204:41: note: Passing null pointer value via 1st parameter 'from'
	return ploadt<PacketType, LoadMode>(m_data + index);
					    ^
    /usr/include/eigen3/Eigen/src/Core/CoreEvaluators.h:204:12: note: Calling 'ploadt'
	return ploadt<PacketType, LoadMode>(m_data + index);
	       ^
    /usr/include/eigen3/Eigen/src/Core/GenericPacketMath.h:462:3: note: Taking true branch
      if(Alignment >= unpacket_traits<Packet>::alignment)
      ^
    /usr/include/eigen3/Eigen/src/Core/GenericPacketMath.h:463:26: note: Passing null pointer value via 1st parameter 'from'
	return pload<Packet>(from);
			     ^
    /usr/include/eigen3/Eigen/src/Core/GenericPacketMath.h:463:12: note: Calling 'pload'
	return pload<Packet>(from);
	       ^
    /usr/include/eigen3/Eigen/src/Core/arch/SSE/PacketMath.h:307:124: note: Passing null pointer value via 1st parameter '__dp'
    template<> EIGEN_STRONG_INLINE Packet2d pload<Packet2d>(const double*  from) { EIGEN_DEBUG_ALIGNED_LOAD return _mm_load_pd(from); }
															       ^
    /usr/include/eigen3/Eigen/src/Core/arch/SSE/PacketMath.h:307:112: note: Calling '_mm_load_pd'
    template<> EIGEN_STRONG_INLINE Packet2d pload<Packet2d>(const double*  from) { EIGEN_DEBUG_ALIGNED_LOAD return _mm_load_pd(from); }
														   ^
    /usr/include/clang/6.0.0/include/emmintrin.h:1593:10: note: Dereference of null pointer
      return *(__m128d*)__dp;
	     ^

Co-authored-by: Nathan Brooks <nathanbrooks@picknik.ai>
Co-authored-by: AndyZe <zelenak@picknik.ai>
In PR #62, I broke the examples, which luckily @AndyZe found by
manually testing.  Broken examples should be tested automatically.
@zultron zultron force-pushed the zultron/2020-09-21-test_example_programs branch from 20d866a to ff00dbc Compare September 21, 2020 18:15
@zultron
Copy link
Contributor Author

zultron commented Sep 21, 2020

seems like load_manifest should be on a new line

Agreed. That's a copy'n'paste from the ROS wiki.

Fixed.

@zultron
Copy link
Contributor Author

zultron commented Sep 21, 2020

I like this approach! Very simple. Not really familiar with doing it this way, though

I couldn't figure out how to run the examples directly. They don't produce gtest XML results, and I didn't want to mess up your simple examples by turning them into gtest cases. If you know another way, I'm curious.

@AndyZe
Copy link
Contributor

AndyZe commented Sep 21, 2020

Tested locally. All 3 tests pass as they should. Will approve/merge when CI passes.

@AndyZe AndyZe force-pushed the andyz/squashed_improvements branch from 848a54f to 479e734 Compare October 22, 2020 20:25
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