-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fix type error in state processor #48
Conversation
Codecov Report
@@ Coverage Diff @@
## master #48 +/- ##
=======================================
Coverage 29.99% 29.99%
=======================================
Files 72 72
Lines 3797 3797
=======================================
Hits 1139 1139
Misses 2658 2658
Continue to review full report at Codecov.
|
src/propulsion/RPM_regulator.cpp
Outdated
@@ -10,8 +10,8 @@ RPM_Regulator::RPM_Regulator(Logger &log) : log_(log), current_index(0), failure | |||
{ | |||
} | |||
|
|||
int32_t RPM_Regulator::calculateRPM(int32_t act_velocity, int32_t act_rpm, int32_t act_current, | |||
int32_t act_temp) | |||
float RPM_Regulator::calculateRPM(float act_velocity, int32_t act_rpm, int32_t act_current, |
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.
This is not quite where the problem is. We use std::round
to obtain an integer, so the return type of int32_t
makes sense! However, we are passing in an int32_t
which is problematic because it should be a floating-point number (as you have correctly identified). However, all our floating-point numbers are of type data::nav_t
(which is just an alias for double
) so you should replace float
with this. Further, I'd say you should fix this issue in #44.
src/propulsion/state_processor.cpp
Outdated
@@ -124,7 +124,7 @@ void StateProcessor::accelerate() | |||
int32_t act_current = calcMaxCurrent(); | |||
int32_t act_temp = calcMaxTemp(controllers); | |||
|
|||
int32_t rpm = regulator.calculateRPM(velocity, act_rpm, act_current, act_temp); | |||
float rpm = regulator.calculateRPM(velocity, act_rpm, act_current, act_temp); |
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.
The issue here is not the return type. RPM is a quantity that is being measured in integer values. However, the member field velocity
(which should be called velocity_
but actually has no right to be a member field in the first place and should instead be replaced with a local variable) is of type float
. The velocity we get from Navigation is of type data::nav_t
so we should stay consistent with that.
Description
Fixes type error in state_processor.cpp from #35
Changes