add Field to hold raw binary message #1
Conversation
|
@denniszollo Thanks for the update on this PR. Congratulations on PR#1 :-) IF this feature is added, it must be optional. As it stands various use cases are going to suffer because of this patch. In particular, MAVExplorer uses mavmemlog to suck all messages into memory for fast processing. It will (almost certainly) never make use of the binary field, but we already have significant problems with memory usage in that tool, and this is going to make it worse. My question still stands: what is the use-case for such a field? |
|
In the current implementation it is optional. As a keyword argument it should add very minimal memory footprint to the DFMessage object (for now it just stores a null pointer if it isn't added). As for the specific usecase: In the SBP driver of Ardupilot we use two mavlink messages to wrap up our own binary protocol. Because of something about the construct library that I didn't quite understand, it was impossible to reconstruct the raw binary stream on the other side of mavlink through the python library as we did in our open source piksi_tools script. As for the general user story: Please let me know if this makes sense. |
|
In the current implementation adding the binary message to the individual messages is optional - but every call to the constructor is passing in the binary message! We would need an option to the DFReader init method to indicate whether the binary field should be populated. However, now I'm further confused as to intent, sorry! DFReader.py looks at dataflash logs, not "tlogs" containing mavlink. The "two mavlink messages" reference you provide above creates DataFlash log messages, not mavlink messages. AFAICS mavlink is not involved here; the SBP library is parsing data read directly from the GPS' serial port. It then logs the parsed message to datalfash using a pair of messages. No mavlink involved. From a conceptual point of view I would actually prefer to be able to reconstruct the original message programatically from the parsed message. This ensures that we are parsing the data correctly and providing it in the correct form. It would also allow you to programatically modify the log files on the way through - to anonymise them, for example. This functionality could be tested with a simple regression suite. I would be happy to work with you on the problems you found in reconstructing an identical message from a parsed one. |
|
Peter, thanks for your generous offer to help me figure out how to use the library based upon the design intent and construct the identical message from the parsed one. I think the problem is that there isn't a concept of pure binary fields (or at least the messages defined in our driver don't use them for the SRB1 and SBR2). Thus, when a "null" character is encountered it is treated as a string terminator and the rest of the data is thrown out. I have an example below with my attempt at an explanation. Sorry that it is so specific to what I was trying to do, but I think it is clear. I've put up a gist of the code with some helpful debugging here: https://gist.github.com/denniszollo/23810caba4da9193c3ff2af8a632ffea Here is a zipped test binary dataflash log: Try and run the script with the first argument as the path to a file. mavlink_decode.py test.bin You will see that the parsed version thinks the length of the first SRB1 d1 field is 28 bytes, however it is supposed to be 32 bytes. When the first null character is encountered, the string is truncated. How can I make a message that has binary that isn't a string? When I look at the format to structure, it seems like there is no way to just wrap up binary: |
|
Would it be possible to change the behavior of this function so that it doesn't remove anything after a null terminator? Should I submit a PR that adds in a new kind of format specifier that is true binary? |
Creates a method to get raw binary from a DFMessage without round-tripping through the python dictionary and constructing routines.