search

samuelpowell / Spinnaker.jl

A Julia interface to the FLIR/PointGrey Spinnaker SDK
MIT License
15 stars 7 forks source link

Suggestion for non-fatal erroring when not built properly (i.e. SDK missing) #61

Closed IanButterworth closed 4 years ago

IanButterworth commented 4 years ago

There may be a better way to handle this. Open to review!

IanButterworth commented 4 years ago

@samuelpowell Given the automerge requirement, it's probably worth enabling travis on this, to check for non-fatality

samuelpowell commented 4 years ago

Okay so does it work? If no Spinnaker found we quietly load but all functions will be broken and error at runtime?

IanButterworth commented 4 years ago

Correct.

After removing the spinnaker .dylib's temporarily:

(v1.3) pkg> build Spinnaker
  Building Spinnaker → `~/Documents/GitHub/Spinnaker.jl/deps/build.log`
 Resolving package versions...
[ Info: No changes

julia> using Spinnaker
[ Info: Precompiling Spinnaker [8e0d2ad3-56b8-53f3-8036-54b674872bef]
┌ Error: Package configuration file missing, run 'Pkg.build("Spinnaker")' to configure, or `Pkg.build("Spinnaker", verbose=true)` to debug.
└ @ Spinnaker ~/Documents/GitHub/Spinnaker.jl/src/Spinnaker.jl:59

julia> CameraList()
ERROR: UndefVarError: libSpinnaker_C not defined
Stacktrace:
 [1] spinCameraListCreateEmpty at /Users/ian/Documents/GitHub/Spinnaker.jl/src/wrapper/spin_api.jl:201 [inlined]
 [2] CameraList() at /Users/ian/Documents/GitHub/Spinnaker.jl/src/CameraList.jl:16
 [3] top-level scope at none:0

I have a PR open to clarify the Info: No changes message, as it's deceptive here, because build.jl did make changes to the build in this case.

IanButterworth commented 4 years ago

It would be nice if we could print a soft error to REPL during the build stage, but I don't know if that's possible

samuelpowell commented 4 years ago

@ianshmean what's going on is this good to go?

IanButterworth commented 4 years ago

Yeah. The only thing is that if you forget to install the Spinnaker SDK, it won't be obvious at julia build time given the error log goes straight to file in deps/build.log rather than being printed.

I'm not aware of a better approach out there, so we should merge and keep an eye on best practice

samuelpowell commented 4 years ago

Does this match with what people do at the moment in other packages?

IanButterworth commented 4 years ago

I based it on CuArrays or one of that family. It should be