make constants explicit in synchronization database #218
Conversation
| LIDAR_FRAME_RATE_HZ = 10 | ||
|
|
||
| # in milliseconds | ||
| RING_CAMERA_SHUTTER_INTERVAL_MS = (1 / RING_CAMERA_FPS) * 1000 # evaluates to 33.3 ms |
There was a problem hiding this comment.
I think that 1000 might be more readable as 1e3.
There was a problem hiding this comment.
(in terms of quickly determining the units conversion)
| @@ -14,6 +14,17 @@ | |||
|
|
|||
| logger = logging.getLogger(__name__) | |||
|
|
|||
| RING_CAMERA_FPS = 30 | |||
There was a problem hiding this comment.
Can we add typing here?
There was a problem hiding this comment.
typing_extensions.Final?
| # Stereo Camera is 5 Hz (once per 200 milliseconds) | ||
| # LiDAR is 10 Hz | ||
| # Since Stereo is more sparse, we look for 200 millisecond on either side | ||
| MAX_LIDAR_RING_CAM_TIMESTAMP_DIFF = RING_CAMERA_SHUTTER_INTERVAL_MS * (1.0 / 2) * (1.0 / 1000) * 1e9 |
There was a problem hiding this comment.
I think we should consider making this more readable. One idea:
MAX_LIDAR_RING_CAM_TIMESTAMP_DIFF = 0.5 * (1e9 / 1e3) * RING_CAMERA_SHUTTER_INTERVAL_MS
I think ideally we would have a function to change units:
to_metric_time(ts: int, src: TimeUnit, dst: TimeUnit)
or something similar.
There was a problem hiding this comment.
MAX_LIDAR_RING_CAM_TIMESTAMP_DIFF = to_metric_time(RING_CAMERA_SHUTTER_INTERVAL_MS / 2), src=Millisecond, dst=Nanosecond)| @@ -14,6 +14,17 @@ | |||
|
|
|||
| logger = logging.getLogger(__name__) | |||
|
|
|||
| RING_CAMERA_FPS = 30 | |||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| Millisecond = TimeUnit.Millisecond |
There was a problem hiding this comment.
What is the convention for renaming enums? Does PEP 8 say anything about this?
Make the frame rate constants more readable/ clear.