-
Notifications
You must be signed in to change notification settings - Fork 3
Hood raised zero #10
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
base: main
Are you sure you want to change the base?
Hood raised zero #10
Conversation
|
Unfortunately github doesn't let me comment lines that didn't change so I'm putting it here as a reminder: Add |
| if(m_hood.GetSupplyCurrent() >= ShooterConstants::zeroingcurrent){ | ||
| m_hood.Set(ControlMode::PercentOutput, 0.0); | ||
| return; | ||
| } | ||
| else if(m_hood.GetSelectedSensorPosition() > ShooterConstants::hoodMax && | ||
| m_angle >= ShooterConstants::hoodMax){ | ||
| m_hood.Set(ControlMode::PercentOutput, 0.0); | ||
| return; | ||
| } | ||
| else if(m_hood.GetSelectedSensorPosition() < ShooterConstants::hoodMin && | ||
| m_angle <= ShooterConstants::hoodMin){ | ||
| m_hood.Set(ControlMode::PercentOutput, 0.0); | ||
| return; | ||
| } | ||
| else { | ||
| m_hood.Set(ControlMode::Position, m_angle); | ||
| } |
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.
I'd suggest refactoring this into:
| if(m_hood.GetSupplyCurrent() >= ShooterConstants::zeroingcurrent){ | |
| m_hood.Set(ControlMode::PercentOutput, 0.0); | |
| return; | |
| } | |
| else if(m_hood.GetSelectedSensorPosition() > ShooterConstants::hoodMax && | |
| m_angle >= ShooterConstants::hoodMax){ | |
| m_hood.Set(ControlMode::PercentOutput, 0.0); | |
| return; | |
| } | |
| else if(m_hood.GetSelectedSensorPosition() < ShooterConstants::hoodMin && | |
| m_angle <= ShooterConstants::hoodMin){ | |
| m_hood.Set(ControlMode::PercentOutput, 0.0); | |
| return; | |
| } | |
| else { | |
| m_hood.Set(ControlMode::Position, m_angle); | |
| } | |
| const double curr_hood_angle = m_hood.GetSelectedSensorPosition(); | |
| const bool hood_overcurrent = m_hood.GetSupplyCurrent() >= ShooterConstants::zeroingcurrent; | |
| const bool hood_too_high = curr_hood_angle > ShooterConstants::hoodMax && | |
| m_angle >= ShooterConstants::hoodMax; | |
| const bool hood_too_low = curr_hood_angle < ShooterConstants::hoodMin && | |
| m_angle <= ShooterConstants::hoodMin; | |
| if (hood_overcurrent || hood_too_high || hood_too_low) { | |
| m_hood.Set(ControlMode::PercentOutput, 0.0); | |
| } else { | |
| m_hood.Set(ControlMode::Position, m_angle); | |
| } |
- make one call to
GetSelectedSensorPosition()and cache the value in a local variable so you're not repeatedly hammering the CAN bus for the same data. - remove the return statements in each branch since the cases are already mutually-exclusive. leaving them in might cause bugs later on if stuff is added after this code block (such as the smartdashboard call).
|
I'd personally avoid mixing and matching between I think it's easier to debug hood issues if the motor is always commanded as |
No description provided.