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

Support for v2 #10

Merged
merged 7 commits into from
Jan 6, 2025
Merged

Support for v2 #10

merged 7 commits into from
Jan 6, 2025

Conversation

cmeissl
Copy link
Collaborator

@cmeissl cmeissl commented Jan 3, 2025

First commit changes the current version (bumping it to 0.1.1) to resolve the native lib using pkg-config and limiting it to 0.1 if the native lib.

Second commit adds support for 0.2, which can explicitly be enabled with a v0_2 feature flag.
In addition the build script enabled linking against 0.2 when the version is detected.
The safe wrapper should handle the i32 <> i64 compatibility just fine and only expose 0.2 features when
the corresponding feature flag is enabled.

Most things are just additions or compatible changes with the exception of the data type of max_pixel_clock_hz in the di_edid_display_range_limits.

To solve this the -sys crate exports the type depending on the actual found native lib version like this:

#[cfg(not(feature = "v0_2"))]
pub type di_edid_display_range_limits_max_pixel_clock_hz = i32;
#[cfg(feature = "v0_2")]
pub type di_edid_display_range_limits_max_pixel_clock_hz = i64;

The safe wrapper will now always use i64 and just use i64::from to convert from the di_edid_display_range_limits_max_pixel_clock_hz value.

TODO

  • Add new CTA types and functions

fixes #9

instead of just linking to libdisplay-info use system-deps,
which will use pkg-config internally, to resolve the native
lib.
this also limits the version to the currently supported one
@cmeissl cmeissl force-pushed the feature/v2 branch 5 times, most recently from 901c059 to eee9743 Compare January 3, 2025 22:18
@cmeissl cmeissl marked this pull request as ready for review January 3, 2025 22:49
@cmeissl cmeissl requested review from ids1024 and Drakulix January 3, 2025 22:49
let deps = system_deps::Config::new().probe().unwrap();
let native_lib = deps.get_by_name("libdisplay-info").unwrap();
let native_version = semver::Version::parse(&native_lib.version).unwrap();
let has_v2 = semver::VersionReq::parse(">=0.2")
Copy link
Member

Choose a reason for hiding this comment

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

Should we error here on >= 0.3, assuming that would be another breaking change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, that is probably a good idea. added a commit to restrict the max version directly in the system_deps

Copy link
Member

@Drakulix Drakulix left a comment

Choose a reason for hiding this comment

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

LGTM!

@cmeissl cmeissl merged commit 367fda6 into main Jan 6, 2025
5 checks passed
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.

Support libdisplay-info 2.0 (or at least don't link against it incorrectly)
3 participants