Validate step size and RTF parameters #740
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Only set them if they are strictly positive. Signed-off-by: Luca Della Vedova <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-gazebo4 #740 +/- ##
===============================================
+ Coverage 65.43% 65.51% +0.08%
===============================================
Files 237 237
Lines 17210 17216 +6
===============================================
+ Hits 11261 11279 +18
+ Misses 5949 5937 -12
Continue to review full report at Codecov.
|
|
I can confirm this PR allows setting each of the parameters separately from CLI without affecting the other parameter. |
peci1
approved these changes
Apr 5, 2021
chapulina
approved these changes
Apr 5, 2021
scpeters
added a commit
that referenced
this pull request
May 19, 2021
* 🎈 3.8.0 (#688) Signed-off-by: Louise Poubel <[email protected]> * Make it so joint state publisher is quieter (#696) Signed-off-by: Michael Carroll <[email protected]> * [BULLET] Making GetContactsFromLastStepFeature optional in Collision Features (#690) * GetContactsFromLastStepFeature made optional Signed-off-by: Tomas Lorente <[email protected]> Co-authored-by: Addisu Z. Taddese <[email protected]> * Add test for thermal object temperatures below 0 kelvin (#621) Signed-off-by: Ashton Larkin <[email protected]> * Scenebroadcaster sensors (#698) * Add sensors to scene broadcaster Signed-off-by: Nate Koenig <[email protected]> * Update src/systems/scene_broadcaster/SceneBroadcaster.cc Co-authored-by: Michael Carroll <[email protected]> * Fix codecheck Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Michael Carroll <[email protected]> * Fix diffuse and ambient values for ackermann example (#707) Signed-off-by: Ammaar Solkar <[email protected]> * 🎈 5.0.0 (#731) Signed-off-by: Louise Poubel <[email protected]> * Support configuring particle scatter ratio in particle emitter system (#674) * set particle scatter ratio through sdf Signed-off-by: Ian Chen <[email protected]> * address feedback Signed-off-by: Ian Chen <[email protected]> * add todo note about merging forward Signed-off-by: Ian Chen <[email protected]> Co-authored-by: Ashton Larkin <[email protected]> * Update PlaybackScrubber description (#733) Signed-off-by: Ammaar Solkar <[email protected]> * Iterate through changed links only in UpdateSim (#678) Signed-off-by: Ashton Larkin <[email protected]> * Do not pass -Wno-unused-parameter to MSVC compiler (#716) Signed-off-by: Silvio Traversaro <[email protected]> * Use Protobuf_IMPORT_DIRS instead of PROTOBUF_IMPORT_DIRS for compatibility with Protobuf CMake config (#715) Signed-off-by: Silvio Traversaro <[email protected]> * Fix component inspector shutdown crash (#724) Signed-off-by: Louise Poubel <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]> * Validate step size and RTF parameters (#740) Only set them if they are strictly positive. Signed-off-by: Luca Della Vedova <[email protected]> * Fix compute_rtfs arguments (#737) Signed-off-by: Caio Amaral <[email protected]> * Fixed collision visual bounding boxes (#746) Signed-off-by: Jenn Nguyen <[email protected]> * Fix CMakelists.txt merge Signed-off-by: Nate Koenig <[email protected]> * ECM's ChangedState gets message with modified components (#742) * ecm's ChangedState to contain modified components Signed-off-by: Jenn Nguyen <[email protected]> * updated log_system test Signed-off-by: Jenn Nguyen <[email protected]> * removed unnecessary calls Signed-off-by: Jenn Nguyen <[email protected]> Co-authored-by: Ian Chen <[email protected]> * fixed particle emitter forward playback (#745) Signed-off-by: Jenn Nguyen <[email protected]> * Merge pull request #730 from ignitionrobotics/particle_emitter Particle emitter based on SDF * 4 7 0 prep (#755) * Prepare for 4.7.0 Signed-off-by: Nate Koenig <[email protected]> * Added placeholder Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]> * Fix 'invalid animation update data' msg for actors (#754) Signed-off-by: Ashton Larkin <[email protected]> * Update benchmark comparison instructions (#766) (#766) Signed-off-by: Ashton Larkin <[email protected]> * [DiffDrive] add enable/disable (#772) * add enable/disable diffdrive Signed-off-by: Guillaume Doisy <[email protected]> * remove debug Signed-off-by: Guillaume Doisy <[email protected]> * do not subscribe to enable if topic is empty Signed-off-by: Guillaume Doisy <[email protected]> * add test Signed-off-by: Guillaume Doisy <[email protected]> * lint and style Signed-off-by: Guillaume Doisy <[email protected]> * change enable type to bool and renamed to enabled Signed-off-by: Guillaume Doisy <[email protected]> * Add odometry publisher system (#547) * Create Initial Odometry Publisher system plugin Add code for initial plugin that gets position from Pose component and calculates velocities based on rolling mean from displacement data. Signed-off-by: Maganty Rushyendra <[email protected]> * Remove Linear and Angular Velocity components Also renames frames in Odometry msg to include model name, and makes various style changes. Signed-off-by: Maganty Rushyendra <[email protected]> * Get World pose instead of pose of robot base frame Signed-off-by: Maganty Rushyendra <[email protected]> * Add documentation for variables and functions Includes minor stylistic changes. Signed-off-by: Maganty Rushyendra <[email protected]> * Check for valid odomTopic and update copyright year Signed-off-by: Maganty Rushyendra <[email protected]> * Add tests for OdometryPublisherSystem and fix velocity calculation bug Swap X and Y linear velocities when calculating odometry velocities relative to robotBaseFrame. Signed-off-by: Maganty Rushyendra <[email protected]> Co-authored-by: ahcorde <[email protected]> * Patch particle emitter2 service (#777) * Patch particle emitter2 service Signed-off-by: Nate Koenig <[email protected]> * Remove condition variable Signed-off-by: Nate Koenig <[email protected]> * Set emitter frame and relative pose Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]> * Preparing for 4.8.0 release (#780) Signed-off-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]> * 👩🌾 Enable Focal CI (#646) Signed-off-by: Louise Poubel <[email protected]> Co-authored-by: Michael Carroll <[email protected]> * [TPE] Support setting individual link velocity (#427) Signed-off-by: claireyywang <[email protected]> Signed-off-by: Ian Chen <[email protected]> Co-authored-by: Ian Chen <[email protected]> Co-authored-by: Louise Poubel <[email protected]> * Don't store duplicate ComponentTypeId in ECM (#751) Signed-off-by: Louise Poubel <[email protected]> * Feature/hydrodynamics (#749) Implement hydrodynamics and thruster plugin. Signed-off-by: Arjo Chakravarty <[email protected]> Co-authored-by: Mabel Zhang <[email protected]> Co-authored-by: Carlos Agüero <[email protected]> * Fix macOS build: components::Name in benchmark (#784) Signed-off-by: Louise Poubel <[email protected]> Co-authored-by: Steve Peters <[email protected]> * Fix ColladaExporter submesh index bug (#763) Signed-off-by: Jorge Perez <[email protected]> * 👩🌾 Fix Windows build and some warnings (#782) Signed-off-by: Louise Poubel <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]> * Prevent crash on Plotting plugin with mutex (#747) Signed-off-by: Louise Poubel <[email protected]> * Bump ign-physics version to 3.2 (#792) Signed-off-by: Louise Poubel <[email protected]> * Bump to ign-msgs 7.1 / sdformat 11.1, Windows fixes (#758) Signed-off-by: Louise Poubel <[email protected]> * Util: Use public API from libsdformat for detecting non-file source (#794) Signed-off-by: Eric Cousineau <[email protected]> * Fix included nested model expansion in SDF generation (#768) * fixed included nested model expansion Signed-off-by: Jenn Nguyen <[email protected]> * added resource path to test Signed-off-by: Jenn Nguyen <[email protected]> * use orig URIs & support multi level nesting Signed-off-by: Jenn Nguyen <[email protected]> * save fuel version when enabled Signed-off-by: Jenn Nguyen <[email protected]> * retrieve uri from map Signed-off-by: Jenn Nguyen <[email protected]> * copy included element Signed-off-by: Jenn Nguyen <[email protected]> * clear attributes before copying include element Signed-off-by: Jenn Nguyen <[email protected]> * Map canonical links to their models (#736) Signed-off-by: Ashton Larkin <[email protected]> * ColladaExporter, export submesh selected (#802) * Export only submesh if selected * Add test case for the PR * Attempting a unified solution Signed-off-by: Jorge Perez <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Michael Carroll <[email protected]> Co-authored-by: Jose Tomas Lorente <[email protected]> Co-authored-by: Addisu Z. Taddese <[email protected]> Co-authored-by: Ashton Larkin <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Nate Koenig <[email protected]> Co-authored-by: Ammaar Solkar <[email protected]> Co-authored-by: Ian Chen <[email protected]> Co-authored-by: Ashton Larkin <[email protected]> Co-authored-by: Silvio Traversaro <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]> Co-authored-by: Luca Della Vedova <[email protected]> Co-authored-by: Caio Amaral <[email protected]> Co-authored-by: Jenn Nguyen <[email protected]> Co-authored-by: G.Doisy <[email protected]> Co-authored-by: Rushyendra Maganty <[email protected]> Co-authored-by: Claire Wang <[email protected]> Co-authored-by: Arjo Chakravarty <[email protected]> Co-authored-by: Mabel Zhang <[email protected]> Co-authored-by: Carlos Agüero <[email protected]> Co-authored-by: Steve Peters <[email protected]> Co-authored-by: Jorge Perez <[email protected]> Co-authored-by: Eric Cousineau <[email protected]> Co-authored-by: Jorge Perez <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🦟 Bug fix
Addresses #536 (comment)
Summary
Calling the
set_physicsservice with only one parameter, i.e.:caused the other parameter to be set to the default value of 0. The function updating the physics parameters was not checking the values received and setting rtf / step size to 0 which would cause undefined behavior and sometimes crash the simulation.
This PR adds a check to the values and only updates them if they are strictly positive.
Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge