Skip to content
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

SensorViewConfiguration missing timestamp #392

Open
caspar-ai opened this issue Jun 17, 2020 · 4 comments
Open

SensorViewConfiguration missing timestamp #392

caspar-ai opened this issue Jun 17, 2020 · 4 comments
Labels
Bug Problems in the build system, build scripts, etc or faults in the interface. SensorModeling The Group in the ASAM development project working on sensor modeling topics.

Comments

@caspar-ai
Copy link
Contributor

Describe the bug

There is a disconnect between the documentation and the implementation with regards Top-Level Messages and SensorViewConfiguration.

Describe how to reproduce the bug

The interface conventions documentation states that all top level messages require an interface as the first field and a timestamp as the second. It also explicitly mentions SensorViewConfiguration as one such example of a top level message.

The definition of a SensorViewConfiguration includes an interface version but does not include a timestamp.

Describe the expected behavior

The expected fix depends on the desired behaviour:

  1. If the SensorViewConfiguration is an exception to the top level rule then I would expect the documentation to be updated, and a comment added inline by the message definition.
  2. If it isn't an exception, a timestamp field should be added.
@caspar-ai caspar-ai added the Bug Problems in the build system, build scripts, etc or faults in the interface. label Jun 17, 2020
@Krishna626
Copy link
Contributor

@caspar-ai Hi Casper, while going through the SensorViewConfiguration definition, I could find the simulation start time at line 191. Isn't it what you are expecting?

@caspar-ai
Copy link
Contributor Author

@Krishna626 I'm not an expert in this area, so I'm not 100% sure. On first read though it doesn't look like it to me.

In particular, compare to, for example, the ground truth definition which defines the timestamp as the current time. Whereas the field you point to on line 191 defines it as the start time of the simulation, i.e. that field would never change throughout the lifetime of the exchange. At least that's how I read it.

@caspar-ai caspar-ai added the SensorModeling The Group in the ASAM development project working on sensor modeling topics. label Oct 27, 2020
@thomassedlmayer
Copy link
Contributor

As this issue was brought up by @PhRosenberger in #764 I tried to get some insight on the problem.

It seems like the part on mandatory 'version' and 'timestamp' fields in top level messages was removed from the documentation at some point. I could imagine that it isn't too important for SensorViewConfiguration to include a timestamp since it is either sent prior to simulation start (during initialization) or contained in another top level interface message which already includes a timestamp.

I further noticed that FeatureData is listed under the chapter "Top-level interfaces" in the documentation while not containing a timestamp as well. Though, each SensorDetectionHeader already contains some information on the measurement time of the data instead of having a top level timestamp for all messages together.

@thomassedlmayer
Copy link
Contributor

Issue was discussed in today's Packaging/Performance meeting:

  • Discussion: Could make sense to add the timestamp based on the following use case: If multiple configs are sent after simulation start, a timestamp would help to identify the order in case the messages get mixed up. But no real use case/problem for anyone right now.
  • No further actions for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Problems in the build system, build scripts, etc or faults in the interface. SensorModeling The Group in the ASAM development project working on sensor modeling topics.
Projects
None yet
Development

No branches or pull requests

3 participants