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.

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

Be more lenient with config file #289

Open
Atemu opened this issue Mar 16, 2024 · 3 comments
Open

Be more lenient with config file #289

Atemu opened this issue Mar 16, 2024 · 3 comments

Comments

@Atemu
Copy link

Atemu commented Mar 16, 2024

The current config file format is very strict. Defaults must be present and the keys of the curve dict must be integers.

Both of these requirements cause considerable complexity when automating deployment of this service which I did in the form of a NixOS module that I plan to upstream.

It'd be great if lactd could assume the defaults if the config file does not include them and allow string representations of temperature number keys.

@ilya-zlobintsev
Copy link
Owner

Could you be more specific about what values exactly do you want there to be defaults for? The minimum required config is already pretty small:

daemon:
  log_level: info
  admin_groups:
  - wheel

gpus:

You can see all of the possible fields here - most of them are already either an Option, or have a default attribute set. I suppose the daemon config could have a default for the entire field.

As for the fan curve map keys - I would like to keep them as numbers, since this way they can be directly deserialized into a map with integer keys as it is used for fan control. Adding support for string keys would mean that the curve would first have to be deserialized into an intermediate map that have an abstract value as the key, and then converted/parsed into integers.

This isn't too hard, but being able to store the things such as the fan curve directly like this was one of the main reasons why the config file uses YAML in the first place, and I don't want to add more ambiguity to the config format.

@ilya-zlobintsev
Copy link
Owner

@Atemu any thoughts on this?

@Atemu
Copy link
Author

Atemu commented Apr 4, 2024

Thanks for the ping; this got lost in the endless sea of tabs ;)

By "defaults" I mean the default config file generated by the daemon on first startup. I assume these are defined in code somewhere.
What would be ideal is the values of the daemon set being set to their defaults unless overridden by an explicit value in the config. If my config was:

daemon:
  log_level: warning

admin_groups and disable_clocks_cleanup aswell as gpus would still be set to their defaults with only the log level being set to warning for instance.

On a theoretical level, I'd prefer integer keys too but the big problem with them is that barely any tooling supports them. For automatically generating the config file, I had to resort to converting to JSON and then back to YAML using a tool that has a special flag for converting number strings to integers (yj/jy):

https://github.com/Atemu/nixos-config/blob/4ad167830d5dba802a317058fd26b05f5a3b7eba/modules/lact/module.nix#L7-L14

This is a rather hammer-like approach and will try to convert any key that looks like a number.

Is there not a way to coerce strings into numbers while de-serialising? Not familiar with the library you're using for that purpose.

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

No branches or pull requests

2 participants