-
Notifications
You must be signed in to change notification settings - Fork 16.8k
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
Logger Metadata: Provide format and unit/multiplier info for log messages #25861
Conversation
cd1878a
to
429ec81
Compare
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.
Very cool!
The commit message needs to start with the subsystem so:
"autotest: "
@peterbarker is the guy who will need to review this properly! |
Definitions of each character are extracted from LogStructure.h Data is extracted by parsing the logging definition struct Also parse WriteMessage() calls for messages not defined in struct Add support to separate log descriptions for messages with same field list Compute derived unit from combination of format, unit and multiplier For XML output the format and derived unit into new attributes Add enumerations to the XML output (bitmasks were already done) For MD,RST,HTML, output either derived unit, 'char[n]', 'bitmask' or 'enum' Fix support for Blimp by adding it to the parse_enum.py lookup table
429ec81
to
1c4d420
Compare
Thanks. Commit message updated as proposed. |
Were is some extra info, would be nice if we could combine it in:
|
@amilcarlucas - that seems like it could be useful information to include. I suppose the way to do it is to include something like this in the message definition comments, alongside the other info: // @LogRate SCHED_LOOP_RATE if ATTITUDE_FAST is set else 10Hz
// @LogBitMaskField NTUN and PID Personally I would prefer this to be raised in a separate proposal/issue, to avoid adding extra things into this PR, and to give others an opportunity to voice any strong opinions on the field naming and page rendering before implementation. |
Yes, I agree, let's get this one in first |
Where do the generated files get stored? Are they available on firmware.ardupilot.org ? They should. |
They end up here: https://autotest.ardupilot.org/LogMessages/ thanks to: Tools/scripts/build_log_message_documentation.sh |
It would be better if they would also end up here: That way even stable (older) releases would have consistent documentation. The same applies to the |
Can you add html anchors to each message so that they can be referenced directly from an external link? |
It looks like anchors may already be created, e.g.: https://ardupilot.org/copter/docs/logmessages.html#gpa |
@Hwurzburg what do you think about versioning this, like I posted three posts up? |
@amilcarlucas I dont think that has a lot of value for normal users....since they dont mess with logs much..unlike parameters which does have a lot of value for versioning...and devs/sophisticated users dont really need it, they can git blame if they really need to find when a log message was added/changed.. |
If it gets automatically generated by the release script and stored at https://firmware.ardupilot.org/Blimp/stable-4.4.4/ together with a apm.pdef.xml file for that version it will help some developers. And does not need any wiki change. |
when its implemented I can add a note in the wiki about how to find the old versions |
@rmackay9 are you OK with this? Should I implement it? |
The aim of this PR is to enable information about the units of logger message fields to be included on the web pages.
(This was one of the things that I would have found useful as a newbie!)
The format, unit and multiplier are combined to derive the actual unit of the value as displayed by the tools:
For the XML output:
For the .md, .rst and .html output:
** some messages on the web page already require horizontal scrolling, and I didn't want to make this worse
** I thought that strings/bitmasks/enums were useful to know about, on such fields without units
** I felt that further information (such as the raw data type, base unit and multiplier) could be confusing.
Additionally, I added support for separating log descriptions for messages with same field list (such as the PIDx messages), so that in the future they can each have a different description without having to repeat the field lists. e.g.:
I think I've understood correctly how both MAVExplorer and UAVLogViewer work in how they decode field values, such that the units presented are consistent with the values that these tools provide to users. My understanding is as follows:
I've tested this by running parse.py for each vehicle type (['Rover', 'Sub', 'Copter', 'Plane', 'Tracker', 'Blimp']) and checking the outputs look sensible, including looking at a selection of fields in some of my own recordings and comparing against the units shown by UAVLogViewer when selecting plot parameters.
In testing I found that the "--vehicle Blimp" option already generated an error before I started, so I have fixed this by adding Blimp to the lookup table in parse_enum.py.
I had some observations from this:
This took me a bit of digging around, so feel to correct any misunderstandings, or let me know if further explanation is needed!