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

RSDK-3545 - add sensor process #177

Closed

Conversation

kim-mishra
Copy link
Collaborator

@kim-mishra kim-mishra commented Jun 28, 2023

Ticket

  • SensorProcess polls the lidar to get the next sensor reading and adds it to the mapBuilder by calling addSensorReading in a loop and exits if the context provided is canceled. It also starts the background log process
  • startBackgroundLogAggregator logs the failure counts on a time interval provided by the caller
  • addSensorReading checks if the metadata from the GRPC call has a timestamp, if it does it calls addSensorReadingFromReplaySensor otherwise It calls addSensorReadingFromLiveReadings
  • addSensorReadingFromReplaySensor continuously calls cartofacade.AddSensorReading until it can acquire lock, and increments the failure count of this time stamp each time it fails
  • addSensorReadingFromLiveReadings calls cartofacade.AddSensorReading once, and skips the reading if it cannot acquire the lock, and increments the failure count of this time stamp once
  • There is also a change to the testhelper file so that we can mock a replay sensor that returns a corrupted time stamp that would not be able to be parsed

Questions:

  1. Right now, I keep track of the number of failures for each time stamp and on a frequency that can be set by the caller, log for each timestamp. Is this sufficient?

@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Jun 28, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 29, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 29, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 29, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 29, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 29, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 29, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 29, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 30, 2023
@kim-mishra kim-mishra changed the title Rsdk 3545 add sensor process RSDK-3545 - add sensor process Jun 30, 2023
Comment on lines 141 to 148
func incrementLogCount(params Params, t time.Time) {
_, ok := params.LogFailures[t]
if !ok {
params.LogFailures[t] = 1
return
}
params.LogFailures[t]++
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think when we log the failures, it will not always be up to date because there is no lock right now. I thought about adding a lock, but think it would cause performance issues and that given the aggregation of failures it is not as crucial to get the exact number of failures at that instant logged. Thoughts?

@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jun 30, 2023
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Jul 5, 2023
}

buf := new(bytes.Buffer)
err = pointcloud.ToPCD(reading, buf, 0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
err = pointcloud.ToPCD(reading, buf, 0)
err = pointcloud.ToPCD(reading, buf, pointcloud.PCDBinary)

@nicksanford
Copy link
Collaborator

PR moved to: #185

@nicksanford nicksanford closed this Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test This pull request is marked safe to test from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants