bldc
bldc copied to clipboard
vedderb
Firmware version/info + Support for VESC BMS State
I realize they're two seemingly unrelated commits, but they're in the same PR because the BMS commit will cause merge conflicts if applied separately... I can resubmit separately if you prefer.
-
3 new APIs to allow packages to query HW Name, FW Name, and FW Version (as a string!)
-
Transmit BMS state via CAN bus, and send BMS state as part of COMM_BMS_GET_VALUES
- state defaults to UNKNOWN (=255)
- fault defaults to NONE (=0)
I've now also added a get_values API to get the complete bms_values struct
It is better if op_state_unknown is the first one if that is what is shown for BMSes that do no support it. The VESC Tool PR also needs to be updated for that. I do like that the BMS faults are generic and not vendor specific here - should be the same in the VESC Tool pr.
For the firmware version getter it is better to just give the numbers and not create a string. You can give the function three int pointers as arguments that then are set to the firmware version numbers.
Changes have been made, and commits updated get_version now uses 3 int pointers BMS_OP_STATE_UNKNOWN is now 0
Btw, reordering Kevin's existing enum is going to be a major PITA for him. This means that users running an older version of the tool will get wrong op-state reported. Could turn into a support nightmare for him...
Fwiw, UNKNOWN=255 would have also worked in all cases as long as we initialize state to UNKNOWN. So if you'd reconsider that it would be a huge help...
If that helps that is ok, but wouldn't the problem still be there as the faults are already there?
I had a closer look at the opcodes, and I'm not so happy with them in general. They seem to overlap a bit with the faults and many of them could be enabled at the same time. For example, the express-bms I'm working on can be charging, balancing, and load enabled at the same time. Also, what is the difference between init and idle? What is external? To me that seems very specific to some solution and not generic. Looking at the faults again, many of them also seem non-generic and I wonder what they even mean, but that is ok with fault codes as you just need to use the ones that make sense for you every time a fault occurs. The op-codes are of the nature where you have to pick one and most of the time I would not know which one to pick.
One way to deal with the opcodes would be to make them a bitfield and assign different bits to certain states. Then you can have any combination. It would also make sense to make something vendor-specific that is independent of what is present in the VESC code.
@EnnoidMe What are your thoughts on this matter? I'd be willing to work on adding support in both firmware's but think we should come to a consensus first regarding spec.
@vedderb if we can't come to a consensus on this would it at least be possible to add to bms_get_values to vesc_c_if.h so vesc-pkg c packages can access individual cell values and temps (needed for pushback)?
@vedderb if we can't come to a consensus on this would it at least be possible to add to bms_get_values to vesc_c_if.h so vesc-pkg c packages can access individual cell values and temps (needed for pushback)?
The problem with providing that to vesc_c_if is that I cannot change the bms value struct anymore after that as that would break binary compatibility. I have been updating it quite a few times already, so that would be a bit too much of a restriction. For now I suggest using the lbm-extensions to provide BMS-values to the c library if needed. There are a few ways to do that, the simplest would be to register an extension from C that lbm calls in a periodic thread and updates the bms values.
Closing it for now, we shall make another attempt at improved BMS communication in 6.05