-
Notifications
You must be signed in to change notification settings - Fork 14
fix(esp-box): Fix example IMU vector calculation on ESP-BOX #474
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the ESP-BOX example to rotate the gravity and computed vectors when running on the original ESP-BOX, ensuring consistent output across ESP-BOX and ESP-BOX-3.
- Introduce a
box_type
check to conditionally swap and flip the gravity vector. - Apply the same conditional rotation logic to the
vx
andvy
components.
Comments suppressed due to low confidence (1)
components/esp-box/example/main/esp_box_example.cpp:249
- There are no tests covering the new IMU orientation adjustment; consider adding unit or integration tests to verify the gravity vector and computed vector rotations for different box types.
auto box_type = box.box_type();
|
||
auto box_type = box.box_type(); | ||
if (box_type == espp::EspBox::BoxType::BOX) { | ||
std::swap(gravity_vector.x, gravity_vector.y); | ||
gravity_vector.y = -gravity_vector.y; // flip y axis | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider extracting the box-type check and vector rotation logic into a shared helper function or moving the box_type
declaration closer to both uses to reduce duplicated code and improve readability.
auto box_type = box.box_type(); | |
if (box_type == espp::EspBox::BoxType::BOX) { | |
std::swap(gravity_vector.x, gravity_vector.y); | |
gravity_vector.y = -gravity_vector.y; // flip y axis | |
} | |
gravity_vector = adjust_gravity_vector(gravity_vector, box.box_type()); |
Copilot uses AI. Check for mistakes.
@@ -246,6 +246,12 @@ extern "C" void app_main(void) { | |||
auto orientation = imu->get_orientation(); | |||
auto gravity_vector = imu->get_gravity_vector(); | |||
|
|||
auto box_type = box.box_type(); | |||
if (box_type == espp::EspBox::BoxType::BOX) { | |||
std::swap(gravity_vector.x, gravity_vector.y); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The swap-and-flip pattern is duplicated for both the gravity vector and the (vx, vy) calculations; extracting this transformation into a reusable helper would avoid duplication and make future adjustments easier.
Copilot uses AI. Check for mistakes.
Description
esp-box/example
to rotate the gravity vector depending on the box type, since the original ESP-BOX has a different IMU orientation on board.Motivation and Context
Ensures the output of the example is the same across ESP-BOX and ESP-BOX-3
How has this been tested?
Build and run
esp-box/example
on ESP-BOX and ensure that in the default screen orientation, the gravity vectors on screen point in the right direction.Screenshots (if appropriate, e.g. schematic, board, console logs, lab pictures):
Types of changes
Checklist:
Software
.github/workflows/build.yml
file to add my new test to the automated cloud build github action.