Skip to content

Conversation

vulpes2
Copy link

@vulpes2 vulpes2 commented Feb 12, 2025

Need some help on getting the conditional include to work, this obviously will not compile on non-darwin platforms.

  1. The conditional compilation feature has been deprecated in cython so we can't use that anymore.
  2. The recommended alternative (verbatim C macros) is not the most elegant solution since it still forces you to define things like int hid_darwin_get_open_exclusive() on all platforms in python.

Fixes #178

@bearsh
Copy link
Contributor

bearsh commented Feb 13, 2025

what about putting platform specific functions into a platform specific submodule?

@prusnak
Copy link
Member

prusnak commented Feb 13, 2025

I think we should just do this change on all platforms not only on macos

include_dirs=[embedded_hidapi_include, embedded_hidapi_macos_include],

and cython will take care of it correctly = include functions if they can be resolved and not if they cannot.

I am going to push another commit into the PR branch to test the hypothesis

@prusnak
Copy link
Member

prusnak commented Feb 13, 2025

Hm, ok, that didn't work :-)

let's get back to the drawing board

@bearsh
Copy link
Contributor

bearsh commented Feb 13, 2025

AFAIK cython does not resolve symbols but only creates c code. so if a function is used in cython code, the linker expects it to be present somewhere which is not the case for platform specific functions in libusb as it does not provide stubs for it.

I think the cleanest solution is to add a darwin subpackage. this could also be done for other platform specific functions.
I a common api is required, a python wrapper could be added.

@prusnak
Copy link
Member

prusnak commented Feb 13, 2025

I think the cleanest solution is to add a darwin subpackage.

I agree this is the cleanest solution, but not sure if it is also a preferred one.

ChatGPT says we can use this trick:

in the .pyx file:

DEF IS_MACOS = False

IF IS_MACOS:
    cpdef macos_specific_function():
        # Implementation using macOS APIs
        print("This is macOS-specific")
ELSE:
    cpdef macos_specific_function():
        raise NotImplementedError("macos_specific_function is available only on macOS")

in the setup.py:

is_macos = sys.platform == "darwin"

extensions = [
    Extension("mymodule",
              ["mymodule.pyx"],
              define_macros=[("IS_MACOS", "1")] if is_macos else [])
]

@vulpes2 will you update your PR to try this trick?

@bearsh
Copy link
Contributor

bearsh commented Feb 13, 2025

Conditional compilation is deprecated and should not be used (as @vulpes2 mentioned in #194 (comment)): https://docs.cython.org/en/latest/src/userguide/migrating_to_cy30.html#deprecated-def-if

@prusnak
Copy link
Member

prusnak commented Feb 13, 2025

Conditional compilation is deprecated and should not be used (as @vulpes2 mentioned in #194 (comment)): https://docs.cython.org/en/latest/src/userguide/migrating_to_cy30.html#deprecated-def-if

Right. If @vulpes2 wants to create a new model - please do so. And see how complex the change is.

@vulpes2
Copy link
Author

vulpes2 commented Feb 14, 2025

Not sure if it's possible to split these into a platform-specific subpackage, will investigate in that approach.

@vulpes2
Copy link
Author

vulpes2 commented Feb 14, 2025

This has turned out to be incredibly complicated due to my lack of experience in python packaging, spent a few hours trying and couldn't figure it out. Here are some main issues that I ran into and couldn't resolve, maybe someone with more python packaging experience can provide some pointers.

  1. Ideally the darwin-specific function declarations can be moved to darwin/darwin_hid.pxd, and darwin/darwin_hid.pyx can have a class that inherits from the base device class. I couldn't figure out how to make this a submodule, maybe I need to declare it as another Extension? Adding the new .pyx file in sources in the existing Extension seems to have no effect.
  2. An __init__.py might be required to make sure hid.device automatically imports the extended device class in darwin/darwin_hid.pyx on darwin instead of the base class. I can't get that to run because it's not included in the final installation. Attempting to include it in MANIFEST.in and/or inserting it at various places in setup.py either have no effect or results in build failures.
  3. There are no good working examples on how to set this up with cython specifically. Tried checking out a few other projects that use cython like sklearn or pillow, they're all super complicated and weren't helpful at all.

@vulpes2
Copy link
Author

vulpes2 commented Jul 2, 2025

After a bit of time I finally figured this out. The documentation was terrible so it's not so obvious at first glance. Will push my changes in a bit.

@vulpes2 vulpes2 marked this pull request as ready for review July 2, 2025 23:24
@vulpes2
Copy link
Author

vulpes2 commented Jul 2, 2025

This is ready for review now, I've tested this on macOS and linux. The Darwin-specific functions are not defined when it's built on linux, and setting exclusive access to 0 on macOS will make it possible to open HID stuff that are being used by the system now. @prusnak

@vulpes2
Copy link
Author

vulpes2 commented Jul 2, 2025

IF is technically deprecated but there really isn't any straightforward way to do this. At the moment it's still working fine, so we should probably just make it work for now, and deal with that deprecation issue at a later date.

@bearsh
Copy link
Contributor

bearsh commented Jul 8, 2025

I think I've hacked a possible solution without IF. it platform specific subclasses which then gets exported as hid.device. I just need to clean it up a little but and push it somewhere...

@bearsh
Copy link
Contributor

bearsh commented Jul 15, 2025

I think I've hacked a possible solution without IF. it platform specific subclasses which then gets exported as hid.device. I just need to clean it up a little but and push it somewhere...

here a first version master...bearsh:cython-hidapi:platform-subclasses

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Darwin: enable non-exclusive device usage

3 participants