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

simIncludeRW didn't allow specify certain RW friction parameters #816

Merged
merged 3 commits into from
Sep 21, 2024

Conversation

schaubh
Copy link
Contributor

@schaubh schaubh commented Sep 21, 2024

  • Review: By commit
  • Merge strategy: Merge (no squash)

Description

When using simIncludeRW.py the user can specify custom fCoulomb, fStatic and cViscous values. However, if the RW templates also specify these the custom values were overwritten.

Verification

All tests pass. Expanded RW unit test to check if a custom friction value is set and retained.

Documentation

Updated release notes and known issues document.

Future work

None

Before these properties were over-written if the RW template specified them.  Now they are either defaulting to zero if nobody specifies them, set to the RW template value if only the template is used, and set to the specific value if the user wants to.
@schaubh schaubh added the bug Something isn't working label Sep 21, 2024
@schaubh schaubh self-assigned this Sep 21, 2024
@schaubh schaubh requested a review from a team as a code owner September 21, 2024 17:12
Copy link
Contributor

@andrewmorell andrewmorell left a comment

Choose a reason for hiding this comment

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

Looks good, just left one comment wondering about commenting/uncommenting unit test methods in main.

@schaubh schaubh merged commit 65efb46 into develop Sep 21, 2024
9 checks passed
@schaubh schaubh deleted the feature/simIncludeRW_fix branch September 21, 2024 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants