Skip to content

Add adapter power on/off functionality - #31

Closed
DreamTone wants to merge 9 commits into
e-tho:mainfrom
DreamTone:add_adapter_power
Closed

Add adapter power on/off functionality#31
DreamTone wants to merge 9 commits into
e-tho:mainfrom
DreamTone:add_adapter_power

Conversation

@DreamTone

Copy link
Copy Markdown
Contributor

I'm submitting a pull request to add adapter power on/off functionality to the project. The pull request is a little large as it was pretty involved and required a degree of modifying existing functionality as well as adding new functionality. I've maintained the existing project patterns the best I can and broke down the commits into reasonable chunks so it's easier to follow.

The general idea for adding this pull request is that iwdcan control the power of the adapter (physical) and enable/disable the Wi-Fi/AP device (logical). Leaving the adapter power out of the project means the user has to iwctl adapter <name> set-property Powered on or off manually. If the project adds this functionality it's a ease-of-use upgrade, helps the project to more comprehensively align with iwctl's functionality and allows iwmenu to operate properly regardless of the adapter and device states.

The general approach was as follows :

  1. Differentiate the adapter and the device within the project by explicitly referring to "Powering On/Off the Adapter" and "Enabling/Disabling the Wi-Fi/AP Device". This alludes to the user and within the code that the device is logical and the adapter is physical without having to explain it.
  2. Add an adapter menu which has the option "Power On Adapter", which performs the function then proceeds to the device menu (if the device is disabled) or to the regular menus (if the device is enabled).
  3. Modify the settings menu to have the option to "Power Off Adapter", which performs the function and proceeds to the adapter menu.
  4. Modify the device menu to have the option to "Power Off Adapter", which performs the function and proceeds to the adapter menu.
  5. Handle "Power Off Adapter -> Power On Adapter -> Power Off Adapter..." loop to ensure the user can't enter the regular menus unless both the adapter is powered on and the device is enabled.
  6. Add logic to the beginning of run to handle launching iwmenu regardless of the adapter and device states.
  7. Ensure pressing Esc or losing focus exits gracefully.

Possible behaviors to address in future commits:

  • The behavior without the --interaction flag may need some conditionals added (for self.running or stay_in_settings_menu). The functionality works fine, but you may prefer the behavior to exit after "Power Off Adapter" is selected.
  • After disabling the device from the settings menu, enabling the device when "Station Mode" is on, navigates to the settings menu, where as enabling the device when "AP Mode" is on, navigates to the main menu. I haven't dug into this yet, but I think it's existing behavior. Hence referring to "normal menus" in the general approach.
  • May need to add a behavior to provide a notification if "Power On Adapter" fails to let the user know to check for and turn on any physical network kill switches (this is an assumption, I don't have a physical kill switch so I don't know the behavior).

I'd appreciate any feedback. Let me know if you have any questions about the commits.

@e-tho

e-tho commented Aug 28, 2026

Copy link
Copy Markdown
Owner

First of all, that's not how you split commits. A commit should be a logical change, a step in the development process that leaves the project in a working state. I should be able to build the project successfully at each of your commits.

Secondly, since you've mentioned multiple times that you're a beginner and seeing how verbose this PR message is, it is quite obvious that you rely heavily on AI assistance. I don't blame you for that, but it is important to understand everything that's produced, and since as a beginner you probably don't, I don't think handling significant features yourself is the best way to help on open source projects at this point. Filling well-documented issues or providing targeted fixes is a much better way to help, since it avoids introducing bugs, making bad architectural choices, and burdening maintainers with code reviews. Learning through experience is better done in personal projects.

I also mentioned to you that device-level control is fine for iwmenu; it's a handy frontend, its scope isn't to replicate every setting of iwctl. Users who specifically want a hardware switch obviously know how to use the command line. I'm not against adding this feature, but at least filling an issue to talk about it before implementing it would be a better way to proceed.

@DreamTone

Copy link
Copy Markdown
Contributor Author

Ah apologies. Thank you for the feedback.

I aimed to keep the commits buildable. I just learned I should do linting checks prior to commits. I also see I should split them more logically.

And yes, this stretched me well beyond my comfort zone. However, I developed this by looking at the existing structures in the project and debugging constantly. So if it appears amature-like, that's because I tried my best to not to stretch beyond what I could reference from given my limited knowledge.

Next time I'll stick to documenting an issue first. Would providing a link to a forked branch in an issue be considered inappropriate?

@e-tho

e-tho commented Aug 28, 2026

Copy link
Copy Markdown
Owner

Providing a link to a forked branch would amount to the same thing, you're generating code you don't understand and asking someone qualified to review it. I'm not your teacher; most open-source maintainers already do this in their spare time, you're just adding to their burden. I think you should just study, learn best practices, and gain experience on personal projects. Open pull requests only when you're confident in the quality of the code you're producing and understand every aspect of it.

@e-tho e-tho closed this Aug 28, 2026
@DreamTone
DreamTone deleted the add_adapter_power branch August 31, 2026 00:00
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.

2 participants