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

replay: fix issue when original logfile had topics with zero timstamps #11708

Merged
merged 1 commit into from
Mar 25, 2019

Conversation

CarlOlsson
Copy link
Contributor

@CarlOlsson CarlOlsson commented Mar 22, 2019

Describe problem solved by the proposed pull request
If the original logfile contained a topic which at some point was logged with a zero timestamp next_file_time would be set to zero and in the next iteration of the for loop next_file_time is updated to the timestamp of the next topic in the _subscriptions list.

The result was that the topics which were before the zero timestamp topic in the _subscriptions list would never be published until the end. This was not visible in most log analysis software since the timestamp would still be correct but the order and timing of the published topics would be wrong.

Additional context
Another thing which makes replay not work as expected is the fact that the logger only tries to subscribe to a topic once a second. So e.g. when a VTOL transitions to cruise flight there will be a time right after transition when the fw_pos_controller publishes the virtual attitude setpoint but it is not logged for up to one second

Verified

This commit was signed with the committer’s verified signature. The key has expired.
pocke Masataka Pocke Kuwabara
Signed-off-by: CarlOlsson <[email protected]>
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

If the original logfile contained a topic which at some point was logged with a zero timestamp

So you mean a zero timestamp not at the beginning, after already valid timestamp(s)?

@CarlOlsson
Copy link
Contributor Author

So you mean a zero timestamp not at the beginning, after already valid timestamp(s)?

Yes exactly

@mhkabir mhkabir merged commit ed2d4f6 into PX4:master Mar 25, 2019
@bkueng
Copy link
Member

bkueng commented Mar 26, 2019

Yes exactly

Is this in upstream, and if so, can we fix that as well?

@CarlOlsson
Copy link
Contributor Author

No I don't think so, for us it was the virtual_attitude_sp after transition to cruise. I am fixing this now but in order to make sure that the functionality stays the same I needed to be able to replay the controllers. Awesome feature btw

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants