nmcli: two fixes needed to make wifi.wake-on-wlan settings work properly #5431
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ansibullbot
added
bug
felixfontein
added
check-before-release
|
Thanks for your contribution! Can you please add a changelog fragment and potentially also add tests for this (to avoid regressions)? The tests can be found in tests/unit/plugins/modules/net_tools/test_nmcli.py. Thanks! |
|
I added ChangeLog fragments. I unfortunately don't have time to deal with tests. |
ansibullbot
added
needs_ci
felixfontein
changed the title
|
I've changed the fragments to conform to our standard. I don't use or know nmcli so I hope someone with more knowledge will review the code changes. |
|
Please note that in #5461 the collection repository was restructured to remove the directory tree in plugins/modules/, and the corresponding tree in tests/unit/plugins/modules/. Your PR modifies files in this directory structure, and unfortunately now has some conflicts, potentially because of this. Please rebase with the current |
ansibullbot
added
needs_rebase
|
Rebased. |
ansibullbot
removed
needs_rebase
|
@ThomasGebert I cleaned up the earlier code and added two more commits because I found two more bugs while doing final testing on this. :-/ Can you take another look? |
jikamens
added a commit
to jikamens/community.general
that referenced
this pull request
Feb 22, 2023
ansibullbot
added
the
needs_revision
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
ThomasGebert
suggested changes
Feb 23, 2023
|
I think with the addresses change pulled onto a different branch this PR is now ready to merge. |
ansibullbot
added
needs_revision
|
@ThomasGebert if you're happy I'll merge. |
ansibullbot
removed
the
needs_revision
felixfontein
removed
the
check-before-release
Backport to stable-5: 💚 backport PR created✅ Backport PR branch: Backported as #6050 🤖 @patchback |
patchback bot
pushed a commit
that referenced
this pull request
Feb 24, 2023
…rly (#5431) * nmcli: Convert current value of wifi.wake-on-wlan before comparing The new value of wifi.wake-on-wlan is specified as an integer, but in the nmcli output it's specified as a hex string followed by a textual description of it. Therefore, to determine properly whether it's being changed we need to pull the hex string out of the current value, convert it into an integer, and finally convert the integer back to a string so that we can compare it to the new specified value. Without this change, whenever wifi.wake-on-wlan is specified in the module arguments the module will think the value is being changed even when it isn't. * nmcli: Handle wifi options correctly when connection type not specified When an nmcli task does not specify the connection type and the module ask nmcli for it, the module needs to convert nmcli's `802-11-wireless` to `wifi`, the term for this connection type used by the module. * nmcli: Correctly detect values changed to the integer 0 If the user specifies a value of 0 (without quotes) in a task, we should interpret that as an actual value, not empty, when comparing the new value to the old one. Otherwise we incorrectly conclude that there was no change. * Changelog fragment for #5431 (cherry picked from commit 490899f)
Backport to stable-6: 💚 backport PR created✅ Backport PR branch: Backported as #6051 🤖 @patchback |
|
@jikamens thanks a lot for fixing this! |
patchback bot
pushed a commit
that referenced
this pull request
Feb 24, 2023
…rly (#5431) * nmcli: Convert current value of wifi.wake-on-wlan before comparing The new value of wifi.wake-on-wlan is specified as an integer, but in the nmcli output it's specified as a hex string followed by a textual description of it. Therefore, to determine properly whether it's being changed we need to pull the hex string out of the current value, convert it into an integer, and finally convert the integer back to a string so that we can compare it to the new specified value. Without this change, whenever wifi.wake-on-wlan is specified in the module arguments the module will think the value is being changed even when it isn't. * nmcli: Handle wifi options correctly when connection type not specified When an nmcli task does not specify the connection type and the module ask nmcli for it, the module needs to convert nmcli's `802-11-wireless` to `wifi`, the term for this connection type used by the module. * nmcli: Correctly detect values changed to the integer 0 If the user specifies a value of 0 (without quotes) in a task, we should interpret that as an actual value, not empty, when comparing the new value to the old one. Otherwise we incorrectly conclude that there was no change. * Changelog fragment for #5431 (cherry picked from commit 490899f)
felixfontein
pushed a commit
that referenced
this pull request
Feb 24, 2023
…e wifi.wake-on-wlan settings work properly (#6050) nmcli: two fixes needed to make wifi.wake-on-wlan settings work properly (#5431) * nmcli: Convert current value of wifi.wake-on-wlan before comparing The new value of wifi.wake-on-wlan is specified as an integer, but in the nmcli output it's specified as a hex string followed by a textual description of it. Therefore, to determine properly whether it's being changed we need to pull the hex string out of the current value, convert it into an integer, and finally convert the integer back to a string so that we can compare it to the new specified value. Without this change, whenever wifi.wake-on-wlan is specified in the module arguments the module will think the value is being changed even when it isn't. * nmcli: Handle wifi options correctly when connection type not specified When an nmcli task does not specify the connection type and the module ask nmcli for it, the module needs to convert nmcli's `802-11-wireless` to `wifi`, the term for this connection type used by the module. * nmcli: Correctly detect values changed to the integer 0 If the user specifies a value of 0 (without quotes) in a task, we should interpret that as an actual value, not empty, when comparing the new value to the old one. Otherwise we incorrectly conclude that there was no change. * Changelog fragment for #5431 (cherry picked from commit 490899f) Co-authored-by: Jonathan Kamens <[email protected]>
felixfontein
pushed a commit
that referenced
this pull request
Feb 24, 2023
…e wifi.wake-on-wlan settings work properly (#6051) nmcli: two fixes needed to make wifi.wake-on-wlan settings work properly (#5431) * nmcli: Convert current value of wifi.wake-on-wlan before comparing The new value of wifi.wake-on-wlan is specified as an integer, but in the nmcli output it's specified as a hex string followed by a textual description of it. Therefore, to determine properly whether it's being changed we need to pull the hex string out of the current value, convert it into an integer, and finally convert the integer back to a string so that we can compare it to the new specified value. Without this change, whenever wifi.wake-on-wlan is specified in the module arguments the module will think the value is being changed even when it isn't. * nmcli: Handle wifi options correctly when connection type not specified When an nmcli task does not specify the connection type and the module ask nmcli for it, the module needs to convert nmcli's `802-11-wireless` to `wifi`, the term for this connection type used by the module. * nmcli: Correctly detect values changed to the integer 0 If the user specifies a value of 0 (without quotes) in a task, we should interpret that as an actual value, not empty, when comparing the new value to the old one. Otherwise we incorrectly conclude that there was no change. * Changelog fragment for #5431 (cherry picked from commit 490899f) Co-authored-by: Jonathan Kamens <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
ISSUE TYPE
COMPONENT NAME
nmcli
ADDITIONAL INFORMATION
There are two commits in this PR, both of which are necessary to make wifi.wake-on-wlan settings work properly with this module, and one of which is necessary to make any wifi.* setting work properly. See the descriptions of the fixes in the individual commits.