-
Notifications
You must be signed in to change notification settings - Fork 135
Update BatteryState.msg with Battery Node Information #299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: rolling
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,14 +41,23 @@ float32 charge # Current charge in Ah (If unmeasured NaN) | |
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 | ||
uint8 power_supply_status # The charging status as reported. Values defined above | ||
uint8 power_supply_health # The battery health metric. Values defined above | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
uint32 discharge_fault # The current discharge fault of the battery, if there is one | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment about charge_fault this seems like we could cover it inside the power supply health. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, same as above |
||
|
||
float32[] cell_voltage # An array of individual cell voltages for each cell in the pack | ||
# If individual voltages unknown but number of cells known set each to NaN | ||
# If individual voltages unknown, but number of cells known, set each to NaN | ||
float32[] cell_temperature # An array of individual cell temperatures for each cell in the pack | ||
# If individual temperatures unknown but number of cells known set each to NaN | ||
# If individual temperatures unknown, but number of cells known, set each to NaN | ||
string location # The location into which the battery is inserted. (slot number or plug) | ||
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 | ||
Comment on lines
+58
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you give me examples for those fields to fill in? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 As for the firmware version, this tends to be vendor/manufacturer specific and there is no standard. NEC has additional information with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Example EDS definition:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah okay. Using my example Inventus battery above, this would populate my fields:
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One more note, it may be important to include a field for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Two things to consider here.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the semantic difference between this and the serial number?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Serial number: unique to that battery, should never change.
ID: not unique, configurable, like an IP address or a CAN ID