Skip to content

[primitives] Add input_required option to PassThrough - #24929

Open
castor639 wants to merge 1 commit into
RobotLocomotion:masterfrom
castor639:passthrough-input-required
Open

[primitives] Add input_required option to PassThrough#24929
castor639 wants to merge 1 commit into
RobotLocomotion:masterfrom
castor639:passthrough-input-required

Conversation

@castor639

@castor639 castor639 commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Resolves #21727 .


This change is Reviewable

@ggould-tri ggould-tri added component: system framework System, Context, and supporting code release notes: breaking change This pull request contains breaking changes labels Sep 1, 2026

@ggould-tri ggould-tri left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Assigning +a:@sherm1 as systems framework area lead; feel free to delegate. Noted as release-notable since at superficial glance I think this is a breaking API change to the primitives.

@ggould-tri made 1 comment.
Reviewable status: LGTM missing from assignee sherm1(platform), needs at least two assigned reviewers (waiting on sherm1).

@sherm1

sherm1 commented Sep 1, 2026

Copy link
Copy Markdown
Member

@drake-jenkins-bot test this please

@sherm1 sherm1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The code looks fine and I don't think it's a breaking change, but I am unclear on the motivation for this change? Do you have a use case for it? Please explain why we should make this change now.

@sherm1 made 1 comment.
Reviewable status: LGTM missing from assignee sherm1(platform), needs at least two assigned reviewers.

@sherm1 sherm1 added release notes: feature This pull request contains a new feature and removed release notes: breaking change This pull request contains breaking changes labels Sep 1, 2026
@sherm1

sherm1 commented Sep 1, 2026

Copy link
Copy Markdown
Member

@drake-jenkins-bot retest this please

@jwnimmer-tri

Copy link
Copy Markdown
Collaborator

For context -- this is a fix for #21727.

@sherm1

sherm1 commented Sep 3, 2026

Copy link
Copy Markdown
Member

OK, thanks Jeremy. @castor639 when a PR addresses some issue, please put the issue number in the PR description, preceded by GitHub magic like "Fixes" or "Resolves" so that when the PR is merged the issue is automatically closed.

@sherm1 sherm1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Feature :lgtm:, with one nit
+a:@SeanCurtis-TRI for platform review per rotation (Tuesday)

@sherm1 reviewed 6 files and all commit messages, and made 2 comments.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee SeanCurtis-TRI(platform) (waiting on castor639 and SeanCurtis-TRI).


systems/primitives/pass_through.h line 56 at r1 (raw file):

  /// connected will throw. If false (the default), the zero default value is
  /// used instead.
  /// @pydrake_mkdoc_identifier{1args_vector_size}

nit: these "1args" mkdoc names seem wrong for these 2-arg methods. Not clear to me why you need the arg count in the name anyway. Consider just vector_size, value, and abstract_model_value as the mkdoc names.

@jwnimmer-tri jwnimmer-tri removed the component: system framework System, Context, and supporting code label Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release notes: feature This pull request contains a new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add an option to require the input of PassThrough system

5 participants