[PI - 260] Add spline interpolation #64
Conversation
borednuna
force-pushed
the
feature/implement-spline-interpolation
branch
2 times, most recently
from
7c99d7d to
e942fd2
Compare
| @@ -0,0 +1,54 @@ | |||
| // Copyright (c) 2023 ICHIRO ITS | |||
| struct PointTime | ||
| { | ||
| float point; | ||
| float time; | ||
| }; | ||
|
|
||
| SmoothSpline(); | ||
| explicit SmoothSpline(const std::vector<PointTime>& point_times); | ||
| ~SmoothSpline(); | ||
|
|
||
| void add_point(const Polynom& polynom); | ||
| void add_point(double value, double t); | ||
| Polynom polynomial_fit( | ||
| double pos1, double pos2, double vel1, double vel2, double acc1, double acc2, double t); | ||
|
|
||
| private: | ||
| std::vector<float> points; |
There was a problem hiding this comment.
I suggest not mixing between float and double. Mixing them may add more type conversions, and the double precision in double will be useless if it's being operated with a float.
| std::vector<Polynom>& Spline::get_splines() | ||
| { | ||
| return splines; | ||
| } |
There was a problem hiding this comment.
If you're exposing the splines as a reference, why not simply remove the get_splines function and expose the splines as a public property instead?
| void Spline::add_spline(const Polynom& polynom) | ||
| { | ||
| splines.push_back(polynom); | ||
| } |
There was a problem hiding this comment.
If splines is exposed as a public property, we can simply call splines.push_back directly. There's no need to declare another add_spline function.
| double Polynom::get_value(double value) | ||
| { | ||
| return get_value(value, OrderType::POSITION); | ||
| } |
There was a problem hiding this comment.
Instead of this, we can simply use the other function but declared it like this:
double Polynom::get_value(double value, int derivative_order = OrderType::POSITION);| Polynom::Polynom() | ||
| { | ||
| } |
There was a problem hiding this comment.
Do we really need to have this constructor?
| SmoothSpline::SmoothSpline() {} | ||
|
|
||
| SmoothSpline::~SmoothSpline() {} |
There was a problem hiding this comment.
Do we really need to have these constructor and destructor?
| Spline::Spline() | ||
| { | ||
| } | ||
|
|
||
| Spline::~Spline() | ||
| { | ||
| } |
There was a problem hiding this comment.
Do we really need to have these constructor and destructor?
| std::vector<double> coefficients; | ||
| coefficients.resize(6); |
There was a problem hiding this comment.
FYI you can do this:
| std::vector<double> coefficients; | |
| coefficients.resize(6); | |
| std::vector<double> coefficients(6); |
| Polynom polynom(coefficients); | ||
| return polynom; |
There was a problem hiding this comment.
FYI you can do this:
| Polynom polynom(coefficients); | |
| return polynom; | |
| return Polynom(coefficients); |
Jira Link: https://ichiro-its.atlassian.net/browse/PI-260
Description
Added spline and polynomial classes that can be used for calculations
Type of Change
How Has This Been Tested?
Checklist:
feature/JIRA-ID-SHORT-DESCRIPTIONif has a JIRA ticketenhancement/SHORT-DESCRIPTIONif has/has no JIRA ticket and contain enhancementhotfix/SHORT-DESCRIPTIONif the change doesn't need to be tested (urgent)