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

Generate valid topics depending on world name #163

Closed
wants to merge 1 commit into from

Conversation

chapulina
Copy link
Contributor

Part of gazebosim/gz-sim#239 is making sure that all topics can handle entity names with spaces. This can be the case for the world.

This makes use of the AsValidTopic function to convert world names to valid ones whenever possible.

@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Dec 30, 2020
@codecov
Copy link

codecov bot commented Dec 30, 2020

Codecov Report

Merging #163 (275f77b) into ign-gui3 (c80515d) will increase coverage by 0.00%.
The diff coverage is 61.53%.

Impacted file tree graph

@@            Coverage Diff            @@
##           ign-gui3     #163   +/-   ##
=========================================
  Coverage     60.31%   60.32%           
=========================================
  Files            20       20           
  Lines          2510     2523   +13     
=========================================
+ Hits           1514     1522    +8     
- Misses          996     1001    +5     
Impacted Files Coverage Δ
src/plugins/world_stats/WorldStats.cc 94.59% <40.00%> (-2.58%) ⬇️
src/plugins/world_control/WorldControl.cc 79.20% <75.00%> (-0.29%) ⬇️

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 c80515d...275f77b. Read the comment docs.

@chapulina
Copy link
Contributor Author

There's little value in getting this into Citadel because the string components serialization in Ignition Gazebo isn't working properly with whitespaces. This is fixed on Dome though, so I retargeted the PR on #164. We can backport that to Citadel in the future if we see the need.

@chapulina chapulina closed this Dec 30, 2020
@chapulina chapulina deleted the chapulina/3/valid_topics branch December 30, 2020 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant