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

Simplified conditional compilation #138

Merged
merged 5 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ windows-native = [

[dependencies]
libc = "0.2"
cfg-if = "1"

[target.'cfg(target_os = "linux")'.dependencies]
udev = { version = "0.7", optional = true }
Expand Down
116 changes: 50 additions & 66 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,48 +58,68 @@
//! [`HidDevice`] handles can access the same physical device. For backward compatibility this is
//! an opt-in that can be enabled with the `macos-shared-device` feature flag.

extern crate libc;
#[cfg(all(feature = "linux-native", target_os = "linux"))]
extern crate nix;

#[cfg(target_os = "windows")]
use windows_sys::core::GUID;

mod error;
mod ffi;

#[cfg(hidapi)]
mod hidapi;

#[cfg(all(feature = "linux-native", target_os = "linux"))]
#[cfg_attr(docsrs, doc(cfg(all(feature = "linux-native", target_os = "linux"))))]
mod linux_native;
#[cfg(target_os = "macos")]
#[cfg_attr(docsrs, doc(cfg(target_os = "macos")))]
mod macos;
#[cfg(target_os = "windows")]
#[cfg_attr(docsrs, doc(cfg(target_os = "windows")))]
mod windows;

#[cfg(feature = "windows-native")]
#[cfg_attr(docsrs, doc(cfg(all(feature = "windows-native", target_os = "windows"))))]
mod windows_native;

use libc::wchar_t;
use std::ffi::CStr;
use std::ffi::CString;
use std::fmt;
use std::fmt::Debug;
use std::sync::Mutex;
use cfg_if::cfg_if;

pub use error::HidError;

#[cfg(hidapi)]
use crate::hidapi::HidApiBackend;
#[cfg(all(feature = "linux-native", target_os = "linux"))]
use linux_native::HidApiBackend;
#[cfg(all(feature = "windows-native", target_os = "windows"))]
use windows_native::HidApiBackend;
cfg_if! {
if #[cfg(all(feature = "linux-native", target_os = "linux"))] {
//#[cfg_attr(docsrs, doc(cfg(all(feature = "linux-native", target_os = "linux"))))]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why comment out these cfg_attrs, and not the ones below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The attribute is used to document that a certain function is limited to certain configurations:
image

The native backends should not appear to the public api and therefore the attribute seemed pretty useless. However I just commented them out for now to make it easier to reenable them in case I am wrong and they did actually serve a purpose that I missed.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think you are right. We can just remove them completely, since its backend code.

mod linux_native;
use linux_native::HidApiBackend;
} else if #[cfg(all(feature = "windows-native", target_os = "windows"))] {
//#[cfg_attr(docsrs, doc(cfg(all(feature = "windows-native", target_os = "windows"))))]
mod windows_native;
use windows_native::HidApiBackend;
} else if #[cfg(hidapi)] {
mod hidapi;
use hidapi::HidApiBackend;
} else {
compile_error!("No backend selected");
}
}

// Automatically implement the top trait
cfg_if! {
if #[cfg(target_os = "windows")] {
#[cfg_attr(docsrs, doc(cfg(target_os = "windows")))]
mod windows;
use windows::GUID;
/// A trait with the extra methods that are available on Windows
trait HidDeviceBackendWindows {
/// Get the container ID for a HID device
fn get_container_id(&self) -> HidResult<GUID>;
}
trait HidDeviceBackend: HidDeviceBackendBase + HidDeviceBackendWindows + Send {}
impl<T> HidDeviceBackend for T where T: HidDeviceBackendBase + HidDeviceBackendWindows + Send {}
} else if #[cfg(target_os = "macos")] {
#[cfg_attr(docsrs, doc(cfg(target_os = "macos")))]
mod macos;
/// A trait with the extra methods that are available on macOS
trait HidDeviceBackendMacos {
/// Get the location ID for a [`HidDevice`] device.
fn get_location_id(&self) -> HidResult<u32>;

/// Check if the device was opened in exclusive mode.
fn is_open_exclusive(&self) -> HidResult<bool>;
}
trait HidDeviceBackend: HidDeviceBackendBase + HidDeviceBackendMacos + Send {}
impl<T> HidDeviceBackend for T where T: HidDeviceBackendBase + HidDeviceBackendMacos + Send {}
} else {
trait HidDeviceBackend: HidDeviceBackendBase + Send {}
impl<T> HidDeviceBackend for T where T: HidDeviceBackendBase + Send {}
}
}


