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

Add HAROS integration ament_haros #192

Open
max-krichenbauer opened this issue Sep 13, 2019 · 5 comments
Open

Add HAROS integration ament_haros #192

max-krichenbauer opened this issue Sep 13, 2019 · 5 comments
Labels
question Further information is requested

Comments

@max-krichenbauer
Copy link

Hello!

A previous discussion to provide a integration package for the analysis tool HAROS ( https://github.com/git-afsantos/haros ) for catkin (haros_catkin https://github.com/rosin-project/haros_catkin )
spawned the suggestion by some people to provide the same for ROS2 in the form of a new ament_lint package.
The discussion is here: rosin-project/haros_catkin#24
and the development repository here: https://github.com/esol-community/ament_lint/tree/ament_haros
It's still waiting on some PRs to HAROS itself, so it's not ready for integration yet (although already fully functional),
but I would like to start the discussion about this integration here and get input from people.

Thanks so much for your time.
Best regards,
Max

@dirk-thomas dirk-thomas added the question Further information is requested label Sep 13, 2019
@dirk-thomas
Copy link
Contributor

It would be good to describe on a high level how the tool works and what it does.

Also relevant for the integration would be how the dependency HAROS should be distributed.

@max-krichenbauer
Copy link
Author

@dirk-thomas Thank you for the comment and sorry for the late reply.
Basically, HAROS parses ROS workspaces and tests the source code files for code_quality standards. It can also use several additional static analysis tools through plug-ins and can create a web-based report (though I think that feature is applicable here.)
It's the same tool used in haros_catkin which is already available for kinetic and melodic. I'm not sure if the same distribution method would apply to ament_lint

@dirk-thomas
Copy link
Contributor

HAROS parses ROS workspaces and tests the source code files for code_quality standards.

For each linter in this repo there are two packages:

  • one which provides a command line tool / Python API to run the linter
  • another to provide a CMake API to add the linter to a package.

Some of the linters are then selected to run as the set of default linters if a packages chooses to do so. So in general the linter should work on a single package - not on a whole workspace.

It's the same tool used in haros_catkin which is already available for kinetic and melodic.

From the manifest I could only find the following dependencies: cccc and cppcheck. Does HAROS provide any additional linters atm? I am asking since there is already a cppcheck based linter package in this repo and cccc isn't really a linter but only gathers some statistics.

All current linter packages have in common that they generate a xUnit compatible result file. I am not sure how applicable that is for something like cccc. Does it offer any criteria which would qualify the statistics to "fail" a test?

For the pure report generating part I am not sure ament_lint is the right place / framework.

@max-krichenbauer
Copy link
Author

max-krichenbauer commented Sep 24, 2019

Thank you for the comment!

For each linter in this repo there are two packages:

  • one which provides a command line tool / Python API to run the linter
  • another to provide a CMake API to add the linter to a package.

Yes, that is the way I implemented it as well. The packages are ament_haros and ament_cmake_haros respectively, following the convention of the other linters.

Some of the linters are then selected to run as the set of default linters if a packages chooses to do so. So in general the linter should work on a single package - not on a whole workspace.

Yes, of course. Using HAROS is selected on a per-package level by adding it to the package.xml and CMakeLists.txt files, just like the other linters in this repo.

From the manifest I could only find the following dependencies: cccc and cppcheck. Does HAROS provide any additional linters atm? I am asking since there is already a cppcheck based linter package in this repo and cccc isn't really a linter but only gathers some statistics.

Yes, including lizard, radon and ccd. Please see https://github.com/git-afsantos/haros_plugins
It also produces a html report that can be displayed in the Wiki.
Development for that happens here: https://github.com/droter/roswiki

All current linter packages have in common that they generate a xUnit compatible result file. I am not sure how applicable that is for something like cccc. Does it offer any criteria which would qualify the statistics to "fail" a test?

HAROS uses the ROS code_quality metrics to decide pass or fail.

For the pure report generating part I am not sure ament_lint is the right place / framework.

I think the idea is that it integrates seamlessly into the build farm and then shows up in the wiki page of the packages as well, so it would be convenient to have an easy way of adding it to one's packages. Not sure if it has to be in this repository though. Just wanted to start the discussion.

@max-krichenbauer
Copy link
Author

Update: I realized the misunderstanding about per-package selection vs. workspace-wide use may be due to the variable names.

I pushed a commit that makes it more clear (without changing the behavior):
args.paths[0] is expected to be the directory of the package that requested ament_haros.
This package_dir is searched for the actual package name(s). (Since the folder may be named differently).
The found package name(s) are added to the HAROS project file. This is how target packages are communicated to HAROS.
The workspace root is also detected and passed to HAROS. This is a requirement of HAROS, but the packages to be analyzed are taken from the project yaml file and thus limited to the one that requested it.

I hope this clarifies things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants