Conversation
…or6d struct to vortex utils
…t each solver is more independent and will do perform own calculations
… factory class and descriptions for them
…s before. Will add tests whenever i find time again
… now), will clean up the code and fix warm start for next time
…ench in order to keep feasability and warm starting the solver after initializing
…s node tests fail :(
f792d80 to
a531809
Compare
motion/thrust_allocator_auv/include/thrust_allocator_auv/allocator_config.hpp
Outdated
Show resolved
Hide resolved
…ubious and not testing the expected functionality. These are mostly tests for moments as sometimes the desired wrench and actual wrench will not physically be able to match given the constraints.
| /** | ||
| * @brief The Allocator class structure that the solvers will inherit and | ||
| * override. | ||
| */ | ||
| class Allocator { |
There was a problem hiding this comment.
Consider naming it IAllocator or similar to make it clear it is an interface class
motion/thrust_allocator_auv/include/thrust_allocator_auv/allocator.hpp
Outdated
Show resolved
Hide resolved
motion/thrust_allocator_auv/include/thrust_allocator_auv/allocator.hpp
Outdated
Show resolved
Hide resolved
motion/thrust_allocator_auv/include/thrust_allocator_auv/allocator_config.hpp
Outdated
Show resolved
Hide resolved
| class Factory { | ||
| public: | ||
| /** | ||
| * @brief A function that makes an allocator based on a string | ||
| * defining the desired type. | ||
| * | ||
| * @param allocator_type String of the desired allocator type. | ||
| * @return unique pointer to the created allocator. | ||
| */ | ||
| static std::unique_ptr<Allocator> make_allocator( | ||
| const std::string& allocator_type, | ||
| const AllocatorConfig& config); | ||
| }; |
There was a problem hiding this comment.
Why is this a class? Why not a free function inside a namespace?
There was a problem hiding this comment.
This is a good question, i was inspired by the concept of factories and decided to use it here. Its something along the lines that thrust_allocator does not create itself. The factory is sent an order for a QP allocator and makes a qp allocator not a pseudoinverse allocator type of thing
motion/thrust_allocator_auv/include/thrust_allocator_auv/pseudoinverse_allocator.hpp
Outdated
Show resolved
Hide resolved
| Eigen::MatrixXd calculate_pseudoinverse(const Eigen::MatrixXd &T, | ||
| const Eigen::MatrixXd &W); |
There was a problem hiding this comment.
Generally, if a class method doesnt have any dependencies to the class it would be better off as a free function. guideline
motion/thrust_allocator_auv/include/thrust_allocator_auv/qp_allocator.hpp
Outdated
Show resolved
Hide resolved
| void QPAllocator::formulate_as_qp(const AllocatorConfig &cfg) { | ||
| const int r = cfg.input_weight_matrix.rows(); // number of thrusters = 8 | ||
| const int n = cfg.slack_weight_matrix.rows(); // degrees of freedom = 6 |
There was a problem hiding this comment.
Why are the comments stating the values? Surely either a) the comments are wrong and they can be other values, or b) the comments are right and they are better off as constexpr values? 😄 In either case the comments are goners
There was a problem hiding this comment.
this was mostly to remind myself what the actual value was :D
| // create and set the square term matrix. | ||
| Eigen::MatrixXd phi = Eigen::MatrixXd::Zero(r + n, r + n); | ||
| phi.block(0, 0, r, r) = cfg.input_weight_matrix; | ||
| phi.block(r, r, n, n) = cfg.slack_weight_matrix; |
There was a problem hiding this comment.
Just be aware that potential readers may not have access to the book, so names like r and n, and greek letters may not be very descriptive 😄
There was a problem hiding this comment.
r and n are commented above as number of thrusters and degrees of freedom. Fossens book is at the office
| if (!casadi_solver_initialized_) { | ||
| throw std::runtime_error("CasADi QP solver not initialized " | ||
| "(formulate_as_qp_CasADi not called)."); | ||
| } |
There was a problem hiding this comment.
perhaps using std::optional would be safer and easier to handle than throwing?
There was a problem hiding this comment.
I will look up the difference and look into this
motion/thrust_allocator_auv/include/thrust_allocator_auv/qp_allocator.hpp
Outdated
Show resolved
Hide resolved
the qp allocator and refactored some code because the node dies everytime i try to launch it and use it in the simulator
Refactored the thrust allocator module to be easier to build upon. Added more documentation in the readme file and some basic unit testing of the thrust allocator functionality. Added a new solver using CasADi and Fossen's formulation of the QP thrust allocation problem. Whenever i find time again i will add more comprehensible unit testing.
Lastly it would be nice to have simulator testing and perform some analysis on computation time vs accuracy tradeoff for both pseudoinverse and the QP solver but that is outside of the scope of this PR.