pub type HidResult<T> = Result<T, HidError>;
pub const MAX_REPORT_DESCRIPTOR_SIZE: usize = 4096;
Expand Down Expand Up @@ -467,42 +487,6 @@ trait HidDeviceBackendBase {
}
}

/// A trait with the extra methods that are available on macOS
#[cfg(target_os = "macos")]
trait HidDeviceBackendMacos {
/// Get the location ID for a [`HidDevice`] device.
fn get_location_id(&self) -> HidResult<u32>;

/// Check if the device was opened in exclusive mode.
fn is_open_exclusive(&self) -> HidResult<bool>;
}

/// A trait with the extra methods that are available on macOS
#[cfg(target_os = "windows")]
trait HidDeviceBackendWindows {
/// Get the container ID for a HID device
fn get_container_id(&self) -> HidResult<GUID>;
}

#[cfg(not(any(target_os = "macos", target_os = "windows")))]
trait HidDeviceBackend: HidDeviceBackendBase + Send {}
#[cfg(target_os = "macos")]
trait HidDeviceBackend: HidDeviceBackendBase + HidDeviceBackendMacos + Send {}
#[cfg(target_os = "windows")]
trait HidDeviceBackend: HidDeviceBackendBase + HidDeviceBackendWindows + Send {}

/// Automatically implement the top trait
#[cfg(not(any(target_os = "macos", target_os = "windows")))]
impl<T> HidDeviceBackend for T where T: HidDeviceBackendBase + Send {}

/// Automatically implement the top trait
#[cfg(target_os = "macos")]
impl<T> HidDeviceBackend for T where T: HidDeviceBackendBase + HidDeviceBackendMacos + Send {}

/// Automatically implement the top trait
#[cfg(target_os = "windows")]
impl<T> HidDeviceBackend for T where T: HidDeviceBackendBase + HidDeviceBackendWindows + Send {}

pub struct HidDevice {
inner: Box<dyn HidDeviceBackend>,
}
Expand Down
2 changes: 1 addition & 1 deletion src/windows.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use windows_sys::core::GUID;
pub use windows_sys::core::GUID;
use crate::{HidDevice, HidResult};

impl HidDevice {
Expand Down
8 changes: 4 additions & 4 deletions src/windows_native/descriptor/encoder.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::windows_native::descriptor::typedefs::{Caps, LinkCollectionNode};
use crate::windows_native::descriptor::types::{ItemNodeType, Items, MainItemNode, MainItems};
use crate::windows_native::error::{WinError, WinResult};
use crate::windows_native::utils::PeakIterExt;
use super::typedefs::{Caps, LinkCollectionNode};
use super::types::{ItemNodeType, Items, MainItemNode, MainItems};
use super::super::error::{WinError, WinResult};
use super::super::utils::PeakIterExt;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not really a fan of relative paths for imports, when its necessary to go up multiple levels.
Its much clearer with one glance from where the item was imported with the absolute path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this change to keep the backend more self-contained and not depend on the name it was imported under. My original idea was to do something like this:

#[cfg_attr(feature = "linux-native", path = "linux_native.rs")]
#[cfg_attr(feature = "windows-native", path = "windows_native/mod.rs")]
mod platform;

Which obviously broke those imports.

After changing my approach to cfg_if I decided to nevertheless keep it. I can revert the paths if you want.


pub fn encode_descriptor(main_item_list: &[MainItemNode], caps_list: &[Caps], link_collection_nodes: &[LinkCollectionNode]) -> WinResult<Vec<u8>> {
// ***********************************
Expand Down
13 changes: 6 additions & 7 deletions src/windows_native/descriptor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@ mod tests;
use std::collections::HashMap;
use std::ffi::c_void;
use std::slice;
use crate::ensure;
use crate::windows_native::descriptor::encoder::encode_descriptor;
use crate::windows_native::descriptor::typedefs::{Caps, HidpPreparsedData, LinkCollectionNode};
use crate::windows_native::descriptor::types::{BitRange, ItemNodeType, MainItemNode, MainItems, ReportType};
use crate::windows_native::error::{WinError, WinResult};
use crate::windows_native::hid::PreparsedData;
use crate::windows_native::utils::PeakIterExt;
use encoder::encode_descriptor;
use typedefs::{Caps, HidpPreparsedData, LinkCollectionNode};
use types::{BitRange, ItemNodeType, MainItemNode, MainItems, ReportType};
use super::error::{WinError, WinResult};
use super::hid::PreparsedData;
use super::utils::PeakIterExt;


pub fn get_descriptor(pp_data: &PreparsedData) -> WinResult<Vec<u8>> {
Expand Down
2 changes: 1 addition & 1 deletion src/windows_native/descriptor/tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::fs::read_to_string;
use crate::windows_native::descriptor::get_descriptor_ptr;
use super::get_descriptor_ptr;

#[test]
fn test_01() {
Expand Down
2 changes: 1 addition & 1 deletion src/windows_native/descriptor/typedefs.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::mem::size_of;
use crate::windows_native::descriptor::types::BitRange;
use super::types::BitRange;

// Reverse engineered typedefs for the internal structure of the preparsed data taken from
// https://github.com/libusb/hidapi/blob/master/windows/hidapi_descriptor_reconstruct.h
Expand Down
7 changes: 3 additions & 4 deletions src/windows_native/dev_node.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use std::ptr::null_mut;
use windows_sys::Win32::Devices::DeviceAndDriverInstallation::{CM_Get_DevNode_PropertyW, CM_Get_Parent, CM_LOCATE_DEVNODE_NORMAL, CM_Locate_DevNodeW, CR_BUFFER_SMALL, CR_SUCCESS};
use crate::ensure;
use crate::windows_native::error::{check_config, WinError, WinResult};
use crate::windows_native::string::U16Str;
use crate::windows_native::types::{DeviceProperty, PropertyKey};
use super::error::{check_config, WinError, WinResult};
use super::string::U16Str;
use super::types::{DeviceProperty, PropertyKey};

#[repr(transparent)]
#[derive(Debug, Copy, Clone, Eq, PartialEq)]
Expand Down
12 changes: 6 additions & 6 deletions src/windows_native/device_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ use windows_sys::Win32::Devices::Properties::{DEVPKEY_Device_CompatibleIds, DEVP
use windows_sys::Win32::Foundation::{BOOLEAN, HANDLE};
use windows_sys::Win32::Storage::EnhancedStorage::{PKEY_DeviceInterface_Bluetooth_DeviceAddress, PKEY_DeviceInterface_Bluetooth_Manufacturer, PKEY_DeviceInterface_Bluetooth_ModelNumber};
use crate::{BusType, DeviceInfo, WcharString};
use crate::windows_native::dev_node::DevNode;
use crate::windows_native::error::WinResult;
use crate::windows_native::hid::{get_hid_attributes, PreparsedData};
use crate::windows_native::interfaces::Interface;
use crate::windows_native::string::{U16Str, U16String, U16StringList};
use crate::windows_native::types::{Handle, InternalBusType};
use super::dev_node::DevNode;
use super::error::WinResult;
use super::hid::{get_hid_attributes, PreparsedData};
use super::interfaces::Interface;
use super::string::{U16Str, U16String, U16StringList};
use super::types::{Handle, InternalBusType};

fn read_string(func: unsafe extern "system" fn (HANDLE, *mut c_void, u32) -> BOOLEAN, handle: &Handle) -> WcharString {
//Return empty string on failure to match the c implementation
Expand Down
5 changes: 2 additions & 3 deletions src/windows_native/hid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ use std::ffi::c_void;
use std::mem::{size_of, zeroed};
use windows_sys::core::GUID;
use windows_sys::Win32::Devices::HumanInterfaceDevice::{HIDD_ATTRIBUTES, HidD_FreePreparsedData, HidD_GetAttributes, HidD_GetHidGuid, HidD_GetPreparsedData, HIDP_CAPS, HidP_GetCaps, HIDP_STATUS_SUCCESS};
use crate::ensure;
use crate::windows_native::error::{check_boolean, WinError, WinResult};
use crate::windows_native::types::Handle;
use super::error::{check_boolean, WinError, WinResult};
use super::types::Handle;

pub fn get_interface_guid() -> GUID {
unsafe {
Expand Down
9 changes: 4 additions & 5 deletions src/windows_native/interfaces.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
use std::ptr::{null, null_mut};
use windows_sys::core::GUID;
use windows_sys::Win32::Devices::DeviceAndDriverInstallation::{CM_GET_DEVICE_INTERFACE_LIST_PRESENT, CM_Get_Device_Interface_List_SizeW, CM_Get_Device_Interface_ListW, CM_Get_Device_Interface_PropertyW, CR_BUFFER_SMALL, CR_SUCCESS};
use crate::ensure;
use crate::windows_native::error::{check_config, WinError, WinResult};
use crate::windows_native::hid::get_interface_guid;
use crate::windows_native::string::{U16Str, U16StringList};
use crate::windows_native::types::{DeviceProperty, PropertyKey};
use super::error::{check_config, WinError, WinResult};
use super::hid::get_interface_guid;
use super::string::{U16Str, U16StringList};
use super::types::{DeviceProperty, PropertyKey};

pub struct Interface;

Expand Down
36 changes: 17 additions & 19 deletions src/windows_native/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
//! The implementation which uses the C library to perform operations
//! The implementation which directly uses the win32 api to perform operations

macro_rules! ensure {
($cond:expr, $result:expr) => {
if !($cond) {
return $result;
}
};
}

mod types;
mod error;
Expand All @@ -24,27 +32,17 @@ use windows_sys::Win32::Foundation::{GENERIC_READ, GENERIC_WRITE, INVALID_HANDLE
use windows_sys::Win32::Storage::FileSystem::{CreateFileW, FILE_FLAG_OVERLAPPED, FILE_SHARE_READ, FILE_SHARE_WRITE, OPEN_EXISTING, ReadFile, WriteFile};
use windows_sys::Win32::System::IO::{CancelIo, DeviceIoControl};
use windows_sys::Win32::System::Threading::ResetEvent;
use crate::{DeviceInfo, HidDeviceBackendBase, HidDeviceBackendWindows, HidError, HidResult};
use crate::windows_native::dev_node::DevNode;
use crate::windows_native::device_info::get_device_info;
use crate::windows_native::error::{check_boolean, Win32Error, WinError, WinResult};
use crate::windows_native::hid::{get_hid_attributes, PreparsedData};
use crate::windows_native::interfaces::Interface;
use crate::windows_native::string::{U16Str, U16String};
use crate::windows_native::types::{Handle, Overlapped};
use super::{DeviceInfo, HidDeviceBackendBase, HidDeviceBackendWindows, HidError, HidResult};
use dev_node::DevNode;
use device_info::get_device_info;
use error::{check_boolean, Win32Error, WinError, WinResult};
use hid::{get_hid_attributes, PreparsedData};
use interfaces::Interface;
use string::{U16Str, U16String};
use types::{Handle, Overlapped};

const STRING_BUF_LEN: usize = 128;

#[macro_export]
macro_rules! ensure {
($cond:expr, $result:expr) => {
if !($cond) {
return $result;
}
};
}


pub struct HidApiBackend;
impl HidApiBackend {
pub fn get_hid_device_info_vector(vid: u16, pid: u16) -> HidResult<Vec<DeviceInfo>> {
Expand Down
2 changes: 1 addition & 1 deletion src/windows_native/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use std::str::Utf8Error;
use windows_sys::core::PCWSTR;
use windows_sys::Win32::Devices::Properties::{DEVPROP_TYPE_STRING, DEVPROP_TYPE_STRING_LIST, DEVPROPTYPE};
use crate::WcharString;
use crate::windows_native::types::DeviceProperty;
use super::types::DeviceProperty;

#[repr(transparent)]
pub struct U16Str([u16]);
Expand Down
4 changes: 2 additions & 2 deletions src/windows_native/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use windows_sys::Win32::Foundation::{CloseHandle, FALSE, HANDLE, INVALID_HANDLE_
use windows_sys::Win32::System::IO::{GetOverlappedResultEx, OVERLAPPED};
use windows_sys::Win32::System::Threading::{CreateEventW, INFINITE};
use windows_sys::Win32::UI::Shell::PropertiesSystem::PROPERTYKEY;
use crate::{BusType, ensure};
use crate::windows_native::error::{WinError, WinResult};
use crate::BusType;
use super::error::{WinError, WinResult};

#[allow(clippy::missing_safety_doc)]
pub unsafe trait DeviceProperty {
Expand Down
Loading