Roll Pitch Yaw Calculations #326
Conversation
| - hal_nxp | ||
| - littlefs | ||
| - loramac-node | ||
| - hal_st |
There was a problem hiding this comment.
Note github diff doesn't show that it's present above. This is just deleting a duplicate entry
There was a problem hiding this comment.
Pull Request Overview
This PR adds new math functionality to calculate roll, pitch, and yaw from sensor fusion quaternions and updates the build configuration and project manifest accordingly.
- Added math utilities in both header and source files (n_math_utils.h/.cpp)
- Updated CMakeLists.txt and Kconfig to enable the new math feature
- Modified west.yml to include additional repositories (lz4 and zscilib)
Reviewed Changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| west.yml | Updates manifest by adding lz4 and zscilib repositories |
| lib/f_core/math/n_math_utils.cpp | Introduces the quaternion-to-euler conversion with unit conversion |
| lib/f_core/math/CMakeLists.txt | Configures build for the new math module |
| lib/f_core/Kconfig | Adds a new configuration option for math functionality |
| lib/f_core/CMakeLists.txt | Updates subdirectory inclusions to support the new math and radio modules |
| include/f_core/math/n_math_utils.h | Provides the header for the new math utility with documentation |
Files not reviewed (2)
- app/backplane/sensor_module/.idea/editor.xml: Language not supported
- app/backplane/sensor_module/.idea/vcs.xml: Language not supported
Comments suppressed due to low confidence (1)
lib/f_core/math/n_math_utils.cpp:11
- There's a mismatch between the implementation and the header documentation: the header indicates that the output angles are in radians, but the implementation converts them to degrees. Please update the documentation or the conversion logic so that the behavior is consistent.
roll = euler.x * multiplier;
|
|
||
| void NMathUtils::CalculateRollPitchYaw(zsl_quat inputQuat, float& roll, float& pitch, float& yaw) | ||
| { | ||
| static constexpr multiplier = 180 / ZSL_PI; |
There was a problem hiding this comment.
Missing type for the constant 'multiplier'. Consider specifying a type (e.g., 'float') or using 'auto' to ensure proper type deduction.
| static constexpr multiplier = 180 / ZSL_PI; | |
| static constexpr double multiplier = 180 / ZSL_PI; |
There was a problem hiding this comment.
robots kinda cooking with this one
|
|
||
| void NMathUtils::CalculateRollPitchYaw(zsl_quat inputQuat, float& roll, float& pitch, float& yaw) | ||
| { | ||
| static constexpr multiplier = 180 / ZSL_PI; |
There was a problem hiding this comment.
robots kinda cooking with this one
| * @brief Calculate roll, pitch, and yaw from a quaternion. | ||
| * | ||
| * @param[in] inputQuat The input quaternion. | ||
| * @param[out] roll Output roll angle in radians. |
There was a problem hiding this comment.
either this comment is wrong or the implementation is wrong? You have a multiplier of 180/pi which turns radians into degrees in the implementation but say it comes out as radians?
Description
Calculates roll, pitch and yaw based on the output of a sensor fusion quaternion
Type of change
How Has This Been Tested?
Standard math, so just check and make sure it compiles for now. Once we integration test we can fully validate.
Checklist: