Conversation
mpkarpov-ui
left a comment
There was a problem hiding this comment.
I like the changes overall, just a couple questions though
| ;lib_deps = | ||
| ; Eigen |
There was a problem hiding this comment.
Curious why eigen is killed here
There was a problem hiding this comment.
Should undo this getting deleted
There was a problem hiding this comment.
Guessing these are just organizational / formatting changes so lgtm
| enum class LED { | ||
| BLUE = 0, | ||
| RED = 1, | ||
| ORANGE = 2, | ||
| GREEN = 3 | ||
| }; | ||
|
|
||
| /** | ||
| * The ESP32 has two physical cores, which will each be dedicated to one group of threads. | ||
| * The DATA_CORE runs the GPS thread, as well as the threads which read from the sensor_data struct (e.g. SD logging). | ||
| */ | ||
| #define DATA_CORE ((BaseType_t) 1) | ||
| enum class Channel { | ||
| A = 0, | ||
| B = 1, | ||
| C = 2, | ||
| D = 3 | ||
| }; | ||
|
|
||
| /** | ||
| * Macro for declaring a thread. Creates a function with the name suffixed with `_thread`, annotated with [[noreturn]]. | ||
| * | ||
| * @param name The name of the task. | ||
| * @param param The single parameter to input into the thread. | ||
| */ | ||
| #define DECLARE_THREAD(name, param) [[noreturn]] void name##_thread(param) | ||
| enum class Camera { | ||
| Side = 0, | ||
| Bulkhead = 1 | ||
| }; |
There was a problem hiding this comment.
Any reason we're switching to enum classes instead of defines here?
There was a problem hiding this comment.
It is better than the #DEFINE pattern
| float roll_rate_hz = std::clamp(std::abs(orientation.angular_velocity.vx) / (2.0f*static_cast<float>(PI)), 0.0f, max_roll_rate_hz); | ||
| float roll_rate_hz = std::clamp(std::abs(orientation.angular_velocity.vx) / (2.0f*static_cast<float>(M_PI)), 0.0f, max_roll_rate_hz); |
There was a problem hiding this comment.
I thought PI and M_PI had different values?
There was a problem hiding this comment.
Maybe I'm just misremembering
There was a problem hiding this comment.
I thought M_PI was something like ~ 0.27, might be mistaken though
No description provided.