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

6 ➡️ 7 (main) #1446

Merged
merged 21 commits into from
Apr 25, 2022
Merged

6 ➡️ 7 (main) #1446

merged 21 commits into from
Apr 25, 2022

Conversation

chapulina
Copy link
Contributor

6 ➡️ 7

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

azeey and others added 14 commits April 5, 2022 15:26
As discussed in #1158, a segfault occurs during exit because Views created as a result of an ECM query by plugins have their destructors stored in the shared library of the plugin. For GUI plugins, the plugins are unloaded from memory before the ECM is destructed, so when it's time to destruct the Views, a segfault occurs because the pointer to the virtual destructor is invalid. This PR is fixes the problem by making View a regular class instead of a template. This ensures that the destructor of View is stored in the core library of ignition-gazebo. As a result, the ECM can be destructed after GUI plugins have been unloaded.

Signed-off-by: Addisu Z. Taddese <[email protected]>

Co-authored-by: Michael Carroll <[email protected]>
Co-authored-by: Ashton Larkin <[email protected]>
Signed-off-by: Jose Luis Rivero <[email protected]>

Co-authored-by: Louise Poubel <[email protected]>
To avoid conflicts between different major versions, add the major version to the svg filename.
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Jose Luis Rivero <[email protected]>

Co-authored-by: Jose Luis Rivero <[email protected]>
Signed-off-by: Jose Luis Rivero <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>

Co-authored-by: Jose Luis Rivero <[email protected]>
Co-authored-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
* Fix CMake version examples and bump plugin version

Signed-off-by: methylDragon <[email protected]>

* Fix example CMake version

Signed-off-by: methylDragon <[email protected]>

* Fix typo in docs

Signed-off-by: methylDragon <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Co-authored-by: Jose Luis Rivero <[email protected]>
This PR adds some infrastructure for being able to create comms systems. A comms system lets you send and receive data under the constrains of a given comms model. The comms model specifies when and how messages should be delivered to its destinations.

Subscribers create a regular callback via Ignition Transport and send a request to a centralized broker to bind an address, the robot attached to the address and the topic name used as a callback. Publishers fill a Dataframe message specifying the addresses of the sender and destination and set the payload. Then, publishers need to send via Ignition Transport the message to the centralized broker.

Besides the general infrastructure, this PR contains two systems: PerfectComms and CommsEndpoint.

PerfectComms is an example of a comms system. As required, it implements the ICommsModel interface and it always delivers the messages to their destinations.

CommsEndpoint is a helper system that can be optionally attached to a model (see examples/worlds/comms.sdf) and lets you assign an address to a robot and the subscription topic. The system automatically connects with the broker and bind this address/robot for you. You could do this process manually if needed but I think it's convenient for most of the users.

Signed-off-by: Carlos Agüero <[email protected]>

Co-authored-by: Ian Chen <[email protected]>
Co-authored-by: Arjo Chakravarty <[email protected]>
@chapulina chapulina added the needs upstream release Blocked by a release of an upstream library label Apr 14, 2022
@mjcarroll
Copy link
Contributor

focal is busted missing dataframe.

@github-actions github-actions bot added the 🌱 garden Ignition Garden label Apr 14, 2022
@chapulina
Copy link
Contributor Author

Yup, this needs gazebosim/gz-msgs#244 to go in first and then some nightlies

caguero and others added 3 commits April 14, 2022 10:06
* RFComms system

Signed-off-by: Carlos Agüero <[email protected]>
Co-authored-by: Ian Chen <[email protected]>
Co-authored-by: Arjo Chakravarty <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
Signed-off-by: Louise Poubel <[email protected]>
@chapulina chapulina removed the needs upstream release Blocked by a release of an upstream library label Apr 19, 2022
@codecov
Copy link

codecov bot commented Apr 19, 2022

Codecov Report

Merging #1446 (e399431) into main (1083313) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1446   +/-   ##
=======================================
  Coverage   35.01%   35.01%           
=======================================
  Files          44       44           
  Lines        2356     2356           
=======================================
  Hits          825      825           
  Misses       1531     1531           

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 1083313...e399431. Read the comment docs.

@chapulina
Copy link
Contributor Author

I'm not sure what's up with the failure on Jenkins Ubuntu:

Errors were encountered while processing:
 /tmp/apt-dpkg-install-M4iyFG/023-libignition-common5-core-dev_4.999.999+nightly+git20220420+1r1b1ad37b31be684b3679d8b92cca957addeaab2f-1~focal_amd64.deb

GitHub actions was able to install all dependencies just fine, and I can't reproduce the issue locally. I've already tried cleaning the workspace. Maybe there's some other caching going on? CC @j-rivero

@chapulina
Copy link
Contributor Author

failure on Jenkins Ubuntu

Should be fixed by

There are ign-common5 nightlies building, then we can retrigger CI here:

https://build.osrfoundation.org/job/ign-common5-debbuilder/

@chapulina
Copy link
Contributor Author

CI status:

  • Actions Focal: Only a pre-existing test failure:
    • INTEGRATION_scene_broadcaster_system
  • Action Jammy: new test failures. Jammy CI is being introduced by this branch, so I think we should merge it like this and address the test failures in a separate PR.
    103 - INTEGRATION_follow_actor_system (SEGFAULT)
    139 - INTEGRATION_model_photo_shoot_default_joints (SEGFAULT)
    141 - INTEGRATION_model_photo_shoot_random_joints (SEGFAULT)
    173 - INTEGRATION_scene_broadcaster_system (Failed)
    209 - testFixture_TEST.py (Subprocess aborted)
    
  • Windows: only warnings, as expected
  • Homebrew: pre-existing failures:
    • INTEGRATION_scene_broadcaster_system
    • UNIT_SimulationRunner_TEST
  • Jenkins Ubuntu: Infra is broken right now with nvidia-container-cli: initialization error: nvml error: driver/library version mismatch: unknown.

Copy link
Contributor

@adityapande-1995 adityapande-1995 left a comment

Choose a reason for hiding this comment

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

In order for 2e8892a to work,

gazebosim/gz-sensors#186 and
gazebosim/gz-sensors#212 would also need to be forward ported from ign-sensors

@chapulina
Copy link
Contributor Author

In order for 2e8892a to work,

Thanks, I think those have already been ported on these PRs

A convenient way to check what hasn't been ported yet is to click on the links on this table.

Copy link
Contributor

@adityapande-1995 adityapande-1995 left a comment

Choose a reason for hiding this comment

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

Looks good, hopefully no errors other than these : #1446 (comment)

@chapulina chapulina merged commit 7063d41 into main Apr 25, 2022
@chapulina chapulina deleted the chapulina/6_to_7 branch April 25, 2022 15:34
@j-rivero j-rivero mentioned this pull request Sep 16, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants