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.

Sign up for GitHub

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

Jump to bottom

armbian-bsp-cli: fix 10-wifi-disable-powermanagement.rules #7130

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

armbian-bsp-cli: fix 10-wifi-disable-powermanagement.rules #7130

wants to merge 6 commits into from

Conversation

alexl83
Copy link
Contributor

@alexl83 alexl83 commented Aug 23, 2024 •
edited
Loading

Description

armbian-bsp-cli: fix 10-wifi-disable-powermanagement.rules

  • corrected iw path
  • made rule better targeted at wifi devices
  • switched from %k (kernel) to $name variable to better integrate with user-provided rules

How Has This Been Tested?

  • Tested new rule with actual wifi devices --> udeavdm test /sys/class/net/wlan0 output:
Queued commands:
       RUN{program} : /usr/sbin/iw dev wlan0 set power_save off

Checklist:

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings

@alexl83
armbian-bsp-cli: fix 10-wifi-disable-powermanagement.rules
0308243 
 - corrected "iw" path
 - made rule better targeted at wifi devices
 - switched from %k (kernel) to $name variable to better integrate with user-provided rules
@alexl83 alexl83 requested a review from a team as a code owner August 23, 2024 20:40
@github-actions github-actions bot added size/small PR with less then 50 lines BSP Board Support Packages labels Aug 23, 2024
@alexl83 alexl83 changed the title armbian-bsp-cli: fix 10-wifi-disable-powermanagement.rules armbian-bsp-cli: fix 10-wifi-disable-powermanagement.rules Aug 23, 2024
@alexl83 alexl83 force-pushed the fix_wifi_powermanagenet_rules branch from 6124132 to d45f5f0 Compare August 24, 2024 21:42
igorpecovnik
igorpecovnik reviewed Aug 27, 2024
View reviewed changes
packages/bsp/common/etc/udev/rules.d/10-wifi-disable-powermanagement.rules
@@ -1 +1 @@
KERNEL=="wlan*", ACTION=="add", RUN+="/sbin/iw dev %k set power_save off"
SUBSYSTEM=="net", ACTION!="remove", ENV{DEVTYPE}=="wlan", RUN+="/usr/sbin/iw dev $name set power_save off"
Copy link
Member

Choose a reason for hiding this comment

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

Is path actually needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it wouldn't find iw without path - probably something related to udev sandboxing or some restriction of sorts

Copy link
Member

Choose a reason for hiding this comment

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

OK. got it. Other problem could be that different userland have different location - check for jammy, noble, bookworm, trixie

Copy link
Contributor Author

@alexl83 alexl83 Aug 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

OK. got it. Other problem could be that different userland have different location - check for jammy, noble, bookworm, trixie

You're right Pal, bookworm and Jammy have /sbin/iw but rule without absolute path won't work
need to find a way..

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, classic :)

@igorpecovnik
Copy link
Member

What to do with this? extend to cover both locations?

@alexl83
Copy link
Contributor Author

alexl83 commented Sep 15, 2024 •
edited
Loading

What to do with this? extend to cover both locations?

Bookworm and Trixie have different prefixes also for sh - /bin/sh and /usr/bin/sh so even calling sh -c wouldn't solve
I'm thinking about adding an helper script but I'm not a fan; let me convert this PR to draft, I'll come up with some ideas...

@alexl83 alexl83 marked this pull request as draft September 15, 2024 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@igorpecovnik igorpecovnik igorpecovnik left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
BSP Board Support Packages size/small PR with less then 50 lines
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@alexl83 @igorpecovnik