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

Non-blocking paths request #555

Merged
merged 2 commits into from
Jan 14, 2021
Merged

Conversation

chapulina
Copy link
Contributor

This is inspired by #550, even though it may not solve the underlying problem reported there.

Change the request for resource paths from the GUI to the server to be asynchronous so the GUI isn't blocked.

With this change, there's a chance that the GUI tries to load resources before it has all paths from the server. But considering that the request is done before other blocking or time consuming requests, like the world name and initial state, I think it would be hard for it not to be responded in time.


If you look at the commit history, I initially also added a button to the GUI that allowed the user to trigger a new request on demand. Then thinking about it, I removed the button because the original request should be fulfilled before that one anyway, so I don't think we'd have a problem of getting no response - we may get a delayed response and the button doesn't change that.

If we want to be very thorough, maybe a button that tries to reload all resources again would be more useful, because we may get new paths added later. But such a button would be non-trivial to implement, so I'll just abandon this idea until we have a strong use case.

Signed-off-by: Louise Poubel <louise@openrobotics.org>
…ough even if asked too early

Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina chapulina added the GUI Gazebo's graphical interface (not pure Ignition GUI) label Jan 13, 2021
@github-actions github-actions bot added the 🏰 citadel Ignition Citadel label Jan 13, 2021
@codecov
Copy link

codecov bot commented Jan 13, 2021

Codecov Report

Merging #555 (1e9cc49) into ign-gazebo3 (d5e65d9) will increase coverage by 0.00%.
The diff coverage is 80.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##           ign-gazebo3     #555   +/-   ##
============================================
  Coverage        77.51%   77.51%           
============================================
  Files              208      208           
  Lines            11526    11527    +1     
============================================
+ Hits              8934     8935    +1     
  Misses            2592     2592           
Impacted Files Coverage Δ
src/gui/PathManager.cc 89.47% <77.77%> (+0.58%) ⬆️
src/ServerPrivate.cc 81.46% <100.00%> (ø)

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 d5e65d9...1e9cc49. Read the comment docs.

@chapulina chapulina merged commit 4124dea into ign-gazebo3 Jan 14, 2021
@chapulina chapulina deleted the chapulina/3/paths_non_blocking branch January 14, 2021 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel GUI Gazebo's graphical interface (not pure Ignition GUI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants