JaneliaSciComp / NIDAQ.jl

National Instruments Data Acquisition Interface
Other
46 stars 16 forks source link

make it automerge-able #30

Open bjarthur opened 3 years ago

bjarthur commented 3 years ago

https://github.com/JuliaRegistries/General/pull/26469#issuecomment-747087503

DylanMMarques commented 2 years ago

Hello,

I am using this package and I am having some problems by the fact that it is not automerge. The issue is related with:

try
  global ver
  global major, minor, update  # bug in Julia v0.5 on windows?
  major = Ref{UInt32}(0)
  ccall((:DAQmxGetSysNIDAQMajorVersion,NIDAQmx),Int32,(Ref{UInt32},),major)
  minor = Ref{UInt32}(0)
  ccall((:DAQmxGetSysNIDAQMinorVersion,NIDAQmx),Int32,(Ref{UInt32},),minor)
  update = Ref{UInt32}(0)
  ccall((:DAQmxGetSysNIDAQUpdateVersion,NIDAQmx),Int32,(Ref{UInt32},),update)
  ver = "$(major[]).$(minor[]).$(update[])"
catch
  error("can not determine NIDAQmx version.")
end

try
  include("constants_V$ver.jl")
  include("functions_V$ver.jl")
catch
  error("NIDAQmx version $ver is not supported.")
end

I think that this code can be simply replaced by:

 include("constants_V20.1.0.jl")
 include("functions_V20.1.0.jl")

This has additional advantage that the code would be able to run with any NIDAQmx versions. I quickly compared the files functions_V20.1.0.jl against the old versions of functions_Vver.jl and the older versions of NIDAQmx should run correctly using functions_V20.1.0.jl. The only difference between functions_V20.1.0.jl and the older version is a few additional functions. The same applies to constants_Vver.jl

By having the functions_Vver.jl and constants_Vver.jl generated with the latest version of NIDAQmx, the code should work with the older versions assuming that NIDAQmx is back version compatible.

Thanks, Dylan

bjarthur commented 2 years ago

i can see how this change would conveniently allow users to use whatever version of the drivers they happen to have installed, but my concern is that someday they might not be backwards compatible.

what i don't understand is why you're having problems with automerge. that is related to releasing a new version of NIDAQ.jl, which we do everytime we update the driver support. have your forked it?

the much better solution IMO is to use binary builder to automatically install the DAQmx drivers.

DylanMMarques commented 2 years ago

My problem is not directly the automerge, but related with that. I am creating a package using NIDAQ.jl and I want it to be able to precompile on computers which do not have DAQmx drivers installed. My package controls various hardware and I only want to require the DAQmx installation if the NIDAQ.jl module is needed. I also want to document the package using Documenter.jl and using the automatic deploy feature which require NIDAQ.jl to be precompiled without the drivers installed (or to be installed automatically).

I totally agree with you that the binary builder to automatically install the drivers would be a much better alternative. Unfortunately, I don't know much about it, but sounds perfect for everything to be installed automatically.

bjarthur commented 2 years ago

would conditionally using NIDAQ using Requires.jl solve you problem?

bjarthur commented 2 years ago

if not, then the only other thing i can think of is creating a DAQmx artifact using BinaryBuilder.jl and specifying lazy=true in the Artifact.toml file. see CUDA.jl for an example of how to do this.