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

Throw on nonexistent/invalid SimDevice name in SimDeviceSim constructor #5041

Merged
merged 1 commit into from
Feb 3, 2023
Merged

Throw on nonexistent/invalid SimDevice name in SimDeviceSim constructor #5041

merged 1 commit into from
Feb 3, 2023

Conversation

rzblue
Copy link
Member

@rzblue rzblue commented Feb 2, 2023

No description provided.

@rzblue rzblue requested a review from a team as a code owner February 2, 2023 20:05
@Starlight220
Copy link
Member

What's the rationale for this?
Besides, it should be consistent with getting a nonexistent/wrongly-typed field.

@rzblue rzblue force-pushed the simdevicesim-error-handle branch from 0a68fb1 to 24b1662 Compare February 2, 2023 21:04
@rzblue
Copy link
Member Author

rzblue commented Feb 2, 2023 •
edited
Loading

Currently creating a SimDeviceSim at a non-existant name will result in, essentially, a poisoned object that will never work as it will have a handle of 0. In java (not sure about c++), this will result in an NPE when a user tries to use the return value of getDouble and friends, which isn't an obvious error.

This fails fast rather than delaying until the first use.

@Starlight220
Copy link
Member

So getting a nonexistent/wrongly-typed property should fail-fast as well.

@rzblue
Copy link
Member Author

rzblue commented Feb 2, 2023

Probably, yeah. I can add those as well.

@PeterJohnson PeterJohnson merged commit d991f6e into wpilibsuite:main Feb 3, 2023
PeterJohnson added a commit to PeterJohnson/allwpilib that referenced this pull request Feb 5, 2023
@PeterJohnson
Revert "[wpilib] Throw on nonexistent SimDevice name in SimDeviceSim …
b1403e5 
…constructor (wpilibsuite#5041)"

This reverts commit d991f6e.
This was referenced Feb 5, 2023
Merged
Open
PeterJohnson added a commit that referenced this pull request Feb 5, 2023
@PeterJohnson
[wpilib] Revert throw on nonexistent SimDevice name in SimDeviceSim (#…
8230fc6 
…5053)

This breaks current vendor use of SimDeviceSim.

This reverts commit d991f6e (#5041).
Starlight220 pushed a commit to Starlight220/allwpilib that referenced this pull request Mar 2, 2023
@rzblue @Starlight220
[wpilib] Throw on nonexistent SimDevice name in SimDeviceSim construc…
4777489 
…tor (wpilibsuite#5041)

Previously this would just create a object that was otherwise non-functional.
Starlight220 pushed a commit to Starlight220/allwpilib that referenced this pull request Mar 2, 2023
@PeterJohnson @Starlight220
[wpilib] Revert throw on nonexistent SimDevice name in SimDeviceSim (w…
36b9f1c 
…pilibsuite#5053)

This breaks current vendor use of SimDeviceSim.

This reverts commit d991f6e (wpilibsuite#5041).
@rzblue rzblue deleted the simdevicesim-error-handle branch June 21, 2023 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PeterJohnson PeterJohnson PeterJohnson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rzblue @Starlight220 @PeterJohnson