-
Notifications
You must be signed in to change notification settings - Fork 0
Initial Battery Monitor Subsystem #2
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
Conversation
src/main/java/frc/robot/subsystems/battMon/BattMonHardware.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/robot/subsystems/battMon/BattMonHardware.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/robot/subsystems/battMon/BattMonHardware.java
Outdated
Show resolved
Hide resolved
src/main/java/frc/robot/subsystems/battMon/BattMonHardware.java
Outdated
Show resolved
Hide resolved
| //Refresh data and graph it | ||
| io.updateInputs(inputs); | ||
|
|
||
| //Check for dangerous outputs NOTE all of these values are temporary |
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.
You aren't actually using your state machine here...
In NORMAL
- calculate the DANGER and WARNING temperature limits based on the current drawn (tLim = temp * slope + offset), WARNING = XX below DANGER
- check if temp is greater than DANGER
- set appropriate alert
- calculate recoveryTemp based on current DANGER limit and save it off
- transition to DANGER
- check if temp is greater than WARNING
- set appropriate alert
- calculate recoveryTemp based on current WARNING limit and save it off
- transition to WARNING
In WARNING:
- calculate current DANGER threshold based on current
- check if temp is above DANGER threshold if so:
- save off recoveryTemp (xx deg below current DANGER threshold)
- set appropriate alerts
- change state to DANGER
- check if temp is below the saved recoveryTemp, if so:
- clear the appropriate alerts
- switch states back to NORMAL
In DANGER:
- check if the current temp is below the saved recoveryTemp if so:
- switch back to NORMAL
- clear the appropriate alerts
| io.updateInputs(inputs); | ||
|
|
||
| //Check for dangerous outputs NOTE all of these values are temporary | ||
| if (inputs.batt1Output > 1000){ |
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.
If we are trying to have a low battery threshold, that alert should be set based on a constant, not a magic # in code
| private Alert safeAgainAlert = new Alert("SAFE Temp low. Limits removed", AlertType.kInfo); | ||
|
|
||
| @Override | ||
| public void periodic() { |
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.
you are going to want a running average of the temperature to work off of as that signal is quite noisy
we may end up wanting averages of voltages/currents as well
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.
Could use this class in wpilib: https://github.wpilib.org/allwpilib/docs/release/java/edu/wpi/first/math/filter/LinearFilter.html#movingAverage(int)
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.
Would want to make all of the decisions about current limiting on the averaged temp
Should record the averaged temp as an output
|
|
||
| public class BattMonConstants {} | ||
| public class BattMonConstants { | ||
| //All of these aren't right and will need to be determined |
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 think you are probably going to need the following constants:
For voltage/current measurements (replacing volt where appropriate):
- kVolt1
- kDutyVolt1
- kVolt2
- kDutyVolt2
Then the following constants can be calculated in the constants file using those 4 constants (replacing batt and volt with the appropriate naming)
- kBattVoltSlope
- kBattVoltOffset
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.
Temperature is going to need 2 constants that we can get straight from the datasheet of the sensor
Temp = (dutyCycle - 0.32)/0.0047
| import edu.wpi.first.wpilibj.Alert; | ||
| import edu.wpi.first.wpilibj.Alert.AlertType; | ||
|
|
||
| public class BattMonSubsystem extends SubsystemBase { |
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.
going to need a getState() method for the robotStateSubsystem to interact with this one
|
|
||
| // Check for dangerous outputs NOTE all of these values are temporary | ||
| switch (curState) { | ||
| case NORMAL: |
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.
Normal could go to either warning or danger temps, right now only accounts for warning
| break; | ||
| } | ||
|
|
||
| case DANGER: |
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.
Danger should recover back to normal
| safeAlert.set(true); | ||
| curState = battMonState.NORMAL; | ||
| } else { | ||
| break; |
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.
as opposed to putting break in else statement, just put at end of particular switch case
| public static double kBreakerTemp1 = 0.32; | ||
| public static double kBreakerTemp2 = 0.0047; | ||
|
|
||
| // Low Battery Thresholds |
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.
Not 100% sure about what these constants are used for
| public static final double kHysteresis = 20; | ||
| public static final double kWarningOffset = 5; | ||
|
|
||
| public static final double kPDPHigh = 42; |
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.
Not 100% sure about what these constants are used for
| private Alert safeAgainAlert = new Alert("SAFE Temp low. Limits removed", AlertType.kInfo); | ||
|
|
||
| @Override | ||
| public void periodic() { |
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.
Could use this class in wpilib: https://github.wpilib.org/allwpilib/docs/release/java/edu/wpi/first/math/filter/LinearFilter.html#movingAverage(int)
| private Alert safeAgainAlert = new Alert("SAFE Temp low. Limits removed", AlertType.kInfo); | ||
|
|
||
| @Override | ||
| public void periodic() { |
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.
Would want to make all of the decisions about current limiting on the averaged temp
Should record the averaged temp as an output
Improved code morale with threat of deletion
No description provided.