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.

Sign up for GitHub

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

Jump to bottom

Refactor GazeboSimulator class #163

Merged
merged 7 commits into from
Mar 31, 2020
Merged

Refactor GazeboSimulator class #163

merged 7 commits into from
Mar 31, 2020

Conversation

diegoferigo
Copy link
Collaborator

As discussed in #158, we only support the deterministic mode of the simulator. The refactored GazeboSimulator class has some logic that I used to test the non-deterministic mode that could be used in the future if we plan to put more effort to implement it.

In general, the assumptions are:

  • Non-deterministic mode is not supported (that means iterations=0)
  • World files loaded with GazeboSimulator::loadSdfWorld can contain only one world

@diegoferigo diegoferigo self-assigned this Mar 29, 2020
@diegoferigo diegoferigo mentioned this pull request Mar 29, 2020
Closed
9 tasks
@diegoferigo diegoferigo force-pushed the refactor/new_gazebo_simulator branch 2 times, most recently from 2ef6f6e to 497fa96 Compare March 31, 2020 10:04
@diegoferigo diegoferigo marked this pull request as ready for review March 31, 2020 10:13
@diegoferigo
Copy link
Collaborator Author

Ready to be reviewed @traversaro @raffaello-camoriano @paolo-viceconte

@diegoferigo diegoferigo force-pushed the refactor/new_gazebo_simulator branch from 497fa96 to 4ac1c19 Compare March 31, 2020 11:32
traversaro
traversaro approved these changes Mar 31, 2020
View reviewed changes
Copy link
Contributor

@traversaro traversaro left a comment

Choose a reason for hiding this comment

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

Minor typo

cpp/scenario/gazebo/src/GazeboSimulator.cpp Outdated

// Setup signals callbacks.
// It must be done after the creation of the simulator since
// we oveeride override their callbacks.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// we oveeride override their callbacks.
// we override their callbacks.

Copy link
Collaborator Author

@diegoferigo diegoferigo Mar 31, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks! Fixed in https://github.com/robotology/gym-ignition/compare/4ac1c19cfc61c75681c181b79d4790b7712e5de1..7c80ee690a16c8e5bf81fbc3f1f2689253f76e14

@diegoferigo diegoferigo force-pushed the refactor/new_gazebo_simulator branch from 4ac1c19 to 8eb5705 Compare March 31, 2020 14:16
@diegoferigo diegoferigo force-pushed the refactor/new_gazebo_simulator branch from 8eb5705 to 7c80ee6 Compare March 31, 2020 14:21
@diegoferigo diegoferigo merged commit 4f5052f into robotology-legacy:refactoring Mar 31, 2020
@diegoferigo diegoferigo deleted the refactor/new_gazebo_simulator branch March 31, 2020 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@traversaro traversaro traversaro approved these changes

@paolo-viceconte paolo-viceconte Awaiting requested review from paolo-viceconte

@raffaello-camoriano raffaello-camoriano Awaiting requested review from raffaello-camoriano

Assignees

@diegoferigo diegoferigo

Labels
complexity::medium component::scenario::gazebo pr::resolution::merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@diegoferigo @traversaro