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

Update CI config #83

Merged
merged 8 commits into from
May 5, 2021
Merged

Conversation

JStech
Copy link
Contributor

@JStech JStech commented May 4, 2021

@codecov
Copy link

codecov bot commented May 4, 2021

Codecov Report

Merging #83 (f8e0547) into master (034f951) will increase coverage by 6.42%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #83      +/-   ##
==========================================
+ Coverage   72.75%   79.16%   +6.42%     
==========================================
  Files          10        9       -1     
  Lines         554      499      -55     
==========================================
- Hits          403      395       -8     
+ Misses        151      104      -47     
Impacted Files Coverage Δ
...t/handeye_calibration_solver/handeye_solver_base.h
...ye_calibration_target/src/handeye_target_aruco.cpp 87.78% <0.00%> (+4.45%) ⬆️
..._calibration_target/src/handeye_target_charuco.cpp 88.78% <0.00%> (+5.76%) ⬆️
..._calibration_solver/src/handeye_solver_default.cpp 64.62% <0.00%> (+6.89%) ⬆️
...t/handeye_calibration_target/handeye_target_base.h 88.08% <0.00%> (+7.59%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 034f951...f8e0547. Read the comment docs.

@rhaschke rhaschke changed the title Use industrial_ci UNDERLAY Update CI config May 4, 2021
Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took the chance to unify the CI config with that of the MoveIt main repo.
I also added a Noetic build. However, this fails. Please have a look!

.github/workflows/industrial_ci_action.yml Outdated Show resolved Hide resolved

name: CI

on: [push, pull_request]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While updating this, do we want to add the pre-release test here too?

.github/workflows/ci.yaml Show resolved Hide resolved
@JStech
Copy link
Contributor Author

JStech commented May 5, 2021

I also added a Noetic build. However, this fails. Please have a look!

That test failure is because OpenCV 3.2, the default version for Ubuntu 18.04, has a buggy ArUco board pose detector. See discussion here, #5. I added a check to use the incorrect pose for 3.2 and the correct pose otherwise.

Copy link
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my point of view, this is ready to merge.
I'm not sure though, that we should consider wrong results for the unit tests against OpenCV 3.2. Alternatively, we could skip the test completely (in this case). However, then the remaining code isn't tested as well. Hence, I'm fine with the current solution as well.

@JStech JStech merged commit 54ba48f into master May 5, 2021
@JStech JStech deleted the pr-industrial-ci-underlay-second-attempt branch May 31, 2021 12:53
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.

None yet

4 participants