Skip to content
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.

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

incorrect use of WinDLL #261

Open
carandraug opened this issue Jan 9, 2023 · 3 comments
Open

incorrect use of WinDLL #261

carandraug opened this issue Jan 9, 2023 · 3 comments

Comments

@carandraug
Copy link
Collaborator

We use ctypes.WinDLL to load all shared libraries in Windows but that's wrong. We should be using ctypes.CDLL even on Windows for some libraries.

We have been using WinDLL under the incorrect assumption that's what one needs to do in Windows. However, the choice is not about the underlying OS, it's about the calling convention used by the functions. We should be using WinDLL to access functions that use the stdcall calling convention and CDLL to access functions that use the standard C calling convention (called cdecl on Windows).

If I understood this correctly, we need to check the header files and see if in Windows the functions are defined with __stdcall (may happen indirectly such as when the WINAPI macro is used). If not, then we should be using CDLL even on windows. It seems that using the wrong one will not immediately lead to issues but may lead to weird unpredictably issues later.

I've checked the header files and seems we need to change for Alpao and BMC mirrors, for Hamamatsu cameras, and for Linkam stages:

device (module) checked header needs fix
alpao mirrors (microscope/_wrappers/asdk.py) yes probably
BMC mirrors (microscope/_wrappers/BMC.py) yes probably
hamamatsu cameras (microscope/_wrappers/dcamapi4.py) yes probably
mirao mirrors (microscope/_wrappers/mirao52e.py) yes no (MIRAOCALL expands to __stdcall)
pvcam cameras (microscope/cameras/pvcam.py) yes no (PV_DECL expands to __stdcall on Windows)
andor SOLIS (microscope/cameras/atmcd.py) yes maybe (uses WINAPI on one version but nothing on other)
andor SDK3 cameras (microscope/cameras/_SDK3.py) yes no (AT_EXP_CONV expands to WINAPI)
linkam stage (microscope/stages/linkam.py) yes probably

However, I no longer have access to any of these. Could someone who does have access test it?

@dstoychev
Copy link
Collaborator

Tested the ALPAO mirror wrapper. Replacing WinDLL with CDLL on line 29 works fine!

SDK = ctypes.WinDLL("ASDK")

@carandraug
Copy link
Collaborator Author

Thank you for testing Danny. I've pushed e837dfc. I've also asked @thomasmfish by email whether he could check the Linkam stage.

@iandobbie
Copy link
Collaborator

Tom Fish says that Diamond is using CDLL for the linkam stage there and it works for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants