Update BatteryState.msg with Battery Node Information #299
Conversation
Signed-off-by: Christian McCrave <[email protected]>
Signed-off-by: Christian McCrave <[email protected]>
| string manufacturer # The name of the manufacturer of the battery | ||
| string serial_number # The best approximation of the battery serial number | ||
|
|
||
| string hw_version # The hardware version of the battery | ||
| string sw_version # The software version of the battery | ||
| string fw_version # The firmware version of the battery |
There was a problem hiding this comment.
could you give me examples for those fields to fill in?
There was a problem hiding this comment.
Absolutely! I'm not sure if an example using CAN is helpful, but all batteries I've used in robotics so far have talked over CAN, so I will use that as an example. Two of our most commonly used battery types are manufactured by NEC and Inventus, so they will be the example batteries.
The CANopen standard has two objects detailing the hw and sw versions, devices should always populate the standard dictionary indexes, and both our batteries do. The object at 0x1009 is likely to be populated as Manufacturer Hardware Version and 0x100A should be Manufacturer Software Version.
As for the firmware version, this tends to be vendor/manufacturer specific and there is no standard. NEC has additional information with ApplicationFirmwareRevision reporting at 0x2102. Inventus reports this at 0xD000sub0x20 but does not report the objects details in their EDS.
There was a problem hiding this comment.
Example EDS definition:
[1009]
ParameterName=Manufacturer Hardware Version
ObjectType=0x7
DataType=0x0009
AccessType=const
DefaultValue=0203
PDOMapping=0
[100A]
ParameterName=Manufacturer Software Version
ObjectType=0x7
DataType=0x0009
AccessType=const
DefaultValue=02301002
PDOMapping=0
There was a problem hiding this comment.
I assume @fujitatomoya was wondering what sort of text would go into those fields. Even if there is no standard for it, some examples should be available I guess.
There was a problem hiding this comment.
Ah okay. Using my example Inventus battery above, this would populate my fields:
hw_version: "00000000"
sw_version: "00000008"
fw_version: raw output is "31304203" which requires an ASCII conversion to "B10"
Useful for us to know the fw_version specifically because we have multiple firmware versions that can come from the vendor and certain desirable behaviors are only on B10.
There was a problem hiding this comment.
One more note, it may be important to include a field for charge_fault and discharge_fault. Battery vendors often have fault codes that could be important to push upstream as well, as it can actually disable charging or discharging in some circumstances.
There was a problem hiding this comment.
These fields and the manufacturer one above seem to be much better candidates for using Diagnostics reports instead of this message. This message is designed to be an abstraction of a battery such that you can use it generically without knowing specifically what's under the hood. The diagnostics provide an opportunity to enable visualization of debugging if you need to update a firmware version or know what manufacturer's hardware is installed.
As an example a node using the generic battery interface is not expected to be doing logic based on the firmware version so we don't want to include it in the generic interface.
There was a problem hiding this comment.
Two things to consider here.
- I think in general I agree with you and what I've added here may be too detailed for a high level message. A consideration should be made for breaking these out into a detailed message instead, to reserve this for a lower level use case. I would then believe that we could make a BatteryInfo message of sorts, that contains:
uint32 id # Identifier for the battery
string manufacturer # The name of the manufacturer of the battery
string hw_version # The hardware version of the battery
string sw_version # The software version of the battery
string fw_version # The firmware version of the battery
- Firmware version does play a huge role in how batteries can function.
For example, here at Clearpath there is a certain FW Version for NEC batteries that will actually prevent them from transmitting CAN data, it's a known bad FW Version and must be updated. This would be a diagnostic based piece of information. However we have known differences in how certain battery charging behaviors work depending on the firmware loaded into them (bypassing full modules to charge series/parallel modules in a larger pack). This would be more on the controls side of information.
Signed-off-by: Christian McCrave <[email protected]>
| float32 capacity # Capacity in Ah (last full capacity) (If unmeasured NaN) | ||
| float32 design_capacity # Capacity in Ah (design capacity) (If unmeasured NaN) | ||
| float32 percentage # Charge percentage on 0 to 1 range (If unmeasured NaN) | ||
| uint32 id # Identifier for the battery |
There was a problem hiding this comment.
What is the semantic difference between this and the serial number?
There was a problem hiding this comment.
Serial number: unique to that battery, should never change.
ID: not unique, configurable, like an IP address or a CAN ID
| uint8 power_supply_technology # The battery chemistry. Values defined above | ||
| bool present # True if the battery is present | ||
|
|
||
| uint32 charge_fault # The current charge fault of the battery, if there is one |
There was a problem hiding this comment.
These seem to be in a potential alternative value to the power_supply_health instead of a new field.
Related for these new fields the proposed ones don't have clear interpretation and wouldn't be acceptable as defined.
There was a problem hiding this comment.
I'd be okay with that, but we should break out charge_fault and discharge_fault in a message somewhere for more specific information. All batteries should have fault codes, which is important diagnostic information.
| bool present # True if the battery is present | ||
|
|
||
| uint32 charge_fault # The current charge fault of the battery, if there is one | ||
| uint32 discharge_fault # The current discharge fault of the battery, if there is one |
There was a problem hiding this comment.
Same comment about charge_fault this seems like we could cover it inside the power supply health.
| string manufacturer # The name of the manufacturer of the battery | ||
| string serial_number # The best approximation of the battery serial number | ||
|
|
||
| string hw_version # The hardware version of the battery | ||
| string sw_version # The software version of the battery | ||
| string fw_version # The firmware version of the battery |
There was a problem hiding this comment.
These fields and the manufacturer one above seem to be much better candidates for using Diagnostics reports instead of this message. This message is designed to be an abstraction of a battery such that you can use it generically without knowing specifically what's under the hood. The diagnostics provide an opportunity to enable visualization of debugging if you need to update a firmware version or know what manufacturer's hardware is installed.
As an example a node using the generic battery interface is not expected to be doing logic based on the firmware version so we don't want to include it in the generic interface.
|
@tfoote can I ask where you think the line is in the abstraction? We have a lot functionality around support for battery drivers in field. Custom SoC estimation, fault handling, protective actions, etc. We do this from a higher level battery management system. Hardware/firmware versions become relevant to us in a few contexts:
Can this be in diagnostics? Sure. We do that. But it would be nice to have it in a meaningfully defined interface. Though perhaps not every field is relevant. Would something like |
|
If you'd like to define a more detailed battery info message that could be a good approach. For the core message as it stands, that's the abstraction for usage. It is what things like a battery monitor widget will show. It can answer questions like What is it's state of charge? Will I be able to get power from it for how long? All battery widgets will subscribe and implement it. This is content that's expected to change significantly over time. Then a BatteryInfo parallel one with all the manufacturer specifics that are useful for configuration debugging and are expected to be generally constant within a run except occasionally changing makes sense to standardize more than general conventions in diagnostics. But this can be a much lower rate message and also less widely consumed. |
5 participants
Description
The BatteryState.msg struct is missing a few possible useful pieces of informaton for downstream systems.
Is this user-facing behavior change?
It shouldn't affect anything user-facing, but the additions can added to user-facing changes, such as diagnostics.
Did you use Generative AI?
No.
Additional Information
This shouldn't be a breaking change, just adding additional members to a message. This felt like the least invasive while still allowing our system to maintain a ROS message rather than decouple and generate our own.