Skip to content

3 ➡️ 4#108

Merged
chapulina merged 5 commits intoign-sensors4from
chapulina/3_to_4
Mar 18, 2021
Merged

3 ➡️ 4#108
chapulina merged 5 commits intoign-sensors4from
chapulina/3_to_4

Conversation

@chapulina
Copy link
Contributor

➡️ Forward port

Port ign-sensors3 to ign-sensors4.

Branch comparison: ign-sensors4...ign-sensors3

Note to maintainers: Remember to Merge with commit (not squash-merge or rebase)

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

nkoenig and others added 5 commits February 8, 2021 12:49
Signed-off-by: Nate Koenig <nate@openrobotics.org>

Co-authored-by: Nate Koenig <nate@openrobotics.org>
Fix macOS/windows tests that failed to load library (#60)

* Add workflow for macos-latest
* Set test env to help find plugins

This fixes tests on macOS and windows that were failing to
find and/or load a sensor component library.

In windows CI and the macOS workflow (which runs `make test`
before `make install`) tests were failing with
the message "Unable to find sensor plugin path".
This is fixed by setting the IGN_PLUGIN_PATH in cmake to
the build folder containing the compiled plugins.

In the macOS jenkins build (which runs `make test` after
`make install`) tests were failing with the message
"SDF sensor type does not match template type". It was
difficult to track down, but it appears to be caused by
a failure to properly dlopen all the shared libraries
linked by the component plugins when a test finds an
installed component library, rather than one from the build
folder. It is fixed by setting DYLD_LIBRARY_PATH to include
the location of the installed libraries.

Fixes #4.

* Remove redundant AddPluginPaths calls from tests

They don't work on windows, so just depend on the
environment variables set in cmake instead.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>

Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina requested a review from iche033 as a code owner March 12, 2021 06:29
@github-actions github-actions bot added the 🔮 dome Ignition Dome label Mar 12, 2021
@codecov
Copy link

codecov bot commented Mar 12, 2021

Codecov Report

Merging #108 (603c035) into ign-sensors4 (c999f98) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##           ign-sensors4     #108   +/-   ##
=============================================
  Coverage         76.13%   76.13%           
=============================================
  Files                23       23           
  Lines              2388     2388           
=============================================
  Hits               1818     1818           
  Misses              570      570           

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 c999f98...603c035. Read the comment docs.

Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

Segfaults in some tests

	 13 - INTEGRATION_camera_plugin (SEGFAULT)
	 15 - INTEGRATION_depth_camera_plugin (SEGFAULT)
	 17 - INTEGRATION_gpu_lidar_sensor_plugin (SEGFAULT)
	 19 - INTEGRATION_rgbd_camera_plugin (SEGFAULT)
	 21 - INTEGRATION_thermal_camera_plugin (SEGFAULT)

@chapulina
Copy link
Contributor Author

Segfaults in some tests

Those tests have been failing since macOS actions were introduced in #60. The issue tracking these failures, #62, is closed. Maybe we need a new ign-cmake release with gazebosim/gz-cmake#125? CC @scpeters

@adlarkin adlarkin mentioned this pull request Mar 17, 2021
@scpeters
Copy link
Member

Segfaults in some tests

Those tests have been failing since macOS actions were introduced in #60. The issue tracking these failures, #62, is closed. Maybe we need a new ign-cmake release with ignitionrobotics/ign-cmake#125? CC @scpeters

the failing macOS tests in our jenkins builds are tracked in #66 and #67

I'm not sure why the actions tests are seg-faulting

@scpeters
Copy link
Member

Segfaults in some tests

Those tests have been failing since macOS actions were introduced in #60. The issue tracking these failures, #62, is closed. Maybe we need a new ign-cmake release with ignitionrobotics/ign-cmake#125? CC @scpeters

the failing macOS tests in our jenkins builds are tracked in #66 and #67

I'm not sure why the actions tests are seg-faulting

actually it's probably due to lack of GPU support in the actions runners. I wouldn't block this on those macOS test failures in actions; we should probably disable those if the actions runner doesn't have GPU hardware

Copy link
Contributor

@adlarkin adlarkin left a comment

Choose a reason for hiding this comment

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

Approving this since @scpeters says the MacOS failures aren't blocking

@chapulina chapulina merged commit f85cf62 into ign-sensors4 Mar 18, 2021
@chapulina chapulina deleted the chapulina/3_to_4 branch March 18, 2021 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🔮 dome Ignition Dome

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants