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

[wpiutil] RFC: Add mechanism to register global reset functions #4890

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

virtuald
Copy link
Member

@virtuald virtuald commented Jan 1, 2023

Intent would be to get rid of most of the wpi::impl/frc::impl functions I've added for RobotPy and provide a way for teams to reset wpilib between unit tests as well. Would also provide a mechanism for vendor state to be reset if they participated.

Let's start by talking about this, and if we think this isn't terrible, then I can actually replace the existing reset functions I'm aware of with this and add those to the PR.

CC @Starlight220

@virtuald virtuald requested a review from PeterJohnson as a code owner January 1, 2023 01:30
@virtuald virtuald changed the title [wpiutil] Add mechanism to register global reset functions [wpiutil] RFC: Add mechanism to register global reset functions Jan 1, 2023
@virtuald
Copy link
Member Author

virtuald commented Jan 1, 2023

/format

@virtuald virtuald marked this pull request as draft January 1, 2023 01:54
@Starlight220
Copy link
Member

The concept looks good!
One thing I'm not sure about is how Java would work with this, regarding @PeterJohnson 's comment here. Wouldn't this create dangling references as well?

@virtuald
Copy link
Member Author

virtuald commented Jan 1, 2023

Java has its own versions of all the singletons I'm thinking about, so it would need its own implementation of this sort of thing. I am not going to implement that.

@virtuald
Copy link
Member Author

virtuald commented Jan 2, 2023

Couple things I ran into when baking this:

  • NetworkTables is actually a bit more involved than I had thought through at first. Likely you'd want to reset the default instance and destroy every other instance -- which I don't even do for my tests.
  • There are some robotpy-specific things that I need to do when some of these are reset, which I thought I'd be able to do by inserting my own priorities, but that isn't actually the case.
  • It's not clear to me if the ordering I have defined here is really the Right Order. I currently follow this order in RobotPy, but I have changed it when crashes occurred.

@virtuald
Copy link
Member Author

virtuald commented Jan 2, 2023

/format

@virtuald
Copy link
Member Author

virtuald commented Jan 2, 2023

I don't understand why tidy/format are failing here.

@calcmogul calcmogul added the component: wpiutil WPI utility library label Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: wpiutil WPI utility library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants