search

wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.05k stars 611 forks source link

[wpiutil] Remove RuntimeDetector and greatly simplify RuntimeLoader #6600

Closed spacey-sooty closed 3 months ago

spacey-sooty commented 3 months ago

Resolves https://github.com/wpilibsuite/allwpilib/issues/6598

ThadHouse commented 3 months ago

We can just use System.loadLibrary() where the old calls were. CombinedRuntimeLoader will not work for loading individual libraries.

RuntimeLoader.loadLibrary() might need to stay to give a better error message, but it'd be much simpler then what was already there.

spacey-sooty commented 3 months ago

We can just use System.loadLibrary() where the old calls were. CombinedRuntimeLoader will not work for loading individual libraries.

RuntimeLoader.loadLibrary() might need to stay to give a better error message, but it'd be much simpler then what was already there.

Based off debugging my typo in the AprilTagJNI I'd say the error message isn't that bad but they could be worse in different cases idk

PeterJohnson commented 3 months ago

Isn't the error message for raw loadLibrary() just "can't load native library"? I believe the reason we had the more descriptive error was for the (fairly common) case when users didn't have an updated MSVC runtime library installed.

spacey-sooty commented 3 months ago

Yes the error message is just can't load library name here if the updated MSVC runtime is an issue I can add back a simpler version of RuntimeLoader.loadLibrary()

PeterJohnson commented 3 months ago

Let’s do that, yes.

PeterJohnson commented 3 months ago

Do any vendor libraries use the removed functionality?

ThadHouse commented 3 months ago

I don't think so. They shouldn't be doing so anyway if they are.