8000 Fix sim versionEntry NT table path by edf42001 · Pull Request #569 · PhotonVision/photonvision · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix sim versionEntry NT table path #569

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Nov 13, 2022
Merged

Fix sim versionEntry NT table path #569

merged 5 commits into from
Nov 13, 2022

Conversation

edf42001
Copy link
Contributor
@edf42001 edf42001 commented Nov 3, 2022

We haven't tested if this resolves the issue yet

@edf42001 edf42001 requested a review from a team as a code owner November 3, 2022 23:58
@mcm001
Copy link
Contributor
mcm001 commented Nov 4, 2022

Potentially closes #568

@edf42001
Copy link
Contributor Author
edf42001 commented Nov 5, 2022

I'm trying to test the fix, but don't know how to point VSCode to compile with the new code instead of the released version from vendordeps

@mcm001
Copy link
Contributor
mcm001 commented Nov 7, 2022

You should be able to add mavenLocal in build.gradle, and then change the version in the vendordep to dev-v2023.1.1-beta-5-5-g8706ea91 or whatever

@amquake
Copy link
Member
amquake commented Nov 8, 2022

works for me in sim 👍

@edf42001
Copy link
Contributor Author

Having never used gradle before, I got as far as discovering that ./gradlew publishToMavenLocal creates a vendoreps json for the most recent commit, but directly putting the generated json into the project does not work, because it isn't pointing to the Jars that would be built on my computer, and I don't know how to configure it to do so.

Does the master branch need to be merged into this one to keep it up to date?

I don't know why the checks failed with Could not find org.ajoberstar.grgit:grgit-core:3.0.0. Doesn't seem like it would have anything to do with these changes.

In conclusion, I need more info on using gradle and maven before testing this on my machine, so it's nice someone else was able to verify that the fix works.

mcm001
mcm001 previously approved these changes Nov 11, 2022
View reviewed changes
@mcm001
Copy link
Contributor
mcm001 commented Nov 11, 2022

You're right those errors were random, rerunning CI now

@mcm001
Copy link
Contributor
mcm001 commented Nov 11, 2022

Testing locally is a bit annoying, you'll have to add maven local your robot projects repositories {} block like this, I think



repositories {
    mavenLocal()
}

@mcm001 mcm001 added this to the 2023 Beta 6 milestone Nov 11, 2022
@amquake
Copy link
Member
amquake commented Nov 11, 2022

The bit Matt described is now on the build instructions docs

@edf42001
Copy link
Contributor Author

I successfully followed the instructions and was able to compile (after updating the sim to use 3D poses from 2D). But now I get a runtime error, and am curious if something wrong with my build or if others have had the issue:

Error at org.photonvision.SimVisionSystem.processFrame(SimVisionSystem.java:159): Unhandled exception: java.lang.NoSuchMethodError: 'double edu.wpi.first.math.Vector.dot(edu.wpi.first.math.Vector)'
        at edu.wpi.first.math.geometry.Quaternion.times(Quaternion.java:82)
        at edu.wpi.first.math.geometry.Translation3d.rotateBy(Translation3d.java:141)
        at edu.wpi.first.math.geometry.Transform3d.inverse(Transform3d.java:137)
        at org.photonvision.SimVisionSystem.processFrame(SimVisionSystem.java:159)
        at org.photonvision.SimVisionSystem.processFrame(SimVisionSystem.java:147)

@amquake
Copy link
Member
amquake commented Nov 11, 2022

@edf42001 This would probably be a better discussion for the discord, but that issue is due to bundling the 3d classes for users with wpilib 2022. The 2022 classpath overwrites the new bundled Vector class which causes that error. The solution is using the 2023 beta or copy-pasting + overwriting the new geometry classes(~10 files) into the 2022 project.

@shueja
Copy link
Contributor
shueja commented Nov 12, 2022

For the record, we were warming up to do a final update for 2022 and move to the 2023 beta when this tag change released, so we still plan to get back to that soon(tm).

@mcm001
Copy link
Contributor
mcm001 commented Nov 12, 2022

Imo we should be good to merge as-is? @edf42001 unless you wanna hold off

@edf42001
Copy link
Contributor Author 8000

I successfully got the 2023 beta, but ran into problems simulating because RevLib was out of date and some issues with wpiHal. So, I'll leave this here for now with amquake having tested it. I'll leave it up to the owners to merge according to the planned release schedule. But let me know if you need anything else from me.

@edf42001
Copy link
Contributor Author

Imo we should be good to merge as-is? @edf42001 unless you wanna hold off

Oh wow you sent this right while I was typing mine. Of course go ahead.

gerth2
gerth2 approved these changes Nov 13, 2022
View reviewed changes
Copy link
Contributor
@gerth2 gerth2 left a comment

Choose a reason for hiding this comment

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

Looks like what I would have expected - I haven't fully tested yet, but since others have I'll mark it as good to go.

@gerth2 gerth2 merged commit f193a23 into PhotonVision:master Nov 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gerth2 gerth2 gerth2 approved these changes

@mcm001 mcm001 mcm001 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2023 Beta 6
Development

Successfully merging this pull request may close these issues.
5 participants
@edf42001 @mcm001 @amquake @shueja @gerth2
0