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

Component Unit Tests #99

Closed
5 tasks done
GalexY727 opened this issue Jan 31, 2024 · 13 comments · Fixed by #112 or #141
Closed
5 tasks done

Component Unit Tests #99

GalexY727 opened this issue Jan 31, 2024 · 13 comments · Fixed by #112 or #141
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed subsystem A physical subsystem on the robot
Milestone

Comments

@GalexY727
Copy link
Member

GalexY727 commented Jan 31, 2024

User Story

With the closure of #27, we need to make sure that each and every subsystem is realisitally able to move. We are going to be getting an alpha bot very soon, and our number one priority should be to not break anything too on our first code run. I think it's possible for it to work first try, so let's find out how to get that done

Acceptance Criteria

  • Each component needs to be able to move in sim. Our motors are capible of running in simulation, we just need to make sure we are logging them correctly and that our position conversion factors make the movement reasonable.
    • Doubling off of the above, we need to have soft stops to limit our components. A soft stop is a limit in the code that stops the motor from going any further, even though it physically might be able to. The easiest way to enforce a soft stop is by using MathUtil.clamp(value, min, max), with constants representing our min and max value.
  • Gear ratios
    • Yeah. they exist. We need constants in the code for them so that we can run things like setPositionConversionFactor or setVelocityConversionFactor and all we need to do is plug in the required value to get that done. Notice that Climb logic -> Main (#97) #98 outputs in meters for the climb's desired height. The goal for this bullet is for us to turn meters into motor rotations.
  • Unit tests
    • Each subsystem should have a "unit test" that it runs when we initially get the robot to SLOWLY (think around 10 seconds) move it from one end of its limit to another, just so that we can test things like negative signs without telling the motor to go full throttle.
  • PID (where applicable)
    • For motors that run a PID controller (shooter, elevator, climb) we need to have those constants set to a reasonable gain before telling the motor to move at all. I recommend starting with a P of 0.01 and an I and D of 0. PID can really easily tell our motor to go faster than we initially intended for it to go.

Thankfully, since we have sim, we are able to ask the motor what it thinks its simulated position or velocity is, which is a great start to seeing if our logic is telling it to do the right things or not.

@GalexY727 GalexY727 added enhancement New feature or request help wanted Extra attention is needed subsystem A physical subsystem on the robot labels Jan 31, 2024
@GalexY727 GalexY727 added this to the Piece Control milestone Jan 31, 2024
@GalexY727 GalexY727 changed the title Verification of driving all components Component Unit Tests Jan 31, 2024
@GalexY727 GalexY727 moved this to Ready in Crescendo 2024 Jan 31, 2024
@Oliver-Cushman
Copy link
Contributor

Ideally should we make different testing branches for different subsystems or should these tests exist under one unit test branch? By the way I have some ideas to try out on sim so I think I am going to assign this to myself for now.

@Oliver-Cushman Oliver-Cushman self-assigned this Jan 31, 2024
@PatribotsProgramming
Copy link
Member

I wanted to mention this page on WPILib for unit tests, we don't need to follow it but its something that caught my eye.

@Jacob1010-h
Copy link
Member

I wanted to mention this page on WPILib for unit tests, we don't need to follow it but its something that caught my eye.

I really like this!! I'll start writing some unit tests in edd

@Jacob1010-h
Copy link
Member

Ideally should we make different testing branches for different subsystems or should these tests exist under one unit test branch? By the way I have some ideas to try out on sim so I think I am going to assign this to myself for now.

I think we should make unit tests for different subsystems in different branches so we don't cludder up main

@GalexY727
Copy link
Member Author

Please keep simplicity in mind when writing these if you plan on using the interfaces. I want unit tests to be the simplest part of the code which anyone can understand.

@GalexY727
Copy link
Member Author

Ideally should we make different testing branches for different subsystems or should these tests exist under one unit test branch? By the way I have some ideas to try out on sim so I think I am going to assign this to myself for now.

I think we should make unit tests for different subsystems in different branches so we don't cludder up main

An alternative to this would be to have a second robotContainer file that contains all of the code for just testing everything and nothing that would run on the main bot. This way it would be up to date with the test of the robot and it won't be a pain to keep up to date as the subsystems get more complex.

@GalexY727 GalexY727 pinned this issue Feb 1, 2024
@Oliver-Cushman Oliver-Cushman linked a pull request Feb 2, 2024 that will close this issue
@Oliver-Cushman Oliver-Cushman removed a link to a pull request Feb 2, 2024
@GalexY727 GalexY727 linked a pull request Feb 2, 2024 that will close this issue
@github-project-automation github-project-automation bot moved this from Ready to Done in Crescendo 2024 Feb 2, 2024
@GalexY727 GalexY727 reopened this Feb 3, 2024
@github-project-automation github-project-automation bot moved this from Done to Ready in Crescendo 2024 Feb 3, 2024
@GalexY727
Copy link
Member Author

wrongly closed whoopies
unit tests are just on main now cuzz uh yeah

@Jacob1010-h
Copy link
Member

Jacob1010-h commented Feb 3, 2024

? Ok. What abt making all the different branches?

Oh did you still want to use wpilib s junit things? @GalexY727

@Jacob1010-h Jacob1010-h moved this from Ready to In progress in Crescendo 2024 Feb 3, 2024
@Jacob1010-h
Copy link
Member

@GalexY727 Did u want to use the junits or just make some other form of testing?

@GalexY727
Copy link
Member Author

GalexY727 commented Feb 3, 2024

While JUnit tests are really powerful, I think that they are more attuned to a public repository rather than a private one like our robot. We know that most of our code works because we generally test in sim before PRing, and that is our substitution of a JUnit test. Most of these checkboxes are kind-of done, but I want this issue to serve as a final to-do list to check off before we click enable on our first real robot deployment. The last thing we want to do is break a falcon (cough cough supernurds)

@Oliver-Cushman Oliver-Cushman moved this from In progress to In review in Crescendo 2024 Feb 7, 2024
@PatribotsProgramming PatribotsProgramming linked a pull request Feb 9, 2024 that will close this issue
@github-project-automation github-project-automation bot moved this from In review to Done in Crescendo 2024 Feb 10, 2024
@GalexY727 GalexY727 unpinned this issue Feb 17, 2024
@Jacob1010-h
Copy link
Member

While JUnit tests are really powerful, I think that they are more attuned to a public repository rather than a private one like our robot. We know that most of our code works because we generally test in sim before PRing, and that is our substitution of a JUnit test. Most of these checkboxes are kind-of done, but I want this issue to serve as a final to-do list to check off before we click enable on our first real robot deployment. The last thing we want to do is break a falcon (cough cough supernurds)

I feel like this is no longer the case, after working with them for #288, I think that for testing math, JUnits are very powerful. For general application though, sim is still da best.

@GalexY727
Copy link
Member Author

While JUnit tests are really powerful, I think that they are more attuned to a public repository rather than a private one like our robot. We know that most of our code works because we generally test in sim before PRing, and that is our substitution of a JUnit test. Most of these checkboxes are kind-of done, but I want this issue to serve as a final to-do list to check off before we click enable on our first real robot deployment. The last thing we want to do is break a falcon (cough cough supernurds)

I feel like this is no longer the case, after working with them for #288, I think that for testing math, JUnits are very powerful. For general application though, sim is still da best.

Although that math was shown to work, #288 still failed the "unit test" of running it in sim and seeing if the output was as expected. While this did come down to tuning constants, the JUnit files would be fully encapsulated in testing the entire functionality in sim. Again, not saying JUnits are worthless, but I don't think that they are necessary when we look at the other resources we have access to.

@Jacob1010-h
Copy link
Member

While JUnit tests are really powerful, I think that they are more attuned to a public repository rather than a private one like our robot. We know that most of our code works because we generally test in sim before PRing, and that is our substitution of a JUnit test. Most of these checkboxes are kind-of done, but I want this issue to serve as a final to-do list to check off before we click enable on our first real robot deployment. The last thing we want to do is break a falcon (cough cough supernurds)

I feel like this is no longer the case, after working with them for #288, I think that for testing math, JUnits are very powerful. For general application though, sim is still da best.

Although that math was shown to work, #288 still failed the "unit test" of running it in sim and seeing if the output was as expected. While this did come down to tuning constants, the JUnit files would be fully encapsulated in testing the entire functionality in sim. Again, not saying JUnits are worthless, but I don't think that they are necessary when we look at the other resources we have access to.

Agreed, it's better to test via implementation then with fake data that didn't prove it would work with my implementation. But then again, I probably could've written my test differently with that In mind.

I think that this is the major separating factor: to make junit tests valuable, you need to build with them in mind. Because that wasn't a concern for us, Junits prove useless for us

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed subsystem A physical subsystem on the robot
Projects
Archived in project
4 participants