feat(pro): added support for initializing the fans to a new mode#474
Merged
jonasmalacofilho merged 11 commits intomainfrom Oct 17, 2022
Merged
feat(pro): added support for initializing the fans to a new mode#474jonasmalacofilho merged 11 commits intomainfrom
jonasmalacofilho merged 11 commits intomainfrom
Conversation
Member
Author
|
Re: #472 (comment)
|
1c3465e to
f14fcda
Compare
jonasmalacofilho
approved these changes
Oct 17, 2022
| self._hwmon.driver) | ||
|
|
||
| if fan_modes: | ||
| _LOGGER.warning('kernel driver does not support the `fan mode` options, ignoring') |
Member
There was a problem hiding this comment.
FYI I might still tweak this message, for consistency with other hwmon drivers and docs/developer/hwmon.md.
| raise ValueError(f'invalid direction: {direction!r}') | ||
|
|
||
|
|
||
| def fan_mode_parser(value: Optional[str], max_fans: Optional[int] = None) -> dict: |
Member
There was a problem hiding this comment.
After merging this, we should look into adding more type annotations, and checking them (again).
IIRC one issue is our use of:
class Base:
def some_method(arg1, arg2, **kwargs):
raise NotImplementedError()and each subclass having their implementations with slightly different signatures:
def some_method(arg1, arg2, arg3=None, arg4=None, **kwargs):
# ...(If we end up deciding to continue not using type annotations, they should then be removed from this function).
Member
|
Thanks! |
jonasmalacofilho
added a commit
that referenced
this pull request
Oct 17, 2022
This file contains hidden or 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
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.
Related: #472
This will add some cli flags to enable / disable / change fan modes on a commander pro device
TODO:
Checklist:
liquidctl.8Linux/Unix/Mac OS man pagedocs/*guide.mddevice guidesNew CLI flag?
extra/completions/New device?
extra/linux/71-liquidctl.rules(instructions in the file header)en)New driver?
docs/developer/protocol/