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

Updat MongoDBBridge for timestamp. #227

Open
wants to merge 6 commits into
base: noetic
Choose a base branch
from

Conversation

abmuslim
Copy link

No description provided.

@abmuslim
Copy link
Author

Hi @Sanic, I pulled a request.
Have a look.
Thank you


n_.getParam("ts",time_stamp); // getting the value of "ts" from the command arg and store in time_stamp.
n_.deleteParam("ts"); // deleting the "ts" variable.

MEASURE_TIME;
Copy link
Contributor

Choose a reason for hiding this comment

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

MEASURE_TIME should always be at the beginning of a method

auto it= std::find(frames.begin(), frames.end(), time_stamp); // finding if the queued timestamp is in the frames vector

if(it== frames.end()){
outWarn("Timestamp Queried is not in the frame or Timestamp Query not enabled");
Copy link
Contributor

Choose a reason for hiding this comment

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

This is triggered on every iteration when the ts parameter is not set by the user.
Could you please change the method so that it checks if the timestamp parameter is set and only then do the required steps?

@Sanic
Copy link
Contributor

Sanic commented Mar 23, 2022

I think that it would also be reasonable to check that if the timestamp parameter is set and no frame for this timestamp exists, that you then already return false in line 98. Basically like you do it in Line 113.

@abmuslim
Copy link
Author

Hi @Sanic, Kindly have a look at the updated MongoDBBridge.cpp file.

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.

2 participants