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
279 stars 200 forks source link

[photonlib] bring Python to parity w/Java #1565

Open LucienMorey opened 1 week ago

LucienMorey commented 1 week ago

4774 has obviously been super into updating the Python side of this over the past few days ;)

I thought I would outline the work remaining before next year. Hopefully you can give us an idea of deadlines for things/whether you want them at all.

Tasks:

Optional tasks:

The scariest one IMO, is the rewrite of some of the Python constructors we put in with our sim stuff. Getting an idea of a deadline for that would be very helpful.

mcm001 commented 1 week ago

My current posture is to play fast and loose with Python support this beta cycle, and bias towards merging anything that's marked ready for review with green CI if asked. I'd like have dust settled by early/mid December so I'm not trying to fix maven servers while skiing like last year, and I'm going to be pushing us into maintanance mode starting mid December/early January for the season. Obviously it's fantastic to have developers with bandwidth to fixup our Python code and I'd like to make that as easy as possible

(and if we break Python, I'm sure all 4 users will be sad)

LucienMorey commented 1 week ago

(and if we break Python, I'm sure all 4 users will be sad)

You're right! Me and my 3 friends would be sad.

Great to hear that is your overall stance. We will do our thing and try to keep you updated as best as possible. If you have any particular callouts for other things or things that you guys want on your docs side while we are wreaking havoc, please sing out!

LucienMorey commented 1 week ago

How will people know that we are making Python great again if you change the title :rofl:

mcm001 commented 1 week ago

Python also does not have a way to run a TimeSync server right now. That's gunna be a fun one to fix (and a must-fix lol)

LucienMorey commented 1 week ago

Python also does not have a way to run a TimeSync server right now. That's gunna be a fun one to fix (and a must-fix lol)

I don't know how to do that one yet. It depends on wpinet and most of that doesn't have bindings from my reading.

EDIT: I am going on a journey to figure out how to generate the bindings over in robotpy

spacey-sooty commented 1 week ago

Python also does not have a way to run a TimeSync server right now. That's gunna be a fun one to fix (and a must-fix lol)

I don't know how to do that one yet. It depends on wpinet and most of that doesn't have bindings from my reading.

EDIT: I am going on a journey to figure out how to generate the bindings over in robotpy

The TSP is I believe very performance sensitive, it might make sense to bind to the native implementation

LucienMorey commented 1 week ago

Python also does not have a way to run a TimeSync server right now. That's gunna be a fun one to fix (and a must-fix lol)

I don't know how to do that one yet. It depends on wpinet and most of that doesn't have bindings from my reading. EDIT: I am going on a journey to figure out how to generate the bindings over in robotpy

The TSP is I believe very performance sensitive, it might make sense to bind to the native implementation

Ok. I also have no idea to do that. Isnt it going to be a massive issue though? The whole reason we stepped in to make python things for the sim was because the binding approach wasn't working out with #1438.

EDIT: Also, isn't all the time syncing stuff trying to determine the jitter on the connection rather than the latency of the pipeline? The time-sensitive part is between getting a sample and stamping it. The TSP server should be able to handle the rest if that is deterministic, right?

spacey-sooty commented 1 week ago

Python also does not have a way to run a TimeSync server right now. That's gunna be a fun one to fix (and a must-fix lol)

I don't know how to do that one yet. It depends on wpinet and most of that doesn't have bindings from my reading.

EDIT: I am going on a journey to figure out how to generate the bindings over in robotpy

The TSP is I believe very performance sensitive, it might make sense to bind to the native implementation

Ok. I also have no idea to do that. Isnt it going to be a massive issue though? The whole reason we stepped in to make python things for the sim was because the binding approach wasn't working out with #1438

Yeah wrapping python code for the roboRIO is a pain. I'm not really sure how the best way to approach this is for this case... if you can get a python implementation working well then we'd probably be good to use that.

mcm001 commented 1 week ago

The protocol itself is stupor easy to implement in pure python. True that its performance sensitive, but path of least resistance first is always good.

This issue with python bindings we hit was specifically around simulation. Robotpy just wasn't really set up for us to be depending on cscore or OpenCV from our native libs. This is just a chunk of code that only depends on wpinet and wpiutil. I think you could generate robotpy bindings for only photon-targeting potentially without too much trouble as long as we keep OpenCV out of targeting.

mcm001 commented 1 week ago

I would start with https://github.com/gerth2/robotpy-photonvision , update it and rip out photon lib and see where that gets you.

LucienMorey commented 1 week ago

@mcm001, whatever we decide to go with, what is the pathway for testing these things to know they work? Discussions around timing in this space are one of my personal bugbears because it's nice in theory and impossible to refute, but I don't think we are operating on a timescale that this is a performance killer in most cases. Is there a plan for any automated testing in any of the other languages that we can port to make sure things work to the required performance level

LucienMorey commented 1 week ago

Leaving a note for myself - There is a bad return at the end of PhotonCameraSim.process. The layout of PhotonPipelineMetadata is different to the C++ counterpart, so args are being shoved in places they shouldn't be

mcm001 commented 1 week ago

Is there a plan for any automated testing in any of the other languages

Requirements? In my engineering project?? Naaaaaaaahh

At some point I plan to use a Rio-driven led to blink at 1 PPS and measure end to end timing accuracy and precision. Nothing automated yet. I bet a native python thing gets us over the hump this year though.