Conversation
ManushPatell
requested review from
BlakeFreer,
CameronBeneteau,
TylerStAmour,
langei and
samparent97
as code owners
* Check for changes only in certain directories * Fix static code analysis workflow * Fix build-test-projects.yml * Add test files for PR analysis * Remove test files from repo * Reduce number of pre-commit checks * Reduce number of PR analysis checks
|
I've written out the command codes that were on the 2950B datasheet. I know some codes have multiple versions where some include When writing the implementation of each command code, I strongly urge to recheck my hex conversion because there's a chance that 1/52 codes are wrong by 1 or more bits. Additionally, the codes |
* Added generate git hash script * Added platformio configs to use generate_git_hash.py as a new pre script file * Added sending of git hash in lvcontroller * Added 1hz task and git hash send to front controller * Added 1hz task and git hash send to tms * Used env variable for finding include directory of project
Takes way to long. We should not be checking static analysis in precommit since it takes several minutes. This should only be a github check
|
A problem I've faced is what happens if the BMS ignores a command or write function. We've implemented the CCNT (Counter), that tracks the number of write/command functions that have been successfully received by the BMS. Whenever, a read function is performed, we check if our CCNT count is equal to what the bms is sending back. This is all great until we actually get an offset of 1 or more between the expected and actual value. In that case it will always be false, possibly flooding our CAN bus (I'm assuming we'll be creating CAN messages for this). We can send the message and offset it back, is that the best practice here? Any inputs? @BlakeFreer @HarshPatel08 |
|
Is CCNT a concept that you created? I don't see why it's needed. Everything is communicated over SPI/isoSPI (not CAN). Unlike CAN, SPI is an extremely reliable protocol. You can assume that every command succeeds. When you start testing the actual hardware, either that assumption will be proved correct or you'll know what failures actually need detecting |
| return true; | ||
| } | ||
|
|
||
| uint16_t LvBms::commandPec15(const uint8_t* data, int len) const { |
There was a problem hiding this comment.
Nice bit wise implementation. this is how CRC circuits work in hardware, but software implementations usually use a lookup table method since it's far more efficient for a CPU
https://en.wikipedia.org/wiki/Computation_of_cyclic_redundancy_checks
This website can generate the CRC table for lots of polynomials. I'm assuming (but not 100% sure) that your particular CRC is there
https://crccalc.com/?crc=123456789&method=&datatype=ascii&outtype=hex
If it doesn't include your CRC, keep your current implementation
| } | ||
|
|
||
| void LvBms::freezeResultRegisters() { | ||
| std::array<uint8_t, 2> cmd = |
There was a problem hiding this comment.
It looks like you always call split_message and pass the uint8 array into sendControlCmd. I think a better division of the abstraction would be just to pass the command enum into sendControlCmd and do the splitting internally. This would make most of your command functions super simple, just calling sendControlCmd(enum_value)
Since this makes each command functions trivial, you may decide to remove all of the class methods which are just command names and let the user call lvbms.sendCommand(enum_value).
There was a problem hiding this comment.
That's a really good suggestion, just implemented that!
Just to clarify above. I know we're not using CAN to send/receive commands/data, but we should be using it to create a message that tells us if the PEC or CCNT are not equal. An example of this is when the program CCNT is not equal to the BMS CCNT. This can happen when a command is ignored by the BMS, forcing it to be off by 1 unless we change the program CCNT ourselves.
I want to acknowledge this error by sending a message via CAN, that the CCNT's are off, and then reset them to match so we don't get this error constantly.
It isn't something I created. It's used in the data-sheet, where write/command calls should increment the count, read should compare the counts and reset/startup calls should set the value back to 0. I think it's pretty useful to find if something is consistently dismissed by the BMS but beyond that point, it's hard to tell which function call is being dismissed if it's an irregular event |
PRs can now be approved by anyone in the organization.
6 participants
Just wanted to make the initial set up with Harsh on call. I'm going to grind through this after wednesday but he has time right now so I wanted to show him the functions on the datasheets and a template to work on.