Refactor/type rename #647
Merged
Refactor/type rename #647
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #647 +/- ##
==========================================
- Coverage 35.63% 35.58% -0.06%
==========================================
Files 38 38
Lines 2256 2254 -2
Branches 686 686
==========================================
- Hits 804 802 -2
Misses 1273 1273
Partials 179 179
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
jorgenfj
requested review from
Andeshog and
kluge7
and removed request for
Andeshog
Andeshog
reviewed
Dec 20, 2025
Contributor
Andeshog
left a comment
Andeshog
left a comment
There was a problem hiding this comment.
Feel like it would be nice to also change the variable names here to better reflect the type
Contributor
Author
|
Aren't most of the functions taken directly from algorithms in the Fossen book. Won't it be more clear in this case to name them eta and nu when comparing them to the equations in the book? |
Contributor
|
Not everyone has access to the book while reading the code, and as you said, not everyone knows what |
Contributor
Author
|
Should the function arg names for the dp specific functions also be updated? |
Contributor
|
yeah the same applies there |
Andeshog
reviewed
Dec 21, 2025
Comment on lines
32
to
34
| Eigen::Vector6d calculate_tau(const vortex::utils::types::PoseEuler& eta, | ||
| const vortex::utils::types::PoseEuler& eta_d, | ||
| const vortex::utils::types::Twist& nu); |
Andeshog
reviewed
Dec 21, 2025
|
|
||
| if (std::abs(J.determinant()) < tolerance) { | ||
| spdlog::error("J(eta) is singular"); | ||
| spdlog::error("J(pose) is singular"); |
Contributor
There was a problem hiding this comment.
Just say "J is singular" instead probably
anbit-adhi
pushed a commit
that referenced
this pull request
Jan 6, 2026
* waypoint mode handling * update tests with new message * initial package setup * ros interface function declaration * node setup * working prototype * reentrant cb, multithreaded * single threaded impl * conv threshold action goal * default thresholdref conv value * removed switching logic * removed timer execution * sim test utils * waypoint_manager_test setup * no rendering test arg * waypoint tests, timeout error * test refactor * format * rename utils package * test suite and description * first waypoint test * removed unused function * renamed service field to priority. Added simple tests * waypoint manager readme * uniform attitude naming convention * fix pr requests * update tests with new service fields * four corner test * update util func name * update with new action def * removed failing build type * test dependencies * ignore failing yasmin package * remove __init__ files * quat_to_euler in make_pose helper * added __init__ file * removed sim deps for test packages * added action shutdown handling * removed waypoint manager set setup * added waypoint manager node tests * waypoint manager 4 corner sim test * added missing launch testing test dependency * add sleep for topic discovery * fix action member field name * updated to new utils type names * renamed variables to match types * update function arg to reflect vortex type * update variable name in tests * renamed function arg names
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Renamed vortex util types to reflect the changes in: vortexntnu/vortex-utils#22
This should probably be merged after the waypoint manager pull request