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

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

Open
11 of 14 tasks
LucienMorey opened this issue Nov 12, 2024 · 13 comments
Open
11 of 14 tasks

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

LucienMorey opened this issue Nov 12, 2024 · 13 comments
Labels
enhancement New feature or request

Comments

@LucienMorey
Copy link
Contributor

LucienMorey commented Nov 12, 2024

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.

@LucienMorey LucienMorey added the enhancement New feature or request label Nov 12, 2024
@mcm001
Copy link
Contributor

mcm001 commented Nov 12, 2024

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
Copy link
Contributor Author

(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!

@mcm001 mcm001 changed the title Make Python great again! [photonlib] bring Python to parity w/Java Nov 13, 2024
@LucienMorey
Copy link
Contributor Author

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

@mcm001
Copy link
Contributor

mcm001 commented Nov 14, 2024

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
Copy link
Contributor Author

LucienMorey commented Nov 14, 2024

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
Copy link
Member

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
Copy link
Contributor Author

LucienMorey commented Nov 14, 2024

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
Copy link
Member

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
Copy link
Contributor

mcm001 commented Nov 14, 2024

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
Copy link
Contributor

mcm001 commented Nov 14, 2024

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

@LucienMorey
Copy link
Contributor Author

@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
Copy link
Contributor Author

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
Copy link
Contributor

mcm001 commented Nov 15, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants