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

Create new PhotonStructDecoder #181

Merged
merged 8 commits into from
Nov 12, 2024

Conversation

mcm001
Copy link
Contributor

@mcm001 mcm001 commented Sep 11, 2024

Closes #181

Still TODO:

  • Verify schema hashes
  • Actually plumb this into the rest of the code
  • Allow PhotonStructDecoder to use normal Struct types as nested members
  • Guarantee that float64 x; and double x; hash to the same thing

It's not pretty, but for an MVP it's not bad
image
#181

@mcm001
Copy link
Contributor Author

mcm001 commented Sep 12, 2024

I can't drag and drop Translation/Transform3ds onto the field, but it seems like all other decoding works. the bytes consumed member also needs to be trimmed off.
https://github.com/user-attachments/assets/5d2ac990-479b-4545-9616-df84dd5b814d

@mcm001 mcm001 changed the base branch from main to new-ui September 13, 2024 05:34
@mcm001
Copy link
Contributor Author

mcm001 commented Sep 13, 2024

This should be like 90% of the way to feature complete, but I haven't had a chance to go through and clean-up code yet.

@jwbonner jwbonner changed the base branch from new-ui to main November 11, 2024 16:01
@jwbonner
Copy link
Member

I updated to the latest commit on main, and did a little bit of cleanup. Is there anything major still missing? If not, I'm planning an AdvantageScope release later today and it would be nice to include this feature given the PhotonVision release yesterday.

@mcm001
Copy link
Contributor Author

mcm001 commented Nov 11, 2024

I don't think there are any blocking features missing. I did want to add schema hash checking, but if I'm getting the schema UUID straight from the source already I don't see a reason to double verify it. I'd like to test once more with photon beta 1 before merging out of paranoia then I'm good to go (I'll be out at work for the next 10 hours fya), or we can just send it as is.

Thank you for cleaning this up!

@jwbonner
Copy link
Member

No problem, I'm not planning to release anything until later tonight anyway.

@jwbonner
Copy link
Member

I did a quick test with the PhotonVision beta running in test mode, and it seems to look OK? I'll probably go ahead and merge unless you have an objection.

Screenshot 2024-11-11 at 11 15 49 PM

@mcm001
Copy link
Contributor Author

mcm001 commented Nov 12, 2024

Works for me in test mode here. Seeeeennnnnndddd it

@mcm001 mcm001 marked this pull request as ready for review November 12, 2024 04:18
@jwbonner jwbonner merged commit b6ab389 into Mechanical-Advantage:main Nov 12, 2024
15 checks passed
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.

2 participants