FRC2706 / 2019-2706-Robot-Code

The main robot code for the FIRST 2019 challenge: Deep Space
MIT License
2 stars 0 forks source link

Rcvi integration #99

Closed RachelLucyshyn closed 5 years ago

RachelLucyshyn commented 5 years ago

Summary of Changes

For the feature to work properly, the following must hold i. the robot must have a heading that is "most facing" the desired target compared to other targets at the time driver assist is requested ii. the robot must be powered up at the start of the match at the heading specified in degrees by the ROBOT_START_ANGLE attribute of the Config class, where this heading is relative to a horizontal line extending across the field and positive heading is measured CCW

NOTE: MOTION CONTROL FUNCTIONALITY IS CURRENTLY AWAITING COMPLETION

Testing Performed

Environment: Plyboy Regression tests in DriverAssistVisionTest.java

KyleRAnderson commented 5 years ago

The automated tests for this branch are also failing on our linux environment (which runs tests in a linux image built to run wpilib stuff).

This is usually caused by improper resetting of subsystems. For the most part this is easy enough to fix, if you add calls to Util.resetSubsystems() in the right spots (usually the @Before annotated method).

If you want to see what this looks like pretty much all the tests have this in them.

robertlucyshyn commented 5 years ago

This is a response to @KyleRAnderson to this comment (because I did not know how to respond to the comment directly):

The automated tests for this branch are also failing on our linux environment (which runs tests in a linux image built to run wpilib stuff). This is usually caused by improper resetting of subsystems. For the most part this is easy enough to fix, if you add calls to Util.resetSubsystems() in the right spots (usually the @Before annotated method). If you want to see what this looks like pretty much all the tests have this in them.

I attempted to add the following (similar to what other tests were doing):

@Before
public void setUp() throws NoSuchFieldException, IllegalAcI cessException {
     Util.resetSubsystems();
}

but I get a null pointer exception when the test is run. I suspect is it complaining about the DriveBase because it is somehow needed when the PigeonIMU class is mocked. I have noticed that the other tests that call Util.resetSubsystems() use DriveBase.getInstance() when they are running a test but I don't think that is applicable in the current situation as far as I can tell.

At the moment, I'm looking at the Linux test and it is passing so perhaps Util.resetSubsystems() is not required.

Also, I notice that the tests are showing failures in the tests testLiveWindowAndScheduler and testStates. I don't see how these failures are related to the code we are adding. I have found that when I compile, testLiveWindowAndScheduler and testStates always fail the first time but then they pass the second time.

Let me know if you need further changes.

KyleRAnderson commented 5 years ago

Tests are probably failing because of improper reset between test classes. Probably fixable with calls to Util.resetSubsystems() in the right places.

robertlucyshyn commented 5 years ago

@KyleRAnderson @eandr127 I think we've addressed all items that have been raised so far. There is still one item related to the Xbox stuff in Config.java that we would prefer you to do. We have also gone through all conversations and marked the ones that we clearly addressed as resolved. Please go through the remaining ones and mark them as resolved if you think they are or advise us what additional changes are needed. Please advise us of the next steps to get this pull request completed. Thanks.

Update: I just saw the approval. Do I just push the "Merge pull request" button? Could there be any merge conflicts at this point?

Update2: Rachel just pointed out it says no merge conflicts so I'm pushing the button...