More tests #62
More tests #62
Conversation
| @@ -369,12 +478,64 @@ def test_max_altitude(self, data_processor): | |||
| ], | |||
| ) | |||
| def test_calculate_rotations(self, data_packets, expected_value): | |||
There was a problem hiding this comment.
I want some more cases on this test. Another case should involve e.g. different axis of gravity vector, different gravity sign, no gyro data, etc.
There was a problem hiding this comment.
I'm gonna wait on this one for will
tests/test_data_processor.py
Outdated
| assert rotations[1][-1] == pytest.approx(expected_value[1]) | ||
| assert rotations[2][-1] == pytest.approx(expected_value[2]) | ||
|
|
||
| def test_initial_orientation(self): |
There was a problem hiding this comment.
ideally this test should also be paramterized so it covers literally all 6(?) gravity cases. The two cases you chose are sufficient so this isn't necessary but still.
There was a problem hiding this comment.
it was a pain to write these and I figured the cases covered everything, but you're probably right. I think I'll also wait on will for this one.
Co-authored-by: Harshil <[email protected]>
|
Regarding some of the unit tests with rotating acceleration vectors, I will most likely have to do another pass over the math. It most likely wont work given a flipped dataset where +Z is vertical. For the time being, if you are giving it normal compensated accelerations, gravity vectors, and quaternions that appear on the existing datasets, tests should pass and everything will work fine. I will most likely need to work on #63 a bit first so that I can fix the math with flipped datasets. I can be responsible for updating and passing those tests once I get that sorted out, and add some case scenarios for different vertical orientations. |
|
You mean only the rotated accelerations would have an incorrect value? If that is the case, the unit tests should've already caught them in a way. Anyway, the unit tests should actually test every possible orientation and configuration, and we need to be sure the values are correct (cause it's hard to do the math manually). Do you know if there's an online calculator or something so we can double check? |
|
No, It has to due with how the IMU would log data depending on orientation. With the rocket being -Z vertical (like interest launch) you would expect on launch pad assuming directly vertical:
From this, we rotate the acceleration, which gives a vector pretty close to [0, 0, -9.8]
When you do the same operations, I believe the acceleration would come out to [0, 0, -9.8], but we want it to be positive. |
|
@wlsanderson @harshil21 what do you want to do with this PR? I would prefer to merge this as it is, then make a new branch for fixing the math and improving the tests for them. Alternatively, we could make a branch from this fixing the math, merge it into this, then merge this into #47, but then I feel like we will have way too many branches and conflicts down the line and I don't want to deal with that. Merging #47 with main is already going to be a headache as is |
alright I get it now. Can you make sure to get 6 different data sets (csv files) for all the orientations? Simply running |
Yeah a separate PR after merging this is the way to go, to keep things atomic.
dw, there wouldn't be too many conflicts (I already solved most yesterday) |
In this PR I: