Skip to content

chore: update to rolling#42

Merged
bjv-capra merged 1 commit intomicro-ROS:masterfrom
bjv-capra:fix/ci
Nov 22, 2022
Merged

chore: update to rolling#42
bjv-capra merged 1 commit intomicro-ROS:masterfrom
bjv-capra:fix/ci

Conversation

@bjv-capra
Copy link
Copy Markdown
Collaborator

@bjv-capra bjv-capra commented Nov 17, 2022

Based on rclc's

@bjv-capra bjv-capra force-pushed the fix/ci branch 3 times, most recently from 11910ae to 8ffaddf Compare November 17, 2022 14:20
@bjv-capra
Copy link
Copy Markdown
Collaborator Author

@ralph-lange perhaps you could give it a view? I'm not sure I'm doing it right.

@ralph-lange
Copy link
Copy Markdown
Contributor

Hi @bjv-capra, since there is currently one master branch used for all ROS distros together (instead of separate branches named foxy, galactic, humble), I propose to test all distros together in this workflow. See https://github.com/boschresearch/fmi_adapter/blob/master/.github/workflows/build_and_test.yml#L16 for an example for a matrix for all ROS distros and Ubuntu versions. In the present case, docker_image has to be added as third dimension.
But even if separate branches are used, I would keep the matrix so that the workflow file (with just one entry) for maintenance reasons - all variable values in one place.

@bjv-capra
Copy link
Copy Markdown
Collaborator Author

Thanks! I'll give it a go.
WRT docker_image, I'm not even sure it's needed when runs-on is set. I'm just wondering if steps such as running apt-get install perhaps require a more specific docker_image.

@bjv-capra bjv-capra force-pushed the fix/ci branch 2 times, most recently from 2e7a3c5 to cb2162a Compare November 21, 2022 10:41
@bjv-capra
Copy link
Copy Markdown
Collaborator Author

@ralph-lange seems I have two different errors.
First, on the humble pipeline I get. Which is odd, considering I based myself on your example from https://github.com/boschresearch/fmi_adapter/blob/rolling/.github/workflows/build_and_test.yml#L16

Error: Input humble was not a valid ROS 2 distribution for 'target-ros2-distro'. Valid values: dashing,eloquent,foxy,galactic,rolling

The second error seems to be the msgs package not being available to another of the packages while building. Strangely, it works fine for both Foxy and rolling.

Comment thread .github/workflows/ci.yml
Copy link
Copy Markdown
Contributor

@ralph-lange ralph-lange left a comment

Choose a reason for hiding this comment

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

Not sure yet why the CI Action failed for Galactic, but Humble should be solved by upgrading to the latest version of action-ros-ci. Let's test this first before looking into the problem with Galactic.

Comment thread .github/workflows/ci.yml Outdated
Copy link
Copy Markdown
Contributor

@ralph-lange ralph-lange left a comment

Choose a reason for hiding this comment

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

Hi @bjv-capra, my mistake, the sudo is required now since no container is used any longer. Please excuse my wrong comment.

Comment thread .github/workflows/ci.yml Outdated
Comment thread .github/workflows/ci.yml Outdated
@bjv-capra
Copy link
Copy Markdown
Collaborator Author

No problem, I highly appreciate the help with this

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Nov 22, 2022

Codecov Report

Base: 34.44% // Head: 42.85% // Increases project coverage by +8.41% 🎉

Coverage data is based on head (df4692b) compared to base (be37125).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff             @@
##           master      #42      +/-   ##
==========================================
+ Coverage   34.44%   42.85%   +8.41%     
==========================================
  Files         222        2     -220     
  Lines       16185      133   -16052     
  Branches     5142       21    -5121     
==========================================
- Hits         5575       57    -5518     
+ Misses       6115       61    -6054     
+ Partials     4495       15    -4480     
Impacted Files Coverage Δ
...sgs/srv/detail/micro_ros_self_test__type_support.c
..._diagnostic_updater/micro_ros_diagnostic_updater.c
...iagnostic_updater/test/test_diagnostic_updater.cpp
...stics/micro_ros_common_diagnostics/src/hwmonitor.c
...stics/micro_ros_common_diagnostics/src/hwmonitor.c
...iagnostic_updater/test/test_diagnostic_updater.cpp
...iagnostic_updater/test/test_diagnostic_updater.cpp
..._diagnostic_updater/micro_ros_diagnostic_updater.c
...stics/micro_ros_common_diagnostics/src/hwmonitor.c
...ail/micro_ros_diagnostic_key_value__type_support.c
... and 214 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ralph-lange
Copy link
Copy Markdown
Contributor

Hi @bjv-capra, I've analyzed the log output for Galactic for an hour this morning and cannot explain the error message. I've also compared it with a manual build under Galactic and checked the _exported_targets variable in install/micro_ros_diagnostic_msgs/share/micro_ros_diagnostic_msgs/cmake/ament_cmake_export_targets-extras.cmake. When comparing the lists at using all available rosidl_typesupport... of the Foxy build and the Galactic build, I did not find a difference either.
Do you have any idea?

@bjv-capra
Copy link
Copy Markdown
Collaborator Author

Hi @ralph-lange, I've also done a similar process as yours, and was not able to understand why it would fail. It has me quite puzzled.
I'm not able to reproduce EXACTLY in the same context as the CI runner, but I don't see any strange issue, at least locally.
I'm a bit unsure on how to proceed as I'd like for the CI to be available ASAP, so I can rely on my following PRs building correctly.
What about dissabling Galactic? I know, a bit radical but, I don't see much other alternative at the moment.

@bjv-capra bjv-capra linked an issue Nov 22, 2022 that may be closed by this pull request
Copy link
Copy Markdown
Contributor

@ralph-lange ralph-lange left a comment

Choose a reason for hiding this comment

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

Hi @bjv-capra, you could do a force merge and then check whether the ROS 2 Build Farm builds the repo successfully, cf. https://build.ros2.org/job/Gdev__micro_ros_diagnostics__ubuntu_focal_amd64/.

@bjv-capra
Copy link
Copy Markdown
Collaborator Author

I'll first clean up the commits a bit, then I'll merge. So I'll have to force push here first 👍
Thanks! Fingers crossed

Signed-off-by: Bart Jimenez Vera <bjv@capra.ooo>
@bjv-capra bjv-capra merged commit 1ec7462 into micro-ROS:master Nov 22, 2022
@bjv-capra bjv-capra deleted the fix/ci branch November 22, 2022 14:32
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.

Pipelines failing

3 participants