Track Day Let's Pray#4
Conversation
tmb5932
left a comment
tmb5932
left a comment
There was a problem hiding this comment.
Just some questions, mainly about leftover todo comments
|
|
||
| void TrackMCuC::mcDischargingState() { | ||
| if (stateChanged) { | ||
| // TODO: Tell MC to discharge |
There was a problem hiding this comment.
i feel like this might be important to have implemented, but ill take your word for it otherwise...
There was a problem hiding this comment.
This would improve safety, but I just checked the datasheet and realized we can't do it with our current configuration, so we'll have to wait until the semester for this
There was a problem hiding this comment.
cmon man, you know im the one looking at VCU this semester. Stop giving me more things to do...
There was a problem hiding this comment.
To be clear, no one should ever use this code again after Sunday. You should delete this branch after you make something useful
| * @param priv[in] The MCuC instance that contains the queue the message is to be added to. Must be an vcu::MCuC* | ||
| */ | ||
| void powertrainCANInterrupt(io::CANMessage& message, void* priv) { | ||
| void accessoryCANInterrupt(io::CANMessage& message, void* priv) { | ||
| auto* mcuc = (vcu::MCuC*) priv; | ||
| if (mcuc != nullptr) { |
There was a problem hiding this comment.
Reasoning to change this to accessory can interrupt? Also the docs comment on line 145 still says "powertrain CAN line"
There was a problem hiding this comment.
I did not mean to edit this file. That's my bad
There was a problem hiding this comment.
ok, well the same changes are made in targets/REV3-Hardmon/main.cpp. I assume those are also accidental
There was a problem hiding this comment.
As this is merging into a branch, and seemingly just for this Trackday, I assume this wont end up in main? so doesnt really matter if these are reverted, as they arent being used?
mjh9585
left a comment
mjh9585
left a comment
There was a problem hiding this comment.
Looks good, just one change for the future.
| case 0x2D0A: | ||
| highestCellTemp = payload[6]; | ||
| lowestCellVoltage = (((uint16_t) payload[0]) << 8) + payload[1]; | ||
| break; | ||
| case 0x2C0A: | ||
| bmsMasterTemp = (((uint16_t) payload[0]) << 8) + payload[1]; | ||
| break; |
There was a problem hiding this comment.
Can you explain a little more where these values come from? Defining the values as constants would help make this more readable.
aclowmclaughlin
left a comment
aclowmclaughlin
left a comment
There was a problem hiding this comment.
Looks good. Sorry took so long to review (I know the race is probably already happening). Stuff came up but I knew that Heller and Travis had it covered.
4 participants
VCU implementation for the 2025 track day