Skip to content

Conversation

@Meow404
Copy link
Collaborator

@Meow404 Meow404 commented Feb 7, 2026

This change is Reviewable

@xuanhien070594
Copy link
Contributor

multibody/lcs_factory.cc line 123 at r1 (raw file):

        planar_normal_direction_per_contact_.push_back(
            options_.planar_normal_direction.value());
      } else {

As we don't need this else branch, we can initialize the vector planar_normal_direction_per_contact_ with all default values { }, then assign new value when n_friction_directions_per_contact_[i] == 1. In addition, can we use single for loop to reduce code duplication?

Copy link
Contributor

@xuanhien070594 xuanhien070594 left a comment

Choose a reason for hiding this comment

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

@xuanhien070594 reviewed 9 files and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Meow404).

Copy link
Collaborator Author

@Meow404 Meow404 left a comment

Choose a reason for hiding this comment

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

@Meow404 made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @xuanhien070594).


multibody/lcs_factory.cc line 123 at r1 (raw file):

Previously, xuanhien070594 (Hien Bui) wrote…

As we don't need this else branch, we can initialize the vector planar_normal_direction_per_contact_ with all default values { }, then assign new value when n_friction_directions_per_contact_[i] == 1. In addition, can we use single for loop to reduce code duplication?

Done.

Copy link
Contributor

@xuanhien070594 xuanhien070594 left a comment

Choose a reason for hiding this comment

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

:lgtm:

@xuanhien070594 reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Meow404).

@github-actions
Copy link

LCOV of commit af98d68 during C3 Coverage #69

Summary coverage rate:
  lines......: 91.9% (1444 of 1571 lines)
  functions..: 77.4% (130 of 168 functions)
  branches...: 52.2% (1196 of 2290 branches)

Files changed coverage rate:
                                        |Lines       |Functions  |Branches    
  Filename                              |Rate     Num|Rate    Num|Rate     Num
  ============================================================================
  multibody/lcs_factory.cc              | 4.8%    313|3573%    15|    -      0
  multibody/lcs_factory.h               |33.3%      3|1200%     1|    -      0
  multibody/lcs_factory_options.h       | 6.9%     29| 800%     2|    -      0

@Meow404 Meow404 merged commit 7e99d56 into main Feb 11, 2026
4 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants