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

sensord: improve sensor frequency stability and timestamp accuracy #33257

Closed
wants to merge 10,000 commits into from

Conversation

deanlee
Copy link
Contributor

@deanlee deanlee commented Aug 11, 2024

This PR addresses sensor frequency instability and improves timestamp accuracy by introducing the get_latest_event_time function.

Key Changes:

  1. Improved Frequency Stability: get_latest_event_time continuously reads from the GPIO file descriptor until all available events are retrieved. By clearing the event queue, it prevents poll() from returning immediately in the next loop, avoiding duplicate sensor data reads and ensuring a stable message frequency.
  2. Enhanced Timestamp Accuracy: Provides precise timestamps for sensor events by reading all pending events and calculating the latest timestamp.
  3. Improved Error Handling: Manages EINTR for read operations to ensure robustness and stability, preventing interruptions from disrupting the event reading process.

jamperezmondragon and others added 30 commits July 20, 2024 15:54
* Update main_es.ts

* Update main_es.ts

* Update Spanish Translation [(commaai#32995)](commaai#32995)
…tructor (commaai#33034)

* Add CameraConfig struct for initializing CameraState in constructor

* Update system/camerad/cameras/camera_qcom2.h

---------

Co-authored-by: Adeeb Shihadeh <adeebshihadeh@gmail.com>
Update Python packages and pre-commit hooks

Co-authored-by: Vehicle Researcher <user@comma.ai>
* casadi wheel

* ci

* test 312

* test with new aarch64 build

* use release wheels

* assert

* bool

* try this

* maybe

* work

* use final wheel
bump submodules

Co-authored-by: Vehicle Researcher <user@comma.ai>
* working

* multiprocessing

* fix that

* print services

* all services + fix

* less verbose

* start readme

* segment range

* cleanup

* update readme + fix bug in 'all'

* cleanup + update readme

* update readme

* cleanup

* cleanup

* rm frame_iter

* cleanup

* staticmethod

* proc kill

* split files

* fix range with hevc vids

* update reamde + add prompt

* readme

* readme

* readme
* espActive: `IMMEDIATE_DISABLE` -> `SOFT_DISABLE`

* only stock long

* just soft disable for now

---------

Co-authored-by: Shane Smiskol <shane@smiskol.com>
* op

* change this

* juggler

* options

* fix

* submodules

* typo

* venv

* clean + install
fix wayland requestActivate warning in fullscreen mode
* use pull_request_target

* env for name
@Quantizr
Copy link
Contributor

Reran on the testing closet looking at gyroscope.timestamp and accelerometer.timestamp, there is no significant difference in either max or relative standard deviation of timings between this PR and master (shown in the attached picture). Nor is there a significant difference in p5, p25, median, p75, p95, or avg (coming to testing closet soon).

The service timings for accelerometer and gyroscope are calculated by taking the diff of the timestamp attribute of the messages corresponding to the sensors in the order in which they appear in the logs. Interestingly, in both this PR and in master, a few of those diffs are negative, indicating that some of the messages are out of order, which would then be affecting both max and RSD.

Logs are already sorted by logMonoTime prior to calculations. Not sure if the correct behavior of the testing closet would be to sort by timestamp before calculating diffs or to throw an event that timestamps are out of order.

image

sshane and others added 13 commits August 16, 2024 23:57
* rm import linter, we're done!

* revert

* uv lock

* Revert "uv lock"

This reverts commit 5e46f48.
* move most of /car

* move some car tests

move some car tests

* fix selfdrive/car/tests

* fix selfdrive/controls tests

* fix the rest of the selfdrive tests

* bump opendbc

* fix all tests

* few more non-test references

* remove opcar and move docs to car

fix these debugging scripts

fix docs

* bump opendbc and panda

forgot panda
cleaning up readme

Co-authored-by: Sameh <Sameh Mohamed>
* agnos: decompress max_length

* flash last chunk after eof

* don't decompress more than length

* cleanup

---------

Co-authored-by: Adeeb Shihadeh <adeebshihadeh@gmail.com>
* stash

* no other leaks! pm.send grows memory usage by ~20mb but that's it

* undo

* clean up
* can?

* car still makes sense

* bump

* opendbc

* ?

* bump

* revert
* Update Python packages

* fix opendbc

---------

Co-authored-by: Vehicle Researcher <user@comma.ai>
Co-authored-by: Adeeb Shihadeh <adeebshihadeh@gmail.com>
commaai#33252)

* encapsulate frequency management

* apply reviews

* early return, avoiding unnecessary calculations

* simplify avg freq calc
@adeebshihadeh
Copy link
Contributor

@deanlee the timings are the same. Any other reason to merge this?

@deanlee
Copy link
Contributor Author

deanlee commented Aug 20, 2024

Although the test results show no difference, this is still a more robust way to read events.

@deanlee deanlee marked this pull request as draft August 20, 2024 02:01
@deanlee deanlee force-pushed the sensord_improve_time_accuracy branch from c195cc3 to 7ad90f3 Compare August 20, 2024 03:06
@deanlee
Copy link
Contributor Author

deanlee commented Aug 23, 2024

Since I don't have the device with me right now, I need to clarify something: Is the ts in gpioevent_data measured in nanoseconds since boot or nanoseconds since the epoch? We are currently treating it as nanoseconds since the epoch, but it seems that most GPIO returns use nanoseconds since boot.

Furthermore, instead of using poll, we might be able to use RateKeep to periodically read eventdata, clear the queue, and then send the sensor data. If this approach works, it could help stabilize the frequency.

@maxime-desroches
Copy link
Contributor

This PR was closed because of a git history rewrite.
Please read #33399 for what to do with your fork and your PRs.

@deanlee deanlee deleted the sensord_improve_time_accuracy branch September 13, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.