Skip to content

Fix: Added rfkill support as a feature flag #298

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

manthanabc
Copy link
Contributor

Combines this commit commit and #254 .
works on arch linux with mediatek MT7921 wifi module.
wifi stops resting as expected with --no-default-features flags, although i am not able to test with a rfkill switch yet

@manthanabc manthanabc marked this pull request as draft March 8, 2025 22:44
@Shinyzenith
Copy link
Member

Looks good, would you mind adding some relevant documentation for this flag too?

@manthanabc manthanabc marked this pull request as ready for review March 19, 2025 22:37
@Shinyzenith
Copy link
Member

The quoted commit seems to be gone. That's odd.

@manthanabc
Copy link
Contributor Author

The quoted commit seems to be gone. That's odd.

It was accessible just a few days ago, was similar to this pr moved rfkill support to a feature flag. anyway should'nt be too important since the changes are identical

@Shinyzenith
Copy link
Member

Instead of NO_DEFAULT_FEATURES can we just make the flag NO_RFKILL_SW_SUPPORT? Because we might have a different set of defaults down the line and I want the make packagers life as easy as possible

@manthanabc
Copy link
Contributor Author

Instead of NO_DEFAULT_FEATURES can we just make the flag NO_RFKILL_SW_SUPPORT? Because we might have a different set of defaults down the line and I want the make packagers life as easy as possible

I don't think we can have such flag without changing the features to no_rfkill_support but according to cargo book features are intended to be additive only, link.
otherwise dependancies may break (But should'nt really be a problem in case of swhkd ig 🤔)

Otherwise we could add another feature which includes all features other than rfkill, called no_rfkill_sw_support but that would be redundant

@Shinyzenith
Copy link
Member

If they're additive can we add ENABLE_RFKILL_SW_SUPPORT?

@manthanabc
Copy link
Contributor Author

If they're additive can we add ENABLE_RFKILL_SW_SUPPORT?

That would be identical to current changes, the core issues is

We either add rfkill switch support by default (current approach), requiring arch users to pass --no-default followed by all other defaults they wish to have (As far as i can tell this is the only way to opt out of a default feature in cargo)

Or we make support of rfkill switch disabled by default, making rfkill switch support opt out by default for all users.

Or we just ignore the cargo guide, and go with NO_RFKILL_SW_SUPPORT. This should not cause any issues since no other crate uses swhkd as dependency 😅

I think just going with the third option would be cleanest for now

@Shinyzenith
Copy link
Member

Sounds good, I'm fine with approach 3

Copy link
Member

@Shinyzenith Shinyzenith left a comment

Choose a reason for hiding this comment

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

@newtoallofthis123 Can you double check things before I go ahead with the merge?

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

Successfully merging this pull request may close these issues.

3 participants