-
Notifications
You must be signed in to change notification settings - Fork 206
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
[photonlib] bring Python to parity w/Java #1565
Comments
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) |
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! |
How will people know that we are making Python great again if you change the title 🤣 |
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? |
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. |
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. |
I would start with https://github.com/gerth2/robotpy-photonvision , update it and rip out photon lib and see where that gets you. |
@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 |
Leaving a note for myself - There is a bad return at the end of |
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. |
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:
OpenCVHelp
test harness - started with Add Python test harness for openCVHelp class #1557visionSystemSim
test based on contribution from fixed rotation3d constructor #1553, also going to be handled through Add Python test harness for openCVHelp class #1557PhotonCameraSim.process.
The layout ofPhotonPipelineMetadata
is different to the C++ counterpart, so args are being shoved in places they shouldn't be [python] Fix population of metadata #1578Optional tasks:
visionSystemSim_test.py
PhotonCameraSim.submitProcessedFrame
can be tidied to just have a default second arg simplify function args to use default rather than being None #1595solvePNP_Square
andsolvePNP_SQPNP
should make calls to the login module instead of print add missing log and use logger over print #1596PhotonCameraSim.consumeNextEntryTime
. we are playing with ints and floats and casting and it could be bad if things grow and we arent carefulThe 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.
The text was updated successfully, but these errors were encountered: