Fix message iteration by index and range

[vkresch]

Reference to a related issue in the repository

For message iteration there is currently a bug in which the range cannot be specified (#426).

Add a description

What is this change?
This PR replaces hte while loop iteration with the for loop iteration over the messages inspired from the osi-validator.

What does it fix?
It fixes the index specification of OSI trace files. (Fixes #426)

How has it been tested?
I ran manual test with an osi trace file to get messages by index. I also tested the conversion from *.txt -> *.osi and made the *.osi file also readable.

Mention a member

@tpajenkamp-dspace let me know if this PR fixes your issue. Please test it with your trace file and let me know if it works.

Check the checklist

• My code and comments follow the style guidelines and contributors guidelines of this project.
• I have performed a self-review of my own code.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings.
• I have added tests that prove my fix is effective or that my feature works.
• New and existing unit tests / travis ci pass locally with my changes.

[tpajenkamp-dspace]

@vkresch Thanks for the update, now get_message_by_index works without problems.

However, there is a bug in get_messages_in_index_range if more than one message is extracted. I created a comment directly at the offending code line. Currently, I get:

google.protobuf.message.DecodeError: Truncated message.

This also affects other methods like get_messages which call get_messages_in_index_range.

Also, I noticed another possible problem in this method which is technically another issue and may be somewhat intended. When the generator returned by get_messages_in_index_range is consumed (reaches StopIteration), the underlying file is closed so that following calls to most methods cause ValueError: seek of closed file or similar errors.

[tpajenkamp-dspace]

self._int_length must not be subtracted

[vkresch]

For some reason I cannot reproduce the bug with more than one message extraction with my osi file. Can you maybe share your osi file if possible? Try to validate your osi file with osi visualizer or the osi validator. Can you visualize your file in osi visualizer?

The other issue is the generator property of python. Generators generate the elements on the fly and throw them away. A way to prevent it would be creating a deep copy of the generator as an iterator (making copies is of course performance heavy but would for now work as a quick fix).

[vkresch]

Maybe an error exceptions needs to be added so that the user knows whats going on. Good point!

[tpajenkamp-dspace]

First of all, I would argue with logic. The first message_offset is 0. Every message offset indicates the first byte of the message length indicator of its following serialized message. Thus, it is the byte right after the previous binary message (first offset excluded). The length indicator offset is already taken into account by the computation of rel_begin. By also subtracting self._int_length from the next message offset to compute rel_end, an incomplete serialized message is read missing the last 4 bytes. The next operation starts at the next message_offset entry so that these 4 bytes are skipped and never processed.

The osi-validator suffers from the same problem.

I have no appropriate means for file uploads, but this simple script generates an OSI trace file. It uses the same mechanism to pack the osi messages as txt2osi.py. The bug is triggers in the last line. If I apply my proposed fix, it works.

import struct
from osi3.osi_version_pb2 import InterfaceVersion, current_interface_version
from osi3.osi_sensorview_pb2 import SensorView

import OSITrace

filePath = "osi_SensorView_tmp.osi"

sv = SensorView()
sv.version.CopyFrom(InterfaceVersion.DESCRIPTOR.file.GetOptions().Extensions[current_interface_version])
with open(filePath, "wb") as fp:
    for i in range(15):
        sv.timestamp.seconds = 0
        sv.timestamp.nanos = i * 5
        msgBin = sv.SerializeToString()
        filePath = r"_osi_SensorView_tmp.osi"
        fp.write(struct.pack("<L", len(msgBin)))
        fp.write(msgBin)

osiTrace = OSITrace.OSITrace()
osiTrace.from_file(filePath, type_name="SensorView")
msgs = list(osiTrace.get_messages())

[vkresch]

Yes you are right! I checked with different files. But I don't get the "Truncated error message" (is this maybe some edge case?). When decoding there are missing fields when I keep the self._int_length subtraction happening see output below:

(I just created two readable outputs with and without the int subtractions and compared both files with meld. The right one should be the correct one!)

I will adjust it and will also add a meaningful error message when the message generator reaches the end! Thanks for reporting this issue!

[tpajenkamp-dspace]

With the latest commit, it works for me. Thank you.

[vkresch]

@jdsika please merge. Thanks ;)