DeepBlueRobotics / DeepBlueSim

MIT License
8 stars 0 forks source link

Add libdeepbluesim #74

Closed brettle closed 3 months ago

CoolSpy3 commented 3 months ago

I'm not sure if you've looked into it, but I did some digging into the build error.

It looks like WpilibTools is being loaded as part of controller and libdeepbluesim, and the two are conflicting (if you remove the controller subproject, the build passes). On windows, WpilibTools tries to make a cast of ExtractEmbeddedWindowsHelpers. ExtractEmbeddedWindowsHelpers_Decorated is a class created by Gradle which extends ExtractEmbeddedWindowsHelpers. However, because WpilibTools is loaded twice, it registers two versions of the extractEmbeddedWindowsHelpers task (both in the root project) EDIT: It actually checks to see if the task is already registered, and, if it is, skips the registration. Then, whichever one was loaded first second attempts to cast the version of the task loaded by the second first, which fails because the two are from different class loaders.

(Side note: If you move just the plugin declaration to plugin/build.gradle, the error becomes apparent, with a message stating that the extractEmbeddedWindowsHelpers cannot be recreated a second time.)

I attempted a quick-fix by moving all of the WpilibTools code to plugin/build.gradle. This fixes the build, but causes the natives to not be loaded at runtime. I might look at it a bit more tomorrow, but I'm not sure how far I'll get, so feel free to investigate as well. However, for now, I'm gonna call it a night.

brettle commented 3 months ago

I actually hadn't even seen that there was a build failure. Many thanks for the late night investigation!

Feel free to keep investigating. I probably won't look into it right away. Consider opening an issue with wpilib-tool-plugin and perhaps providing a minimal test case.

As a workaround, would it help if libdeepbluesim.jar was built as part of the controller project (ie not as a separate project)? There are various constant values (eg names of NT entries) that are shared between the 2 jars, so having DeepBlueSim.jar depend on libdeepbluesim.jar might not be a bad thing. I'd like to keep libdeepbluesim.jar as a thin jar if possible and if DeepBlueSim.jar is going to include libdeepbluesim.jar, I'd prefer if it didn't also include any transitive dependencies (since it really just needs those few constants).

Another tangentially related thing that might be worth investigating is having the plugin generate/update Webot's runtime.ini file for the controller. That might obviate the need to extract the native libs if it can be used to point Webots at them directly.

CoolSpy3 commented 3 months ago

I actually managed to find a workaround by declaring the WpilibTools dependency in :plugin. I've got a thought for a possible fix to the plugin, so I'll work on a PR for WpilibTools.

brettle commented 3 months ago

@CoolSpy3, just in case you missed it, this is ready for review.

CoolSpy3 commented 3 months ago

@CoolSpy3, just in case you missed it, this is ready for review.

Yeah, I've been working my way through it. I've gotten a bit sidetracked atm by a Webots bug that popped up during testing, but I've got 1 more file to review and a bunch of logging suggestions that I'm moving to another PR. Should, hopefully, be done by EOD.

brettle commented 3 months ago

As is, this breaks console output when run as an extern controller.

Won't the extern controller just read whatever logging.properties that is in the current working directory? If so, why is that a problem?

CoolSpy3 commented 3 months ago

Won't the extern controller just read whatever logging.properties that is in the current working directory? If so, why is that a problem?

Specifically, I'm concerned about the VSCode run/debug task. I've gotten into the habit of using it when debugging the controller, and not having command line output makes debugging harder. ATM, the working directory for that task is the project root, so logging.properties does not exist, causing nothing to be logged. My initial thought was that we could configure it with -Djava.util.logging.config.file, however, it occurs to me now that what we could do is change the working directory for that task to be the controller directory, which actually might make sense to do anyways. (Although having a cli option might be useful as well.)

brettle commented 3 months ago

Specifically, I'm concerned about the VSCode run/debug task. I've gotten into the habit of using it when debugging the controller, and not having command line output makes debugging harder. ATM, the working directory for that task is the project root, so logging.properties does not exist, causing nothing to be logged.

Fwiw, I thought that would cause the default logging.properties from ~/wpilib/2024/jdk/conf/ to be used. Looks like that puts INFO level logs to the console.

change the working directory for that task to be the controller directory, which actually might make sense to do anyways.

Sounds good. I've attempted to do that in my latest commit, but running the debug task is failing for me for unrelated reasons, so I can't test at the moment. If my commit isn't what you are looking for, please make the change yourself since you are in a position to test it.

(Although having a cli option might be useful as well.)

If you real want it I'm not going to stop you, but seems like it is convenient to know that the controller is running from the dir that it would run from as a non-extern controller.