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

[cmd] Fix sysid crashing when passed nullptr or {} #6508

Merged
merged 5 commits into from
May 1, 2024
Merged

[cmd] Fix sysid crashing when passed nullptr or {} #6508

merged 5 commits into from
May 1, 2024

Conversation

r4stered
Copy link
Contributor

No description provided.

@r4stered r4stered requested a review from a team as a code owner April 10, 2024 18:53
@r4stered
Copy link
Contributor Author

/format

@r4stered r4stered requested review from PeterJohnson and a team as code owners April 10, 2024 19:04
@r4stered r4stered requested a review from a team as a code owner April 10, 2024 20:22
@PeterJohnson
Copy link
Member

/format

spacey-sooty
spacey-sooty reviewed Apr 12, 2024
View reviewed changes
Copy link
Contributor

@spacey-sooty spacey-sooty left a comment

Choose a reason for hiding this comment

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

This resolves #6507 right?

@calcmogul calcmogul linked an issue Apr 12, 2024 that may be closed by this pull request
C++ SysIdRoutine Mechanism does not support null log function #6507
Closed
@calcmogul calcmogul changed the title [commands] fix sysid crashing when passed nullptr or {} #6507 [commands] fix sysid crashing when passed nullptr or {} Apr 12, 2024
@calcmogul calcmogul changed the title [commands] fix sysid crashing when passed nullptr or {} [cmd] fix sysid crashing when passed nullptr or {} Apr 12, 2024
@calcmogul calcmogul changed the title [cmd] fix sysid crashing when passed nullptr or {} [cmd] Fix sysid crashing when passed nullptr or {} Apr 12, 2024
@PeterJohnson
Copy link
Member

Please rebase this / retarget this to the main branch.

@r4stered r4stered changed the base branch from development to main April 22, 2024 13:39
@r4stered
Copy link
Contributor Author

r4stered commented Apr 22, 2024 •
edited
Loading

Please rebase this / retarget this to the main branch.

I was able to cherry-pick my commits into my forked main. Should I make a new PR? I am not sure how to change this PR to merge from my main

@spacey-sooty
Copy link
Contributor

It doesn't need to be from your main just against wpilibsuite main

@PeterJohnson PeterJohnson added breaking Introduces a breaking change. component: command-based WPILib Command Based Library labels May 1, 2024
@PeterJohnson PeterJohnson merged commit 0afc35f into wpilibsuite:main May 1, 2024
25 checks passed
chauser pushed a commit to chauser/allwpilib that referenced this pull request May 30, 2024
@r4stered @chauser
[commands] Fix C++ SysIdRoutine crashing when passed nullptr or {} (w…
e62ff0b 
…pilibsuite#6508)

Flattens parameter from a `std::optional<std::function<...>>` to just a `std::function<...>`.  This is a breaking change but a trivial one for teams to fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@calcmogul calcmogul calcmogul left review comments

@spacey-sooty spacey-sooty spacey-sooty left review comments

@PeterJohnson PeterJohnson PeterJohnson approved these changes

Assignees
No one assigned
Labels
breaking Introduces a breaking change. component: command-based WPILib Command Based Library
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

C++ SysIdRoutine Mechanism does not support null log function
4 participants
@r4stered @PeterJohnson @spacey-sooty @calcmogul