EKF API and test #33
EKF API and test #33
Conversation
| @@ -0,0 +1,57 @@ | |||
| #pragma once | |||
There was a problem hiding this comment.
Where is the file header? Reference other header files and add it please
| #pragma once | ||
| #include <stdio.h> | ||
| #include <math.h> | ||
| #define STATE_SIZE 9 |
There was a problem hiding this comment.
Please leave a comment explaining what this is
/** @brief [DESC] */
| #include <math.h> | ||
| #define STATE_SIZE 9 | ||
| #define MEASUREMENT_SIZE 6 //(x, y, z), (ax, ay, az), (a, b, y) | ||
| #define DT 1 |
There was a problem hiding this comment.
What is DT? Leave a comment like above
/** @brief [DESC] */
| #include <stdio.h> | ||
| #include <math.h> | ||
| #define STATE_SIZE 9 | ||
| #define MEASUREMENT_SIZE 6 //(x, y, z), (ax, ay, az), (a, b, y) |
There was a problem hiding this comment.
Please reformat the comment. Not sure what the comment means either, don't really see the relation?
|
|
||
|
|
||
| //state matrix | ||
| double x[STATE_SIZE]={}; |
There was a problem hiding this comment.
Why are these variables in the header file? Can we move them to the C file
| double Q[STATE_SIZE][STATE_SIZE]={}; //9x9 matrix w/covariance in position, velocity, and orientation angle noise | ||
|
|
||
|
|
||
| double R[MEASUREMENT_SIZE][MEASUREMENT_SIZE]= |
There was a problem hiding this comment.
You have a lot of matrices. Can you put them all into a storage struct? Having them stored together in a storage will make it much cleaner
| double S_inv[MEASUREMENT_SIZE][MEASUREMENT_SIZE] = {0}; | ||
| double H_T[STATE_SIZE][MEASUREMENT_SIZE] = {0}; | ||
| double P_H_T[STATE_SIZE][MEASUREMENT_SIZE] = {0}; | ||
| double y[MEASUREMENT_SIZE] = {0}; |
There was a problem hiding this comment.
Are all of these required? Or can some of them be reused. Your API is for embedded devices, we don't want to cause a stack overflow by allocating too much memory from 1 function call
| double result{STATE_SIZE}[STATE_SIZE]; | ||
| transponse_matrix(A, expected); | ||
| TEST_ASSERT_TRUE(compare_matrices(result, expected)); | ||
| } |
There was a problem hiding this comment.
This test doesn't make sense. Youre transposing A, putting the result into 'expected', but not doing anything with 'result'? I don't think this is actually testing the code
| TEST_ASSERT_FLOAT_WITHIN(EPSILON, 1 + 0.5 * 1 * DT * DT, x[0]); | ||
| TEST_ASSERT_FLOAT_WITHIN(EPSILON, 1 + 0.5 * 2 * DT * DT, x[1]); | ||
| TEST_ASSERT_FLOAT_WITHIN(EPSILON, 1 + 0.5 * 3 * DT * DT, x[2]); | ||
| } |
There was a problem hiding this comment.
You copy pasted the exact same test twice
| TEST_ASSERT_FLOAT_WITHIN(EPSILON, expected_x[i], x[i]); | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
You need more test cases. You need to cover the edge cases and use non-standard input values. You should also be isolating diferent situations, for example a measurement with no y or z position. and only x position. Or a test with only velocity and 0,0,0 for position. etc.
No description provided.