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

Add python simulation #1532

Merged
merged 82 commits into from
Nov 10, 2024
Merged

Conversation

LucienMorey
Copy link
Contributor

We are doing the whole native python thing. We think we are most of the way through since we can run it with our robot code in sim but no data is coming through just yet. Hopefully if anyone else has other progressive we can share.

@LucienMorey LucienMorey changed the title Add python Simulation Add python simulation Nov 7, 2024
@mcm001
Copy link
Contributor

mcm001 commented Nov 7, 2024

Yall are absolutely insane for crunching through 1500 lines of language translation, love it. I was attempting to go the robotpy bindings route to side-step needing to re-implement photon sim in Python but hit build system roadblocks. If this works this'll be pretty awesome to see upstreamed.

@LucienMorey LucienMorey force-pushed the tdb/add_python_sim branch 2 times, most recently from 9e8ebc8 to e2c7510 Compare November 7, 2024 10:51
@LucienMorey
Copy link
Contributor Author

@mcm001 @spacey-sooty I think you're the right ones to ping here...

Some of this can and probably should be split out if you want.

  • I have extended the message generator stuff to make packing functions in python. We needed it for the sim
  • 153ca26 Identifies an inconsistency in naming with C++ and Java the message uses a lowercase 't' when the python class and every usage of it has uppercase so we get exceptions

Do you want these things prepped and sent in outside this PR?

In other news, we are driving around in our sim with simulated vision and results returning to our code. There is still more to go, but it is very close!

@spacey-sooty
Copy link
Member

@mcm001 @spacey-sooty I think you're the right ones to ping here...

Some of this can and probably should be split out if you want.

* I have extended the message generator stuff to make packing functions in python. We needed it for the sim

* [153ca26](https://github.com/PhotonVision/photonvision/pull/1532/commits/153ca269d6a809b259f2fb8ab0015430df5a05bf) Identifies an inconsistency in naming with C++ and Java the message uses a lowercase 't' when the python class and every usage of it has uppercase so we get exceptions

Do you want these things prepped and sent in outside this PR?

In other news, we are driving around in our sim with simulated vision and results returning to our code. There is still more to go, but it is very close!

Im prolly not the right person to ask but having these as seperate PRs would make this more reviewable so would probably be good.

Thanks for the incredible work, you'll have to show me at SCR

@mcm001
Copy link
Contributor

mcm001 commented Nov 7, 2024

Yeah let's split out the Packet autogen and spelling changes into their own PR for reviewability

@LucienMorey
Copy link
Contributor Author

LucienMorey commented Nov 9, 2024

I have rebased this to get more up-to-date ci. It will need to be rebased again after #1541 gets merged because it contains those changes at the beginning of the history right now

done

@LucienMorey LucienMorey force-pushed the tdb/add_python_sim branch 3 times, most recently from fd28c1d to 8080073 Compare November 10, 2024 11:35
@LucienMorey LucienMorey marked this pull request as ready for review November 10, 2024 12:17
@LucienMorey LucienMorey requested a review from a team as a code owner November 10, 2024 12:17
@LucienMorey
Copy link
Contributor Author

LucienMorey commented Nov 10, 2024

Well this is as ready to go as it can be. We have ported all the things we and probably most teams will need, including the test harness.

What do you want done about the lint job? #1545 broke things and I don't know how to fix it. I have tried going to both the same and a newer version of wpiformat. I cant get the same diff as the CI and if I manually change and run the tool it reverts things. I think this whole job is now pretty likely broken.

@mcm001
Copy link
Contributor

mcm001 commented Nov 10, 2024

The "wpiformat" workflow uploads a patch file you can apply with git apply, but I've pushed formatting fixes for ya

@mcm001
Copy link
Contributor

mcm001 commented Nov 10, 2024

Oh or apparently I don't have permission to do that. Apply this patch file: https://github.com/PhotonVision/photonvision/actions/runs/11765371019/artifacts/2168245903

@LucienMorey
Copy link
Contributor Author

Woot! Is anything else needed from us for the time being @mcm001?

Copy link
Contributor

@mcm001 mcm001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of these days, CI will be green and yet the code is broken. But it is not this day

@mcm001 mcm001 merged commit b3d74e5 into PhotonVision:master Nov 10, 2024
33 checks passed
@LucienMorey
Copy link
Contributor Author

@mcm001 I know you made a release a few hours ago, but would you be willing to make another one with the sim stuff, or is there a cycle time we have to wait on? I want to make life easier for the kids on my team so they don't have to install dev builds if possible

@mcm001
Copy link
Contributor

mcm001 commented Nov 10, 2024

Do we publish dev builds of photonlib to pypi? If not yeah I'm open to a quirk patch. When is your need by date?

@LucienMorey
Copy link
Contributor Author

Do we publish dev builds of photonlib to pypi? If not yeah I'm open to a quirk patch. When is your need by date?

It seems like you make things automatically. 21 hours ago lines up with what you were doing right? it seems older than whats in the github releases somehow
image

One of our last offseason projects is setting up a camera on a turret for single camera with massive effective fov action. We are working on it sporadically now, so whenever you are ready to drop a patch it would be appreciated.

@mcm001
Copy link
Contributor

mcm001 commented Nov 10, 2024

Yee we have got tags automated -- looks like we don't push artifacts for every commit to main like Java and C++ though. I'll drop a tag once the dust settles a bit more.

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

Successfully merging this pull request may close these issues.

5 participants