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

Wheel slip system #134

Merged
merged 31 commits into from
Sep 17, 2020
Merged

Wheel slip system #134

merged 31 commits into from
Sep 17, 2020

Conversation

JShep1
Copy link

@JShep1 JShep1 commented May 13, 2020 •
edited
Loading

Ready for review

@JShep1 JShep1 requested review from azeey and chapulina as code owners May 13, 2020 00:43
@JShep1 JShep1 added the 📜 blueprint Ignition Blueprint label May 13, 2020
@JShep1 JShep1 marked this pull request as draft May 13, 2020 00:45
@JShep1 JShep1 added the enhancement New feature or request label May 13, 2020
@codecov
Copy link

codecov bot commented May 13, 2020 •
edited
Loading

Codecov Report

Merging #134 into ign-gazebo2 will increase coverage by 5.30%.
The diff coverage is 62.50%.

Impacted file tree graph

@@               Coverage Diff               @@
##           ign-gazebo2     #134      +/-   ##
===============================================
+ Coverage        53.78%   59.08%   +5.30%     
===============================================
  Files              121      118       -3     
  Lines             5838     4864     -974     
===============================================
- Hits              3140     2874     -266     
+ Misses            2698     1990     -708
Impacted Files Coverage Δ
src/LevelManager.cc 41.91% <40.00%> (+0.29%) ⬆️
...de/ignition/gazebo/components/SlipComplianceCmd.hh 100.00% <100.00%> (ø)
src/SdfGenerator.cc 95.61% <100.00%> (+0.07%) ⬆️
src/SimulationRunner.hh 22.22% <0.00%> (-77.78%) ⬇️
src/SimulationRunner.cc 63.12% <0.00%> (-7.62%) ⬇️
include/ignition/gazebo/components/Component.hh 82.14% <0.00%> (-4.77%) ⬇️
src/Barrier.cc 96.15% <0.00%> (-3.85%) ⬇️
...e/ignition/gazebo/detail/EntityComponentManager.hh 93.16% <0.00%> (-0.86%) ⬇️
src/systems/physics/Physics.hh
src/systems/scene_broadcaster/SceneBroadcaster.hh
... and 1 more

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 0af2c5f...58b63a8. Read the comment docs.

John Shepherd added 2 commits May 14, 2020 13:39
Add test init and resolve comments
2621219 
Signed-off-by: John Shepherd <[email protected]>
updated tests
b747497 
Signed-off-by: John Shepherd <[email protected]>
John Shepherd and others added 13 commits May 18, 2020 18:39
add tricycle uphill test
96c3df7 
Signed-off-by: John Shepherd <[email protected]>
Add partial tire drum test
8a5765b 
Signed-off-by: John Shepherd <[email protected]>
Add triball drift sdf
5437795 
Signed-off-by: John Shepherd <[email protected]>
Sdf changes
9441bda 
Signed-off-by: John Shepherd <[email protected]>
@scpeters
initial version of wheel_slip system copied from apply_joint_force
b411ea0
@azeey
Port essential parts of WheelSlipPlugin
c67d44c
Add test init and resolve comments
420002e 
Signed-off-by: John Shepherd <[email protected]>
updated tests
6da8157 
Signed-off-by: John Shepherd <[email protected]>
add tricycle uphill test
9777693 
Signed-off-by: John Shepherd <[email protected]>
Add partial tire drum test
9a94f53 
Signed-off-by: John Shepherd <[email protected]>
Add triball drift sdf
b8c8bf5 
Signed-off-by: John Shepherd <[email protected]>
Sdf changes
8ebdb83 
Signed-off-by: John Shepherd <[email protected]>
merge conflict
8f73337 
Signed-off-by: John Shepherd <[email protected]>
@JShep1 JShep1 marked this pull request as ready for review May 26, 2020 23:51
@JShep1 JShep1 requested a review from iche033 as a code owner May 26, 2020 23:51
@JShep1 JShep1 changed the base branch from ign-gazebo2 to master May 27, 2020 18:20
@JShep1 JShep1 changed the base branch from master to ign-gazebo2 May 27, 2020 18:20
chapulina
chapulina reviewed May 28, 2020
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

I did a first pass and left some suggestions

examples/worlds/trisphere_cycle_wheel_slip.sdf Show resolved Hide resolved
include/ignition/gazebo/components/SlipComplianceCmd.hh Outdated Show resolved Hide resolved
src/systems/physics/Physics.cc Outdated Show resolved Hide resolved
include/ignition/gazebo/components/SlipComplianceCmd.hh Outdated Show resolved Hide resolved
src/systems/physics/Physics.cc Outdated Show resolved Hide resolved
src/systems/wheel_slip/WheelSlip.cc Show resolved Hide resolved
src/systems/wheel_slip/WheelSlip.cc Outdated Show resolved Hide resolved
src/systems/wheel_slip/WheelSlip.cc Outdated Show resolved Hide resolved
src/systems/wheel_slip/WheelSlip.cc Outdated Show resolved Hide resolved
src/systems/wheel_slip/WheelSlip.hh Outdated Show resolved Hide resolved
azeey
azeey requested changes Jul 23, 2020
View reviewed changes
test/integration/wheel_slip.cc Outdated Show resolved Hide resolved
test/integration/wheel_slip.cc Show resolved Hide resolved
Add test fixes
5a92b81 
Signed-off-by: John Shepherd <[email protected]>
azeey
azeey requested changes Aug 21, 2020
View reviewed changes
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

This looks great! I left some comments that should be easy to address

include/ignition/gazebo/components/SlipComplianceCmd.hh Outdated Show resolved Hide resolved
src/systems/physics/Physics.cc Show resolved Hide resolved
src/systems/physics/Physics.cc Outdated Show resolved Hide resolved
src/systems/physics/Physics.cc Outdated Show resolved Hide resolved
src/systems/wheel_slip/WheelSlip.cc Outdated Show resolved Hide resolved
src/systems/wheel_slip/WheelSlip.cc Show resolved Hide resolved
test/integration/wheel_slip.cc Outdated Show resolved Hide resolved
test/integration/wheel_slip.cc Outdated Show resolved Hide resolved
test/integration/wheel_slip.cc Show resolved Hide resolved
src/systems/wheel_slip/WheelSlip.hh Outdated Show resolved Hide resolved
@azeey azeey mentioned this pull request Aug 22, 2020
Merged
4 tasks
John Shepherd added 3 commits August 25, 2020 13:02
@chapulina
Copy link
Contributor

⚠️ This requires a release of DART and of ign-physics1.

@chapulina chapulina added the needs upstream release Blocked by a release of an upstream library label Sep 3, 2020
@nkoenig nkoenig requested a review from azeey September 17, 2020 13:21
@azeey
Copy link
Contributor

azeey commented Sep 17, 2020

I've added a fix to the diff_drive_skid test failure in #357.

azeey
azeey approved these changes Sep 17, 2020
View reviewed changes
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

LGTM! The I don't think any of the test failures in CI are caused by this PR.

@nkoenig nkoenig merged commit 53f2636 into ign-gazebo2 Sep 17, 2020
@nkoenig nkoenig deleted the wheel_slip_system branch September 17, 2020 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chapulina chapulina chapulina left review comments

@nkoenig nkoenig nkoenig approved these changes

@azeey azeey azeey approved these changes

@iche033 iche033 Awaiting requested review from iche033

Assignees

@JShep1 JShep1

Labels
📜 blueprint Ignition Blueprint enhancement New feature or request needs upstream release Blocked by a release of an upstream library
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@JShep1 @chapulina @azeey @nkoenig @scpeters