search

PhotonVision / photonvision

PhotonVision is the free, fast, and easy-to-use computer vision solution for the FIRST Robotics Competition.
https://photonvision.org
GNU General Public License v3.0
260 stars 169 forks source link

Add sequence ID, capture, publish and recieve timestamp to PhotonPipelineResult #1305

Closed mcm001 closed 2 months ago

mcm001 commented 3 months ago

Closes #1304

srimanachanta commented 3 months ago

Would this be a good time to make that change that includes the pipeline name in the result. Not sure if that overhead is worth.

mcm001 commented 3 months ago

Maybe just the index number is a good compromise?

TheTripleV commented 2 months ago

We also sent across the capture timestamp in system time for future syncing.

lgtm otherwise.

gerth2 commented 2 months ago

Fine as is, comments: --> Timestamp caveats of "sometimes NT4 did not sync" - needs some integration thought later this summer. --> Will break python as-is - OK, since theoretically pybind updates get that actually fixed --> Sequence ID has some functional overlap with heartbeat. Maybe consider obsoleting heartbeat, or doing something else to save a byte or two in the messages?

mcm001 commented 2 months ago

Ah whoops I can fix python. heartbeatPublisher is a good fallback since it shows up directly in Glass instead of inside a message. And yeah we should have a larger NTP discussion, but this is a good first step