Mavdumplog: Handle TLog files saved with .log extension #900
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
peterbarker
reviewed
Jan 9, 2024
peterbarker
reviewed
Jan 10, 2024
tools/mavlogdump.py
Outdated
| mlog_type = str(type(mlog)) | ||
| isbin = "DFReader_binary" in mlog_type | ||
| islog = "DFReader_text" in mlog_type # NOTE: "islog" does not mean a tlog | ||
| istlog = "mavmmaplog" in mlog_type |
There was a problem hiding this comment.
This should only ever be a dataflash log - if you look at the class you can see it's a DFLOG parser.
So something very weird is going on here :-)
There was a problem hiding this comment.
mavlogdump handles either DF binary, DF text or Mavlink/Tlog format files, and the class of the variable returned by mlog = mavutil.mavlink_connection(...) will reflect this.
I added a print on these flags during testing with all 3 file types (including the TLog named both as .log and .tlog), and the right flag gets set.
shancock884
force-pushed
the
iss710-is-text-log
branch
2 times, most recently
from
413415e
to
ecbe7c1
Compare
DFReader: is_text_log should return false if binary Mavlogdump: use class type not file ext to determine input format, to allow .log to be either text or tlog Update description text in mavlogdump file header
shancock884
force-pushed
the
iss710-is-text-log
branch
from
ecbe7c1
to
471c60b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If a binary file is passed to the DFReader_is_text_log function, there is a good chance that it will throw an exception, as it attempts to interpret the binary data as a UTF8 string. It should simply return false to say "No, its not a DF Text log". This then allows the TLog reader to handle the file instead.
I was bitten by this when I did a file-naming mistake of my own, but then spotted that issue #710 mentioned the same thing, so thought I would put the proposed fix forward.
The main part of the fix is to simply catch the UnicodeDecodeError exception in DFReader_is_text_log, and return false, as proposed by @booo in the issue description.
A secondary part is that mavlogdump makes some assumptions about the file type based on the file extension, which may not hold true, if (for example) .log extension is used for a Mavlink/Tlog file.
One example of this is that 'dump --verbose' previously only did a verbose dump with a .tlog file extension - even if the file really IS a tlog with a different extension.
So, I am proposing to use the class type returned by mavutil.mavlink_connection instead to determine the type of the file being dumped.
I also thought that the summary text you get when typing 'mavlink.py -h' was not very clear or complete, so have updated it.
I tested this by downloading one of the tlogs from Audupilot autotest, renaming as .log, and running it through "mavlogdump" and "mavlogdump --verbose". Both work without crashing and generating the correct output.
fix #710