Skip to content
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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Fix type error in state processor #48

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8bc067e
Changed calculateRPM to return float
TomLonergan03 Oct 18, 2021
32b80bb
Changed calculateRPM to take nav_t for velocity
TomLonergan03 Oct 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/propulsion/RPM_regulator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Member

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.

int32_t act_temp)
{
int32_t opt_rpm = calculateOptimalRPM(act_velocity);
if (act_temp <= MAX_TEMP) {
Expand Down
5 changes: 2 additions & 3 deletions src/propulsion/RPM_regulator.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,9 @@ class RPM_Regulator {
* @param act_rpm - average rpm of all the motors
* @param act_current - max current (mA) drawn out of all the motors
* @param act_temp - max temperature out of all the motors
* @return int32_t - the optimal rpm which the motors should be set to.
* @return float - the optimal rpm which the motors should be set to.
*/
int32_t calculateRPM(int32_t act_velocity, int32_t act_rpm, int32_t act_current,
int32_t act_temp);
float calculateRPM(float act_velocity, int32_t act_rpm, int32_t act_current, int32_t act_temp);

/**
* @brief Get the Failure boolean
Expand Down
2 changes: 1 addition & 1 deletion src/propulsion/state_processor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

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.


log_.INFO("MOTOR", "Sending %d rpm as target", rpm);

Expand Down