Charger#1
Conversation
| #include <cstddef> | ||
| #include <cstdint> | ||
|
|
||
| namespace soclookuptable { |
There was a problem hiding this comment.
Could you move the lookup function here? It would be easier to follow. Also, be sure to mention that the two tables are interlinked, where you're mapping from the first table to the second table.
There was a problem hiding this comment.
Also, at the top of the namespace would be a good place to define what the SOC acronym means.
| if (!module.connected || !module.cellDataValid) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Is this intentional to ignore bad cells? Might want to add a comment explaining why it's okay to ignore.
|
|
||
| for (size_t i=0; i < soclookuptable::kNumLookUpPoints - 1; i++) { | ||
| if (cellMv >= soclookuptable::kVoltageTable[i] && cellMv <= soclookuptable::kVoltageTable[i+1]) { | ||
| // linear interpolation |
There was a problem hiding this comment.
Might be clearer to say "linear interpolation to map between the two tables."
| std::array<adbms6830::BMSInterface::SiliconIdReadback, constants::kModuleCount> moduleSiliconIds{}; | ||
| }; | ||
|
|
||
| struct StateOfCharge { |
There was a problem hiding this comment.
Could you document what this structure is used for?
| soc.minCellMv = lowestCellMv; | ||
| soc.maxCellMv = highestCellMv; | ||
| // convert Mv to V | ||
| soc.totalPackVoltageMv = static_cast<uint16_t>(totalCellMv * 0.001); |
There was a problem hiding this comment.
Why are you converting to volts if it's called totalPackVoltageMv?
| mutex_t gBmsDataMutex; | ||
| volatile bool gBmsDataReady = false; | ||
| bool gCan0Ready = false; | ||
| bool chargingMode = false; |
There was a problem hiding this comment.
This should probably be volatile (remember that there's two threads running, so you need to carefully think through synchronization).
2 participants
Making this so I can leave notes on the code.