Skip to content

Update CI config#83

Merged
JStech merged 8 commits into
masterfrom
pr-industrial-ci-underlay-second-attempt
May 5, 2021
Merged

Update CI config#83
JStech merged 8 commits into
masterfrom
pr-industrial-ci-underlay-second-attempt

Conversation

@JStech

@JStech JStech commented May 4, 2021

Copy link
Copy Markdown
Contributor

@codecov

codecov Bot commented May 4, 2021

Copy link
Copy Markdown

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

@rhaschke rhaschke left a comment

Copy link
Copy Markdown
Contributor

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!

Comment thread .github/workflows/industrial_ci_action.yml Outdated
Comment thread .github/workflows/ci.yaml

name: CI

on: [push, pull_request]

Copy link
Copy Markdown
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?

Comment thread .github/workflows/ci.yaml
@JStech

JStech commented May 5, 2021

Copy link
Copy Markdown
Contributor Author

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.

@rhaschke rhaschke left a comment

Copy link
Copy Markdown
Contributor

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.

4 participants