search

OpenSimulationInterface / open-simulation-interface

A generic interface for the environmental perception of automated driving functions in virtual scenarios.
Other
267 stars 125 forks source link

Bug in OSI Docu on Tracefiles #760

Closed PhRosenberger closed 6 months ago

PhRosenberger commented 7 months ago

Describe the bug

The page in our documentation on OSI trace files seems broken. There seems to be a duplicate in heading and text. @philipwindecker is the documentation build pipeline the problem here?

Here the page in our docu: https://opensimulationinterface.github.io/osi-antora-generator/asamosi/latest/interface/architecture/trace_file_formats.html Here the source for that page: https://github.com/OpenSimulationInterface/open-simulation-interface/blob/master/doc/architecture/trace_file_formats.adoc

philipwindecker commented 7 months ago

That IS a weird behavior. I will have to investigate.

philipwindecker commented 7 months ago

The problem seems to come from the part "$$__$$". This seems to trigger some html passthrough behavior (according to https://github.com/asciidoctor/asciidoctor-latex/issues/69). However, it is unclear to me whether the intended output should be __ or $$__$$ or $__$. Please clarify

philipwindecker commented 7 months ago

Messages are separated by pass:[<code>&#36;&#36;__&#36;&#36;</code>]. is my suggested fix.

philipwindecker commented 7 months ago
PhRosenberger commented 7 months ago

However, it is unclear to me whether the intended output should be __ or $$__$$ or $__$. Please clarify

The ascissdoc $$__$$ there is rendered as __. So I would go for this. @pmai is that the correct separator for plain text files? - I only have experience with .osi and .txth files..

ClemensLinnhoff commented 7 months ago

I have some further questions about the trace file formats in general.

  1. Why are there two different formats (.osi and .txt), what are the use-cases for each? So when should I use .osi and when should I use .txt?
  2. In the documentation for the .txt format it says "Plain-text trace file" but is this really true? The example in osi-validation is not plain text. So is this example wrong or the documentation?
  3. Does an example on how to generate a .txt trace file exist somewhere?

@pmai or @jdsika could you shed some light on this? Then we can also put the answers in the documentation to make the trace file usage more clear.

thomassedlmayer commented 7 months ago

I agree that the 'plain text' statement for txt files in the documentation is misleading. I don't get why they are called 'txt' files at all. I think the main difference between txt and osi was supposed to be that txt should contain these message delimiter/separators ($$__$$). The osi2txt.py argument parser description also says "Convert a serialized osi/txt trace file to a readable txth output" which indicates that the txt format was supposed to be a binary format all along.

The OSITrace.py file could probably be used to reverse engineer how a separated osi file should actually look like. Without having looked at the code in depth I assume that it is just the usual serialized binary format with these separators in between top level messages.

I think that this separated binary format does not help a lot as we have wrappers like SensorView/SensorData/etc. which can be serialized one by one anyways. So I don't really get the advantage of the separator in our case. It could be some kind of performance improvement if you could deserialize field by field until the separator. But I don't know if this was actually used like this by anyone?

I agree that we should fix this as it generate all kind of confusion, see #726. I think in this issue the txt2osi was misinterpreted as possibility to convert txth (actual human readable) files to binary osi files.

philipwindecker commented 7 months ago

So, I will wait until this discussion is resolved before I implement any fix here? Or will the group fix this? If the notation is removed, we do not need to fix this at all (from an Antora perspective).

Let me know (best: directly mention me) when I need to become active again, I might not see this otherwise right now. :)

PhRosenberger commented 7 months ago

Ok, I will take it to one of the next CCB meetings to discuss it there, as well.

PhRosenberger commented 6 months ago

We just decided in the CCB to get rid of the .txt files anyways, as this is some leftover from the ancient times of OSI that no one actually is or should be using. This will then also solve the docu issue.

ClemensLinnhoff commented 6 months ago

Suggestion for discussion:

Get rid of current .txt format, as decided. But also change the current .txth format to .txt. Then we would have a clear and very intuitive separation:

I suppose this might be a bit confusing for people who are used to .osi, .txt and .txth. But for new people this will remove a lot of potential confusion, when they are confronted with the (not really existing) .txth format.

Furthermore, we can get rid of the txt2osi.py. Then we could rename osi2read.py to osi2txt.py and potentially add a txt2osi.py to convert a plain text trace file to a binary trace. This would allow to change individual values in the trace very easily for testing.

ClemensLinnhoff commented 6 months ago

Alternative: Get rid of txt file altogether and introduce a yml or json format.

PhRosenberger commented 6 months ago

Alternative: Get rid of txt file altogether and introduce a yml or json format.

I would propose to put this request in a new issue for suggestion and close this issue here, when the PR #777 gets merged.