BatteryStatus.msg - docs update

[hamishwillee]

This fixes up the BatteryStatus UORB topic to match the evolving doc standard (for similar, see #24662).

Solution

• Single space for comments etc
• Capitalize comments
• Enum values after the field that uses them
• Uses the following markup
  • [@enum <value>] to mark allowed values
  • [<value>] to mark units
  • [@range <minValue>,<maxValue>] to mark ranges - inclusive
  • [@invalid <value>] to mark the invalid/not-supplied value

Changelog Entry

For release notes:

Fix battery_status message definition

Other comments

@MaEtUgR See inline notes

[MaEtUgR]

Thanks for having a stab. I think you opened the can of worms. This message needs to be revised, it's on my list... battery info, verify the fields make sense and have actual high rate use.

It's why I started with a less debated message in #24662 with of course also much less value.

[hamishwillee]

I plan to add a battery info message for such properties that don't change as quickly and don't need to be published at high rate and also double check what the fields are good for instead of just having one field per driver that provides some random additional information about the battery not overlapping with anything else.

@MaEtUgR Excellent.

The MAVLink equivalent that needs to be published is https://mavlink.io/en/messages/common.html#BATTERY_INFO

I also want to publish this https://mavlink.io/en/messages/development.html#BATTERY_STATUS_V2

[hamishwillee]

@MaEtUgR I think this is good for another look. I updated the top of file comment and merged my suggestions. There are open questions, but they are around specific docs, not "the doc format". This changes no information, so we're net better off.

I guess we should create issues against things like #24789 (comment)

[hamishwillee]

@MaEtUgR This is good to go - I plan to self-merge without approval tomorrow. But I'd prefer other eyes.

[MaEtUgR]

I had started earlier, but this message needs extensive cleanup and I got sidetracked. These are my immediate comments. This is a good step forward and we’ll need to keep iterating with the battery beyond the documentation changes 👍

[MaEtUgR]

Thank you for addressing my comments.
Let's not hold this back because of details even though it will change again anyways. As I wrote it's a very good documentation improvement step 🙏

[dakejahl]

Jumping in here without having read any of the previous conversation. We spent a lot of time on the DroneCAN battery messages and I would consider them to be the "definitive reference". I would at least match the fields/names to the DroneCAN spec

PR for reference, the messages haven't changed
dronecan/DSDL#7

[hamishwillee]

Thanks @MaEtUgR

[MaEtUgR]

@dakejahl Generally that's my goal. I'd just make sure we only carry fields in uORB that currently provide any value. I started with the serial number in this pr: #24960 It matches the definition of the DSDL you linked. I'll follow up with more cleanup and adaption of the fields such that we can review more easily and stay on top.

[MaEtUgR]

I'm sorry, I didn't see that the messages check in CI failed on the last commit:

I quickly talked to Beat and it's most likely because the order of the fields warning and faults got swapped in this pr. I reviewed in detail and was aware that the order of the two fields changes but presumed it wouldn't make a difference because it does not within PX4 and also not for the constants in ROS. I also missed the ROS messages CI check because some other ROS check had failed on every pull request, all bad excuses 🙈

My suggestion is that I finish #24960 which will fix the problem because it includes bumping the version and a translation that deals with the different field order correctly 🏃